Skip to content

fix(swap): require in-vault review before signing headless /execute#266

Closed
BitHighlander wants to merge 2 commits into
developfrom
fix/swap-execute-requires-review
Closed

fix(swap): require in-vault review before signing headless /execute#266
BitHighlander wants to merge 2 commits into
developfrom
fix/swap-execute-requires-review

Conversation

@BitHighlander

Copy link
Copy Markdown
Collaborator

The problem

POST /api/v2/swap/execute (the BEX/headless swap path) signed on the device with no Vault reviewheadlessExecuteSwap called executeSwap directly with NOOP_PUSH_SUBSTAGE. It was a deliberate "device is the only trust gate" design (see the BEX handoff doc), with sign-off deferred to the vault owner.

That's inadequate for swaps specifically: the firmware can only render "send X to <addr>" — the destination is a router/inbound vault address and the swap intent lives in an opaque memo. The device physically cannot show "you are swapping X for ~Y on network Z." Hitting "swap" from the extension went straight to device signing with no Vault display of what was being signed.

The fix (reuse SwapDialog review, swaps-only)

headlessExecuteSwap no longer signs directly. It drives Vault's real SwapDialog to its review screen, seeded with the swap, and blocks until the user approves on-screen — the dialog's own confirm → device-sign → trackSwap path runs — or cancels.

  • The dialog's swap-cmd handler already accepts a CAIP as the asset key (findAssetByKey: x.asset === key || x.caip === key) and buffers it until its swappable list loads, so seeding from the fromCaip/toCaip pair works.
  • New Bun-side waiter over the existing publishSwapUiState mirror (awaitSwapUiState) observes the dialog reaching review → submitted, mirroring the WalletConnect requestSigningApproval gate.
  • Singleton guard: 409 if a review is already in flight or a swap is already open in Vault.

BEX-contract impact (intended)

  • /execute still blocks and returns {txid}, but only after on-screen approval (the 5-min client timeout already covers this).
  • The signed quote is Vault's authoritative re-quote; the caller's /quote becomes a pre-estimate.
  • A user cancelling/timing out in Vault returns 409 (not a txid) — the BEX must handle it.

Scope

Swaps only. sweep/execute, TON, zcash shielded, and generic tx sign+broadcast still reach the device headlessly — lower risk (the device can show address+amount there) — and are flagged for a separate audit, not bundled here.

Test plan

  • tsc --noEmit clean (only pre-existing minimatch ambient error)
  • bun test swap unit suites — 182 pass
  • Device smoke: call /api/v2/swap/execute (or hit swap from BEX) → Vault SwapDialog pops at review with the correct from/to/network/amount/fees/memo → approve → device signs → {txid} returned; cancel in Vault → 409; second concurrent call → 409.

Files: src/bun/index.ts (drive + await), src/bun/rest-swap.ts + src/bun/rest-api.ts (comments/contract), docs/HANDOFF_bex_swap_headless_endpoints.md (correction note).

POST /api/v2/swap/execute (the BEX/headless path) signed on the device with
no vault GUI in the loop — relying solely on firmware confirmation. For swaps
that's inadequate: the firmware can only render "send X to <addr>", which hides
the swap intent (the destination is a router/inbound vault address and the
intent is in an opaque memo). The user hit "swap" from the extension and it
went straight to device signing with no vault review of what was being signed.

Fix: headlessExecuteSwap no longer calls executeSwap directly. It drives
vault's real SwapDialog to its review screen, seeded with the swap (the
dialog's swap-cmd handler already accepts a CAIP as the asset key), and blocks
until the user approves on-screen — the dialog's own confirm → device-sign →
trackSwap path runs — or cancels.

- New Bun-side waiter over the publishSwapUiState mirror (awaitSwapUiState)
  to observe the dialog reaching review → submitted, mirroring the
  WalletConnect requestSigningApproval gate.
- Singleton guard: 409 if a review is already in flight or a swap is already
  open in vault.
- Vault re-quotes; that authoritative quote is what's signed. The caller's
  /quote response becomes a pre-estimate. User cancel/timeout → 409.

Scope: swaps only. sweep/TON/zcash/generic sign+broadcast still reach the
device headlessly (lower risk — device can show address+amount) and are
flagged for a separate audit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ypass)

The in-vault review gate was defeatable: an authenticated REST client could
POST /api/v2/swap/execute to open the review, then POST /api/v2/swap/confirm —
which sent kind:'confirm' and made SwapDialog call handleExecuteSwap() with no
human ever clicking. Signing must only ever be initiated by a real on-screen
"Approve & Swap" gesture.

- SwapDialog: the 'confirm' swap-cmd no longer calls handleExecuteSwap; it is
  ignored with a warning. The on-screen button is the sole signing trigger.
- rest-swap: /api/v2/swap/confirm now returns 403 (programmatic confirmation
  not permitted). Callers drive to review via /execute (or /open + /advance)
  and the user approves on-screen.

Addresses PR #266 review finding 1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@BitHighlander

Copy link
Copy Markdown
Collaborator Author

Superseded by #268 (merged), which closes the same /api/v2/swap/execute headless hole via the central approval-overlay gate — the same raw-sign overlay WalletConnect and the other BEX routes use. Dropping this SwapDialog-drive approach to avoid double-gating /execute. Thanks!

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.

1 participant