fix(backend)!: derive OnRamper signed wallet addresses from caller principal#13167
fix(backend)!: derive OnRamper signed wallet addresses from caller principal#13167AntonioVentilii wants to merge 15 commits into
Conversation
…into feat/onramper-derive-server-side
…into feat/onramper-derive-server-side
…server-side # Conflicts: # src/backend/src/signer/service.rs
|
✅ No security or compliance issues detected. Reviewed everything up to a9eedad. Security Overview
Detected Code Changes
|
There was a problem hiding this comment.
Pull request overview
Closes an OnRamper URL-signing oracle by making the backend derive the authenticated caller’s receiving addresses (BTC/ETH/ICP/SOL) server-side and signing only those, removing all caller-supplied wallet-address inputs and updating the frontend + generated bindings accordingly.
Changes:
- Backend
sign_onramper_widget_urlis now argument-less +async, derives caller addresses via management-canister public-key reads, and addsAddressDerivationFailed. - Frontend no longer derives/sends wallet addresses; it just calls the signing endpoint with the user identity and appends
signed_queryverbatim. - Regenerates Candid + TS declaration artifacts and updates product docs to state the negative guarantee (cannot sign attacker-chosen addresses).
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/src/types/onramper.rs | Removes request type; adds AddressDerivationFailed and updates error docs. |
| src/frontend/src/tests/lib/utils/onramper.utils.spec.ts | Updates unit tests for the argument-less signing call and backend-supplied signed query. |
| src/frontend/src/lib/utils/onramper.utils.ts | Removes wallet args; calls backend signing endpoint with identity only. |
| src/frontend/src/lib/types/onramper.ts | Removes now-dead OnRamper wallet address/wallet types. |
| src/frontend/src/lib/types/api.ts | Removes OnRamper signing request parameter types. |
| src/frontend/src/lib/components/onramper/OnramperWidget.svelte | Drops address-store wiring; relies on backend-derived addresses. |
| src/frontend/src/lib/canisters/backend.errors.ts | Maps new AddressDerivationFailed error variant. |
| src/frontend/src/lib/canisters/backend.canister.ts | Updates canister wrapper to call sign_onramper_widget_url() with no args. |
| src/frontend/src/lib/api/backend.api.ts | Updates API wrapper to call the no-arg canister method. |
| src/declarations/backend/backend.factory.did.js | Regenerated backend IDL factory (no-arg endpoint; type removals). |
| src/declarations/backend/backend.factory.certified.did.js | Regenerated certified IDL factory (no-arg endpoint; type removals). |
| src/declarations/backend/backend.did.d.ts | Regenerated TS declarations for breaking Candid changes + new error variant. |
| src/declarations/backend/backend.did | Regenerated Candid for breaking endpoint signature and type cleanup. |
| src/backend/tests/it/onramper.rs | Updates integration tests for caller-bound, server-derived signing. |
| src/backend/src/signer/service.rs | Adds ETH/SOL principal→address derivation via management-canister pubkey reads (schema-aware). |
| src/backend/src/signer/mod.rs | Re-exports new signer helpers used by OnRamper service wiring. |
| src/backend/src/onramper/service.rs | Derives caller networkWallets server-side and signs only those. |
| src/backend/src/lib.rs | Removes obsolete request-type import. |
| src/backend/src/api/onramper.rs | Makes sign_onramper_widget_url async + no-arg, binds to msg_caller(). |
| src/backend/backend.did | Regenerated backend Candid mirror (breaking endpoint signature and type cleanup). |
| docs/ai/PRODUCT.md | Documents Buy (OnRamper) behavior + explicit anti-oracle guarantee. |
…re is caller-bound
…' into feat/onramper-derive-server-side
|
Superseded by a cleaner decomposition of the same fix, switched to the validate-ownership design (the FE keeps sending its derived addresses; the backend re-derives from the caller principal, verifies they match, and signs its own derived values — rejecting on mismatch or underivable network). This is non-breaking (the request shape is unchanged), so it splits cleanly:
Encoders already merged in #13166. Closing this in favor of that stack. |
Motivation
sign_onramper_widget_urlwas an open signing oracle: it HMAC-signed thewallets/network_wallets/wallet_address_tagsparameters exactly as supplied by any authenticated caller, without checking the addresses belonged tomsg_caller(). An attacker could get the backend to sign their own address and phish a victim into funding it via a valid, OISY-branded OnRamper URL — the HMAC is OnRamper's sole URL-integrity check, so a valid signature reads as OISY authorization.This closes the oracle by deriving the caller's own receiving addresses server-side and signing only those. A caller can now only ever obtain a signature bound to their own wallet.
Spec:
docs/ai/spec-driven-development/specs/2026-06-22-fix-onramper-signing-oracle.md(#13164). Stacked on #13166 (the address encoders).Changes
async.sign_onramper_widget_urltakes no request; it derives the caller's BTC/ETH/ICP/SOL receiving addresses frommsg_caller()and signs them asnetworkWallets. Thecaller_is_not_anonymousguard and per-caller rate limiter are unchanged.signer/service.rs): ETH (secp256k1 schema1+ keccak/EIP-55) and SOL (Ed25519 schnorr + base58) reuse the samecanister_id = cfs_canister_idpattern as the existing BTC derivation; ICP is localprincipal2account. These are pubkey reads — no chain-fusion-signer call, noallow_signing/allowance flow, no signing fee. A chain is omitted if its derivation fails (ICP always succeeds), andAddressDerivationFailedis returned only if nothing derives.wallets/network_wallets/wallet_address_tagsfrom the request (type removed); added theAddressDerivationFailederror variant;backend.did+src/declarations/backend/regenerated vianpm run generate.buildOnramperLinkand the canister/api layer call the endpoint with the identity only;OnramperWidget.sveltedrops the address-store wiring; the now-deadmapOnramperNetworkWalletsutil andOnramper*Wallet*types are removed.docs/ai/PRODUCT.mdgains a Buy (OnRamper) section with the explicit negative guarantee.Tests
signer::servicecover the pubkey→address encoding against known vectors.tests/it/onramper.rs): anonymous rejection,SecretNotConfigured, rate limit, deterministic signing of the caller's own derived addresses, and a positive anti-oracle proof — two distinct callers get different signed content (a caller can only sign their own addresses).onramper.utils.spec.tsrewritten for the argument-less call; affected buy specs pass.cargo fmt/lint.rust.sh/lint.did.sh,npm run check(clean except a pre-existing unrelatedsol-instructions-system.utils.tserror not in this diff), scoped ESLint, and the affected vitest specs all pass. The heavy pocketIC integration suite runs in CI.Parity note: byte-for-byte parity between the backend-derived addresses and the wallet's own addresses must be confirmed on staging before the buy flow is un-gated — there is no signer wasm in the test harness to assert it automatically (by design — production never calls the signer).
BREAKING CHANGE:
sign_onramper_widget_urlno longer acceptswallets/network_wallets/wallet_address_tagsand takes no arguments; it derives and signs the caller's own addresses. Clients must stop sending wallet parameters (frontend binding regenerated in this PR).