Skip to content

fix(provenance): make one length floor the whole substitution rule - #6551

Merged
icecrasher321 merged 4 commits into
stagingfrom
fix/causal-projection-beats-value-matching
Aug 11, 2026
Merged

fix(provenance): make one length floor the whole substitution rule#6551
icecrasher321 merged 4 commits into
stagingfrom
fix/causal-projection-beats-value-matching

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

What

A secret literal shorter than 8 characters is no longer substituted anywhere.

Eight was already the floor for matches inside a larger token. Below it, a second tier still substituted whenever the hit sat on a word boundary — standing alone, delimited, or as the whole value — on the theory that those positions made the hit unambiguous.

Position is not the variable that matters. A hit on 7 is uninformative wherever it sits, because the value space is ten.

Why

That tier caused two production incidents:

  • _raw_idx = 7 was rewritten to _raw_idx = {{WEEKLY_OWNWORK_TTL}} for the one shard whose index collided with a TTL variable's value. Shards 0–6 and 8–15 were untouched, which is the fingerprint of a value collision rather than a code path.
  • 2,000 boolean had_error cells became [REDACTED_SECRET] because a *_ENABLED variable held false.

Each was patched with a per-value exception list (true/false/null, then almost single digits). The floor subsumes both, so the lists are deleted — this removes exceptions rather than adding one.

What else goes

With no literal below the floor reaching a matcher, the tier's machinery is unreachable and is deleted with it:

  • ResolvedSecretMatchPolicy and getResolvedSecretMatchPolicy
  • isWordBoundaryMatch / satisfiesResolvedSecretMatchPolicy and their code-point helpers
  • the mode: 'detect' | 'render' matcher option, which existed only to select between the tiers

One constant, MIN_SUBSTITUTABLE_LITERAL_LENGTH, now governs the whole question. Net −350 lines.

The trade, stated explicitly

A secret shorter than 8 characters is no longer redacted from logs or model-visible content. Substitution cannot hide a value that short — an observer who can read the surrounding text can enumerate it.

Two tests pinned the old tier (projects a short secret standing alone or delimited…, keeps content and the control error safe from active one-character values). They are rewritten to pin the new rule, not deleted, so the trade stays visible in the suite rather than vanishing.

Entropy remains rejected as a criterion, for the reason already documented: an all-f HMAC key scores 0.00 bits/char, a zero-padded AWS key id 1.02. Length is the criterion.

Review notes

Most of the diff is test fixtures. Every short stand-in secret ('x', 'Test', '123', 'E', 'A', 'F', '!') was lengthened past the floor along with the content it is matched against, so each test still exercises its original shape. The fixture updates are where a real regression could hide — worth a skim for any case where a lengthened fixture no longer exercises what the test name claims.

Verification

  • tsc --noEmit clean
  • check:api-validation passed
  • 18,671 tests passing across executor/, lib/, tools/, providers/, app/api/, background/
  • Full suite: 22,718 passing. search-modal.test.tsx fails on this branch and on staging alike — it has no import path to any changed code and the file is untouched here. diff-engine.test.ts is the known load-flake (passes in isolation).

🤖 Generated with Claude Code

A literal shorter than eight characters is no longer substituted anywhere. It
was already the floor for matches inside a larger token; below it a second tier
still substituted whenever the hit sat on a word boundary — standing alone,
delimited, or as the whole value — on the theory that those positions made the
hit unambiguous.
Position is not the variable that matters. A hit on `7` is uninformative wherever
it sits, because the value space is ten. That tier rewrote `_raw_idx = 7` into
`_raw_idx = {{WEEKLY_OWNWORK_TTL}}` for the one shard whose index collided with a
TTL variable, and turned 2,000 boolean `had_error` cells into `[REDACTED_SECRET]`
because a `*_ENABLED` variable held `false`. Each was patched with a per-value
exception list; the floor subsumes both, so the lists are deleted.
With no literal below the floor reaching a matcher, the tier's machinery is
unreachable and goes with it: the match-policy type, its classifier, the word
boundary test, and the detect/render mode that existed only to select between
them. One constant now governs the question.
The cost is explicit and accepted: a secret shorter than eight characters is no
longer redacted from logs or model-visible content. Substitution cannot hide a
value that short — an observer who can read the surrounding text can enumerate
it. Two tests that pinned the old tier are rewritten to pin this, rather than
deleted, so the trade stays visible.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vercel

vercelBot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedAug 11, 2026 7:14pm

Request Review

@cursor

cursorBot commented Aug 11, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes core secret redaction across executor, logs, traces, copilot, and providers; mis-substitution risk drops but secrets under 8 characters may leak in display paths.

Overview
Stops substituting secret literals under 8 characters in logs, model egress, and copilot projection. The old split—full substring match for long values, word-boundary-only for short ones—is gone, along with ResolvedSecretMatchPolicy, boundary helpers, and matcher mode: 'detect' | 'render'.

MIN_SUBSTITUTABLE_LITERAL_LENGTH (8) and isNonIdentifyingSecretLiteral (length-only, replacing true/false/null blocklists) now decide whether a literal enters matchers at all. Trace projection and model egress matchers no longer pass mode: 'render'.

Accepted trade: PINs, single-character secrets, and values like false stay plaintext in projected surfaces instead of causing false rewrites (e.g. shard indices, boolean columns). Tests were updated with longer fixtures and expectations that document the new behavior.

Reviewed by Cursor Bugbot for commit eeb1001. Configure here.

@greptile-apps

greptile-appsBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR applies the eight-character minimum to all resolved-secret substitutions and removes the obsolete boundary-match tier.

  • Replaces the short-literal exception set with one minimum-length predicate.
  • Simplifies matcher construction and removes detect/render modes and word-boundary machinery.
  • Updates fixtures to remain above the substitution floor.
  • Corrects the post-projection byte-limit test so it reaches and isolates the final JSON wire-size check.

Confidence Score: 5/5

The PR appears safe to merge.

The previously reported byte-limit test gap is fixed: the registry now records the cataloged literal, projection succeeds within the decoded-content budget at 20 bytes, and only the final 25-byte JSON wire-size check rejects at that limit, so no blocking failure remains.

Important Files Changed

FilenameOverview
apps/sim/executor/utils/resolved-secret-match-policy.tsConsolidates substitution eligibility into a single eight-character minimum, matching the explicitly documented tradeoff.
apps/sim/executor/utils/resolved-secret-matcher.tsRemoves boundary-policy and matcher-mode handling while filtering short literals during matcher construction.
apps/sim/executor/utils/resolved-secret-content-projection.test.tsThe prior fixture mismatch is fixed, and limits 20 and 25 now distinguish content-walk accounting from final serialized JSON size enforcement.
apps/sim/executor/utils/resolved-secret-content-projection.tsAdapts model projection matcher creation to the unified substitution policy.
apps/sim/lib/logs/execution/trace-secret-projection.tsAligns trace projection with the unified matcher behavior without introducing an unresolved failure.

Reviews (3): Last reviewed commit: "test(provenance): make the byte-limit te..." | Re-trigger Greptile

Comment threadapps/sim/executor/utils/resolved-secret-content-projection.test.ts Outdated
Comment threadapps/sim/executor/utils/resolved-secret-content-projection.test.ts Outdated
…d out
Greptile caught the byte-limit test: lengthening the catalog fixture without the
`recordResolved` value beside it left them mismatched, so the registry latched and
`projectResolvedSecretModelJsonContent` returned `{ safe: false }` at its
completeness guard — before any alias projection. The test passed while covering
nothing it is named for. It now records the same literal it catalogs, sizes the
limit between the raw bytes and the projected bytes, and asserts both directions
so a limit applied to the wrong side fails it.
Auditing every changed test for the same shape — a `recordResolved` value that
does not match its own catalog entry — found one more, pre-dating this branch: the
legacy-memory test drew its teeth from substituting the one-character secret `x`
inside `Box`, which the floor no longer substitutes, so it too had become vacuous.
Its fixture is now a full-length secret that appears in the message, which is what
makes "not projected" meaningful.
The three remaining mismatches are deliberate: those tests are about a resolution
that fails to verify.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cursor caught the third instance of the same shape: the fixture `TOK` is three
characters, so the floor drops it and the matcher is empty. The assertion was
`'Bearer {{TOKEN}}'` in and out, which an empty matcher satisfies exactly as well
as working label protection — the test could no longer tell them apart.
Use a fixture over the floor whose label still contains its own plaintext, and
project the bare plaintext first. That control fails if the matcher is inert, so
the atomic-label assertion beside it can only pass for the right reason.
Auditing every test on this branch for the shape — all fixtures below the floor —
returned twenty-four, but the rest are sound: most assert that nothing is
substituted, which the floor makes more certain rather than less, and the
remainder run through the resolver's causal path, which never consults a matcher.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

@cursor review

Three limits can reject this value and my previous fix still tripped the wrong
one. `projectResolvedSecretModelJsonContent` checks the raw encoding, then walks
the content against a running budget, then re-encodes the projected object — and
only the last is what the test is for. At 16 the walk charged the key `a` and
then measured the 17-byte alias against the remaining 15, so it failed before the
re-encoding ran; at 25 everything passed. Neither assertion touched the check.
Twenty is the band that isolates it: the walk admits the alias against its
remaining 19, so a rejection there can only come from re-encoding the 25-byte
result. Asserting the content projection succeeds at the same limit pins that,
and deleting the re-encoding check now fails the test rather than leaving it
green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

@cursor review

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit fdfb439. Configure here.

@icecrasher321
icecrasher321 merged commit b879960 into stagingAug 11, 2026
50 of 51 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/causal-projection-beats-value-matching branch August 11, 2026 21:55
Sign up for freeto 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

@icecrasher321