fix(demo): fail-closed gate for submit_safe_tx_signature under demo mode (#775)

[graciangabriel8]

Closes #775

What was wrong

submit_safe_tx_signature is registered with a plain handler(...) (src/index.ts:2215-2229). In makeDemoDispatch (src/index.ts:1138+) it matches none of the three intercepted shapes — it is not sign_/pair_ledger_/prepare_-prefixed, and it is absent from both ALWAYS_GATED_EXPLICIT and CONDITIONALLY_GATED_EXPLICIT — so under isDemoMode() the dispatcher falls through to return realHandler(args).

The real handler (src/modules/safe/actions.ts:287+) then calls kit.proposeTransaction(...) / kit.confirmTransaction(...) — a live HTTP write to the Safe Transaction Service. A pending Safe multisig tx gains a signature, visible to every co-signer and one step closer to execution, while the user believes demo mode means nothing real happens.

Reachability (issue step 1)

The issue asked whether a demo user can reach this at all post-#772. Yes. The tool consumes no device signature — it derives an approved-hash sender signature from the signer address (encodeApprovedHashSignature). Its only precondition is the on-chain read approvedHashes(signer, safeTxHash) != 0, which is satisfied by an approval mined before demo was entered, or produced from the Safe Web UI / by a co-signer entirely outside this server. #772's device-signing gates therefore do not contain it.

What changed

1. src/demo/index.ts — added "submit_safe_tx_signature" to ALWAYS_GATED_EXPLICIT with inline rationale. Always-gated, not conditionally: every conditionally-gated tool except the broadcast tool runs the real handler in live demo mode, and broadcastSimulationDispatch is bound to the send_transaction handle/simulate flow and cannot wrap an STS POST. The tool has no read-only branch — every successful call writes — so gating loses no inspection-only demo UX. Same always-gate shape as finalize_btc_psbt / prepare_btc_multisig_send, different sink class.

2. test/support/sink-reachability.ts — extended PROP_SINKS with proposeTransaction / confirmTransaction (the issue's step 2: an off-chain-state-write sink category), plus a header-doc entry. Their only call sites in src/ are inside submitSafeTxSignature, so this widens the analysis by exactly the one tool security(demo): submit_safe_tx_signature posts to the off-chain Safe Transaction Service under demo mode (off-chain-write sink class, follow-up to #772) #775 is about — verified by grep across src/ (the other matches are interface declarations in sdk.ts and { op: "proposeTransaction" } diagnostic string literals, neither of which is a call site).

Tests

Falsifier — test/775-safe-tx-service-demo-gate.test.ts (new). Drives the exact production dispatch decision via the extracted makeDemoDispatch factory with a spy realHandler, mirroring test/demo-btc-containment.test.ts exactly.

• RED before the fix: the tool matched neither gate list, so dispatch fell through and the spy was called — the "never invokes the real handler" expectations fail.
• GREEN after: always-gated, spy never called, structured VAULTPILOT_DEMO refusal returned.
• Non-vacuity: a positive control with demo OFF asserts the same dispatch does reach the spy, so the GREEN "not called" is real containment, not a dead wire. Also asserts the gate does not over-reach into get_safe_positions / prepare_safe_tx_execute.

test/demo-sink-gating.structural.test.ts — new case pinning both halves together: submit_safe_tx_signature is detected as sink-reaching (RED if the sink names are dropped from PROP_SINKS) and contained (RED if the gate is reverted). The pre-existing "every sink-reaching tool is contained" invariant now guards it mechanically.

test/support/sink-reachability.sts-matcher.test.ts (new) — positive liveness for the two new sink names against a synthetic parsed AST, mirroring the #778 wc-matcher precedent. Needed because bodyHasSink returns on the first sink found and both names are called inside the same function, so real code only ever exercises proposeTransaction; confirmTransaction would otherwise be an unverified matcher. Includes two non-vacuity negative controls (a different method name; the sink names used as plain string literals, which is the real { op: "proposeTransaction" } shape in actions.ts).

Tests were NOT run locally — Node/npm are not installed on the authoring machine, so nothing was executed and no local result is claimed. CI (Build & Test on Node 20 + 22) is the authoritative check.

Existing tests

None weakened, none updated. test/safe-propose.test.ts calls submitSafeTxSignature directly as a module function rather than through the dispatcher, so demo gating does not affect it. test/support/dump-sink-analysis.test.ts asserts sinks.length >= 4; the sink count only grows. test/support/sink-reachability.wc-matcher.test.ts got a header-comment note only (its "every other sink is exercised end-to-end today" claim would otherwise be stale) — no assertion touched.

Blast radius

Small and demo-only. The gate fires only under isDemoMode(); with demo off the dispatch is an unchanged pass-through. The one user-visible behavior change: a demo user calling submit_safe_tx_signature now gets the standard structured always-gated refusal instead of a live STS POST. The PROP_SINKS change is test-support-only and does not ship in src/.

Residual concerns

• Unverified by execution. No local run; correctness rests on matching the existing test files' framework, imports and helpers exactly. CI decides.
• Sink-set widening semantics. analyzeRegisteredTools() now conflates two sink classes (device-sign/broadcast from security(demo): sign_btc_multisig_psbt + finalize_btc_psbt(broadcast) reach a REAL Ledger signature and mainnet broadcast under demo mode #772, off-chain write from security(demo): submit_safe_tx_signature posts to the off-chain Safe Transaction Service under demo mode (off-chain-write sink class, follow-up to #772) #775) behind one sinkReaching boolean. That is what makes the containment invariant cover this tool, but a future off-chain-write sink whose correct containment is not "always-gate" would need the taxonomy split tracked in arch(substrate): hand-maintained fail-open enumerations are this codebase's default guard habit — 4 instances (#757/#764/#772/#771) #776. Flagged, not solved here.
• Only this tool was gated. No systematic audit of other off-chain API writes across the tool surface was done — the widened walker is the mechanism intended to surface any others, and it currently reports none.
• docs/bug-list-and-proposed-fixes.md still lists security(demo): submit_safe_tx_signature posts to the off-chain Safe Transaction Service under demo mode (off-chain-write sink class, follow-up to #772) #775 under open bugs. That file is a dated snapshot ("Generated 2026-07-20"), and prior merged fixes (security: prepare_swap(toAddress=ATTACKER) drains to the LiFi Diamond unstamped — #760 closed on a partial fix; intent check is tautological against a rogue agent #798/security: INTERMEDIATE_CHAIN_BRIDGES relaxes the chainId gate on a FALSE premise — 1885080386571452 is TRON's on-chain id, not NEAR's; #237 also still broken #799/SEC verdict: ARCHITECTURE.md §3.5 Safe execTransaction safeTxOrigin skips block 4 — UNSOUND, requires mitigation #761) left their entries in place, so it was deliberately not edited here.