Skip to content

fix: redact auth values in error logs, callback reports and server responses - #968

Merged
zeevmoney merged 11 commits into
masterfrom
redact-auth-values-in-logs
Oct 13, 2026
Merged

zeevmoney merged 11 commits into
masterfrom
redact-auth-values-in-logs

Conversation

@zeevmoney

@zeevmoney zeevmoney commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Fixes Issue

N/A

Changes proposed

Keeps auth values (Authorization headers, client/store/master tokens, git and URL credentials) out of log lines, error strings, the callback report payload and HTTP responses, across opal-common, opal-client and opal-server.

opal-common / opal-client

Error formatting

  • New opal_common.logging_utils.errors.describe_error() renders an exception as Type: message with credentials masked. aiohttp's repr() of ClientResponseError / ContentTypeError includes the request headers; str() does not.
  • Used instead of repr(e) in the policy fetcher, the policy updater's transaction error (which also feeds the OPA health-check policy), base_policy_store_client, every OPA and Cedar client error path, the startup limiter, ApiPolicySource and the fetching engine.
  • describe_validation_error() summarizes a pydantic ValidationError as locations and types, without the rejected input. The policy fetcher no longer binds the raw bundle to the log record.

Callback report payload

  • The JSON POSTed to callback URLs omits each entry's fetcher config and inline data and passes entry.url through redact_url. All other fields are kept (update_id, policy_hash, user_data, fetched/saved/hash, entry.topics/dst_path/save_method, subclass fields). The keys are omitted, not set to null, and the payload still parses as DataUpdateReport.
  • The reporter no longer writes the report into the registered callback configs (or the shared default callback config); each request gets its own copy.
  • A non-JSON error body from one callback no longer stops the remaining failures from being logged.

Log scrubbing

  • redact_url_in_text also masks sensitive query params (?token=, &api_key= ...) in free text.
  • New redact_sensitive_text also masks Authorization / Proxy-Authorization values and Bearer/Basic credentials. The loguru patcher applies it to the message and to top-level string extras; CredentialScrubbingStream applies it to the final text.
  • rpc.notify no longer binds the raw update; the data updater's save log redacts the source URL; the inline engine start log masks the value after -a/token flags (argv unchanged).

Responses and errors

  • Unauthorized no longer echoes the presented token in the 401 body.
  • throw_if_bad_status_code reads the body as text and parses JSON if it can, so an HTML error page raises the intended ValueError instead of ContentTypeError.
  • An invalid OPAL_* model/JSON value raises a ValueError that names the key and the error locations, without the value. Invalid CLI option values raise click.BadParameter without the value.

Config printing

  • New sensitive flag on Confi entries (confi.str("X", None, sensitive=True), accepted by every builder). Keys matching TOKEN|SECRET|PASSWORD|PASSPHRASE|API_KEY|PRIVATE_KEY|SSH_KEY count as sensitive too.
  • print-config (Confi.__repr__), debug_repr() and --help show sensitive values as <redacted> and mask URL credentials in other values. OPAL's own token/key entries are marked explicitly.

Validation errors (pydantic v2)

  • hide_input_in_errors=True on the config-bearing models (DataSourceEntry, DataSourceConfig, ServerDataSourceConfig, DataUpdate, CallbackEntry, UpdateCallback, the report models, FetcherConfig, FetchEvent, PolicyBundle, engine options, scope models). .errors() still has the input.
  • repr() of CallbackEntry, UpdateCallback, ServerDataSourceConfig, AccessToken, CedarServerOptions, the scope auth models and the scope policy source masks URLs / tokens / keys.

opal-server

  • GET /scopes omits policy.auth from every item again. The response is a list, so the exclude now addresses each item ({"__all__": {"policy": {"auth"}}}); a bare {"policy": {"auth"}} is ignored for list responses on the current pydantic v2 / FastAPI. GET /scopes/{id} already excluded it.
  • POST /webhook returns repo_url through redact_url (git and API mode), and the webhook log lines redact the repo URL.
  • The gunicorn master's scope preload calls configure_logs() first, so it logs with OPAL's level and redaction instead of loguru's defaults. RedisDB logs the Redis URL redacted.
  • OPAL_NO_RPC_LOGS (default True) is now applied: it raises the websocket RPC/pub-sub libraries' loggers to INFO, since their DEBUG records include message payloads.
  • A malformed broadcast is logged with error locations and types only, not the payload.
  • Redacted URLs in the data-config redirect log, the default callback-report log (entry.url), the policy watcher failure log and the scope API's git error logs.
  • /pubsub_client_info reports values of sensitive query params (token, api_key, ...) as ***.

Check List (Check all the applicable boxes)

  • I sign off on contributing this submission to open-source
  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

N/A

Note to reviewers

  • Tests: every changed site has a test that triggers it with a fake value and checks the text log, a serialize=True sink, and the payload / response / error text where applicable. Test log capture runs without the global loguru patcher, so these tests check the call sites themselves. pytest packages/opal-common packages/opal-client packages/opal-server: 994 passed, 9 skipped.
  • Includes fix(opal-common): keep AccessTokenRequest.ttl's schema numeric on pydantic 2.14 #965's commit so CI runs green on pydantic 2.14; it drops out when fix(opal-common): keep AccessTokenRequest.ttl's schema numeric on pydantic 2.14 #965 merges.
  • CI: tests.yml gets an aggregate CI job (needs every other job and fails unless all succeed; it also checks its own needs list), runs for release/** too, and no longer uses fail-fast. The branch ruleset requires CI and pre-commit.
  • Visible to consumers:
    • the callback payload no longer has entry.config / entry.data, and entry.url is redacted;
    • 401 bodies no longer have detail.token;
    • logged error strings change from repr form to Type: message (also in the OPA health-check policy's error field);
    • print-config / --help mask sensitive values;
    • a bad config value raises ValueError instead of ValidationError;
    • GET /scopes items no longer include policy.auth (as on 0.9.x);
    • the webhook response's repo_url is redacted when the configured URL carries auth;
    • /pubsub_client_info masks sensitive query values.
  • Not changed: GET /scopes/{id} still returns policy.url and the scope's data entries with their config; /data/config still serves DATA_CONFIG_SOURCES to authenticated clients; DataUpdate broadcasts still carry entry configs to subscribed clients.
  • The opal-common / opal-client part is ported to the 0.9 line (pydantic v1) in a separate PR against release/0.9.x.

🤖 Generated with Claude Code

zeevmoney and others added 5 commits October 13, 2026 14:34
- Log aiohttp and other errors as "Type: message" with credentials masked,
  instead of repr(), which includes the request headers.
- Omit each entry's fetcher config and inline data from the report POSTed
  to callback URLs, redact entry URLs, and stop writing the report into the
  registered callback configs.
- Mask Bearer/Basic values, Authorization headers and sensitive query
  params in log messages and string extras.
- Stop echoing the presented token in 401 response bodies.
- Read non-JSON error bodies as text in throw_if_bad_status_code and the
  callback reporter.
- Add a `sensitive` flag to Confi entries; print-config, debug_repr and
  --help show sensitive values as <redacted>.
- Hide rejected input in validation errors of config-bearing models.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A model option that fails validation now raises click.BadParameter with
the error locations and types, and a sensitive option that fails to parse
reports only the exception type. click < 8.2 otherwise echoes the raw
option value, and newer click echoes the error text, which can include it.

Also sign the tokens in unauthorized_response_test per test, so they do
not expire before a long run reaches them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r logs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The global log patcher installed by an earlier configure_logs() call masks
the fake auth header, so asserting on the header alone passed or failed
depending on test order. Assert on the records' message text instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@netlify

netlify Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for opal-docs canceled.

Name Link
🔨 Latest commit d083bcf
🔍 Latest deploy log https://app.netlify.com/projects/opal-docs/deploys/6ace5575e070d6621a71694d

@linear-code

linear-code Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

PER-16925

PER-16823

zeevmoney and others added 4 commits October 13, 2026 15:34
The 3.13 CI leg installs only opal-common and opal-client.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…antic 2.14

pydantic 2.14 renders a timedelta field's validation-mode (input) JSON
schema as an ISO-8601 duration string. ser_json_timedelta keeps only
the serialization-mode (output) schema a number, and earlier 2.x
versions give a number in both modes. FastAPI publishes the input
schema for the /token request body (AccessTokenRequest-Input). So on
2.14 the published OpenAPI said ttl is `{"type": "string", "format":
"duration", "default": "P1Y"}` while the wire still carries float
seconds. A client generated from it would send "P1Y", the value a
pydantic v1 server cannot parse, and access_token_wire_format_test fails
on every CI leg that resolves 2.14.

Set the field's schema to its wire form with a json_schema_extra
callable: type number, no format, default 31536000.0. Measured the same
on pydantic 2.9.2, 2.13.5 and 2.14.1, in validation and serialization
mode; the wire format and numeric input are unchanged.

The new test's validation case reproduces the 2.14 bug. Its
serialization case passes on 2.14 without the fix: it guards the
response schema against a regression.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit e0da42e)
The branch ruleset requires a single "CI" check, which needs every other
job in the Tests workflow and fails unless they all succeed, so jobs and
matrix legs can be renamed without editing the ruleset. The job also
checks that its needs list names every other job.

Run the workflow for pushes and pull requests to release/** as well, and
turn off fail-fast so every Python leg reports its own result.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Stored and raised data-fetch errors use describe_error, so an exception
  with no message (e.g. TimeoutError) is still named.
- Mark DEFAULT_UPDATE_CALLBACKS and DEFAULT_UPDATE_CALLBACK_CONFIG as
  sensitive config entries.
- Mask Bearer/Basic values of 16+ lowercase letters; masking is now
  idempotent next to an existing "***".
- Replace datetime.utcnow() in the policy store transaction timestamps,
  keeping the same naive UTC format.
- Test log capture runs without the global loguru patcher, so redaction
  tests check the call sites themselves.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zeevmoney and others added 2 commits October 13, 2026 18:58
The redirect-loop test servers redirect to a fixed path instead of the
request path, and log assertions match the redacted URL with a regex
instead of a substring, so CodeQL's URL redirection and URL substring
sanitization queries no longer flag the tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CodeQL's URL substring sanitization query flags `"host" in line` checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney
zeevmoney marked this pull request as ready for review October 13, 2026 18:49
Copilot AI balanced review requested due to automatic review settings October 13, 2026 18:49
@zeevmoney
zeevmoney merged commit bfe430e into master Oct 13, 2026
16 checks passed
@zeevmoney
zeevmoney deleted the redact-auth-values-in-logs branch October 13, 2026 18:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

zeevmoney added a commit that referenced this pull request Oct 13, 2026
0.9.10 patch for opal-client and opal-common on the 0.9 line (pydantic
v1): the client and common parts of #968, and Python 3.13 support.

opal-common / opal-client
- Log and store errors as "Type: message" with credentials masked
  (describe_error) instead of repr(), which includes the request headers.
  Used by the policy fetcher and updater, the OPA and Cedar clients, the
  startup limiter, ApiPolicySource and the fetching engine. A timed-out
  fetch is still named (TimeoutError).
- The report POSTed to callback URLs omits each entry's fetcher config
  and inline data and redacts entry.url. The reporter no longer writes
  the report into the registered callback configs.
- The loguru patcher and CredentialScrubbingStream mask Authorization
  values, Bearer/Basic credentials and sensitive query params in log
  messages and string extras.
- 401 bodies no longer echo the presented token.
- throw_if_bad_status_code reads non-JSON error bodies as text, so an
  HTML error page raises the intended ValueError.
- Confi entries take a `sensitive` flag. print-config, debug_repr and
  --help show sensitive values as <redacted>. Invalid OPAL_* values and
  CLI options raise errors that omit the value.
- Credential-bearing models mask URLs, tokens and keys in repr().

Packaging and CI
- opal-common and opal-client admit Python 3.13
  (python_requires ">=3.9,<3.14"). opal-server stays below 3.13. A 3.13
  CI leg installs and tests those two packages with pip.
- tests.yml gets an aggregate CI job, runs for release/** branches and
  no longer uses fail-fast.

Visible to consumers: the callback payload drops entry.config and
entry.data and redacts entry.url; 401 bodies drop detail.token; error
strings change from repr form to "Type: message" (also in the OPA
health-check policy); print-config and --help mask sensitive values; a
bad config value raises ValueError naming the key, without the value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CarlosMion added a commit that referenced this pull request Nov 3, 2026
…ines (0.9.x) (#991)

* fix(opal-server): log update reports with redacted entry URLs (0.9.x)

The default update callback logs the client's report through
_report_for_log, as on master since #968. It leaves out each entry's
config and data, and runs each entry's url through redact_url, so the
dict a serialize=True sink writes in record.extra carries the redacted
URL whatever patcher or stream wrapper the sink runs behind.

This line pins pydantic<2, so the helper uses report.dict(exclude=...)
where master uses model_dump.

server_log_redaction_test.py checks the route with the configure_logs
patcher off and on, through a text sink and a serialize=True sink, and
unit-tests _report_for_log's field set and per-entry redaction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(opal-server): redact the redirect URL in its log line (0.9.x)

get_data_sources_config's external-source branch logs
redact_url(url), as on master since #968. The redirect still goes to
the configured URL with the client's token.

The test checks the log line with the configure_logs patcher off and
on, and the Location header of the redirect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(opal-server): check the patcher-off arm replaces the scrubber

Add test_patcher_off_arm_replaces_an_installed_scrub_patcher. With
_scrub_record_message installed as the global patcher, as earlier test
files leave it in a full run, _captured_logs(scrub=False) must pass a
str extra through unmasked. The redirect test's patcher-off arm is the
only one that depends on the call site's redact_url, and this pins that
the arm runs with the scrubber off.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants