feat(ts): enforce ConsequentBundlePinned via targetProofCid lookup#54
Conversation
Closes the shim-poisoning vector promoted to a normative example in PR #10 (`protocol/specs/2026-04-30-ir-formal-grammar.md`, section "Bridge target pinning: the shim-poisoning vector"). Mirrors the Rust enforcement landed in PR #13. PR #17 already wired the `targetProofCid` field through TS parse/emit/canonicalize; this PR makes the verifier actually reject when the pin doesn't match. The vector this closes: a bridge carries two outbound CIDs into the target side (`targetContractCid`, `targetProofCid`). Without enforcing the second pin, the verifier accepts any `.proof` bundle that happens to contain a contract member matching `targetContractCid`. An attacker can mint a poisoned bundle whose contract member is byte-equal to the bridge's pin; the discharge looks fine, the guarantee is broken. Behavior: - `MementoPool.bundleMembers: Record<bundleCid, memberCid[]>` tracks forward direction. Multi-valued because the same member CID can legitimately appear in two bundles (one honest, one poisoned); last-writer-wins on an inverse map would silently swap them. - `loadAllProofs` populates this from the `.proof` file's content hash on every member it stores. - `BridgeCallSite.bridgeTargetProofCid?: string` carries the pin out of `enumerateBridgeCallsites`. Empty string is treated as absent (mirrors Rust's `filter(|s| !s.is_empty())`). - `resolveBridgeTarget` gates resolution: when the pin is set, the resolved contract member's bundle CID must be in `bundleMembers[expected]`. Mismatch returns `failureReason: "bundle-mismatch"` with `failureMessage` carrying the magic substring `BridgeTargetProofCidMismatch` so cross-impl conformance tests can pattern-match Rust's reject shape. - Missing field (`undefined`) takes the back-compat path: emits a `console.warn` and accepts. New bridges from the current parser must set the field; the warning makes the gap visible. Cache safety: - `resolveBridgeTarget` producer version bumped `@v3 -> @v4` and `serializeInput` now includes `bridgeTargetProofCid` plus a sorted hash of the relevant `bundleMembers` slice. A v3 cache hit cannot silently bypass the new check. - `enumerateBridgeCallsites` producer version bumped `@v3 -> @v4` because the call-site shape changed. Test coverage added (4 cases mirroring Rust PR #13's `tests/resolve_target.rs`): - (a) `rejects when targetProofCid does not match the bundle the contract was loaded from` -- spec scenario A, the shim-poisoning case. Member resolves but its bundle CID differs from the pin. - (b) `rejects when the pinned bundle is not in the pool at all` -- pin references a bundle not loaded. Fail-closed. - (c) `accepts when the contract member is in the pinned bundle` -- spec scenario B. Pin matches. - (d) `accepts a legacy bridge with no targetProofCid and emits a soft warning` -- back-compat path. Sanity check: with the resolveBridgeTarget gate stubbed out, the two reject tests fail and the two accept tests still pass (confirmed by temporarily commenting the gate, running the suite, then restoring). The gate is load-bearing. Test plan: - 746 vitest tests pass (was 742 on origin/main). - Sanity check: gate-removed run reproduces 2 failures. - All existing `enumerateBridgeCallsites` and bridgeEnforcement integration tests still pass with the threading change. Files touched: - implementations/typescript/src/verifier/mementoPool.ts - implementations/typescript/src/verifier/bridgeEnforcement.ts - implementations/typescript/src/claimEnvelope/types.ts - implementations/typescript/src/claimEnvelope/mint.ts - implementations/typescript/src/workflow/producers/loadAllProofs.ts - implementations/typescript/src/workflow/producers/enumerateBridgeCallsites.ts - implementations/typescript/src/workflow/producers/resolveBridgeTarget.ts - implementations/typescript/src/workflow/producers/resolveBridgeTarget.test.ts (new) Related: - Rust analog: PR #13 - Normative spec: PR #10 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 5 minutes and 25 seconds.Comment |
Summary
Closes the shim-poisoning vector promoted to a normative example in #10 (
protocol/specs/2026-04-30-ir-formal-grammar.md, section "Bridge target pinning: the shim-poisoning vector"). The TS verifier now enforcesINVARIANT BridgeDeclaration.ConsequentBundlePinned:This is the cross-impl port of #13 (Rust analog). PR #17 wired the field through TS parse/emit/canonicalize; this PR makes the verifier actually reject when the pin doesn't match.
The shim-poisoning vector this closes
A bridge carries two outbound CIDs into the target side: the antecedent (
targetContractCid) and the consequent (targetProofCid). Without enforcing the second pin, the verifier accepts any.proofbundle that happens to contain a contract member matchingtargetContractCid. An attacker can mint a poisoned bundle whose contract member is byte-equal to the bridge's pin; the discharge looks fine syntactically, the semantic guarantee is broken.This PR adds the one CID-equality check the spec calls "the entire mitigation": after resolving the consequent contract, the verifier checks that the contract is a member of the bundle the bridge pinned, and refuses any other bundle as a substitute.
What changed
MementoPool.bundleMembers: Record<bundleCid, memberCid[]>tracks forward direction. Stored as sorted/deduped arrays per bundle so the cache witness round-trips throughJSON.stringify(Sets do not). Multi-valued because the same member CID can legitimately appear in two bundles (one honest, one poisoned); last-writer-wins on an inverse map would silently swap them.loadAllProofspopulates this from the.prooffile's content hash on every member it stores.BridgeEvidence.body.targetProofCid?: stringplumbed through the runtime envelope shape (claimEnvelope/types.ts) andmintBridgeso producers can emit it.BridgeCallSite.bridgeTargetProofCid?: stringcarries the pin out ofenumerateBridgeCallsites. Empty string treated as absent (mirrors Rust'sfilter(|s| !s.is_empty())).resolveBridgeTargetgates resolution: when the pin is set, the resolved contract member's bundle CID must be inbundleMembers[expected]. Mismatch returnsfailureReason: "bundle-mismatch"withfailureMessagecarrying the magic substringBridgeTargetProofCidMismatchso cross-impl conformance tests pattern-match Rust's reject shape.bridgeEnforcement.tsrunner threadsbridgeTargetProofCidfrom each call site into the resolver call.Behavior on missing field (back-compat)
targetProofCidis REQUIRED by the v1.4 grammar but bridges minted before the field was normative MAY lack it. Mirrors the Rust contract:bridgeTargetProofCid === undefined-> bridge is accepted with a soft warning toconsole.warn(warning: bridge X has no targetProofCid; ConsequentBundlePinned not enforced (back-compat path)). Same shape as Rust'seprintln!.bridgeTargetProofCid === <string>-> ConsequentBundlePinned is enforced fully. Mismatch is fail-closed.New bridges from the current producer MUST set the field; the warning makes the gap visible to operators rather than hiding it.
Cache safety
resolveBridgeTargetproducer version bumped@v3 -> @v4andserializeInputnow includesbridgeTargetProofCidplus a sorted hash of the relevantbundleMembersslice. A v3 cache hit cannot silently bypass the new check.enumerateBridgeCallsitesproducer version bumped@v3 -> @v4because the call-site shape changed.Test coverage added
In
implementations/typescript/src/workflow/producers/resolveBridgeTarget.test.ts(new file). Mirrors Rust PR #13'stests/resolve_target.rs:rejects when targetProofCid does not match the bundle the contract was loaded from-- spec scenario A, the shim-poisoning case. Member resolves but its bundle CID differs from the pin. Must reject.rejects when the pinned bundle is not in the pool at all-- pin references a bundle not loaded. Must reject (fail-closed).accepts when the contract member is in the pinned bundle-- spec scenario B. Pin matches. Must accept.accepts a legacy bridge with no targetProofCid and emits a soft warning-- legacy bridge, no field. Must accept (soft-warning path).Sanity-checked by stubbing the
resolveBridgeTargetgate and confirming the two reject-tests fail while the two accept-tests still pass. The gate is load-bearing.Test plan
vitest runpasses: 746 tests pass (was 742 on origin/main; 4 new tests + same 742)enumerateBridgeCallsitesandbridgeEnforcementintegration tests still green with the threading changegit diff origin/mainis clean)findcommand used; notailto pre-filter command outputFiles touched
implementations/typescript/src/verifier/mementoPool.tsimplementations/typescript/src/verifier/bridgeEnforcement.tsimplementations/typescript/src/claimEnvelope/types.tsimplementations/typescript/src/claimEnvelope/mint.tsimplementations/typescript/src/workflow/producers/loadAllProofs.tsimplementations/typescript/src/workflow/producers/enumerateBridgeCallsites.tsimplementations/typescript/src/workflow/producers/resolveBridgeTarget.tsimplementations/typescript/src/workflow/producers/resolveBridgeTarget.test.ts(new)Related
Pre-existing surface (not introduced here)
tsc --noEmitreports a single new error incircularProof.integration.test.tsbecause myfailureReasonenum gained the"bundle-mismatch"literal and that test was already constructingrowswith weak typing. The same shape error exists onorigin/mainfor the same row (different literal union, same root cause). CI runs vitest, nottsc --noEmit, and all 746 vitest tests pass.Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com