A package name is not a credential name, in any serialization - #299
Merged
Conversation
…zation
0.4.6 fixed "tiktoken==0.11.0" and shipped. It did not fix {"tiktoken": "0.11.0"},
and roar records packages as dict[str, str] keyed by package name, so the published
freeze went on carrying
"tiktoken": "[REDACTED]"
Reproduced on a real job: the on-host checks all passed -- roar 0.4.6 verified by wheel
hash, requirements pinned 0.11.0, the installed distribution was 0.11.0 -- and the
published record was still wrong, because only the serialized form is filtered through
json_named_secret. Row 024 spent a second 3.5-hour run discovering that.
Both rules now share one pattern for a name that DENOTES a credential rather than
merely containing a keyword. Three shapes qualify, and no package name matches any:
UPPERCASE HF_TOKEN, API_KEY, MYTOKEN env vars, POSIX convention
delimited api_key, access-token, token the keyword is its own word
camelCase apiKey, accessToken the capital is the delimiter
tiktoken fails all three: lowercase, "token" undelimited within it, no capital. So do
authlib, keyring, tokenizers and secretstorage, all of which 0.4.6 still redacted in
the JSON form.
This is also strictly better than 0.4.6 in the other direction. That fix dropped
IGNORECASE wholesale and stopped matching lowercase names entirely, so api_key=... and
{"api_key": ...} went unredacted. The delimiter test restores them.
Tests cover both directions in both serializations, including the full record shape as
it is actually written. Negative control: restoring the 0.4.6 json rule fails the
serialized-map case while the string-form cases still pass -- the gap that let this
ship. Full unit suite green, 1234 tests.
Version bumped to 0.4.7 here so the rc branch is release-ready; 0.4.6's release build
failed because the tag was cut ahead of the bump.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A filter that simply stopped redacting would pass every package-survives case and be catastrophically wrong. This asserts both halves of the same artifact: dependency versions come through intact, and credentials sitting beside them in the captured environment do not -- by name shape (uppercase, delimited, camelCase) and by value. Also pins the innocent neighbours: PATH survives, and --depth=14 survives in a command whose HF_TOKEN= is redacted. Negative-controlled in both directions. Neutering the credential-name pattern fails all 22; restoring the 0.4.6 json rule fails only the serialized-map case.
This was referenced Sep 10, 2026
christophergeyer
added a commit
that referenced
this pull request
Sep 10, 2026
…se (#300) * A package name is not a credential name, in any serialization (#299) * fix(filters): a package name is not a credential name, in any serialization 0.4.6 fixed "tiktoken==0.11.0" and shipped. It did not fix {"tiktoken": "0.11.0"}, and roar records packages as dict[str, str] keyed by package name, so the published freeze went on carrying "tiktoken": "[REDACTED]" Reproduced on a real job: the on-host checks all passed -- roar 0.4.6 verified by wheel hash, requirements pinned 0.11.0, the installed distribution was 0.11.0 -- and the published record was still wrong, because only the serialized form is filtered through json_named_secret. Row 024 spent a second 3.5-hour run discovering that. Both rules now share one pattern for a name that DENOTES a credential rather than merely containing a keyword. Three shapes qualify, and no package name matches any: UPPERCASE HF_TOKEN, API_KEY, MYTOKEN env vars, POSIX convention delimited api_key, access-token, token the keyword is its own word camelCase apiKey, accessToken the capital is the delimiter tiktoken fails all three: lowercase, "token" undelimited within it, no capital. So do authlib, keyring, tokenizers and secretstorage, all of which 0.4.6 still redacted in the JSON form. This is also strictly better than 0.4.6 in the other direction. That fix dropped IGNORECASE wholesale and stopped matching lowercase names entirely, so api_key=... and {"api_key": ...} went unredacted. The delimiter test restores them. Tests cover both directions in both serializations, including the full record shape as it is actually written. Negative control: restoring the 0.4.6 json rule fails the serialized-map case while the string-form cases still pass -- the gap that let this ship. Full unit suite green, 1234 tests. Version bumped to 0.4.7 here so the rc branch is release-ready; 0.4.6's release build failed because the tag was cut ahead of the bump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: the true positive and the false positive in one record A filter that simply stopped redacting would pass every package-survives case and be catastrophically wrong. This asserts both halves of the same artifact: dependency versions come through intact, and credentials sitting beside them in the captured environment do not -- by name shape (uppercase, delimited, camelCase) and by value. Also pins the innocent neighbours: PATH survives, and --depth=14 survives in a command whose HF_TOKEN= is redacted. Negative-controlled in both directions. Neutering the credential-name pattern fails all 22; restoring the 0.4.6 json rule fails only the serialized-map case. * style: f-strings for the JSON secret fixtures (ruff UP031) --------- Co-authored-by: Chris Geyer <chris@treqs.ai> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * Tell a version pin from a credential by value and word boundary, not case (#301) * Revert the two attempts to fix this by narrowing the credential-name pattern Both fixes treated "a package version was redacted" as a problem with which NAMES look like credentials, and both narrowed the pattern to exclude package names. Both broke redaction doing it, measured over a 325-case corpus: 0.4.6 dropped IGNORECASE wholesale, so api_key=, hf_token= and authtoken= stopped being redacted at all -- a leak 0.4.5 caught. It also fixed only the "name==version" string form, leaving {"name": "version"} broken, which is the form the record actually serializes. 0.4.7 replaced that with three casing branches (UPPERCASE / delimited-lowercase / camelCase). 42 credential names that 0.4.5 redacted stopped being redacted: Hf_Token, MyToken, Api_Key, APIKey, Github_Token, TOKEN_my, secretValue and the bare Token / Key / Secret / Password / Credential forms. Adding "-" to the delimiter set to catch api-key simultaneously pulled in the *-auth package family, so google-auth, google-auth-oauthlib, dj-rest-auth and social-auth-core had their versions redacted in the serialized form -- the original bug again, in the same serialization, introduced by its own fix. Casing was never the discriminator. This restores the 0.4.5 pattern so the real one can be applied on top of a clean base rather than layered over two abandoned designs. The tests both commits added are kept: they assert the right behavior, they were just enforcing it through the wrong mechanism. * Tell a version pin from a credential by the value and the word boundary, not the case Two guards replace the casing branches the previous attempts relied on: word boundary the keyword must be its own word within the name -- bounded by a delimiter, a case transition, or an edge. "api_key" and "MyToken" qualify; "tiktoken" and "tokenizer" are single words that merely contain one. This is what keeps --tokenizer=gpt2 executable in a recorded command. version guard a PEP 440 version is never a credential. This is what spares google-auth, dj-rest-auth and social-auth-core, whose names DO carry a delimited keyword and which no name-only pattern can get right. Because both guards work on structure rather than case, the pattern no longer excludes any casing, so every credential name 0.4.5 redacted is redacted again -- the 42 that 0.4.7 dropped (Hf_Token, MyToken, Api_Key, APIKey, TOKEN_my, secretValue, bare Token / Key / Secret / Password / Credential) and the lowercase ones 0.4.6 dropped (api_key=, hf_token=, authtoken=). Measured over a 325-case corpus, against both prior baselines: vs 0.4.5 vs 0.4.7 regressions 0 0 new false positives 0 0 false positives fixed 25 3 credentials newly caught 3 43 Tests consolidated from 208 lines / 22 cases to 133 / 76, now asserting every case in all four serializations the record is written in. They fail against each prior version: 0.4.5 (44), 0.4.6 (46), 0.4.7 (37). Unchanged and tracked separately: URL credential rules still enumerate schemes, so postgresql://, mongodb+srv:// and rediss:// leak; bare-userinfo form leaks for every scheme but https/ssh/git; provider shapes cover only hf_/ghp_/sk-/AKIA; and redacted URIs are not parseable because the marker's brackets read as an IPv6 literal. * style: ruff format and fromkeys fixup --------- Co-authored-by: Chris Geyer <chris@treqs.ai> --------- Co-authored-by: Chris Geyer <chris@treqs.ai> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Targets
rc/0.4.7.What 0.4.6 missed
#297 fixed the string form and shipped:
It did not fix the serialized form, and
roarrecords packages asdict[str, str]keyed by package name (core/models/provenance.py—used_packages,installed_packages,packages):json_named_secretcarried the identical flaw —[A-Z_]*(?:KEY|TOKEN|…)[A-Z_]*withIGNORECASE— and was never touched.Reproduced on a real job, not inferred. Every on-host check passed: roar 0.4.6 verified, requirements pinned
0.11.0, installed distribution0.11.0. The published record still read:Every neighbouring package survived. Row 024 spent a second 3.5-hour run finding this, because the first fix was verified against one serialization instead of the artifact.
The fix
Both rules now share one pattern for a name that denotes a credential rather than merely containing a keyword. Three shapes qualify, and no package name matches any:
HF_TOKEN,API_KEY,MYTOKEN— env vars, POSIX conventionapi_key,access-token,token— the keyword is its own wordapiKey,accessToken— the capital is the delimitertiktokenfails all three: lowercase,tokenundelimited within it, no capital. So doauthlib,keyring,tokenizers,secretstorage— all four were still redacted by 0.4.6 in the JSON form.Strictly better than 0.4.6 in the other direction too. That fix dropped
IGNORECASEwholesale, so lowercase names stopped matching entirely andapi_key=…went unredacted. The delimiter test restores them.Verification
json_named_secretrule fails the serialized-map case while every string-form case still passes — precisely the gap that let this ship.Still to do before release, and deliberately not claimed here: an end-to-end run on real compute, publishing and reading back the freeze. That is the check that was skipped for 0.4.6, and the reason this PR exists.
Version
pyproject.tomlbumped to 0.4.7 in this PR sorc/0.4.7is release-ready. 0.4.6's release build failed because the tag was cut ahead of the bump.