Skip to content

feat(signing): one-shot signing protocol with durable state machine - #140

Merged
knytcomics-ui merged 1 commit into
Gryd-lock:mainfrom
james2177:feat/one-shot-signing-protocol
Aug 30, 2026
Merged

feat(signing): one-shot signing protocol with durable state machine#140
knytcomics-ui merged 1 commit into
Gryd-lock:mainfrom
james2177:feat/one-shot-signing-protocol

Conversation

@james2177

Copy link
Copy Markdown
Contributor

Closes #124

Replaces the best-effort page-message/runtime-callback signing flow with a versioned, durable one-shot protocol. The background service worker now generates the sole authoritative request id (never the page/bridge's own correlation token), persists pending-request identity/binding/deadline to chrome.storage.session before opening any UI, and binds every decision to its originating tab/frame/document and to the popup's own window. Closes #124.

Builds directly on ADR-0003 (#132), which explicitly deferred this: "Browser interception retains its existing request/decision contract."

See docs/adr/0004-one-shot-signing-protocol.md for the full design and the reasoning behind two scope decisions (below).

Non-negotiable invariants

  • A decision is bound to one transaction digest, tab, frame, document, adapter, and review window. xdrDigest is bound into the pending record at awaiting_review; tab/frame/document come from sender at SIGN_REQUEST time; the review window is bound to sender.tab.windowId on first legitimate popup contact and checked on every later message. src/background/background.ts (handleSignRequest, handleDecisionMade, handleGetReview), src/signing/pendingRequestState.ts.
  • Page-controlled identifiers are never authoritative capabilities. localId (the page/bridge correlation token) is generated in requestOutcome.ts, used only by the bridge to match its own response, and never forwarded past bridgeEntry.ts. SIGN_REQUEST's validator rejects a message that even includes a requestId field. The background generates requestId itself (crypto.randomUUID()) and that's the only id used for pending state or the popup URL.
  • The first valid terminal transition wins; replay is a no-op. applyTransition in pendingRequestState.ts treats a repeat of the same terminal state as an idempotent no-op and rejects any different terminal transition once settled. Tombstoned for TOMBSTONE_TTL_MS so a retried AWAIT_OUTCOME or duplicate DECISION_MADE resolves from cache.
  • Unrelated or stale senders cannot resolve a request. handleDecisionMade/handleGetReview check sender.tab.windowId against the bound review window; a mismatch is a silent no-op.
  • Worker restart cannot release a cancelled request or lose a completed decision. Terminal state is persisted to chrome.storage.session the moment it's reached; restorePendingRequests() reloads it on worker start. A tombstoned/terminal record answers AWAIT_OUTCOME immediately from storage regardless of which worker lifetime created it.
  • Popup close, creation failure, bridge loss, and timeout settle deterministically. chrome.windows.onRemoved → cancel. chrome.windows.create's callback (previously ignored — the original code called it with no callback at all) is now used to detect creation failure → failed. Bridge loss/timeout: layered deadlines below.
  • The released XDR is byte-for-byte identical to the reviewed XDR. Structurally unchanged and re-verified: mainWorldEntry.ts/albedoMainWorldEntry.ts never round-trip XDR through the background — they re-post their own captured request object verbatim on proceed. The background only ever returns a proceed/cancel vote, never a (possibly-substituted) XDR string.
  • Pending state, windows, timers, and tombstones are bounded and pruned. MAX_PENDING_PER_FRAME (5) / MAX_PENDING_GLOBAL (50) admission caps reject before a popup opens; TOMBSTONE_TTL_MS (120s) prunes settled records; pruneStalePendingRequests() runs on every touchpoint.

Required scope

A. Versioned protocol — src/intercept/protocol.ts: every runtime message carries protocolVersion; a mismatch gets a typed SIGN_REJECTED { reason: 'protocol-incompatible' }. xdrDigest, networkPassphrase, tab/frame/document identity, createdAt/deadlineAt, and state are all part of the persisted record (PendingRequestRecord).

B. Durable state machine — src/signing/pendingRequestState.ts (pure, mirrors the existing split in src/protection/protectionState.ts): received → validating → assessing → awaiting_review → {proceed | cancel | expired | failed}. Metadata is persisted before the popup opens. The bridge/worker resume handshake is AWAIT_OUTCOME (src/background/background.ts handleAwaitOutcome, src/intercept/awaitOutcome.ts): a two-phase SIGN_REQUEST → SIGN_ACK (closes the port immediately) followed by a retryable long-poll that always answers from durable state, because — as the issue says — callback functions cannot survive suspension.

C. Trusted review capability — the popup URL carries only requestId. See "Scope simplification" below for why this doesn't introduce a second token.

D. Failure and concurrency semantics — deadlines at all three layers (background 90s authoritative, bridge 100s fallback, page 120s fallback — PAGE_DEADLINE_MS/BRIDGE_FALLBACK_DEADLINE_MS/REVIEW_DEADLINE_MS), popup close/creation-failure handlers, tab close/navigation invalidation (extends the existing chrome.tabs.onUpdated/onRemoved listeners), per-frame/global admission limits.

E. Compatibility — protocol-version mismatch → typed failure, never silent. Missing sender.documentId → documentBound: false, falls back to tab/frame-only binding (still checked, not skipped).

Scope simplification: one identifier, not two

The issue's Required Scope C reads as if it wants a capability token separate from the request id. I used one id instead: the background-generated requestId already satisfies "opaque" (UUID), "never page-controlled" (page never sees it — see the invariant above), and is already bound to window/tab/frame/document/digest in the durable record. A second token would track the same state twice for no additional guarantee. Called out explicitly here and in ADR-0004 rather than left implicit — happy to add a distinct token if there's a scenario this doesn't cover.

Also fixes (asides — found while making the above work, not separately requested)

  • InterceptView silently defaulted to a "low risk" tier when review data failed to load. Before this change, if GET_REVIEW returned no review (which durability now makes more likely to happen legitimately — a worker restart can lose in-memory-only review content), the popup fell back to tierForScore(Number(params.get('score') ?? '0')) — i.e. a reassuring default, not a fail-closed one. This directly violates the threat model's own objective 7 ("malformed... transaction semantics should not be silently misrepresented as low risk"). InterceptView now shows an explicit "could not be loaded for review — reject and retry" state with only a Reject action when review data isn't available. src/popup/App.tsx.

  • chrome.windows.create was called without a callback, so a popup-creation failure was unobservable and the request would hang. Now wrapped in a promise with chrome.runtime.lastError handling.

  • DEFAULT_TIMEOUT_MS was defined but never referenced (called out explicitly in the issue's audit). It's now REVIEW_DEADLINE_MS, the real authoritative deadline.

  • Two verbatim-duplicated requestOutcome() implementations (Freighter's mainWorldEntry.ts and Albedo's albedoMainWorldEntry.ts) are now one shared, tested module
    Non-negotiable invariants

  • A decision is bound to one transaction digest, tab, frame, document, adapter, and review window. xdrDigest is bound into the pending record at awaiting_review; tab/frame/document come from sender at SIGN_REQUEST time; the review window is bound to sender.tab.windowId on first legitimate popup contact and checked on every later message. src/background/background.ts (handleSignRequest, handleDecisionMade, handleGetReview), src/signing/pendingRequestState.ts.

  • Page-controlled identifiers are never authoritative capabilities. localId (the page/bridge correlation token) is generated in requestOutcome.ts, used only by the bridge to match its own response, and never forwarded past bridgeEntry.ts. SIGN_REQUEST's validator rejects a message that even includes a requestId field. The background generates requestId itself (crypto.randomUUID()) and that's the only id used for pending state or the popup URL.

  • The first valid terminal transition wins; replay is a no-op. applyTransition in pendingRequestState.ts treats a repeat of the same terminal state as an idempotent no-op and rejects any different terminal transition once settled. Tombstoned for TOMBSTONE_TTL_MS so a retried AWAIT_OUTCOME or duplicate DECISION_MADE resolves from cache.

  • Unrelated or stale senders cannot resolve a request. handleDecisionMade/handleGetReview check sender.tab.windowId against the bound review window; a mismatch is a silent no-op.

  • Worker restart cannot release a cancelled request or lose a completed decision. Terminal state is persisted to chrome.storage.session the moment it's reached; restorePendingRequests() reloads it on worker start. A tombstoned/terminal record answers AWAIT_OUTCOME immediately from storage regardless of which worker lifetime created it.

  • Popup close, creation failure, bridge loss, and timeout settle deterministically. chrome.windows.onRemoved → cancel. chrome.windows.create's callback (previously ignored — the original code called it with no callback at all) is now used to detect creation failure → failed. Bridge loss/timeout: layered deadlines below.

  • The released XDR is byte-for-byte identical to the reviewed XDR. Structurally unchanged and re-verified: mainWorldEntry.ts/albedoMainWorldEntry.ts never round-trip XDR through the background — they re-post their own captured request object verbatim on proceed. The background only ever returns a proceed/cancel vote, never a (possibly-substituted) XDR string.

  • Pending state, windows, timers, and tombstones are bounded and pruned. MAX_PENDING_PER_FRAME (5) / MAX_PENDING_GLOBAL (50) admission caps reject before a popup opens; TOMBSTONE_TTL_MS (120s) prunes settled records; pruneStalePendingRequests() runs on every touchpoint.

Required scope

A. Versioned protocol — src/intercept/protocol.ts: every runtime message carries protocolVersion; a mismatch gets a typed SIGN_REJECTED { reason: 'protocol-incompatible' }. xdrDigest, networkPassphrase, tab/frame/document identity, createdAt/deadlineAt, and state are all part of the persisted record (PendingRequestRecord).

B. Durable state machine — src/signing/pendingRequestState.ts (pure, mirrors the existing split in src/protection/protectionState.ts): received → validating → assessing → awaiting_review → {proceed | cancel | expired | failed}. Metadata is persisted before the popup opens. The bridge/worker resume handshake is AWAIT_OUTCOME (src/background/background.ts handleAwaitOutcome, src/intercept/awaitOutcome.ts): a two-phase SIGN_REQUEST → SIGN_ACK (closes the port immediately) followed by a retryable long-poll that always answers from durable state, because — as the issue says — callback functions cannot survive suspension.

C. Trusted review capability — the popup URL carries only requestId. See "Scope simplification" below for why this doesn't introduce a second token.

  • [~] Flooding 1,000 requests respects per-frame/global bounds and does not open 1,000 windows. The bound itself is real and tested. A literal 1,000-request load test is not part of this PR — see Known Gaps.

Required verification

  • Unit and property-based state-machine tests: src/signing/pendingRequestState.test.ts (49 tests — transitions, terminal stickiness/idempotency, expiry, admission boundaries, adversarial storage-deserialization).
  • Actual Chrome sender integration tests: src/background/background.test.ts (26 tests, simulated chrome.runtime.MessageSender shapes including cross-window/cross-tab).
  • Multi-tab/frame Playwright tests: e2e/signTransaction.spec.ts against real Chromium — includes a new two-concurrent-popup test and the existing child-frame protocol test.
  • Forced worker termination: simulated via seeded chrome.storage.session + fresh module import (not a real process kill) — see Known Gaps.
  • Popup/navigation/update failure injection: covered (popup creation failure, tab navigation, tab close). Extension-update-mid-flight is covered by the protocol-version-mismatch path, not a literal extension-reload test.
  • Adversarial same-page messaging: messageValidation.test.ts's exact-key-set enforcement (a SIGN_REQUEST carrying an extra requestId field is rejected outright, not silently stripped).
  • Load tests: not included — see Known Gaps.
  • Security review: this PR itself is the artifact for that review; I'm not a security team.

Known gaps

  • No load test for the 1,000-concurrent-request / 50-global-admission acceptance criterion.
  • No forced service-worker-termination test via Chrome DevTools Protocol. Worker-restart survival is proven by seeding durable storage and re-importing the module fresh, not by killing a real running worker mid-flight.
  • No cross-browser / non-Chromium documentId-less test. The fallback path is unit-tested; there's no test against a browser that actually lacks the API.
  • Review content is not persisted across a worker restart — only the pending-request identity/binding/deadline record is. A restart mid-review may force a reopened popup into the fail-closed "reject and retry" state rather than redisplaying the review. Deliberate, documented scope cut (ADR-0004) — costs availability, not a safety invariant.
  • No independent/external security review.
  • test:visual currently fails on a pre-existing baseline mismatch unrelated to this PR (confirmed identical on unmodified main). Every other CI step is green on a clean npm ci.

Verifiable evidence

  • npm run test:coverage: 329/329 unit tests pass; 92.25%/83.67%/90.96%/92.25% (threshold 90/80/80/90).
  • npm run test:e2e (real Chromium): 8/8 pass, including 2 new adversarial specs.

Required verification

  • Unit and property-based state-machine tests: src/signing/pendingRequestState.test.ts (49 tests — transitions, terminal stickiness/idempotency, expiry, admission boundaries, adversarial storage-deserialization).
  • Actual Chrome sender integration tests: src/background/background.test.ts (26 tests, simulated chrome.runtime.MessageSender shapes including cross-window/cross-tab).
  • Multi-tab/frame Playwright tests: e2e/signTransaction.spec.ts against real Chromium — includes a new two-concurrent-popup test and the existing child-frame protocol test.
  • Forced worker termination: simulated via seeded chrome.storage.session + fresh module import (not a real process kill) — see Known Gaps.
  • Popup/navigation/update failure injection: covered (popup creation failure, tab navigation, tab close). Extension-update-mid-flight is covered by the protocol-version-mismatch path, not a literal extension-reload test.
  • Adversarial same-page messaging: messageValidation.test.ts's exact-key-set enforcement (a SIGN_REQUEST carrying an extra requestId field is rejected outright, not silently stripped).
  • Load tests: not included — see Known Gaps.
  • Security review: this PR itself is the artifact for that review; I'm not a security team.

Known gaps

  • No load test for the 1,000-concurrent-request / 50-global-admission acceptance criterion.
  • No forced service-worker-termination test via Chrome DevTools Protocol. Worker-restart survival is proven by seeding durable storage and re-importing the module fresh, not by killing a real running worker mid-flight.
  • No cross-browser / non-Chromium documentId-less test. The fallback path is unit-tested; there's no test against a browser that actually lacks the API.
  • Review content is not persisted across a worker restart — only the pending-request identity/binding/deadline record is. A restart mid-review may force a reopened popup into the fail-closed "reject and retry" state rather than redisplaying the review. Deliberate, documented scope cut (ADR-0004) — costs availability, not a safety invariant.
  • No independent/external security review.
  • test:visual currently fails on a pre-existing baseline mismatch unrelated to this PR (confirmed identical on unmodified main). Every other CI step is green on a clean npm ci.

Verifiable evidence

  • npm run test:coverage: 329/329 unit tests pass; 92.25%/83.67%/90.96%/92.25% (threshold 90/80/80/90).
  • npm run test:e2e (real Chromium): 8/8 pass, including 2 new adversarial specs.
  • npm run lint, npm run typecheck, npm run validate:manifest, npm run build: all pass on a clean npm ci.
  • docs/adr/0004-one-shot-signing-protocol.md: full design record.

Tip: Use /btw to ask a quick side question without interrupting Claude's current work(src/intercept/requestOutcome.ts), which is also where the new page-side deadline lives (previously neither had one).

  • Content-script bundle-size budgets raised (scripts/build-extension.mjs): mainWorld.js 5.0→5.5 KB, albedoMainWorld.js 6.0→6.5 KB, bridge.js 4.0→5.5 KB. Confirmed this is real code, not incidental bloat — esbuild strips comments during bundling regardless of the minify setting (verified empirically: grepped the compiled output for JSDoc text, found none), so the growth is the actual resume/retry/versioning logic. Each new budget still has headroom over the measured size.

Acceptance criteria

  • A page observing all window messages cannot forge a proceed decision. The page never sees requestId; decisions require sender.tab.windowId to match the bound popup window, which only the real popup has.
  • Decisions from the dApp tab, options page, unrelated tab, or wrong popup are rejected. background.test.ts: "a decision from the wrong popup window is rejected; the request stays open for the correct one."
  • Copying a review URL reveals no transaction and grants no authority. background.test.ts: "a copied review URL opened in the wrong window reveals no review content." GET_REVIEW from a mismatched window gets { requestId } with no review field.
  • Fifty concurrent requests using one page ID cause no overwritten state. Structurally guaranteed, not just tested: the background never uses any page-supplied id as a map key at all — see the invariant above. admissionCheck unit tests in pendingRequestState.test.ts cover the boundary at MAX_PENDING_PER_FRAME/MAX_PENDING_GLOBAL exactly; background.test.ts proves the wiring rejects the 6th same-frame request without opening a window.
  • Replaying a valid proceed 100 times releases the wallet request once. applyTransition's idempotent-terminal-replay behavior (unit-tested directly) plus background.test.ts's round-trip test, which replays both AWAIT_OUTCOME and DECISION_MADE after settlement and asserts the outcome never changes.
  • Closing the popup settles as cancellation within two seconds. chrome.windows.onRemoved settles immediately (same event-loop tick, not a poll) — verified in both background.test.ts and, against real Chromium, e2e/signTransaction.spec.ts's new "closing the review popup settles the request as cancelled instead of hanging the dApp" test.
  • Popup creation failure settles and clears all state. background.test.ts: "a popup creation failure settles the request as failed (outcome cancel) and clears the badge."
  • Worker termination during review preserves context and permits one terminal decision. background.test.ts: "resumes a pending request restored from durable storage after a worker restart" — seeds chrome.storage.session with an awaiting_review record from a "previous" worker lifetime, imports a fresh module, and proves AWAIT_OUTCOME + DECISION_MADE still resolve it exactly once. (Simulated via seeded storage + fresh module import, not a real forced-process-kill — see Known Gaps.)
  • Worker termination after proceed cannot cause a second release. Same mechanism: a restored terminal record answers from the tombstone; background.test.ts's "restored request whose deadline already passed settles expired... with no dangling resolver" covers the adjacent case.
  • Originating-tab navigation or closure invalidates the request. background.test.ts: two dedicated tests (onUpdated with status: 'loading', and onRemoved), both proving a subsequent AWAIT_OUTCOME returns cancel.
  • A missing bridge/background cannot hang beyond the absolute deadline. Three independent, layered deadlines — see Required Scope D. requestOutcome.test.ts proves the page-side one fires via fake timers without any response ever arriving.
  • Changing one XDR byte after review causes digest rejection. The digest (xdrDigest, computed once per SIGN_REQUEST from the reviewed XDR) is bound into the pending record at awaiting_review and is what the popup displays; there is no code path where a different XDR's outcome could be attached to an existing digest-bound record. Not a dedicated adversarial test beyond the existing transactionReview.test.ts fuzz coverage of digest computation — see Known Gaps.
  • Incompatible old content scripts receive a typed failure, not silent bypass. protocolVersion mismatch → SIGN_REJECTED { reason: 'protocol-incompatible' }. background.test.ts + messageValidation.test.ts.

Replaces the page-message/runtime-callback signing flow with a versioned
protocol: the background now generates the sole authoritative request id
(never trusting the page/bridge's own correlation token), persists
pending-request identity/binding/deadline to chrome.storage.session before
opening any UI, and binds every decision to its originating tab/frame/
document and to the popup's own window. Closes Gryd-lock#124.

See docs/adr/0004-one-shot-signing-protocol.md for the full design.

Claude-Session: https://claude.ai/code/session_01EpN7pg6WCWSV8iwYrjFJtB
@knytcomics-ui
knytcomics-ui merged commit 4c3cc11 into Gryd-lock:main Aug 30, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Epic — Establish a lifecycle-safe, origin-bound signing review protocol

2 participants