feat: keep provenance across computed keys resolved at render time - #61
Conversation
`staticMemberInfo` abandoned a member chain the moment it met a computed key that was not a literal, and `wrapCapturedMember` returned false with it. `row[columnId]`, `data[key].status`, and `items[index].name` are ordinary table and list code, so the whole access path was dropped and a primitive read was left with no provenance at all — the same gap the previous commit closed for destructuring, reached a different way. Keeps those segments instead. A dynamic key is hoisted into the existing IIFE alongside the root, so it is evaluated exactly once and in the original inner-to-outer order; the member read and the access path then consume the same parameter and cannot disagree. The path is built as a concatenation only when a key forces it, so a fully static chain emits the identical string literal it did before. Also fixes a reporting bug found while testing this. #registerOrigin joined every non-array key with a dot, so a key that is not a valid identifier produced a path that is not the accessor it claims to be: `rows.row-7.status` reads as a subtraction. formatAccessPath already bracketed those; both now share one identifier pattern. Runtime resolution is unchanged — the emitted path is the grammar parseAccessPath already accepts. Co-Authored-By: Claude Opus 5 <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83257c2fd0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| t.callExpression( | ||
| t.memberExpression(t.identifier("JSON"), t.identifier("stringify")), | ||
| [t.callExpression(t.identifier("String"), [t.cloneNode(key)])], | ||
| ), |
There was a problem hiding this comment.
When a computed key contains secret material—for example, a traced prop or network object read as records[token]—this serializes the raw token into accessPath. The runtime then copies it into ValueOrigin.path, while both display redaction (packages/runtime-core/src/index.ts:1685-1693) and export redaction (packages/runtime-core/src/index.ts:770-775) leave that field untouched, exposing the secret in the inspector, Markdown, and exported traces even when the captured value itself is redacted. Sanitize sensitive dynamic keys before adding them to provenance paths.
AGENTS.md reference: AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| t.callExpression(t.cloneNode(captureIdentifier), captureArguments), | ||
| ), | ||
| [t.cloneNode(member.root)], | ||
| [t.cloneNode(chain.root), ...keyArguments], |
There was a problem hiding this comment.
For a newly supported optional access such as row?.[nextKey()], placing every dynamic key in the IIFE argument list invokes nextKey() before _rowOrigin?.[...]; the original expression skips that call when row is nullish. Multi-segment reads likewise evaluate outer keys before intermediate property reads, rather than interleaving key evaluation and property access, so getters or side-effecting keys can throw, mutate state, or produce a different rendered value only when instrumentation is enabled.
Useful? React with 👍 / 👎.
| t.stringLiteral("["), | ||
| t.callExpression( | ||
| t.memberExpression(t.identifier("JSON"), t.identifier("stringify")), | ||
| [t.callExpression(t.identifier("String"), [t.cloneNode(key)])], |
There was a problem hiding this comment.
When the dynamic key is a Symbol, JavaScript reads the symbol-keyed property, but String(key) turns it into text such as "Symbol(status)". The runtime can then append that string accessor to the registered root origin and report it as confirmed, describing a string property that was never read. Either represent symbol keys losslessly or omit their access paths as unavailable rather than manufacturing a causal chain.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| t.binaryExpression("===", t.unaryExpression("typeof", t.cloneNode(key)), t.stringLiteral("number")), | ||
| concat(concat(t.stringLiteral("["), t.cloneNode(key)), t.stringLiteral("]")), |
There was a problem hiding this comment.
For valid object accesses such as record[-1], record[1.5], record[NaN], or record[Infinity], this number branch emits [-1], [1.5], [NaN], or [Infinity]. However, parseAccessPath in packages/runtime-core/src/index.ts:73 accepts only unsigned decimal digits in unquoted brackets, so it rejects each path and the newly supported read silently loses its confirmed origin. Encode non-index numbers as quoted property strings or extend the parser consistently.
Useful? React with 👍 / 👎.
| t.stringLiteral("["), | ||
| t.callExpression( | ||
| t.memberExpression(t.identifier("JSON"), t.identifier("stringify")), | ||
| [t.callExpression(t.identifier("String"), [t.cloneNode(key)])], |
There was a problem hiding this comment.
A computed key can be user-controlled and arbitrarily large, but this materializes its complete string on every render and the runtime subsequently retains it in accessPath and ValueOrigin.path. Existing event and node budgets do not cap the size of an individual path, so a multi-megabyte lookup key can block rendering and inflate inspections, Markdown, and exported traces. Enforce a path-length limit and report provenance as unavailable instead of retaining an unbounded key.
AGENTS.md reference: AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| t.memberExpression(t.identifier("JSON"), t.identifier("stringify")), | ||
| [t.callExpression(t.identifier("String"), [t.cloneNode(key)])], |
There was a problem hiding this comment.
The inserted JSON and String identifiers resolve in the application's lexical scope. If a component has a parameter, import, or local binding named either one, an otherwise valid row[key] read is transformed into a call to that user value's stringify or call signature, which can throw or create an incorrect path only when CauseScope instrumentation is active. Generate collision-proof helper bindings or reference the intended built-ins without using shadowable application names.
Useful? React with 👍 / 👎.
1 participant
Follow-up to #60. Same gap in the evidence chain, reached a different way.
The gap
staticMemberInfoabandoned a member chain the moment it hit a computed key that was not a literal, andwrapCapturedMemberreturnedfalsewith it. So these emitted no hint at all:The value is a primitive, primitives have no identity, and without an access path there was nothing to recover provenance from. Exactly the failure #60 fixed for destructuring.
Change
Dynamic segments are kept instead of abandoning the chain. Each dynamic key is hoisted into the existing IIFE next to the root:
row[nextKey()].matrix[r][c].The emitted string is the grammar
parseAccessPathalready accepts, so runtime resolution needed no change.Also fixed
Found while writing the end-to-end test, and unrelated to the feature:
#registerOriginjoined every non-array key with a dot, so a key that is not a valid identifier produced a path that is not the accessor it claims to be —formatAccessPathalready bracketed these correctly; the two now share one identifier pattern. This affects paths shown in the inspector and in exported traces, which are meant to be copy-pasteable.Verification
confirmedwith the full path, including one case past the 100-property registration budget.pnpm typecheck,pnpm test: 24/24 tasks.verify:performance: budgets pass (Vite plugin graph 47.7 → 50.5 KiB).verify:public-api: 5 entrypoints, snapshot unchanged.Closing note on the third item
The earlier list also flagged bounded recording (depth 5, 100 properties) as possibly indistinguishable from "no evidence exists". That turned out not to be a real problem. A probe against the runtime showed
#originsAlongAccessPathfalls back to the nearest registered ancestor and appends the remaining path, so a value 8 levels deep or at array index 150 still resolves to aconfirmedorigin with the full path. The budget limits which objects are indexed, not whether the path can be reconstructed. Nothing to fix there.🤖 Generated with Claude Code