feat(wallet): add address-based web sign-in recovery

[Dannnyhamps]

Closes #769

[drips-wave]

@Dannnyhamps Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[vercel]

@Dannnyhamps is attempting to deploy a commit to the miracle656's projects Team on Vercel.

A member of the Team first needs to authorize it.

[Miracle656]

The matchWebAuthnSigner extraction is genuinely good — de-duplicating the inline signer matching out of recover/page.tsx was the right instinct. But there are four hard blockers, and one of them can strand a user's funds.

1. establishFreshFeePayer() destroys an existing fee-payer key

frontend/wallet/lib/feePayer.ts:293:

export function establishFreshFeePayer(): Keypair {
  const keypair = Keypair.random()
  …
  localStorage.setItem(MODE, 'legacy')
  walletSession.setItem(SECRET, keypair.secret())

Unconditional — no check for an existing key. And it is called from /lock, which per app/page.tsx:26-30 is reachable only when a wallet already exists locally. So one click of "Sign in with wallet address" on an existing install replaces a possibly-funded fee-payer secret, and because the mode is now pinned legacy, ensureFeePayer will never re-derive it from PRF again.

That is a fund-stranding shape, not a cosmetic one. It must not overwrite an existing key without an explicit, informed confirmation.

2. It turns mobile CI red

frontend/mobile/lib/recovery.ts:22 adds import { isRegisteredSigner } from '@veil/sdk'. That barrel pulls core.ts → @stellar/stellar-sdk → axios's fetch adapter, which dies under jest-expo:

TypeError: Cannot cancel a stream that already has a reader
  at …/axios/lib/adapters/fetch.js:80:12
  at …/sdk/src/core.ts:13:19

That costs 24 previously-green tests in recovery.test.ts plus your own new suite. Replacing that one import with a local stub makes recovery.test.ts pass 24/24 again, so it is the import and not the environment.

Import from a leaf path rather than the @veil/sdk barrel inside mobile lib/ — the barrel is not safe to pull into the Expo jest environment.

3. The SDK api-surface snapshot was not regenerated

● SDK API Surface › should match the snapshot for main SDK exports
  + "RECOVERY_SIGNER_CASES: object",
  + "isRegisteredSigner: function(arity: 2)",

npx jest -u in sdk/. Separately: RECOVERY_SIGNER_CASES is a test fixture exported from the SDK's public API. That belongs in a test file, not the published surface.

4. Backup restore can never succeed

lock/page.tsx, handleBackupRecovery:

const metadata = JSON.parse(await backupFile.text())
const restored = await decryptBackup(deserializeBackup(metadata), backupPassphrase)

deserializeBackup(blob: string) does its own JSON.parse. Passing an already-parsed object stringifies to "[object Object]" and throws Backup blob is not valid JSON every time. JSON.parse returns any, so tsc cannot catch it. Use deserializeBackup(await backupFile.text()).

It disagrees with the mobile half, which has now merged

#858 merged, and #769 says to take the rule from V191 rather than reinvent it. Right now:

	#858 (mobile)	#851 (web)
Address validation	StrKey.isValidContract	/^C[A-Z2-7]{55}$/
Not-deployed vs unreachable	typed WalletContractNotFoundError	one generic message
Fee-payer	PRF, falling back to random	always random

The regex is the specific failure this batch keeps hitting: 'C' + 'B'.repeat(55) passes it and fails StrKey. A shape regex is not validation.

And the "shared table" acceptance criterion is not met — RECOVERY_SIGNER_CASES is four hex-equality cases over isRegisteredSigner, which the web address path does not even call (it uses matchWebAuthnSigner). Nothing in it covers an invalid address, an undeployed wallet, an unreachable network, or a non-signer passkey, which are exactly the cases where the two platforms currently disagree.

The cleanest fix is what the issue actually asked for: lift #858's validate → resolve → verify sequence into the SDK and have both platforms call it.

One more thing worth knowing

A genuinely fresh device lands on /, which has no address-recovery entry at all; /lock is only reachable with a wallet already in localStorage. So as wired, the feature is unreachable where it is needed and harmful where it is reachable. Worth rethinking the entry point alongside the fixes.

tsc and next build --webpack both pass, and qrcode.react was already a dependency — the build side is fine. It is the behaviour that needs work.

[Miracle656]

Re-read at b67aebd4. Four of the five things I raised are properly fixed, and the SDK extraction is now the shape I asked for. One blocker left, and it's narrow.

1. establishFreshFeePayer() destroying an existing key — addressed

It's gone, replaced by establishRecoveredFeePayer(prf, credentialId, replaceExisting, recoveredKeypair), which reuses the stored secret when the credential ID matches and otherwise throws FeePayerConflictError rather than overwriting. AddressRecovery.tsx catches it and shows a confirmation that says what replacing costs ("may make its funds inaccessible without another backup") before anyone can proceed. recover/page.tsx dropped its resetFeePayer() too. The pinned-mode branches (prf-raw / prf-hkdf / legacy) preserve whichever derivation the wallet was created with, and the paper-recovery path writes the same veil_signer_secret / veil_signer_public_key keys it used to. Six tests in lib/__tests__/feePayer.test.ts cover reuse, conflict, forced replacement and the paper keypair. That's a thorough fix for the one that could strand funds.

2. Mobile CI — addressed, and I verified it

lib/recovery.ts now imports isRegisteredSigner from the leaf sdk/src/recovery/signerRegistry, which touches no Stellar SDK at all. I ran the full mobile suite on your head:

Test Suites: 1 failed, 55 passed, 56 total
Tests:       624 passed, 624 total

recovery.test.ts is back to 24/24. Nothing on main regressed.

3. The API-surface snapshot — addressed

Regenerated, and RECOVERY_SIGNER_CASES is out of the published surface — it's sdk/tests/fixtures/addressRecoveryCases.ts now, which is where a fixture belongs. Full SDK run on your head: 26 suites, 288 tests, 5 snapshots, all passing.

4. deserializeBackup double-parse — addressed

deserializeBackup(await file.text()). Restore can actually succeed now.

5. Disagreement with the mobile half (#858) — addressed

This is the part I'm most pleased with. recoverWalletByAddress() in sdk/src/recovery/signerVerification.ts is the one validate → resolve → verify sequence, and both platforms call it:

• StrKey.isValidContract(address), so 'C' + 'B'.repeat(55) is refused. The shape regex is gone from both recover/page.tsx call sites — the only /^C[A-Z2-7]{55}$/ left in the tree is in pre-existing e2e assertions.
• Typed WalletContractNotFoundError / WalletRecoveryNetworkError / NotVeilWalletError / PasskeyNotRegisteredError / InvalidWalletAddressError, and mobile/lib/signers.ts maps simulation errors onto the same two.
• AddressRecovery is reachable from / via a new /sign-in route and the "Sign in with wallet address or backup" button, which fixes the entry-point problem — the feature is now available on the fresh device it exists for.

I also checked lock/page.tsx, since it shows as 389 changed lines. Normalising quotes, semicolons and whitespace away, the only semantic change is the new "Sign in with another address or backup" button at the end. Behaviour-identical — but please drop the reformat anyway (git checkout upstream/main -- frontend/wallet/app/lock/page.tsx, then re-add the button). It's the unlock and PRF path, and a 300-line cosmetic diff over it is 300 lines nobody can skim.

Separately, matchWebAuthnSigner slices clientDataJSON by byteOffset/byteLength where the old inline code passed .buffer whole — a latent bug quietly fixed on the way past. Nice.

Blocking — the new mobile test can't run

frontend/mobile/lib/__tests__/signerVerification.test.ts is the one failing suite above:

TypeError: Cannot cancel a stream that already has a reader
  at sdk/node_modules/axios/lib/adapters/fetch.js:80:12
  at sdk/node_modules/@stellar/stellar-sdk/lib/http-client/axios-client.js:7:37
  …
  at sdk/src/recovery/signerVerification.ts:21:19

Same failure as last time, one import further out: the test pulls signerVerification.ts, which imports @stellar/stellar-sdk at module scope for StrKey, Contract, TransactionBuilder and SorobanRpc.Server, and that dies under jest-expo. This PR gets no CI (fork), so it looks clean here and would turn Mobile — typecheck & test red the moment it lands.

It also means mobile/lib/signers.ts is now a module no mobile test can import, because it pulls the same file for NotVeilWalletError / WalletContractNotFoundError.

The fix is the one that already worked for isRegisteredSigner: put the error classes in their own leaf module next to signerRegistry.ts — they're plain Error subclasses with no SDK dependency — have signerVerification.ts re-export them, and import from the leaf in mobile/lib/signers.ts and in the mobile test. recoverWalletByAddress itself can stay where it is; the mobile test can exercise it through an injected resolveSigners, which is what it already does, once the import no longer drags StrKey in. (Keeping StrKey out of the leaf matters — that's what pulls the SDK.)

One for the follow-up, not now

recover/page.tsx calls establishRecoveredFeePayer(prf, assertion.id) with no replaceExisting and no FeePayerConflictError catch, so recovering onto a browser that already holds another wallet's fee-payer now dead-ends on the raw conflict message with no way forward. Strictly safer than the old resetFeePayer(), so I'm not holding the PR for it — but /recover should offer the same confirmation AddressRecovery does.

---

On the bigger picture: I asked on #917 for that bundle to be split per issue, and this is the single-issue PR for #769 — it predates #917 by four days rather than being carved out of it, but it's the shape I want, and it shows. Fix the test import, drop the lock/page.tsx reformat, and I'll merge it.

[Miracle656]

Re-checked at df6c359b. The two defects I flagged here are unchanged, and the address check has gone backwards.

The address is no longer validated at all

frontend/wallet/components/AddressRecovery.tsx:62

const selectedAddress = backup?.address ?? address.trim()

No StrKey, no isValidContract, no regex. The shape regex I objected to on
#852 is gone from this branch, but nothing replaced it — so where #852 accepts
56 characters that cannot exist on the network, this accepts anything at all.
Between the two PRs the check went from wrong to absent.

StrKey.isValidContract('C' + 'B'.repeat(55))   // false

That is the one-line fix, and StrKey is already imported in lib/multisig.ts,
lib/simulate.ts and lib/vault.ts.

The assertion is never bound to anything (#851 and #852, same code)

sdk/src/recovery/signerVerification.ts hashes clientDataJSON into the signed
message and never looks inside it:

  parses clientDataJSON : false
  checks "type"         : false   ← not asserted to be "webauthn.get"
  checks "origin"       : false   ← not asserted to be our origin
  checks "challenge"    : false   ← not compared to anything we issued

All it establishes is that some ECDSA signature verifies against a
registered key. That was carried over verbatim from /recover, where the user
had already proven possession another way — but it now sits on a login path,
so an assertion captured once is accepted again, from any origin, forever. A
challenge nobody checks is not a challenge.

This is the one I would fix first wherever this code lands.

establishFreshFeePayer() still mints unconditionally

  feePayer.ts:293  export function establishFreshFeePayer(): Keypair {
  feePayer.ts:294    const keypair = Keypair.random()

Every address or backup recovery overwrites whatever fee payer is stored. A user
with an existing funded one loses it, and the raw secret goes to localStorage,
which is still open finding C3. Read the stored one first and only mint when
there is genuinely nothing there.

Where this stands

The feature is worth having and the mechanism is sound. What is blocking it is a
login path that accepts a replay, an address nobody checks, and a function that
overwrites a funded account — all three small, all three in code that decides who
gets into a wallet.

[Miracle656]

Re-checked at df6c359b. Two of the three things I raised this morning were already fixed, and I was quoting stale code. My mistake, and I'm withdrawing both.

Withdrawn: "the address is no longer validated at all"

I looked at AddressRecovery.tsx:62, saw address.trim(), and stopped there instead of following the call. One line further in:

// sdk/src/recovery/signerVerification.ts:129
if (!StrKey.isValidContract(address)) throw new InvalidWalletAddressError();

That is the right check in the right place — one rule for both clients rather than a regex per screen. And sdk/tests/fixtures/addressRecoveryCases.ts has:

{ name: 'bad StrKey checksum', address: `C${'B'.repeat(55)}`, ..., expected: 'InvalidWalletAddressError' }

which is the exact counterexample I handed you, turned into a test. That was the correct response to the review and I then failed to read it.

Withdrawn: "establishFreshFeePayer() still mints unconditionally"

I quoted feePayer.ts:293 export function establishFreshFeePayer(). That function does not exist on this branch — you replaced it in b67aebd4 on 30 September, five days before I claimed it was unchanged. What is there now reads the stored key first and refuses rather than overwrites:

const existing = peekFeePayerSecret()
if (existing) {
  if (!replaceExisting) {
    if (!storedCredentialId || !recoveredCredentialId || storedCredentialId !== recoveredCredentialId) {
      throw new FeePayerConflictError()
    }
    ...

and Keypair.random() is now reachable only when there is no pinned mode and no PRF — every other path either derives or throws. That is a better fix than the one I asked for, because it distinguishes "nothing stored" from "stored but unreachable" instead of silently preferring a new key.

(walletLocal.setItem(SECRET, ...) in legacy mode still puts a raw secret in localStorage, but that is pre-existing finding C3 across the whole wallet, not something this PR introduces. Not blocking.)

Also fixed since I last looked: the mobile test now imports the module directly rather than the @veil/sdk barrel.

---

One thing still open, and it is the one that matters

sdk/src/recovery/signerVerification.ts — matchWebAuthnSigner hashes clientDataJSON into the signed message and never looks inside it:

const clientDataHash = new Uint8Array(await crypto.subtle.digest('SHA-256', clientDataJSON...))
const message = new Uint8Array(authData.length + clientDataHash.length)
message.set(authData)
message.set(clientDataHash, authData.length)

That is the whole of what the assertion is checked against. Nothing parses the JSON, so:

• challenge is never compared. You do generate a fresh random one at all three call sites — crypto.getRandomValues(new Uint8Array(32)) on web, Crypto.getRandomBytes(32) on mobile. It is issued and then never verified, so an assertion produced against a different challenge verifies identically.
• origin is never checked. An assertion for this credential obtained by any other site verifies here.
• type is never asserted to be webauthn.get.

Taken together, what the function proves is "some ECDSA signature verifies against a key registered on this contract" — not "the holder of that key authenticated here, just now". On /recover that gap was tolerable because possession had been established another way. This PR puts the same function on a sign-in path, which is where a captured assertion becomes a permanent credential.

The fix is to parse clientDataJSON and assert all three before the signature check, with the expected challenge passed in by the caller that generated it. Tests worth having: a correct assertion, one with a stale challenge, and one with a foreign origin.

Housekeeping

frontend/mobile/jest.config.js now declares modulePaths twice:

modulePaths: ['<rootDir>/node_modules'],   // added, with a comment
setupFiles: [...],
modulePaths: ['<rootDir>/node_modules'],   // already there

Same value, so nothing breaks — the second simply wins and the added line does nothing. Keep the comment, drop the duplicate.

Note on CI

The only checks running here are Vercel (failing on fork-deploy authorization, not on your code) and GitGuardian. No test or typecheck job runs on a fork PR, so neither of us can point at CI for this one — please say in the PR what you ran locally and what passed.

One round left on this. The feature is sound and the last three fixes were the right ones; it is the assertion binding and the duplicate config line between here and merge.

https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB