Skip to content

Redact URLs in the http.redirect.rejected log path #70

Description

@Wahbeh-Mohammad

Part of the audit remediation umbrella #67. Severity: HIGH. Requirements: OBS-11, OBS-12, REDIR-28.

Current SDK behavior

The redirect step emits the http.redirect.rejected event when it refuses a redirect. It
passes the decision error to LogEvent.cause(). The logger renders the cause as
name: message. The messages of SchemeDowngradeError and NonReplayableBodyError contain
the raw from and to URLs. The raw URLs can contain userinfo and query-string secrets. The
log record therefore contains the secrets in clear text.

Example record, captured through Cursor and redirectStep():

cause: "SchemeDowngradeError: redirect from 'https://alice:hunter2@example.com/start?access_token=SUPERSECRET' to 'http://example.com/next' would downgrade HTTPS to HTTP"

Every other redirect event (http.redirect.hop, downgradePermitted, loopDetected) sends
its URLs through redactUrl(). The field length cap is 8192, so truncation does not remove
the secret.

Code: packages/core/src/redirect/redirect-step.ts:230, packages/core/src/redirect/errors.ts:28-31,54-57,
packages/core/src/observability/logger.ts:74.

Expected behavior

The SDK does not write a raw URL to any log record. The http.redirect.rejected record shows
userinfo as ***:***@ and each query value as ***, the same as the other redirect events.
The typed error objects can keep the raw URLs in their fromUrl / toUrl properties for
program use.

Notes and leads

  • Smallest fix: build the two error messages from redactUrl(url) output. The fromUrl /
    toUrl properties stay raw.
  • Alternative: emitRejected logs error.name plus redacted URL fields, and never the message.
    This also protects any future error class on this path.
  • The Phase 5b checklist deferred this to Phase 7b under XCUT-19 ("routes every URL field
    through redactUrl()"). Phase 7b redacted the URL fields but not the error-message cause.
  • Test: a seed URL with userinfo and a secret query, a Location that downgrades to http,
    and a non-replayable body with a secret query in Location. Assert the captured record
    contains *** and does not contain the secret. Add the case to
    packages/core/src/redirect/redirect-step.test.ts and consider
    tests/conformance/xcut/security-by-default.conformance.test.ts.
  • Changeset: patch for @dexpace/core.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

area:coreCore HTTP, IO, body, context, encoding: HTTP-* IO-* BODY-* CTX-* UTF-*area:platformConfig, observability, non-functional, hashing: CFG-* OBS-* NFR-* SHA-*securitySecurity impact; see SECURITY.md for private reportstype:bugDefect in shipped behavior

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions