improvement(provenance): name the block behind an unprojected input root - #6890
Conversation
structural-input-root-unprojected fires when a block's config.params throws on the projected inputs — the copy where a secret has been replaced by its placeholder — and no structured projection recovers it. It was reported with the reason and nothing else, so a line told you this had happened somewhere without naming the block, and the caught error was discarded by a bare catch. markIncomplete now takes a structural detail, and this guard passes the block type, tool, input path, and failure class. Names and types only. A coercion that rejects a value tends to quote it, and an input reaching this guard may still hold a resolved secret. Which is also why the json-parse warning a few lines above no longer logs the thrown message: V8 quotes the text it rejected back into it — Unexpected token 's', "sk-live-EX"... is not valid JSON — and that prefix is enough to leak. The field name and its declared type are already in the message, and SyntaxError is the only class JSON.parse throws.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
Also stops a related leak in the same handler: JSON/array parse warnings now log Reviewed by Cursor Bugbot for commit 672d441. Configure here. |
Greptile SummaryThe PR makes provenance incompleteness diagnostics actionable while avoiding resolved-secret disclosure.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/executor/handlers/generic/generic-handler.ts | Adds secret-safe failure attribution to structural projection guards and removes parsed input text from warnings. |
| apps/sim/executor/utils/resolved-secret-trace-registry.ts | Introduces a closed structural-detail payload and preserves canonical incompleteness fields when logging. |
| apps/sim/executor/utils/resolved-secret-trace-registry.test.ts | Verifies detail propagation and protection of canonical diagnostic fields. |
Reviews (2): Last reviewed commit: "fix(provenance): keep a detail from disp..." | Re-trigger Greptile
The detail merged into the incompleteness payload could shadow `reason`.
Spreading it first at the call site protected only the fields added
there; `reason` is added a level up in reportIncompleteness, which
built `{ reason, ...details }`, so a detail carrying that key replaced
the guard literal on the line while the level was still selected from
the real one. `origin` was reachable the same way whenever no importer
origin was set.
Write `reason` last, which protects every caller of that reporter
rather than the one that prompted this, and close the detail to named
fields so neither key is expressible without a cast.
|
@cursor review |
There was a problem hiding this comment.
✅ 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 672d441. Configure here.
Summary
structural-input-root-unprojectedfires when a block'stools.config.paramsthrows on the projected inputs — the copy where a secret has been replaced by its placeholder — and no structured projection recovers it. It was reported with the reason and nothing else, and the caught error was dropped by a barecatch, so the line said this had happened somewhere without naming the block. In prod it has been firing at a baseline of ~1–16/day for over a week with no way to act on it.markIncompletenow takes a structuraldetail, and this guard passes the block type, tool id, input path, and failure class. Its sibling guard (structural-input-projection-incomplete) gets the same treatment.toError(error).message, and V8 quotes the text it rejected back into that message —Unexpected token 's', "sk-live-EX"... is not valid JSON. That value is a resolved input, so the prefix could be a secret. It now logs the failure class; the field name and its declared type were already in the message, andSyntaxErroris the only classJSON.parsethrows.Type of Change
Testing
Tested manually.
bun run lint, all 24 CI audits, andcheck-block-registrypass; type-check clean. 2,387 tests pass across the executor and provenance suites, including two new cases pinning that a supplied detail reaches the line and that it cannot shadow the canonical fields.Note for review: the json-parse leak fix is not covered by a test. The handler builds its logger at import time and the shared
@sim/loggermock returns a fresh instance per call, so the instance the module holds is unreachable from an assertion without either changing the shared mock for every test in the repo or using theresetModules+ dynamic-import pattern the testing rules prohibit. The change itself is one field.Checklist