From 3c7075ab3fa27a3292d77765e3a8c55ac0c4992a Mon Sep 17 00:00:00 2001 From: 1bcMax Date: Mon, 3 Aug 2026 23:41:44 -0500 Subject: [PATCH 1/2] fix(core): a provider wallet could take over x402 payment signing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveFromFiles() consulted ~/./wallet.json before the canonical ~/.blockrun/.session and returned the most recently modified one. Installing another product — or writing one file into the home directory — changed which key resolvePrivateKey() handed to payment signing, which is every paid path in the CLI: api, pay, chat, run, image, video, music, speech, and the data commands. scanWallets() compounded it by reporting each file's self-declared "address" field rather than deriving it. A planted file could therefore name the user's own address while holding a different key, so `blockrun wallet recover` printed the real address twice and gave no indication that the active signer had changed. Demonstrated against the pre-fix build: $ blockrun --json wallet {"address":"0x7099…79C8","source":"provider"} <- attacker's key, not the user's $ blockrun --json wallet recover [{"source":"provider wallet.json","address":"0xf39Fd…92266"}, <- claimed, not derived {"source":"session","address":"0xf39Fd…92266"}] This is the defect blockrun-llm-ts#14 fixed on 2026-07-19. Core kept the pre-fix behavior for three weeks while its header comment claimed to mirror the SDK; its README already documented the correct order, so the contract was right and only the implementation was wrong. - resolveFromFiles() reads .session -> legacy only; discovered wallets never participate in automatic resolution. - scanWallets() derives every address from the discovered key and drops entries whose key is missing or unusable. The file's "address" field is no longer trusted anywhere in the package. - WalletSource drops "provider", which is now unreachable. Breaking for callers narrowing on it. - New listDiscoveredWallets() and adoptWallet(address) give the deliberate migration path the SDK gained in 3.8.0. Adoption matches on the derived address and backs up the outgoing .session first. - CLI gains `wallet list` and `wallet adopt
`; `wallet recover` now orders by true resolution priority and marks exactly one entry active. Both halves are mutation-verified: restoring provider-first resolution turns 4 tests red, and trusting the file's address field turns 3 red, including the one proving a planted address cannot be adopted. 49/49 pass, typecheck clean. --- CHANGELOG.md | 34 +++++++ packages/cli/src/cli.ts | 45 +++++++-- packages/cli/test/commands.test.ts | 44 +++++++++ packages/core/README.md | 13 +++ packages/core/package.json | 2 +- packages/core/src/wallet.ts | 110 +++++++++++++++++++--- packages/core/test/wallet.test.ts | 142 +++++++++++++++++++++++++---- 7 files changed, 352 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06c4eaf..37f754c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,40 @@ All notable changes to the BlockRun CLI are documented here. +## Unreleased — `@blockrun/core` 0.1.0 + +### Security: a provider wallet could take over payment signing + +`resolveFromFiles()` consulted `~/./wallet.json` files **before** the canonical +`~/.blockrun/.session`, and returned the most recently modified one. Installing another +product — or writing a single file into the home directory — therefore changed which key +`resolvePrivateKey()` handed to x402 payment signing, across `blockrun api`, `pay`, `chat`, +`run`, `image`, `video`, `music`, `speech`, and the data commands. `scanWallets()` also +reported each file's self-declared `address` field, so `blockrun wallet recover` would +display an address the file held no key for. + +This is the same defect fixed in `@blockrun/llm` on 2026-07-19 +([blockrun-llm-ts#14](https://github.com/BlockRunAI/blockrun-llm-ts/pull/14)); core kept the +pre-fix behavior while its own header comment claimed to mirror the SDK. Core's README +already documented the correct order — the implementation, not the contract, was wrong. + +- `resolveFromFiles()` now reads `.session` → legacy `wallet.key` only. Discovered provider + wallets never participate in automatic resolution. +- `scanWallets()` derives each address from the discovered private key and drops entries + whose key is missing or unusable. The file's `address` field is no longer trusted anywhere. +- `WalletSource` no longer includes `"provider"` — after this change it was never a reachable + resolution result. **Breaking** for anything narrowing on that member. +- Added `listDiscoveredWallets()` (addresses + source paths, no private keys) and + `adoptWallet(address)`, the deliberate migration path. Adoption matches on the *derived* + address and backs up the outgoing `.session` first, so funds are never stranded. + +### CLI + +- Added `blockrun wallet list` and `blockrun wallet adopt
`. +- `blockrun wallet recover` now reports entries in true resolution order, marks exactly one + `active`, and lists discovered provider wallets as inactive with their file path. Its + `meta.active` previously named a provider wallet that resolution would not actually use. + ## 0.1.1 — 2026-07-17 - Fixed the globally installed `blockrun` executable: npm creates a symlink for diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 1a2cfed..239fbc5 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -15,8 +15,9 @@ import { loadWallet, createWallet, importWallet, + adoptWallet, + listDiscoveredWallets, addressFromKey, - scanWallets, resolveChain, paths, ok, @@ -130,18 +131,46 @@ export function runCore(command: string, args: ParsedArgs): Envelope { } return ok({ address: w.address, privateKey: w.privateKey, source: w.source }); } + if (sub === "list") { + // Wallets belonging to other applications. None of these are active. + const discovered = listDiscoveredWallets(); + if (!discovered.length) return err("wallet", "no wallets discovered from other applications", 404); + return ok( + discovered.map((w) => ({ address: w.address, source: w.source, active: false })), + { hint: "adopt one deliberately with `blockrun wallet adopt
`" } + ); + } + if (sub === "adopt") { + const address = args.rest[1]?.trim(); + if (!address) return err("usage", "usage: blockrun wallet adopt
", 400); + try { + const w = adoptWallet(address); + return ok({ address: w.address, adopted: true }, { hint: "previous wallet backed up in ~/.blockrun/" }); + } catch (e) { + return err("wallet", (e as Error).message, 404); + } + } if (sub === "recover") { - // Show every wallet the resolver can currently see, in priority order. - const found: Array> = []; + // Every key on this machine, in true resolution order. Only env/session/legacy + // can ever be active; discovered provider wallets are shown but never selected. + const found: Array> = []; const env = process.env.BLOCKRUN_WALLET_KEY || process.env.BASE_CHAIN_WALLET_KEY; if (env) found.push({ source: "env", address: safeAddr(env) }); - for (const s of scanWallets()) found.push({ source: "provider wallet.json", address: s.address }); const p = paths(); for (const [src, file] of [["session", p.session], ["legacy", p.legacy]] as const) { if (fs.existsSync(file)) found.push({ source: src, address: safeAddr(fs.readFileSync(file, "utf8").trim()) }); } - if (!found.length) return err("wallet", "no recoverable wallets found (env, provider, session, legacy)", 404); - return ok(found, { active: found[0].source }); + // The resolver stops at the first of the above; everything after is inactive. + const activeSource = found.length ? (found[0].source as string) : null; + for (const [i, entry] of found.entries()) entry.active = i === 0; + for (const w of listDiscoveredWallets()) { + found.push({ source: `provider wallet.json (${w.source})`, address: w.address, active: false }); + } + if (!found.length) return err("wallet", "no recoverable wallets found (env, session, legacy, provider)", 404); + return ok(found, { + active: activeSource, + ...(activeSource ? {} : { hint: "no active wallet — run `blockrun wallet create` or `wallet adopt
`" }), + }); } const w = loadWallet(); if (!w) return err("wallet", "No wallet found. Run `blockrun wallet create`.", 404); @@ -160,7 +189,9 @@ Usage: blockrun [--json|--format ] [--chain base|sol] [args] Wallet & status status wallet + chain overview - wallet [create|import |export --yes|recover] + wallet [create|import |export --yes|list|adopt
|recover] + (list/adopt: wallets from other apps — never active + until you adopt one deliberately) balance USDC balance for the active wallet fund funding address + links chain [base|sol] show or set the payment chain diff --git a/packages/cli/test/commands.test.ts b/packages/cli/test/commands.test.ts index d5ae0fe..a60b892 100644 --- a/packages/cli/test/commands.test.ts +++ b/packages/cli/test/commands.test.ts @@ -74,3 +74,47 @@ test("wallet import → recover → export --yes round-trip", async () => { const rows = rec.ok ? (rec.data as Array<{ source: string; address: string }>) : []; assert.ok(rows.some((r) => r.source === "session" && r.address === ADDR_B)); }); + +test("a discovered provider wallet is listed but never active until adopted", async () => { + const args = (rest: string[]) => ({ ...parseArgs(["wallet", ...rest]), rest }); + // Session currently holds KEY_B/ADDR_B from the round-trip above. + const provDir = path.join(tmp, ".agentcash"); + fs.mkdirSync(provDir, { recursive: true }); + fs.writeFileSync(path.join(provDir, "wallet.json"), JSON.stringify({ privateKey: KEY, address: ADDR })); + + // `wallet` still reports the canonical session wallet, not the newer provider file. + const active = await runCoreCommand("wallet", args([])); + assert.equal(active.ok && (active.data as { address: string }).address, ADDR_B); + + // `wallet list` surfaces it, explicitly inactive. + const listed = await runCoreCommand("wallet", args(["list"])); + const found = listed.ok ? (listed.data as Array<{ address: string; active: boolean }>) : []; + assert.deepEqual( + found.map((w) => [w.address, w.active]), + [[ADDR, false]] + ); + + // `recover` marks session active and the provider entry not. + const rec = await runCoreCommand("wallet", args(["recover"])); + const rows = rec.ok ? (rec.data as Array<{ source: string; address: string; active: boolean }>) : []; + assert.equal(rows.find((r) => r.source === "session")?.active, true); + assert.equal(rows.find((r) => r.source.startsWith("provider"))?.active, false); + assert.equal(rec.ok && rec.meta?.active, "session"); + + // Adoption is the deliberate act that switches it. + const adopted = await runCoreCommand("wallet", args(["adopt", ADDR])); + assert.deepEqual(adopted.ok && adopted.data, { address: ADDR, adopted: true }); + const after = await runCoreCommand("wallet", args([])); + assert.equal(after.ok && (after.data as { address: string }).address, ADDR); +}); + +test("wallet adopt refuses an address no discovered key controls", async () => { + const args = (rest: string[]) => ({ ...parseArgs(["wallet", ...rest]), rest }); + const evil = path.join(tmp, ".evil"); + fs.mkdirSync(evil, { recursive: true }); + // Claims ADDR_B, but holds a key that derives to ADDR. + fs.writeFileSync(path.join(evil, "wallet.json"), JSON.stringify({ privateKey: KEY, address: ADDR_B })); + + const res = await runCoreCommand("wallet", args(["adopt", ADDR_B])); + assert.equal(res.ok, false); +}); diff --git a/packages/core/README.md b/packages/core/README.md index fd98189..200145a 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -32,6 +32,19 @@ import { loadWallet, resolvePrivateKey } from "@blockrun/core"; const w = loadWallet(); // { address, privateKey, source } | null — key never leaves the machine ``` +**Wallets from other applications are never adopted automatically.** `~/./wallet.json` +files are discoverable, but installing another product — or dropping a file into the home +directory — must not be able to change which key BlockRun signs payments with. Adoption is +an explicit act, and matching is done on the address *derived from the discovered key*, so a +file cannot claim an address it holds no key for: + +```ts +import { listDiscoveredWallets, adoptWallet } from "@blockrun/core"; + +listDiscoveredWallets(); // [{ address, source }] — no private keys, nothing active +adoptWallet("0x…"); // copies it to .session, backing up the outgoing wallet first +``` + ### Config (`@blockrun/core/config`) `~/.blockrun` path resolution (override with `BLOCKRUN_HOME`) and chain selection (`resolveChain`). diff --git a/packages/core/package.json b/packages/core/package.json index 9e942c4..8a0f597 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@blockrun/core", - "version": "0.0.3", + "version": "0.1.0", "description": "Shared kernel for all BlockRun products — wallet, x402 payment, config, and the agent-native JSON output contract.", "type": "module", "license": "MIT", diff --git a/packages/core/src/wallet.ts b/packages/core/src/wallet.ts index c91c3aa..864e3de 100644 --- a/packages/core/src/wallet.ts +++ b/packages/core/src/wallet.ts @@ -8,6 +8,12 @@ * 2. ~/.blockrun/.session * 3. ~/.blockrun/wallet.key (legacy) * + * Wallets belonging to OTHER applications (`~/./wallet.json`) are discoverable + * but are never resolved automatically. Installing another product must not silently + * change which key BlockRun signs payments with, and a wallet file dropped into a + * home directory must not be able to redirect spending to an address the user does + * not control. Adoption is always a deliberate act — see `adoptWallet()`. + * * The private key is only ever read locally and used to derive the address / * sign x402 payments — it is never sent to a server. */ @@ -17,7 +23,14 @@ import * as path from "node:path"; import { privateKeyToAccount, generatePrivateKey } from "viem/accounts"; import { paths, homeDir } from "./config.js"; -export type WalletSource = "env" | "provider" | "session" | "legacy"; +/** + * Where a resolved key came from. + * + * There is deliberately no `provider` member: a discovered provider wallet is + * never the result of resolution. Adopting one copies it into `.session`, so from + * then on it resolves as `session`. + */ +export type WalletSource = "env" | "session" | "legacy"; export interface ResolvedKey { privateKey: `0x${string}`; @@ -30,6 +43,14 @@ export interface WalletInfo { source: WalletSource; } +/** A wallet found in another application's directory. Never active until adopted. */ +export interface DiscoveredWallet { + /** Address derived from the discovered key — never the file's `address` field. */ + address: `0x${string}`; + /** Absolute path of the `wallet.json` it came from. */ + source: string; +} + function normalize(raw: string): `0x${string}` { const k = raw.trim(); return (k.startsWith("0x") ? k : `0x${k}`) as `0x${string}`; @@ -59,12 +80,18 @@ export function importWallet(raw: string, opts: { force?: boolean } = {}): Walle /** * Scan `~/./wallet.json` files from any provider (agentcash, etc.), each - * holding `{ privateKey, address }`, most-recently-modified first. Ported - * verbatim from @blockrun/llm so the canonical resolution order is identical. + * holding `{ privateKey, address }`, most-recently-modified first. + * + * The returned `address` is derived from the discovered private key, NOT read from + * the file's `address` field — a wallet file cannot claim an address it holds no + * key for. Entries whose key is missing or unusable are dropped entirely. + * + * Nothing here is active. This is discovery for an explicit migration flow only; + * it must never influence automatic resolution. */ -export function scanWallets(): Array<{ privateKey: string; address: string }> { +export function scanWallets(): Array<{ privateKey: string; address: `0x${string}`; source: string }> { const home = homeDir(); - const results: Array<{ mtime: number; privateKey: string; address: string }> = []; + const results: Array<{ mtime: number; privateKey: string; address: `0x${string}`; source: string }> = []; try { for (const entry of fs.readdirSync(home, { withFileTypes: true })) { if (!entry.name.startsWith(".") || !entry.isDirectory()) continue; @@ -73,8 +100,10 @@ export function scanWallets(): Array<{ privateKey: string; address: string }> { try { const data = JSON.parse(fs.readFileSync(walletFile, "utf-8")); const pk = data.privateKey || ""; - const addr = data.address || ""; - if (pk && addr) results.push({ mtime: fs.statSync(walletFile).mtimeMs, privateKey: pk, address: addr }); + if (!pk) continue; + const derived = addressFromKey(pk); + if (!derived) continue; + results.push({ mtime: fs.statSync(walletFile).mtimeMs, privateKey: pk, address: derived, source: walletFile }); } catch { continue; } @@ -83,13 +112,70 @@ export function scanWallets(): Array<{ privateKey: string; address: string }> { /* ignore */ } results.sort((a, b) => b.mtime - a.mtime); - return results.map(({ privateKey, address }) => ({ privateKey, address })); + return results.map(({ privateKey, address, source }) => ({ privateKey, address, source })); } -/** Resolve a key from files only (no env): provider wallet.json → .session → legacy. */ +/** + * List wallets belonging to other applications, safe to show to a user. + * + * Same discovery as `scanWallets()` but without private keys, so it can be printed + * or returned over a boundary. Adopt one deliberately with `adoptWallet()`. + */ +export function listDiscoveredWallets(): DiscoveredWallet[] { + return scanWallets().map(({ address, source }) => ({ address, source })); +} + +/** + * Adopt a discovered wallet by address, making it the active BlockRun wallet. + * + * This is the deliberate migration path that automatic resolution refuses to take. + * Matching is done against the address *derived from each discovered key*, so a + * wallet file claiming someone else's address can never be selected by it. + * + * The outgoing `~/.blockrun/.session` is backed up beside itself before being + * overwritten, so adopting a wallet cannot strand funds in the old one. + * + * @param address Address to adopt, as listed by `listDiscoveredWallets()` + * @throws If no discovered wallet derives to that address + */ +export function adoptWallet(address: string): WalletInfo { + const wanted = address.trim().toLowerCase(); + const p = paths(); + + for (const entry of scanWallets()) { + if (entry.address.toLowerCase() !== wanted) continue; + + const privateKey = normalize(entry.privateKey); + + // Preserve the outgoing wallet — it may hold funds. + if (fs.existsSync(p.session)) { + const current = fs.readFileSync(p.session, "utf8").trim(); + if (current && normalize(current) !== privateKey) { + const backup = path.join(p.dir, `.session.backup-${Math.floor(Date.now() / 1000)}`); + fs.writeFileSync(backup, current, { mode: 0o600 }); + } + } + + fs.mkdirSync(p.dir, { recursive: true }); + fs.writeFileSync(p.session, privateKey, { mode: 0o600 }); + return { address: entry.address, privateKey, source: "session" }; + } + + const available = listDiscoveredWallets().map((w) => w.address); + throw new Error( + `No discovered wallet controls ${address}. ` + + `Available: ${available.length ? available.join(", ") : "none"}` + ); +} + +/** + * Resolve a key from files only (no env): `.session` → legacy. + * + * Provider `wallet.json` files are deliberately NOT consulted. The canonical + * BlockRun wallet always wins; another application's wallet is adopted only + * through `adoptWallet()`. + */ export function resolveFromFiles(): ResolvedKey | null { - const scanned = scanWallets(); - if (scanned.length > 0) return { privateKey: normalize(scanned[0].privateKey), source: "provider" }; const p = paths(); if (fs.existsSync(p.session)) { const raw = fs.readFileSync(p.session, "utf8").trim(); @@ -105,7 +191,7 @@ export function resolveFromFiles(): ResolvedKey | null { /** * Find the private key, or null if none exists. Canonical BlockRun order, * matching @blockrun/llm's getOrCreateWallet: - * env (BLOCKRUN_WALLET_KEY|BASE_CHAIN_WALLET_KEY) → provider wallet.json → .session → legacy + * env (BLOCKRUN_WALLET_KEY|BASE_CHAIN_WALLET_KEY) → .session → legacy */ export function resolvePrivateKey(env: NodeJS.ProcessEnv = process.env): ResolvedKey | null { const fromEnv = env.BLOCKRUN_WALLET_KEY || env.BASE_CHAIN_WALLET_KEY; diff --git a/packages/core/test/wallet.test.ts b/packages/core/test/wallet.test.ts index 45c24c0..9a0c36e 100644 --- a/packages/core/test/wallet.test.ts +++ b/packages/core/test/wallet.test.ts @@ -1,13 +1,21 @@ -import { test, before, after } from "node:test"; +import { test, before, after, beforeEach } from "node:test"; import assert from "node:assert/strict"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; -import { resolvePrivateKey, loadWallet, scanWallets } from "../src/wallet.js"; +import { + resolvePrivateKey, + loadWallet, + scanWallets, + listDiscoveredWallets, + adoptWallet, + addressFromKey, +} from "../src/wallet.js"; -// Well-known Hardhat account #0 — deterministic key → address. +// Well-known Hardhat accounts — deterministic key → address. const KEY = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"; const ADDR = "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266"; +const OTHER_KEY = "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d"; let tmp: string; const savedEnv = { ...process.env }; @@ -24,38 +32,136 @@ after(() => { fs.rmSync(tmp, { recursive: true, force: true }); }); +/** Each test starts from an empty home so ordering can't hide a regression. */ +beforeEach(() => { + for (const entry of fs.readdirSync(tmp)) { + fs.rmSync(path.join(tmp, entry), { recursive: true, force: true }); + } +}); + +function writeSession(key: string): void { + const dir = path.join(tmp, ".blockrun"); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, ".session"), key); +} + +/** Plant a provider wallet.json. `claimed` fakes the file's self-reported address. */ +function writeProvider(dirName: string, key: string, claimed?: string): string { + const dir = path.join(tmp, dirName); + fs.mkdirSync(dir, { recursive: true }); + const file = path.join(dir, "wallet.json"); + fs.writeFileSync(file, JSON.stringify({ privateKey: key, address: claimed ?? addressFromKey(key) })); + return file; +} + test("no wallet → null", () => { assert.equal(resolvePrivateKey(process.env), null); assert.equal(loadWallet(process.env), null); }); test("~/.blockrun/.session is read as source=session", () => { - const dir = path.join(tmp, ".blockrun"); - fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync(path.join(dir, ".session"), KEY); + writeSession(KEY); const r = resolvePrivateKey(process.env); assert.equal(r?.source, "session"); assert.equal(loadWallet(process.env)?.address, ADDR); }); test("env key wins over the session file (source=env)", () => { + writeSession(KEY); const r = resolvePrivateKey({ ...process.env, BLOCKRUN_WALLET_KEY: KEY }); assert.equal(r?.source, "env"); }); -test("provider wallet.json is scanned and wins over .session (source=provider)", () => { - // ~/.someprovider/wallet.json under the isolated BLOCKRUN_HOME - const provDir = path.join(tmp, ".agentcash"); - fs.mkdirSync(provDir, { recursive: true }); - fs.writeFileSync(path.join(provDir, "wallet.json"), JSON.stringify({ privateKey: KEY, address: ADDR })); - assert.equal(scanWallets()[0]?.address, ADDR); - const r = resolvePrivateKey(process.env); // no env → provider beats .session - assert.equal(r?.source, "provider"); - // cleanup so later tests see the .session path again - fs.rmSync(provDir, { recursive: true, force: true }); -}); - test("normalizes a key without 0x prefix", () => { const r = resolvePrivateKey({ BLOCKRUN_WALLET_KEY: KEY.slice(2) } as NodeJS.ProcessEnv); assert.equal(r?.privateKey, KEY); }); + +// --- Canonical wallet selection (blockrun-llm-ts#14) --------------------------- +// +// A provider wallet.json must never become the active wallet on its own. Before +// this was fixed, installing another product — or dropping a file into the home +// directory — silently redirected x402 payment signing. + +test("a newer provider wallet.json does NOT displace .session", () => { + writeSession(KEY); + writeProvider(".agentcash", OTHER_KEY); // written after .session, so strictly newer + + const r = resolvePrivateKey(process.env); + assert.equal(r?.source, "session"); + assert.equal(r?.privateKey, KEY); + assert.equal(loadWallet(process.env)?.address, ADDR); +}); + +test("a provider wallet.json alone resolves to nothing, not to itself", () => { + writeProvider(".agentcash", OTHER_KEY); + + assert.equal(resolvePrivateKey(process.env), null); + assert.equal(loadWallet(process.env), null); + // ...but it is still discoverable for a deliberate migration. + assert.equal(listDiscoveredWallets().length, 1); +}); + +test("legacy wallet.key is still read when no .session exists", () => { + const dir = path.join(tmp, ".blockrun"); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, "wallet.key"), KEY); + writeProvider(".agentcash", OTHER_KEY); + + const r = resolvePrivateKey(process.env); + assert.equal(r?.source, "legacy"); // legacy still beats a discovered wallet +}); + +// --- Planted address fields --------------------------------------------------- + +test("scanWallets derives the address instead of trusting the file", () => { + writeProvider(".evil", KEY, "0x000000000000000000000000000000000000dEaD"); + + const [found] = scanWallets(); + assert.equal(found.address, ADDR); + assert.notEqual(found.address, "0x000000000000000000000000000000000000dEaD"); +}); + +test("a wallet.json with an unusable key is dropped entirely", () => { + writeProvider(".broken", "not-a-key", ADDR); + assert.deepEqual(scanWallets(), []); + assert.deepEqual(listDiscoveredWallets(), []); +}); + +test("listDiscoveredWallets never returns private keys", () => { + writeProvider(".agentcash", KEY); + const [w] = listDiscoveredWallets(); + assert.equal(w.address, ADDR); + assert.ok(!Object.prototype.hasOwnProperty.call(w, "privateKey")); + assert.ok(w.source.endsWith("wallet.json")); +}); + +// --- Deliberate adoption ------------------------------------------------------ + +test("adoptWallet makes a discovered wallet active and backs up the old one", () => { + writeSession(OTHER_KEY); + writeProvider(".agentcash", KEY); + + const adopted = adoptWallet(ADDR); + assert.equal(adopted.address, ADDR); + assert.equal(adopted.source, "session"); + + // It is now what resolution returns. + assert.equal(resolvePrivateKey(process.env)?.privateKey, KEY); + + // The outgoing key survived, so its funds are not stranded. + const backups = fs.readdirSync(path.join(tmp, ".blockrun")).filter((f) => f.startsWith(".session.backup-")); + assert.equal(backups.length, 1); + assert.equal(fs.readFileSync(path.join(tmp, ".blockrun", backups[0]), "utf8").trim(), OTHER_KEY); +}); + +test("adoptWallet refuses an address no discovered key controls", () => { + writeProvider(".evil", OTHER_KEY, ADDR); // file claims ADDR but holds a different key + assert.throws(() => adoptWallet(ADDR), /No discovered wallet controls/); + // ...and nothing was activated. + assert.equal(resolvePrivateKey(process.env), null); +}); + +test("adoptWallet on an empty machine reports that nothing is available", () => { + assert.throws(() => adoptWallet(ADDR), /Available: none/); +}); From 0b904ea05f0f1014b4bab076f4b094922133d7b9 Mon Sep 17 00:00:00 2001 From: 1bcMax Date: Mon, 3 Aug 2026 23:46:14 -0500 Subject: [PATCH 2/2] ci: verify the code under review, not the last published core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The smoke test packed only the CLI, so npm resolved @blockrun/core from the registry. Two consequences: the step never exercised core changes at all — which is how core drifted three weeks behind the SDK without CI noticing — and any core version bump failed with ETARGET until it had already been published, making the fix in this branch unmergeable on its own. Pack both packages and install them as roots in the same global prefix, so the CLI resolves the core built from this commit. `blockrun --json version` now reports core 0.1.0 from the tarball rather than 0.0.3 from npm. The step also asserts the property this branch fixes, against the packed artifact users actually install: a provider wallet.json must not displace .session, and an address no discovered key controls must not be adoptable. Confirmed to fail against the pre-fix build — the vulnerable CLI reports {"address":"0x7099…79C8","source":"provider"} for that fixture. --- .github/workflows/ci.yml | 35 ++++++++++++++++++++++++++++++++++- .gitignore | 2 ++ CHANGELOG.md | 11 +++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ce6ef4..0078749 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,39 @@ jobs: - name: Published-artifact smoke test run: | mkdir -p .artifacts .smoke-prefix + # Pack core too and install both tarballs as roots. Installing only the CLI + # made npm resolve @blockrun/core from the registry, so this step verified the + # LAST PUBLISHED core rather than the code under review — and any core version + # bump failed here with ETARGET until it had already shipped. + (cd packages/core && pnpm pack --pack-destination ../../.artifacts) (cd packages/cli && pnpm pack --pack-destination ../../.artifacts) - npm install --global --prefix "$PWD/.smoke-prefix" .artifacts/blockrun-cli-*.tgz + npm install --global --prefix "$PWD/.smoke-prefix" \ + .artifacts/blockrun-core-*.tgz .artifacts/blockrun-cli-*.tgz .smoke-prefix/bin/blockrun --json version + + - name: Packed artifact honours canonical wallet selection + run: | + # A provider wallet.json must never displace ~/.blockrun/.session, and must + # never be reported under an address it holds no key for. Asserted against the + # packed artifact because that is what users actually install. + H="$(mktemp -d)" + mkdir -p "$H/.blockrun" "$H/.other" + # Hardhat account #0 — the user's real wallet. + echo "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" > "$H/.blockrun/.session" + sleep 1 # ensure the planted file is strictly newer + # Hardhat account #1's key, falsely claiming account #0's address. + echo '{"privateKey":"0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d","address":"0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266"}' > "$H/.other/wallet.json" + + active=$(BLOCKRUN_HOME="$H" BLOCKRUN_WALLET_KEY= BASE_CHAIN_WALLET_KEY= \ + .smoke-prefix/bin/blockrun --json wallet) + echo "$active" + echo "$active" | grep -q '"address":"0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266","source":"session"' \ + || { echo "::error::a provider wallet.json displaced the canonical wallet"; exit 1; } + + # The planted address must not be adoptable either. err() writes the envelope + # to stderr and exits non-zero, so capture both streams. + adopt=$(BLOCKRUN_HOME="$H" BLOCKRUN_WALLET_KEY= BASE_CHAIN_WALLET_KEY= \ + .smoke-prefix/bin/blockrun --json wallet adopt 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 2>&1 || true) + echo "$adopt" + echo "$adopt" | grep -q '"ok":false' \ + || { echo "::error::adopted an address no discovered key controls"; exit 1; } diff --git a/.gitignore b/.gitignore index df3c9d2..b59096d 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ dist/ *.tgz *.tsbuildinfo .DS_Store +.artifacts/ +.smoke-prefix/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 37f754c..ddcc19f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,17 @@ already documented the correct order — the implementation, not the contract, w `adoptWallet(address)`, the deliberate migration path. Adoption matches on the *derived* address and backs up the outgoing `.session` first, so funds are never stranded. +### CI + +- The published-artifact smoke test packed only the CLI, so npm resolved + `@blockrun/core` from the registry. It was therefore verifying the **last published** + core rather than the code under review — which is how core drifted three weeks behind + the SDK unnoticed — and any core version bump failed the step with `ETARGET` until it + had already shipped. Both packages are now packed and installed as roots. +- The smoke step now asserts the security property against the packed artifact: a + provider `wallet.json` must not displace `.session`, and an address no discovered key + controls must not be adoptable. Verified to fail against the pre-fix build. + ### CLI - Added `blockrun wallet list` and `blockrun wallet adopt
`.