fix: safe in-place upgrade target for pre-fix legacy wallets

[kalepail]

Why

Four mainnet smart-wallet WASM hashes predate the require_auth fix to update_signer (kalepail/passkey-kit@dcc6e3dc9c, 2025-03-27). Anyone who knows a signer key can overwrite that signer on a wallet still running one of them. Wallet instances never upgrade themselves. A private report on 2026-09-17 counted ~1,708 such wallets; the exposure is small in value but is a full takeover per wallet.

The two oldest hashes (0c0a264d…, 19868df3…) store signers in a pre-6a27d48 layout that every later build fails to decode, including ecd990f0…, e45c42b9…, and v1. Upgrading them to any existing post-fix build succeeds as a transaction and then bricks the wallet.

What

• contracts-legacy/ — the last pre-1.0 wallet (e45c42b9…, commit aeb04d7, soroban-sdk 23.0.2, Rust 1.89) with signer reads that accept both pre-1.0 storage layouts, plus an unauthenticated value-preserving migrate_signers(keys) and a get_signer(key) view. Hash c079d3a4136eb6ca68eb724acd3d8af11b0be4a0ed82605925a6dfd4dd83a97c. Uploaded to testnet (tx ae5e9439…); not yet on mainnet.
  Eight tests, including real signed in-place upgrades of the actual mainnet 0c0a264d, b62f6221, and c5509dfa bytecode (fixtures, sha256-pinned) and of the optimized artifact.
• SDK — connectWallet throws LegacyWalletError (code 2006) with the upgrade target and guide URL when a candidate runs vulnerable or patched pre-1.0 code, before birth verification would fail it for a less useful reason. The constructor rejects a known-vulnerable walletWasmHash. New exports for the hash lists.
• SECURITY.md — "Known-vulnerable wallet WASM hashes" section.
• docs/legacy-wallet-upgrade.md — cohorts, the two layouts, the procedure, client compatibility after upgrade.
• CI — verifies the committed canonical artifact contracts-legacy/out/smart_wallet.wasm against the published hash, runs the tests against those exact bytes, and rebuilds on Linux as an informational step. Byte-identical cross-host builds turned out not to be achievable: stellar contract build remaps paths, but rustc's wasm codegen still differs between macOS and Linux (the two CI diffs are in the run logs), so the artifact is committed and its hash asserted rather than rebuilt.

The v1 contract is unchanged.

Review

Four independent reviewers (grok, muse, glm, kimi) reproduced the hash, reran the tests, re-derived the layouts from git and from real on-chain entries, and verdicted "correct, conditional". Their doc corrections are applied. Remaining gaps are operational, not artifact defects: mainnet upload, integrator outreach, and a passkey-capable upgrade page.

After merge

1. stellar contract upload --wasm contracts-legacy/out/smart_wallet.wasm --network mainnet with a funded key, and record it.
2. Integrator outreach with docs/legacy-wallet-upgrade.md.

🤖 Generated with Claude Code

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	cargo/​soroban-sdk@​23.0.2

View full report

[socket-security]

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action	Severity	Alert  (click "▶" to expand/collapse)
Warn		High CVE: The rs-soroban-sdk #[contractimpl] macro calls inherent function instead of trait function when names collide in cargo soroban-sdk-macros CVE: GHSA-4chv-4c6w-w254 The rs-soroban-sdk #[contractimpl] macro calls inherent function instead of trait function when names collide (HIGH) Affected versions: >= 25.0.0 < 25.1.1; >= 23.0.0 < 23.5.2; < 22.0.10 Patched version: 23.5.2 From: contracts-legacy/Cargo.lock → cargo/soroban-sdk@23.0.2 → cargo/soroban-sdk-macros@23.0.2 ℹ Read more on: This package | This alert | What is a CVE? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev. Suggestion: Remove or replace dependencies that include known high severity CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore cargo/soroban-sdk-macros@23.0.2. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

[Copilot]

🟡 Changes recommended

Unresolved moderate findings affect legacy-hash safety, candidate handling, passkey coverage, and migration-test coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a legacy smart-wallet upgrade target, SDK detection and errors, migration guidance, fixtures, and pinned CI verification.

Changes:

• Adds legacy contract support for historical signer layouts and migration.
• Adds SDK hash validation, LegacyWalletError (2006), exports, and tests.
• Adds security documentation, upgrade procedures, release notes, and CI configuration.

File summaries

File	Reviewed changes / final notes
src/kit.ts	Legacy detection and configuration checks. Moderate (3 votes): patched legacy hashes remain ABI-incompatible with v1 deployment. Moderate (1 vote): raw probing can abort candidate handling before birth validation.
src/kit.test.ts	SDK legacy-wallet tests. Moderate (1 vote): tests cover only the first advertised hashes and lack coverage for 19868df3….
src/index.ts	Public exports. Nit (1 vote): README does not list LegacyWalletError.
src/errors.ts	LegacyWalletError and code 2006. Nit (1 vote): clarify that legacy clients cannot call the new migration entry point.
src/constants.ts	Legacy and vulnerable WASM hash identities.
SECURITY.md	Vulnerable hash disclosure and remediation guidance.
README.md	Upgrade-guide documentation link.
docs/legacy-wallet-upgrade.md	Upgrade procedure and compatibility guidance. Nits (1, 2 votes): clarify that migration does not produce v1-compatible u64 storage and align the stated package version.
contracts-legacy/smart-wallet/src/verify.rs	Legacy WebAuthn signature verification.
contracts-legacy/smart-wallet/src/types.rs	Legacy contract types.
contracts-legacy/smart-wallet/src/tests/mod.rs	End-to-end upgrade and migration tests. Moderates (1 vote each): add real Secp256r1 authorization coverage and exercise normalization before signer rewriting in the optimized artifact.
contracts-legacy/smart-wallet/src/tests/fixtures/README.md	Pinned fixture documentation.
contracts-legacy/smart-wallet/src/storage.rs	Storage TTL handling.
contracts-legacy/smart-wallet/src/signer.rs	Support for both signer storage layouts.
contracts-legacy/smart-wallet/src/lib.rs	Legacy upgrade-target implementation. Nit (2 votes): restrict normalization guidance to strict legacy builds. Moderate (1 vote): remove or consistently use the duplicate local types module.
contracts-legacy/smart-wallet/src/context.rs	Authorization context checks.
contracts-legacy/smart-wallet/src/base64_url.rs	Base64url encoding support.
contracts-legacy/smart-wallet/Cargo.toml	Legacy wallet crate configuration.
contracts-legacy/smart-wallet-interface/src/types.rs	Legacy interface types.
contracts-legacy/smart-wallet-interface/src/lib.rs	Legacy contract interfaces.
contracts-legacy/smart-wallet-interface/Cargo.toml	Interface crate configuration.
contracts-legacy/rust-toolchain.toml	Pinned Rust toolchain.
contracts-legacy/Makefile	Legacy build, optimization, testing, and hash commands.
contracts-legacy/Cargo.toml	Legacy workspace configuration.
contracts-legacy/Cargo.lock	Pinned legacy dependencies.
CHANGELOG.md	Legacy-wallet support release notes.
.gitignore	Generated legacy test snapshot exclusions.
.github/workflows/ci.yml	Legacy build, test, formatting, and artifact-hash verification.

Review details

Suppressed comments (8)

contracts-legacy/smart-wallet/src/lib.rs:39

• This declares the new local types module, but every contract module imports its types from smart_wallet_interface::types and no crate::types symbol is referenced. The duplicate types.rs (including a second Error definition) is therefore dead and can drift from the types that actually define the contract ABI; remove it or use one source consistently.

mod types;

contracts-legacy/smart-wallet/src/tests/mod.rs:136

• All end-to-end upgrade authorization entries in this file use Signature::Ed25519; the Secp256r1 cases only exercise storage decoding. The published target's actual passkey authorization path (verify_secp256r1_signature) is therefore not covered, so a regression there could pass CI while the affected passkey wallets cannot upgrade. Add a real WebAuthn/Secp256r1-signed update_contract_code against a legacy fixture.

    let sig = Signature::Ed25519(BytesN::from_array(
        env,
        &sk.sign(payload.to_array().as_slice()).to_bytes(),
    ));
    let signatures = Signatures(map![env, (SignerKey::Ed25519(pk.clone()), sig)]);

contracts-legacy/smart-wallet/src/tests/mod.rs:779

• The optimized-artifact test calls migrate_signers only after the signed update_signer; that update has already rewritten the bare entry into the current layout, so the assertion that migration returns 0 never exercises the new normalization path in the bytes whose hash is shipped. Exercise migrate_signers on a still-bare entry before the update (or use a second bare signer) in this optimized-artifact test.

    assert_eq!(client.migrate_signers(&vec![&env, key.clone()]), 0);

docs/legacy-wallet-upgrade.md:109

• migrate_signers only rewrites the legacy Option<u32> expiration into the wrapped legacy SignerVal; it does not widen it to the v1 Option<u64> representation. Consequently, entries with Some(...) still cannot be decoded by v1, so saying this prepares the wallet for “a strict build” can lead an operator to brick it during a later v1 upgrade. Limit this statement to the strict pre-1.0 builds, or document a separate u32-to-u64 migration.

Step 3 is optional for the wrapped cohort (`migrate_signers` returns 0) and
recommended for the bare cohort, so that the wallet can later move to a
strict build if its operator chooses to.

src/errors.ts:210

• This tells operators that the 0.10.20–0.12.x kit line performs both update_contract_code and migrate_signers, but migrate_signers is a new entry point on this target and is not part of that legacy client interface. The guidance can leave operators with an old client that cannot build the normalization call; say explicitly that the legacy kit is used for the authorized upgrade and that the second call must use a target binding or Stellar CLI/SDK.

        `Upgrade it in place to ${upgradeTarget} with the passkey-kit 0.10.20–0.12.x line ` +
        `(update_contract_code, then migrate_signers), or move its funds out. ` +

src/index.ts:61

• This adds a new public error type, but the README's documented error-class list (README.md:429) is not updated to include LegacyWalletError. Consumers following the public API documentation will not discover the new typed failure or code 2006.

  LegacyWalletError,

src/kit.test.ts:674

• The security tests exercise only KNOWN_VULNERABLE_WALLET_WASM_HASHES[0] and LEGACY_WALLET_WASM_HASHES[0]. A typo or omission in the other advertised identities—especially 19868df3…, which has no byte fixture/end-to-end upgrade test—would let that cohort bypass the intended classification without CI detecting it. Parameterize the constructor/connect checks across every hash and add an exact fixture test for the fourth vulnerable hash, or narrow the advertised support claim.

  const VULNERABLE = KNOWN_VULNERABLE_WALLET_WASM_HASHES[0]!;
  const PATCHED_LEGACY = LEGACY_WALLET_WASM_HASHES[0]!;

src/kit.ts:574

• This now performs a raw current-instance RPC read before the candidate's birth metadata is validated. A stale or caller-supplied candidate with malformed birth data (or a missing instance) can make getContractData throw, aborting connectWallet before it can continue to another candidate; previously birth verification rejected such a candidate as a WalletOwnershipError and continued. Treat a failed legacy probe as a candidate mismatch when appropriate, or preserve the birth-verification-first failure handling while still surfacing the legacy guidance for valid legacy instances.

• Files reviewed: 26/31 changed files
• Comments generated: 3
• Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings affect upgrade security, fixture integrity, SDK configuration, and release readiness.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (7)

.github/workflows/ci.yml:78

• The fixture README lists mainnet SHA-256 values, but this CI step only verifies the newly built target artifact; none of the three checked-in mainnet fixtures are hashed in tests or CI. A modified fixture could therefore make the end-to-end tests exercise different bytecode while still passing, despite the PR describing them as pinned. Add checksum assertions for each fixture here (or in the test setup) using the documented hashes.

      - name: Report the built WASM
        working-directory: contracts-legacy
        run: |
          shasum -a 256 out/smart_wallet.wasm

README.md:583

• This new migration guide makes the existing public caveats contradictory: README.md:520, docs/deployments-2026-09-01.md:86, and docs/migration-v1.md:8 still say there is no legacy/pre-release migration path. Update those statements to distinguish the new operator-only upgrade to the legacy target from the fact that the current SDK still cannot connect to that target.

- **[docs/legacy-wallet-upgrade.md](./docs/legacy-wallet-upgrade.md)** — in-place upgrade for pre-fix legacy wallets (known-vulnerable WASM hashes in [SECURITY.md](./SECURITY.md)).

contracts-legacy/smart-wallet/src/tests/mod.rs:31

• The fixture README calls these byte-exact, SHA-256-pinned mainnet artifacts, but the tests only include_bytes! them and never verify their digests. A changed or corrupted fixture could therefore make the end-to-end upgrade tests pass against code other than the advertised vulnerable hashes. Add a CI/test assertion for each fixture hash.

const WASM_0C0A264D: &[u8] = include_bytes!("fixtures/0c0a264d.wasm");

src/constants.ts:198

• This hash is surfaced by LegacyWalletError as the only in-place remediation target, but the PR states that the artifact is not yet uploaded to mainnet. Publishing this SDK behavior before the upload directs every affected mainnet wallet to a code hash that cannot be installed, leaving the remediation path broken. Gate the release on the mainnet upload and record/verify it before exposing this target to users.

export const LEGACY_UPGRADE_TARGET_WASM_HASH =
  "c079d3a4136eb6ca68eb724acd3d8af11b0be4a0ed82605925a6dfd4dd83a97c";

src/kit.ts:220

• This loop also rejects every acceptedBirthWasmHashes entry that matches a vulnerable deployment hash. That prevents an integrator from accepting the immutable birth code of a wallet that was born on a vulnerable build and later upgraded, even though this list is explicitly separate from current executable-code identities. Keep historical birth hashes out of this safety check; only the deploy hash and current-code allowlist should be blocked.
  src/kit.ts:221
• The guard only rejects the vulnerable list, so walletWasmHash can still be configured to c079d3a4… or another LEGACY_WALLET_WASM_HASHES value. The current generated client then deploys with the v1 __constructor(signer, proof)/upgrade interface against a pre-v1 artifact, while connectWallet rejects the resulting wallet as legacy. Reject all legacy hashes for deployment and current-code allowlists (while keeping historical birth hashes separate) so this configuration cannot create unusable wallets.
  src/kit.ts:227
• The loop also validates acceptedWasmHashes and acceptedBirthWasmHashes, but the error always labels the offending value as walletWasmHash. When a vulnerable hash appears only in one of those lists, the message falsely identifies the deploy hash, which makes the configuration failure misleading. Include the name of the field that supplied the hash (and keep the normalized value in context).

• Files reviewed: 26/31 changed files
• Comments generated: 1
• Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

[Copilot]

🟡 Changes recommended

The legacy discovery path has a critical unresolved issue, and CI, fixture-integrity, and migration-target guidance still require changes.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (7)

contracts-legacy/smart-wallet/src/lib.rs:134

• The target writes the legacy SignerExpiration(Option<u32>) representation, while the v1 interface uses Option<u64> UNIX-second expirations (contracts/smart-wallet-interface/src/types.rs:116); normalizing here therefore does not make a wallet decodable by v1. This also contradicts the upgrade guide's statement that v1 is not a drop-in target. Exclude v1 from this list so operators are not led to believe this migration enables it.

    /// both layouts anyway; this exists so a wallet can later move to a
    /// strict build (`ecd990f0…`, `e45c42b9…`, or v1) without decode failures.

contracts-legacy/smart-wallet/src/tests/fixtures/README.md:13

• The listed SHA-256 values are documentation only; the test suite never hashes the include_bytes! fixtures, so replacing a fixture with different WASM would still let the end-to-end tests pass while no longer testing the claimed mainnet code. Add an executable hash assertion for each fixture (and the target artifact) rather than relying on this table as the pin.

| File | sha256 (= mainnet WASM hash) | Bytes |
|---|---|---|
| `0c0a264d.wasm` | `0c0a264d4cc0b3e79b8533e2a2e1f0ed21501a5a3f9f2455d2f18c232940b865` | 20524 |
| `b62f6221.wasm` | `b62f62217ff256d557513793e9e44317b25b14401a8a6b6149a04d38d72d6c7c` | 23193 |
| `c5509dfa.wasm` | `c5509dfa5f022deb8ae621f073adac5fd788feca0b24d15b5f392ab22c2ff222` | 21660 |

contracts-legacy/smart-wallet/src/tests/mod.rs:33

• These fixtures are described as byte-exact and SHA-256-pinned, but neither the tests nor CI hash the included files against the documented mainnet hashes. A changed fixture would still let the end-to-end tests pass while no longer proving behavior against 0c0a264d, b62f6221, or c5509dfa; add a checksum assertion or CI verification for each fixture before registering it.

/// Mainnet `0c0a264d…`: pre-`6a27d48` layout, no constructor (born via
/// `CreateContract` + an unauthenticated first `add_signer`).
const WASM_0C0A264D: &[u8] = include_bytes!("fixtures/0c0a264d.wasm");
/// Mainnet `b62f6221…`: current layout, has `__constructor`, no auth on
/// `update_signer`.

contracts-legacy/smart-wallet/src/types.rs:2

• This entire module is unused: lib.rs imports every contract type from smart_wallet_interface::types, and no legacy source references crate::types. Keeping a second copy of the ABI types adds dead code and creates a future risk that the two definitions diverge; remove this module/file or switch the contract consistently to it.

use soroban_sdk::{contracterror, contracttype, Address, Bytes, BytesN, Map, Vec};

docs/legacy-wallet-upgrade.md:143

• This lower bound conflicts with the preceding sentence, which correctly identifies 0.17.0 as the current v1 SDK, and with the repository's current package version (0.18.3). Operators on 0.17.x/0.18.x would be told that this SDK still might support the wallet; use 0.17.0 and later here.

`passkey-kit` `0.19.0` and later cannot operate a pre-1.0 wallet, but it

src/kit.ts:221

• This validation only rejects the vulnerable list, but LEGACY_WALLET_WASM_HASHES also contains the patched legacy builds and the upgrade target, which assertNotLegacyWallet unconditionally rejects later. An integrator can therefore configure the current kit with c079d3a4… (or another patched legacy hash) as walletWasmHash/current accepted code; createWallet then builds the v1 constructor/proof ABI against a legacy contract and any resulting wallet cannot be connected by this kit. Reject legacy hashes from normal deployment/current-code configuration as well, or expose a separate explicitly unsupported operator path.
  src/kit.ts:573
• Discovery candidates are explicitly untrusted, but this classifies the current code before verifying the candidate's signed birth claim. A caller can return any known legacy contract address and force LegacyWalletError for a candidate that is not actually associated with this key (or has malformed/mismatched birth data), so the error no longer implies the normal candidate gate passed. Verify birth first, or only retain legacy guidance for a birth-validated candidate.

• Files reviewed: 26/32 changed files
• Comments generated: 4
• Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.