Skip to content

fix(protocol): a hyphen must not decide whether a secret is redacted - #5

Merged
krassx merged 1 commit into
mainfrom
fix/hyphenated-sensitive-keys
Sep 6, 2026
Merged

krassx merged 1 commit into
mainfrom
fix/hyphenated-sensitive-keys

Conversation

@krassx

@krassx krassx commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

The defect

SENSITIVE_KEY_SUBSTRINGS carries api_key and apikey but never api-key. And cookie lives only in SENSITIVE_HEADERS, which matches exactly and covers the header path alone. So inside a JSON body:

{"apiKey":    "sk_live_…"}   redacted
{"x-api-key": "sk_live_…"}   NOT redacted   ← live credential, plaintext
{"api-key":   "sk_live_…"}   NOT redacted
{"cookie":    "session=…"}   NOT redacted

Header-shaped keys appear in bodies constantly — a proxied request, a webhook payload, a captured config object — and a hyphen was the only thing deciding whether a credential shipped.

Two rules, because they carry different risks

The substring denylist is now matched separator-insensitively. One normalisation rather than a hyphenated twin for all 59 entries — a list that would have to be edited in pairs for ever — and it covers . and space spellings at the same time.

The exact header names are consulted too, for the ones with no substring twin: cookie, set-cookie, x-real-ip. They are deliberately not folded into the substring list — as a substring, cookie would flag cookieConsent and cookie_banner_shown, which are ordinary analytics keys. Exact matching costs no false positives, and there is a test asserting those three stay unflagged.

Scope

Broad by design. isSensitiveKey decides redaction for JSON bodies, query pairs, span attributes and local variables, which is why this is its own change rather than a rider on something else. The full gate was run with nothing cached, including e2e — green.

Where it came from

Found while building source-context scrubbing, which was closed as a product decision (#4) — no comparable product scrubs source code, and Sentry's split of scrubbing frame vars while leaving context_line alone is deliberate.

This half stands on its own and is on the other side of that line: it is runtime data, in a place where we already promise scrubbing, and it is the same class Sentry marks pii = "true".

Testing

6 mutations, all caught. The last one is worth recording: stripping a letter instead of a separator passes every other test, because the key and the denylist entry are normalised the same way — so api_key and x-api-key keep matching — while quietly collapsing distinct words together. Drop a and auth becomes uth, which south contains. south / mouth / youth now pin that normalisation removes separators and nothing else.

🤖 Generated with Claude Code

`SENSITIVE_KEY_SUBSTRINGS` carried `api_key` and `apikey` but never `api-key`, and `cookie`
lived only in `SENSITIVE_HEADERS`, which matches exactly and covers the header path alone. So
in a JSON BODY:

  {"apiKey":    "sk_live_…"}   redacted
  {"x-api-key": "sk_live_…"}   NOT redacted
  {"api-key":   "sk_live_…"}   NOT redacted
  {"cookie":    "session=…"}   NOT redacted

Header-shaped keys appear in bodies constantly — a proxied request, a webhook payload, a
captured config object — and the separator was the only thing deciding it.

Two rules now, because they carry different risks:

- the substring denylist is matched SEPARATOR-INSENSITIVELY. One normalisation beats adding a
  hyphenated twin to all 59 entries, which is a list that would have to be edited in pairs for
  ever, and it covers `.` and space spellings at the same time.
- the exact HEADER names are consulted as well, for the ones with no substring twin
  (`cookie`, `set-cookie`, `x-real-ip`). They are deliberately NOT folded into the substring
  list: as a substring, `cookie` would flag `cookieConsent` and `cookie_banner_shown`, which
  are ordinary analytics keys. Exact matching costs no false positives.

Found while building source-context scrubbing (closed as a product decision — nobody in the
field scrubs source code). This half stands on its own: it is RUNTIME data, in a place where
we do promise scrubbing, and it is the same class Sentry marks `pii = "true"`.

Broad by design — `isSensitiveKey` decides redaction for JSON bodies, query pairs, span
attributes and local variables — so the whole gate was run with nothing cached, including
e2e: green.

6 mutations, all caught. The last one needed a new test: stripping a LETTER rather than a
separator passes every other case, because the key and the denylist entry are normalised the
same way — while collapsing distinct words together. Drop `a` and `auth` becomes `uth`, which
`south` contains. `south`/`mouth`/`youth` now pin it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@krassx
krassx merged commit 72b2837 into main Sep 6, 2026
2 checks passed
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.

1 participant