feat(signing): server-side signature verification via Solana RPC#279
Merged
Conversation
Implements Spec 3 from the design follow-up bundle. Closes the trust gap
in POST /api/tool-signing/:flagId/confirm — today the route accepts
whatever signature the client posts at face value. A compromised browser,
buggy client, or replay attack can submit a syntactically-valid garbage
signature and Torque attribution fires for a non-existent transaction.
Adds a two-tier verifier (T1 + T2) gated by a SIPHER_SIG_VERIFY env flag
mirroring the SENTINEL_MODE rollout posture:
strict (code default) — verify; reject pending + 4xx VALIDATION_FAILED
on failure; 503 + Retry-After on RPC unavail
advisory — verify and log on failure; resolve anyway
(used for soak-testing without UX impact)
off — skip verification entirely (legacy behavior)
The .env.example ships SIPHER_SIG_VERIFY=advisory as the safe first-deploy
default so operators can promote to strict after >= 1 week of observation.
Same posture established by SENTINEL_MODE.
T1 (existence): getSignatureStatuses([sig]) → confirmationStatus must be
'confirmed' or 'finalized' AND no err. 'processed' rejected — too early
for telemetry to trust.
T2 (sender binding): getTransaction(sig) → fee-payer (account index 0)
must match entry.wallet. Defensively reads both versioned (staticAccountKeys)
and legacy (accountKeys) message shapes.
T3 (instruction-match against entry.serializedTx) deferred per spec —
residual surface is documented; T1+T2 catch most malicious scenarios.
VerifyResult is a discriminated union so the confirm route's switch on
`reason` pairs cleanly with the PR #277 assertNever guard (third real
PR exercising the pattern).
Timing budget: 3000ms total (Promise.race with timeout). Real Solana RPC
returns getSignatureStatuses ~50-200ms + getTransaction ~200-800ms.
'confirmed' commitment used (not 'finalized') to keep UX latency bounded.
Tests: +22 (1580 → 1602)
- verify-signature: 10 (happy / not_confirmed via null status, err, processed
status / wallet_mismatch / getTransaction returns null / rpc_error on each
call site / timeout)
- tool-signing route: existing 9 base tests preserved (SIPHER_SIG_VERIFY=off
in beforeEach keeps their semantics intact; the verified flag added to
the 200 response means the one strict-equality assertion was tightened
to include verified=false) + 12 new for strict/advisory/off modes +
pre-verifier guards never call verifier
Spec deviations:
- Test file consolidation — Spec proposed new file packages/agent/tests/
sentinel/verify-signature.test.ts (kept) + tests under tests/routes/
tool-signing.test.ts. Existing file is named tool-signing-ROUTES.test.ts;
merged the new mode tests into it rather than creating a duplicate that
would race on the pending-signing module-level Map.
- Env flag default in code is 'strict' (per spec) but .env.example ships
'advisory' for safer first-deploy rollout (also per spec's Phase 1
recommendation). The two are consistent.
Out of scope (follow-ups noted in spec):
- T3 instruction-match (residual wrong-tx-substitution surface)
- Replay dedupe (same sig → multiple flagIds)
- Helius webhook subscription as RPC-polling alternative
Spec: docs/superpowers/specs/2026-05-15-server-side-sig-verification-design.md
Stacked atop: PR #277 (assertNever — merged), PR #278 (tool_signing_expired —
merged). Composes cleanly with the in-flight Spec 4 (claim Phase 2) and Spec 5
(scheduled-op broadcasts) — both reference this verifier in their compose
sections.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
5 tasks
rz1989s
added a commit
that referenced
this pull request
May 17, 2026
PR #280's recursive typecheck (`pnpm -r --filter='!sipher' run typecheck`) exposed a build-order issue that the previous root-only typecheck masked. The agent package imports from @sipher/sdk (workspace dep) whose types live in `dist/index.d.ts` — without building the SDK first, tsc fails with 'Cannot find module @sipher/sdk' for every consumer. Local typecheck passes because dist/ exists from prior builds; CI does fresh install without building. This has been failing on main since PR #280 merged 2 days ago (last green: 7389a3e on PR #279 merge). Add a Build SDK step between Install and Typecheck. Fixes both the Typecheck and Test steps in one shot (tests also import the runtime @sipher/sdk via dist/index.js).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements Spec 3 from PR #276. Closes the trust gap in
POST /api/tool-signing/:flagId/confirm— today the route accepts whatever signature the client posts at face value. A compromised browser, buggy client, or replay attack can submit syntactically-valid garbage and Torque attribution fires for a non-existent transaction.Three-tier design (T1 + T2 shipped, T3 deferred)
getSignatureStatuses→confirmed/finalizedAND no errgetTransaction→ fee payer (accountKeys[0]) === entry.walletT3'processed'confirmation rejected — too early for telemetry to trust.'confirmed'is used for the commitment level (not'finalized') to keep UX latency bounded.SIPHER_SIG_VERIFY rollout posture
Three modes mirroring the
SENTINEL_MODEprecedent:.env.exampleshipsSIPHER_SIG_VERIFY=advisoryas the safe first-deploy default so operators can promote tostrictafter observation — matching the SENTINEL_MODE rollout pattern.Third PR exercising assertNever
VerifyResultis a discriminated union; the confirm route's switch onresult.reasoncomposes with the PR #277 assertNever guard. Adding a new reason (e.g.'tx_invalid_signature') in the future will surface as a typecheck error at every consumer switch. Spec 1's pattern in continued use across the spec chain.Test plan
tests/sentinel/verify-signature.test.ts)'processed'→ not_confirmedtests/routes/tool-signing-routes.test.ts)cd packages/agent && pnpm exec tsc --noEmitcleanverifiedfield)Spec deviations
Test file consolidation. Spec proposed a fresh
tests/routes/tool-signing.test.ts. Existing repo file istool-signing-routes.test.ts. Created the duplicate first, hit module-level state races onpending-signing.ts's Map, then consolidated into the existing file. AddsSIPHER_SIG_VERIFY=offto beforeEach so the original 9 base tests preserve their semantics; the one strict-equality assertion (toEqual({ status: 'accepted' })) was tightened to include the newverified: falsefield.extractFeePayerhelper. Versioned messages exposestaticAccountKeys; legacy messages useaccountKeys. Real Solana RPC withmaxSupportedTransactionVersion: 0returns versioned. Mocks in tests still use the simpleraccountKeysshape. Helper handles both — and a string-vs-object form of the key for paranoia.Out of scope (per spec)
Stacked atop
This is PR 3-of-5 in the spec implementation chain. Specs 4 (claim Phase 2) and 5 (scheduled-op broadcasts) both reference this verifier in their compose sections — landing this enables their cleaner implementation.
Spec reference
docs/superpowers/specs/2026-05-15-server-side-sig-verification-design.md