Fix stack 03: redact camelCase secrets, bound event sanitizer#27
Merged
Conversation
ebarti
force-pushed
the
review-fixes/03-events
branch
from
12424d5 to
1168cb4
Compare
…clic/deep metadata - Redaction missed camelCase/concatenated secret keys (accessToken, refreshToken, authToken, apiKey) because normalization only handled dashes. Now camelCase boundaries are split before the substring/segment checks, so 'accessToken' reaches the 'token' segment rule. Plural/count keys (inputTokens, max_tokens) and the deliberately-emitted auth_source label stay visible. Also extends the substring denylist with credential / private_key / passwd. - Event builders run before safe_emit's try/except, so a cyclic or extremely nested task.metadata made _sanitize raise RecursionError and abort run(), breaking the 'never aborts a runtime' guarantee. _sanitize now bounds recursion depth and detects cycles, emitting truncation markers instead of raising. Refs REVIEW.md §3.4, §3.5.
The segment rule only matched a sensitive segment after splitting on '_', so a
key with no separator and no camelCase boundary (e.g. "accesstoken") never
produced a bare "token" segment and leaked. Also match a sensitive segment as a
trailing suffix, which keeps plural usage counters visible ("inputtokens" ends
with "tokens", not "token").
ebarti
force-pushed
the
review-fixes/03-events
branch
2 times, most recently
from
cb1c5be to
03048df
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked review-fix PR 3/N (base:
review-fixes/02-cancellation).REVIEW.md §3.4, §3.5:
accessToken/refreshToken/authToken/apiKeyin event metadata were not redacted. Now camelCase boundaries are split before the checks. Count keys (inputTokens,max_tokens) and the deliberateauth_sourcelabel stay visible; denylist also gainscredential/private_key/passwd.safe_emit's guard, so a cyclic or extremely nestedtask.metadataraisedRecursionErrorout of the builder._sanitizenow bounds depth and detects cycles, emitting truncation markers instead of raising — honoring the documented "never aborts a runtime" guarantee.Tests: camelCase redaction, cyclic metadata, deep metadata. Full suite 152 passed / 12 skipped; ruff + mypy strict clean.
Round 2 (re-review follow-up)
accesstoken,ACCESSTOKEN,sessiontoken) are now redacted via a trailing-suffix rule on sensitive segments; plural usage counters (inputtokens,totaltokens) stay visible. Regression test added.