test(security): pin the acknowledgedNonProtocolTarget stamp-writer set by identity (#790)

[graciangabriel8]

Closes #790

What changed

One new test file — test/790-stamp-writer-set.test.ts. No production code is touched.

Pre-sign block 4b (src/signing/pre-sign-check.ts, from #786) refuses every stamped transaction to the LiFi Diamond. Its soundness rests on an invariant that, before this PR, lived only in a source comment:

only prepare_custom_call both STAMPS acknowledgedNonProtocolTarget AND can target the LiFi Diamond

Verified true on live main — exactly three writers:

site	enclosing fn	why it is safe for 4b
src/modules/execution/index.ts:1726	prepareCustomCall	the ack-gated arbitrary-call path — exactly what 4b aims at
src/modules/curve/actions.ts:251	buildCurveAddLiquidity	destination validated against the stable_ng factory (get_n_coins > 0) → only ever a Curve pool
src/modules/curve/actions.ts:469	buildCurveSwap	same factory anchor

If a future stamp writer is added that legitimately targets the Diamond, 4b silently over-blocks it (a false refuse — availability, not a drain, as #790 states), and nothing mechanical forces anyone to notice.

The new test walks src/ via the TypeScript AST, collects every site that sets the flag (object-literal property, shorthand property, and = / ??= / ||= / &&= assignment to a property/element access named the flag), and pins the discovered set by identity — file :: enclosingFunction = value. Line numbers are deliberately not part of the pinned key, so an unrelated edit above a site cannot turn the guard red. The failure message stops the editor and asks block 4b's actual question ("can this new writer target the LiFi Diamond?"), and states that the answer, if yes, is a narrower discriminator in 4b — never a widened ack.

Relation to the existing #757 U1 guard — nothing was weakened

test/757-recipient-authorization.test.ts:511 already asserts the writer count is 3, using a line-oriented regex. That test is untouched. This PR adds a separate file rather than folding into it, because the delta #790 asks for is not the count:

• identity, not count — a count-preserving relocation (one writer deleted, a different one added elsewhere) keeps the INCIDENT: pre-sign block 5 checks selector but never arguments — recipient-bearing fns on RECOGNIZED destinations are signable drain paths (Aave/Morpho/Uniswap/Lido) #757 check green and goes RED here;
• AST, not lines — a // comment or a string literal that looks like an assignment can neither false-positive nor mask a real site (the regex has no such awareness);
• it names 4b — INCIDENT: pre-sign block 5 checks selector but never arguments — recipient-bearing fns on RECOGNIZED destinations are signable drain paths (Aave/Morpho/Uniswap/Lido) #757's guard is about provenance (D3); its message points nowhere near the LiFi over-block decision this issue is about.

No existing assertion was changed, relaxed, or removed.

Falsifier

The pin test (src/ contains EXACTLY the pinned stamp writers) is the falsifier for the drift #790 describes: add a fourth acknowledgedNonProtocolTarget = true anywhere under src/, or move one of the three into a different function, and it goes RED with the discovered set printed. On unmodified main the guard does not exist at all, so the drift it catches is currently unguarded (the #757 count check catches only the strict-addition subset).

Three supporting tests keep it from passing vacuously or rotting:

1. non-vacuity anchor — asserts the src/ walk yields > 250 .ts files (≈318 today) and that at least one file still mentions the flag, so a broken walk or a broken pre-filter cannot silently pin an empty set (mirrors demo-sink-gating.structural.test.ts's > 150-tool anchor);
2. detector liveness — runs the collector over a synthetic module exercising every supported write form (object literal, line-wrapped literal, x.flag =, x["flag"] =, arrow-function body) plus the shapes that must NOT count (a === true read, an interface field, a comment), and asserts the exact five detected writers. If the AST matching silently stops firing, this goes RED before the pin can pass emptily;
3. two-way 4b anchor — asserts src/signing/pre-sign-check.ts still contains the dest.kind === "lifi-diamond" && tx.acknowledgedNonProtocolTarget === true refusal, so a rework or removal of 4b surfaces here rather than leaving a guard whose rationale has quietly changed.

Tests were NOT run locally

There is no Node/npm on the authoring machine, so npm test, npm run build and npm run lint were not executed here, and no local result is claimed. CI (Build & Test on Node 20 + 22) is the authoritative check. The file was written to match the repo's proven patterns: the src/ walk and new URL("../src/", import.meta.url).pathname resolution copy test/757-recipient-authorization.test.ts; the import ts from "typescript" + ts.createSourceFile(…, /*setParentNodes*/ true) usage copies test/support/sink-reachability.ts. tsconfig.json excludes test/, so this file is not part of npm run build's typecheck and npm run lint only covers src.

Blast radius

Test-only. No runtime, signing, or tool-surface behaviour changes; nothing imports the new file. Worst case if it is wrong is a red CI check on this PR, not a production defect. Runtime cost is bounded: all src/ files are read, but only the ~7 that literally contain the identifier are parsed.

Residual concerns

• Detection bounds (documented in the file header): writes are matched syntactically. A stamp performed reflectively — Object.assign, a computed key held in a variable, or a spread of an object built elsewhere — is not detected. No such form exists in src/ today; the liveness fixture pins the forms that are covered.
• The pin is a tripwire, not a proof. It forces a human to answer 4b's question; it cannot itself decide whether a new writer can reach the Diamond. The Curve builders' safety still rests on the factory-validation argument, which is asserted in prose here and enforced by their own code, not by this test.
• The 4b anchor is a source-text regex. Reformatting that condition across lines would require updating the anchor; the failure message says so explicitly.
• #757 U1 and this guard now overlap on the strict-addition case. Deliberate — I did not touch another issue's guard to de-duplicate. If maintainers prefer one binding, the INCIDENT: pre-sign block 5 checks selector but never arguments — recipient-bearing fns on RECOGNIZED destinations are signable drain paths (Aave/Morpho/Uniswap/Lido) #757 count assertion is the one this subsumes.

Security review

This touches fund-safety / signing-gate territory (block 4b is the #786 drain fix). This is NOT self-certified as correct or complete — I am not asserting that the pinned set or the 4b invariant is sound, only that the set matches live main today and is now mechanically bound. It needs maintainer security review, including whether the two Curve writers' factory-validation anchor is the right basis for excluding them from 4b's concern.

---

🤖 Generated with Claude Code