Skip to content

fix(provenance): record why a resolved-secret registry became incomplete - #6478

Merged
waleedlatif1 merged 1 commit into
stagingfrom
audit/observability-gaps
Aug 10, 2026
Merged

fix(provenance): record why a resolved-secret registry became incomplete#6478
waleedlatif1 merged 1 commit into
stagingfrom
audit/observability-gaps

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Why

ResolvedSecretTraceRegistry fails closed: when it can no longer vouch for what it projects, it marks itself incomplete. That state is one-wayisPermanentlyIncomplete() never resets — so a single trip fails every later model projection in the run, and the user gets one opaque sentence (... model input could not be safely projected) with no recourse.

Many separate guards can set that state, and none of them recorded which one fired. resolved-secret-trace-registry.ts imported no logger at all, so after the fact there was no way to tell a genuine containment from a matcher that simply could not decide.

What this changes

No behaviour change. Each guard now names itself with a static reason literal:

Reason Guard
untrusted-provenance importProvenance rejects an untrusted/malformed bundle
source-provenance-incomplete upstream bundle already incomplete
entry-decrypt-failed was a bare catch {} that discarded the cause
unverified-resolved-entry resolved value does not verify against the catalog
projection-mismatch same raw value projected two different ways
unresolved-placeholder projected marker with no matching entry
provenance-capacity-exceeded entry-count or serialized-size cap hit
restored-checkpoint-unavailable required restore checkpoint missing
tool-call-scope-mismatch merged tool-call registry has a different scope
constructed-incomplete registry built incomplete by construction
inherited-incomplete-* (×2) a fork carrying a parent's existing incompleteness
value-provenance-* (×3) per-value import guards

Log level

Three levels, defaulting to the safe one:

  • error — reasons that mean something went wrong on a path that should have succeeded: projection-mismatch, entry-decrypt-failed, unverified-resolved-entry, unresolved-placeholder, provenance-capacity-exceeded, tool-call-scope-mismatch, untrusted-provenance, and the two value-provenance-* import guards. None is reachable on a healthy run.
  • warn — the default for everything else: an upstream bundle that already declared itself incomplete, a fork inheriting a parent that reported moments earlier, and unspecified from the ~50 call sites outside this file that have not been audited yet.
  • silentconstructed-incomplete. createIncompleteResolvedSecretTraceRegistry states outright that no trusted catalog was available; it carries nothing actionable.

Defaulting to warn is deliberate. Incompleteness is the designed state wherever there is no catalog to vouch with, and those paths are hot — background/webhook-execution.ts:586 builds an incomplete registry on every webhook execution before replacing it with the real one. Logging that at error would have put a per-webhook error line on a completely healthy path. A reason added later without thought now stays quiet rather than paging someone.

Error still matters for reach: getMinLogLevel() falls back to ERROR under NODE_ENV=production and under test, and helm/sim/templates/deployment-app.yaml sets no LOG_LEVEL at all, so on a default self-hosted chart only error survives. The reasons that indicate a real fault — including projection-mismatch — are the ones that get it.

Decrypt failures are summarised once per import rather than once per entry: one rotated key fails every entry, and a bundle may carry up to MAX_PROVENANCE_ENTRIES of them.

No secret material is logged

Reasons are static literals, the logged inputPath is block/field names, and the remaining fields are cardinalities. The decrypt path records getErrorMessage(error) and whether the entry was named — never the value, the ciphertext, or the entry name. There is a test asserting the secret value and name are absent from the log record.

The failure mode this was built to identify

Driving the real classes reproduces the latch:

const reg = await createResolvedSecretTraceRegistry({ /* SHORT_FLAG = 'true' */ })
reg.recordResolvedAtInputPath('SHORT_FLAG', 'true', ['flag'])
reg.recordResolvedInputProjection(['flag'], 'true', '{{SHORT_FLAG}}')
reg.recordResolvedAtInputPath('SHORT_FLAG', 'true', ['context'])
reg.recordResolvedInputProjection(['context'], 'true', 'true')  // literal, from third-party text
reg.recordTransformedInputProjection(
  { flag: 'true', context: 'true' },
  { flag: '{{SHORT_FLAG}}', context: '{{SHORT_FLAG}}' },
  { targetPaths: [['flag'], ['context']] }
)
// isPermanentlyIncomplete() === true

A secret whose plaintext is very short (a flag, an emoji, a two-letter code) binds to an input path where those same characters occur only incidentally. The resolver records that path's projection as the literal; the handler's transform records it as a placeholder. Same raw value, different projected value, so recordState marks the path incomplete — and incompleteness is one-way, so the rest of the run's model projections fail.

recordResolvedInputProjection is worth noting: it overwrites rawValue/projectedValue without the mismatch guard, so only the handler's later recordState can observe the disagreement.

The behavioural fix is intentionally not in this PR. #6416 solved this class for the logging path with MIN_UNANCHORED_MATCH_LENGTH + word-boundary anchoring, and the same policy plausibly belongs where short values bind to input paths. But resolved-secret-matcher.ts documents its wider detection set as deliberate — "a narrow policy can never talk one of them out of failing closed" — so narrowing the wrong guard would convert a fail-closed into a silent leak. That change needs the author's intent and evidence of which guard actually fires, which is exactly what this PR produces.

Type of Change

  • Bug fix (diagnosability; no behaviour change)

Testing

Registry suite 66/66. Nine new tests cover the error level for genuine faults, the warn default for propagated/unaudited reasons, silence for the by-design construction, the named reason replacing unspecified, the split between a scope mismatch and an already-incomplete merged child, single-report attribution for an arriving-incomplete bundle, once-per-import decrypt summarisation, and the absence of secret material in the record. Each was verified to fail when its specific change is reverted.

Wider run across executor, lib, background, app/api, providers, tools: 15,486 passing, 0 test failures. Typecheck and biome clean. (83 suites fail to collect in my worktree from a local tailwind v4/v3 resolution artifact — byte-identical list on a clean tree with this change stashed, so unrelated to this PR.)

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 10, 2026 1:04am

Request Review

@cursor

cursor Bot commented Aug 10, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Observability-only: incompleteness semantics and secret containment are unchanged; logs use static reasons and field paths, not resolved values.

Overview
Adds structured logging when ResolvedSecretTraceRegistry marks itself or an input path incomplete, without changing fail-closed behavior.

Each guard that trips incompleteness now passes a static reason (e.g. projection-mismatch, unverified-resolved-entry, entry-decrypt-failed) into markIncomplete / markInputPathIncomplete. Reasons classified as originating faults log at error; inherited or upstream-incomplete cases log at warn; constructed-incomplete from createIncompleteResolvedSecretTraceRegistry stays silent on hot paths.

Staged temporary registries ({ staged: true } during per-value provenance filtering) skip registry-level summary logs so the caller reports with input-path context. Decrypt failures during importProvenance emit one aggregated error per import (counts + first error message), and matcher build failures log explicitly. mergeToolCallRegistry now distinguishes tool-call-scope-mismatch from inheriting an already-incomplete child.

New tests mock the logger and cover log levels, reason attribution, decrypt summarisation, and absence of secret material in log payloads.

Reviewed by Cursor Bugbot for commit 8e94bf8. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds structured diagnostics explaining why a resolved-secret trace registry becomes incomplete without changing its fail-closed behavior.

  • Introduces typed incompleteness reasons with error, warning, and silent reporting policies.
  • Attributes registry-wide and input-path failures to their originating guards.
  • Summarizes provenance decryption failures and suppresses redundant staged-registry diagnostics.
  • Adds focused tests for attribution, log levels, cardinality, and exclusion of secret values and names.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/executor/utils/resolved-secret-trace-registry.ts Adds reason-aware incompleteness diagnostics, staged-log suppression, and aggregate decryption-failure reporting while preserving registry state transitions.
apps/sim/executor/utils/resolved-secret-trace-registry.test.ts Adds coverage for reason attribution, severity selection, staged suppression, import-level summarization, and avoidance of logging secret values or names.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Provenance or projection guard] --> B{Registry staged or incompleteness by design?}
  B -->|Yes| C[Update incomplete state without summary log]
  B -->|No| D{Originating fault reason?}
  D -->|Yes| E[Log error with static reason and counts]
  D -->|No| F[Log warning with static reason and counts]
  C --> G[Fail closed for later projection]
  E --> G
  F --> G
  H[Entry decryption failures] --> I[Aggregate failure count and first error]
  I --> J[Emit one detailed import summary]
Loading

Reviews (7): Last reviewed commit: "fix(provenance): record why a resolved-s..." | Re-trigger Greptile

Comment thread apps/sim/executor/utils/resolved-secret-trace-registry.ts
Comment thread apps/sim/executor/utils/resolved-secret-trace-registry.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/executor/utils/resolved-secret-trace-registry.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/executor/utils/resolved-secret-trace-registry.ts
Comment thread apps/sim/executor/utils/resolved-secret-trace-registry.ts Outdated
@simstudioai simstudioai deleted a comment from cursor Bot Aug 10, 2026
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/executor/utils/resolved-secret-trace-registry.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@simstudioai simstudioai deleted a comment from cursor Bot Aug 10, 2026
Comment thread apps/sim/executor/utils/resolved-secret-trace-registry.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot 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 fbc6521. Configure here.

Incompleteness is one-way: once any guard trips, every later model projection in
the run fails and the user is left with a single opaque sentence. Every guard
could set it and none recorded which, in a file that imported no logger at all,
so the cause could not be recovered after the fact.

Name each guard with a static reason literal. Originating causes log at error
because they permanently fail the run and error is the only level that survives
every default the logger falls back to; reasons that merely carry an upstream
fault forward log at warn so one fault does not read as several. The decrypt
catch no longer discards its cause.

No behaviour change. Reasons are static literals and the logged input path is
block/field names; no resolved value is recorded.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot 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 8e94bf8. Configure here.

@waleedlatif1
waleedlatif1 merged commit a554430 into staging Aug 10, 2026
32 checks passed
@waleedlatif1
waleedlatif1 deleted the audit/observability-gaps branch August 10, 2026 01:28
waleedlatif1 added a commit that referenced this pull request Aug 10, 2026
… refusal

A refusal fails closed and reaches the user as one fixed sentence. The guard
that caused it may have tripped many frames — or a whole process — earlier, and
the incompleteness latch is one-way, so by then the causing call has long
returned. #6478 recorded the reason when the guard tripped, but marking
early-returns once a registry is already incomplete, so a run that inherits an
incomplete registry refused with nothing recorded anywhere. That is the case
production actually hits.

Retain reasons on the registry and report them where the refusal happens. The
reason is recorded before the already-incomplete return so a causal chain
accumulates, and before the silence checks so a by-design origin that logs
nothing when marked is still nameable at refusal. Propagation inherits through
markIncomplete's source argument, and copying incomplete input paths carries
their reasons, so a fork cannot latch without its cause.

Route all 68 refusal sites through one choke point that logs boundary, cause,
input path and workspace before throwing. It returns never, so callers still
narrow; messages and thrown types are unchanged at every site. Records carry a
cause discriminator, since a latched registry and a caller-side cross-check of
the projection's own output both arrive here and only the former has reasons.

No behaviour change.
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