fix(security): stop the publication secret gate rejecting public test keys and JWT-shaped identifiers - #1165
Merged
Conversation
The publication secret gate scans agent output in positive-only mode and
fails the node on any hit. Its supplemental JWT rule matched any three
dotted runs of base64url characters (20/10/10 minimum lengths), which also
describes ordinary qualified identifiers: a report citing
`ReentrancyGuardUpgradeable.nonReentrantModifier.lockedStateCheck`, or a
JSON path such as
`auditProfileResolution.settingOrigins.property_priority_threshold`.
The gate rejected such a file after the agent had finished its work.
A JWT's header and payload are base64url-encoded JSON objects. In the
compact form that JWT libraries emit, each opens with '{"' and a letter,
which encodes to "eyJ". The rule now adds a (?=eyJ) lookahead to the
header and payload segments and keeps the old length minimums, so it
matches exactly the old matches whose first two segments begin with
"eyJ". The existing k07 fixture and an HS256 token are still rejected.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…nic and keys Anvil prints the "test test test test test test test test test test test junk" mnemonic and the 10 keys it derives at m/44'/60'/0'/0/0-9 on every startup. Foundry tests and scripts sign with them: forge-std's own test_DeriveRememberKey quotes the mnemonic and key (0). The fail-closed publication gate rejected both. The mnemonic has a valid BIP39 checksum, and `uint256 privateKey = 0xac09...ff80;` satisfies the context-labeled private-key rule. So a generated test that derived or signed with a default Anvil account failed its node's verification. The labeled-key rule now skips those exact 10 keys (compared case-insensitively, with or without 0x), and the mnemonic rule skips a window that is exactly that phrase. The keys and the mnemonic were checked against `anvil` 1.8.3's startup output and `cast wallet private-key --mnemonic-index 0..9`. The keys are public, so exempting them hides nothing. Any other labeled 64-hex key, including key (0) with one digit changed, and any other valid mnemonic, including one that shares the first 11 words, is still rejected. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The mnemonic exemption skipped a window equal to the public phrase with
`continue`, so the word run was never reset. Every later BIP39 word was
then checked in 12- to 24-word windows that overlap the phrase, windows
origin/main never examined because it reset the run after redacting the
phrase. Reproduced on the previous head:
- The gate still rejected the phrase when a whitespace-separated BIP39
word followed it ("... junk one account per index"), and two copies of
it on consecutive lines.
- Redaction weakened next to a real mnemonic. The phrase followed by
"abandon x11 about" on the next line became "test <redacted> abandon
... about", leaving 11 of the real mnemonic's 12 words in the clear.
origin/main redacted both.
A valid window that is exactly the public phrase now ends the run like
any other detected mnemonic; only its redaction range is dropped. The run
therefore evolves as on origin/main, and the rule's output is
origin/main's with those placeholders restored.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The payload (?=eyJ) lookahead removed none of the false positives this
branch targets: the header lookahead alone already stops the rule
matching each dotted identifier fixture. What the payload lookahead did
remove was detection of compact JWE tokens, whose second segment is the
encrypted key, and of JWS tokens whose payload is not compact JSON
('{ "sub": ...' encodes to "eyA"). origin/main detected both. The rule
now requires "eyJ" on the header segment only, and the test pins an
RSA-OAEP JWE and a space-padded payload as still detected.
The CHANGELOG entry drops the claims that real JWTs, other keys and
other mnemonics are all still rejected, and tells cloud operators what
to do when resume recomputes the classification of an allowlisted
variable that holds one of these values.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Comment on lines
+388
to
+390
| if (phrase === PUBLIC_DEVELOPMENT_MNEMONIC) { | ||
| run = []; | ||
| break; |
There was a problem hiding this comment.
Longer mnemonics can pass A valid 24-word private mnemonic can start with the public Anvil phrase. This branch clears the word run after those first 12 words, so the scanner never checks all 24 together. If the remaining 12 words are not a valid mnemonic on their own, the publication gate accepts the private phrase. The previous code kept scanning after the public phrase. How this was verified: The public-prefix branch clears the word run before the 24-word window can be assembled, and the publication gate relies on this scan for mnemonic detection.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/security/src/sensitive-redaction.ts
Line: 388-390
Comment:
**Longer mnemonics can pass** A valid 24-word private mnemonic can start with the public Anvil phrase. This branch clears the word run after those first 12 words, so the scanner never checks all 24 together. If the remaining 12 words are not a valid mnemonic on their own, the publication gate accepts the private phrase. The previous code kept scanning after the public phrase. **How this was verified:** The public-prefix branch clears the word run before the 24-word window can be assembled, and the publication gate relies on this scan for mnemonic detection.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.… limit markdownlint's MD013 in the external static analysis job allows 400 characters per line. The rewritten entry was 408; it is now 398 with the same claims. The job still fails on the file's pre-existing long lines. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Every pull request in this batch inserts its entry at the same place in CHANGELOG.md, so each merge would conflict with the next. The entries are collected into one changelog update instead. Co-Authored-By: Claude Opus 5.5 <[email protected]>
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
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.
Problem
The publication secret gate (
assertArtifactPublicationsContainNoSecrets, run from the workflow verify step and fromverified-output.ts) scans each published file inpositive-onlymode. It fails the node on any hit and does not rewrite bytes. Onorigin/main, three positive rules fire on ordinary Foundry campaign output:`ReentrancyGuardUpgradeable.nonReentrantModifier.lockedStateCheck` reverts on re-entry(report text)auditProfileResolution.settingOrigins.property_priority_threshold(a JSON path)uint256 privateKey = 0xac0974…ff80;(Anvil account 0)string memory mnemonic = "test test … junk";The last two are the canonical Foundry idiom. forge-std's own
test_DeriveRememberKey(lib/forge-std/test/StdCheats.t.sol) quotes the mnemonic and asserts key 0, and main's detector rejects that file. The gate runs after the agent has finished, so a generated test or report shaped like these fails its node.These failures are shown at the detector and gate level (see Verification). I have not reproduced a full campaign killed by them.
Root cause
/\b[A-Za-z0-9_-]{20,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\b/identifies a JWT by shape alone. Three long dotted identifier segments have the same shape.test test test test test test test test test test test junkmnemonic and its first 10 keys (m/44'/60'/0'/0/0-9) on every startup. The mnemonic has a valid BIP39 checksum, andprivateKey = <64 hex>satisfies the label rule.Change
All source edits are in
packages/security/src/sensitive-redaction.ts:(?=eyJ)lookahead, and the old 20/10/10 minimum lengths stay. A JWT header in compact JSON that opens with{"and a letter base64url-encodes toeyJ. The later segments are not constrained, because a JWE's second segment is its encrypted key.PUBLIC_DEVELOPMENT_PRIVATE_KEYSholds the 10 keysanvilprints. The labeled-key rule skips a candidate in this set, compared case-insensitively with or without0x.PUBLIC_DEVELOPMENT_MNEMONIC: when a valid window is exactly this phrase, the mnemonic rule ends the word run as it does for any detected mnemonic but records no redaction range. The words after the phrase are therefore checked exactly as on main.As a yes/no property, detection only narrows. When main's version of a changed pass matches nothing in a string, the new version matches nothing there either. Every match of the new JWT rule also satisfies the old pattern, and the key and mnemonic changes only drop matches; the mnemonic pass ends its word runs exactly where main's does. If main leaves a string unchanged, every pass here therefore receives that same string and also leaves it unchanged. So
containsSensitiveSecretscan go from true to false but not from false to true, and main's fixed points (the checks inmodal/src/worker-diagnostics.tsandrunner.ts) are kept. Redaction spans can differ: forabc-<jwt>, main's JWT rule redacts fromabc-and this rule redacts fromeyJ.Tests:
sensitive-redaction.test.ts, one test per rule:artifacts.test.ts: the gate test gets a must-publish block with a Foundry test file (mnemonic, key 0,vm.deriveKey) and a report line with a dotted identifier. Existing must-reject fixtures k01-k15 are unchanged and still rejected.Since the first revision (review follow-up)
The mnemonic exemption used
continue, which never reset the word run. Two consequences, reproduced on 247c75a:… junk one account per index, and two copies on consecutive lines). 128 of the 2048 words trigger this.<phrase>\n<abandon ×11 about>redacted totest <redacted> abandon … about, leaving 11 of the real mnemonic's 12 words in the clear. Main redacted both.The phrase now ends the run. This also resolves Greptile's second finding (repeated copies growing one run).
The payload
(?=eyJ)lookahead is gone. It stopped none of the false positives above: all three identifier fixtures publish with the header lookahead alone. What it did was drop detection of compact JWE and of JWS tokens with a non-compact payload, both of which main detects (Greptile's first finding).The CHANGELOG no longer says that real JWTs, other keys and other mnemonics are all still rejected, and it now carries the cloud-run note below.
Deliberately not built (and why)
Exact-value matching of configured env values. The workflow gate also byte-matches
sensitiveEnvironmentValues(process.env, …), and that includes any variable named like*PRIVATE_KEY*. An operator who exportedPRIVATE_KEY=<Anvil key 0>will still have artifacts containing that value rejected. I left this alone for three reasons: those values come from the operator's environment, the same helper also feeds diagnostic redaction instart-run.ts,workflow-sync.tsand agent-failure normalization, and I have no reproduction. It is a follow-up if it shows up.forge-std's public Infura key. The provider-keyed RPC URL rule still rejects
https://…infura.io/v3/b9794ad1…inStdChains.sol. Exempting a vendor API key is a separate decision.The
sk-rule matching kebab-case words such assk-learn-pipeline. The ak-/as- vendor pattern matches kebab-case English, silently skipping goal-directed audit lanes #822 fix (exclude-from the suffix) doesn't transfer, because OpenRouter (sk-or-v1-…) and OpenAI project keys (sk-proj-…) are themselves hyphenated.Hardhat's accounts 10-19. Hardhat's documented default derives 20 accounts from the same mnemonic. I did not run Hardhat, and only Anvil's 10 are exempted, as the spec asked.
JWT-shaped tokens this rule no longer matches. Main matched these by shape:
eyJ, such as a space-padded header (eyA…) or a first member name that starts with a digit (eyI…);token_eyJ…), which has no\bbeforeeyJ.JWT libraries emit compact headers. A
Bearer <token>is still caught by the Bearer rule, and configured values are still matched byte for byte. The spec asked foreyJon both the header and the payload. The header alone stops every false positive shown here and gives up less detection.Longer mnemonics that contain the phrase. A valid 15- to 24-word mnemonic containing the public phrase as 12 consecutive words is not flagged, because the phrase ends the run before the longer window completes. Main flagged such a mnemonic only through the public phrase, and it redacted just those 12 words, which left the words carrying the secret bits in the clear. I verified this with a constructed valid 24-word mnemonic that begins with the phrase. Random entropy reproduces the phrase's 132 bits with probability 2^-132, so such a mnemonic arises only by construction. Catching it would need lookahead past the phrase.
Dotted run ids. I first suspected the old JWT rule also broke run-link verification for a dotted
--run-id, and callingprepareWorkflowRunLinkandfinalizeWorkflowRunLinkdirectly does fail on main. ButstartRunrejects such ids earlier (the workflow runner id must match^[a-z0-9_-]{1,64}$), so that path is unreachable, and this PR makes no claim about it.Verification
Discriminating tests. I swapped in
packages/security/src/sensitive-redaction.tsfrom each revision under the current test files:Previous head (247c75a): both changed tests fail.
containsSensitiveSecrets("<phrase> one account per index", [], "positive-only")returns true.I also ran each new assertion on its own. The four that are new since the first revision fail on 247c75a and pass now: prose after the phrase, the real mnemonic on the next line, the JWE, and the space-padded-payload JWS.
origin/main: both tests fail. The phrase is flagged, and
ReentrancyGuardUpgradeable.nonReentrantModifier.lockedStateCheckis flagged. Main detects the JWE and the padded-payload JWS, so those two assertions guard against losing detection rather than discriminate against main.Artifacts gate test with main's security source: it fails with
artifact publication contains sensitive data: generated-tests/AnvilSigner.t.sol(re-run for this revision).This head: security passes 25/25, and all artifacts test files pass 354/354.
Differential fuzz against origin/main. Both builds'
@ultrafuzz/securitydist ran on 6 seeds × 4,000 generated inputs × 2 modes, 48,000 runs in total. The inputs mix the phrase, the keys, labels, random 64-hex, BIP39 words, a real mnemonic,eyJ-prefixed,ey-prefixed and random dotted segments, and punctuation. Half of them are whitespace-joined word streams. Results:Keys. The 10 keys match
cast wallet private-key --mnemonic "test … junk" --mnemonic-index 0..9, in order, which I re-ran for this revision.Corpus check. I scanned a local target checkout (
tiny-vault) with both builds inpositive-onlymode. The scan covered the 69,783 files under 1 MB with no NUL byte, skipping.git,node_modules,outandcachedirectories. It includes the target's.ultrafuzz/runsoutputs, execution snapshots and trusted-CLI closures. Results:StdChains.solandStdChains.t.solcopies (9 each), because of the Infura key.Suites run on this head:
final-report-markdown.test.js: 35/35smithers-diagnostic.test.js: 2/2data-governance.test.js: 10/10generated-workflow-verifier.test.jsandverified-output.test.jswith--test-name-pattern='secret|redact|credential|sensitive': 3/3 and 1/1runtime.test.jswith--test-name-pattern='allowlist|redact|sensitive|classif|cloud agent credential|credential rotation|secret': 5/5worker-diagnostics,public-bundle,recovery-lifecycle,public-eval-diagnostics,worker-result,public-worker): 166/166. An earlier run under heavy machine load timed out 3public-workertests at 30 s, and they timed out the same way on origin/main at that point.config.test.ts,stateful-profiles.test.ts): 59/59Static checks:
npx prettier --checkandnpx eslinton the changed filesCI=1 ESLINT_PLUGIN_DIFF_COMMIT=<merge base> pnpm -w lint:strict:cipnpm --filter @ultrafuzz/security typecheckandpnpm --filter @ultrafuzz/artifacts typecheckpnpm -w knipnode scripts/docs-check.mjsNot run: the full runtime and CLI suites, or an end-to-end campaign.
Risk / compatibility
packages/securitysource changes. No schema, contract description or validator module is touched, so the validator build identity does not rotate.<redacted>.redactedTextSpanCodePointLengthsreturns none, which happens for a message over 16,384 characters or when span inference is ambiguous.node attempt … was already recorded with different immutable datafor that run.createNodeAttemptLedgerEntryand this branch'sreconcileNodeAttemptLedgerEntry, on three messages: a 22,093-character message with the dotted identifier, an 18,089-character one withsigning key: <Anvil key 0>, and a 22,089-character one quotingVM::deriveKey("<phrase>", 0). The short versions of the same messages reconcile through their recorded spans.assertCurrentCloudAgentCredentialEnvironmentrecomputes the classification ofULTRAFUZZ_AGENT_ENV_ALLOWLISTvariables and compares it with the list sealed at compile time.eyJheader (for exampleanvil-fork-node-production.monad-foundation.cloudfront-cdn). Main classified it sensitive and dropped it; this branch forwards it.cloud agent credential classification changed after workflow compilation …; start a new run. I reproduced this by compiling with origin/main'scompileSmithersWorkflowand checking with this branch.ULTRAFUZZ_AGENT_ENV_ALLOWLIST, or unsetting it, passes the check (verified). The CHANGELOG says so.eyJheader, and constructed longer mnemonics that contain the phrase, are no longer caught by these rules (see above).CHANGELOG.mdfor the whole file. origin/main's copy already has 43 longer lines, up to 1,777 characters, and 42 of them are flagged, so any PR that edits the file fails the job. This entry is 398 characters and is not flagged (CI run 36515120221 reports 42 MD013 errors, all on pre-existing lines; gitleaks reports no leaks). This PR does not fix the existing ones. Because that job fails,release-gatesfails too and the full release validation lanes are skipped, so the runtime release lanes have not run on this PR in CI. "PR build and Node.js 24 runtime smoke", which includes the diff-limited strict lint, passes.🤖 Generated with Claude Code
The PR does not appear safe to merge while a constructed longer private mnemonic beginning with the public phrase can pass the publication gate.
Fix with agent prompt
Summary
The PR narrows publication-secret detection to allow Anvil’s published mnemonic and ten development keys, and to avoid treating long dotted identifiers as JWTs. Security and artifact tests cover the intended exemptions and retained detection.
Reviews (4) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."