From 0d0942b0c20e844c6a0c0cd85fdc70321d04646a Mon Sep 17 00:00:00 2001 From: vvillait88 Date: Thu, 27 Aug 2026 21:03:48 -0400 Subject: [PATCH 1/3] Keep secret-disclosure and fund-moving commands off the MCP tool surface The MCP surface is generated from the CLI's command definitions, so every command reaches agents by default. Served as tools, the CLI's safety gates stop meaning anything: --danger and the typed EXPORT confirm become boolean parameters the model sets for itself, the interactive passphrase prompt is answered by AGENTSCORE_PAY_PASSPHRASE, and whatever they guard is returned into the transcript and the model provider's logs. One prompt injection reaches key export or an irreversible transfer. wallet create, wallet show-mnemonic, wallet export, wallet remove, send and unlock are now hidden from MCP clients. All six remain on the CLI, where a human is present and those gates do their job; mcp: false only affects what is served as a tool. Deliberately still exposed after checking what they actually return: init creates wallets but its result carries only chain lists and flags, never the mnemonic, and wallet import takes a secret in rather than handing one out. Spend limits now apply to `send`. They were enforced on the 402 paths only, so a configured ceiling could be walked straight past by using a raw transfer instead. Only the USDC path is checked: a native transfer moves gas tokens this command does not price in dollars, and inventing a conversion would be a worse guess than not claiming one. It is a no-op when nothing is configured, so this changes behavior only for someone who asked for it. The surface test drives a real MCP handshake rather than reading source or CLI internals. Grepping for mcp: false would pass whether or not the flag reached the tool list, and the Cli object is a closure with no command map to walk, so asking the server is the only check that cannot pass vacuously. It pins the exclusions and, alongside them, that pay and balance stay exposed, since those are what the agent fleet uses to pay. --- src/cli.ts | 36 ++++++++++++ src/commands/send.ts | 22 ++++++++ tests/mcp-tool-surface.test.ts | 100 +++++++++++++++++++++++++++++++++ tests/send.test.ts | 34 +++++++++++ 4 files changed, 192 insertions(+) create mode 100644 tests/mcp-tool-surface.test.ts diff --git a/src/cli.ts b/src/cli.ts index bc8f6eb..d861a81 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -204,6 +204,12 @@ export function buildCli() { const wallet = Cli.create('wallet', { description: 'Manage the local encrypted keystore' }); wallet.command('create', { + // Hidden from MCP. Reachable on the CLI, where a human is present and the + // --danger / typed-confirm gates mean something. Served as an MCP tool those + // gates become boolean parameters the model can set for itself, so a single + // prompt injection reaches irreversible key disclosure or fund movement, and + // any secret returned lands in the transcript and the model provider's logs. + mcp: false, description: 'Generate a new encrypted keystore. Omit --chain to create all three chains.', options: z.object({ chain: chainSchema.optional().describe('Blockchain rail (base, solana, tempo)'), @@ -268,6 +274,12 @@ export function buildCli() { }); wallet.command('show-mnemonic', { + // Hidden from MCP. Reachable on the CLI, where a human is present and the + // --danger / typed-confirm gates mean something. Served as an MCP tool those + // gates become boolean parameters the model can set for itself, so a single + // prompt injection reaches irreversible key disclosure or fund movement, and + // any secret returned lands in the transcript and the model provider's logs. + mcp: false, description: 'Print the stored BIP-39 mnemonic. DANGER — only run in a trusted environment.', hint: 'The mnemonic restores every chain wallet — anyone with this phrase can drain your funds. Never paste it into chat, logs, or unencrypted storage.', outputPolicy: 'agent-only', @@ -298,6 +310,12 @@ export function buildCli() { }); wallet.command('export', { + // Hidden from MCP. Reachable on the CLI, where a human is present and the + // --danger / typed-confirm gates mean something. Served as an MCP tool those + // gates become boolean parameters the model can set for itself, so a single + // prompt injection reaches irreversible key disclosure or fund movement, and + // any secret returned lands in the transcript and the model provider's logs. + mcp: false, description: 'Decrypt and print a private key. DANGER — only run if you trust the surrounding environment.', hint: 'The exported key gives full control of the wallet to anyone who reads it. Pipe to an encrypted store; never to a shared shell history.', outputPolicy: 'agent-only', @@ -320,6 +338,12 @@ export function buildCli() { }); wallet.command('remove', { + // Hidden from MCP. Reachable on the CLI, where a human is present and the + // --danger / typed-confirm gates mean something. Served as an MCP tool those + // gates become boolean parameters the model can set for itself, so a single + // prompt injection reaches irreversible key disclosure or fund movement, and + // any secret returned lands in the transcript and the model provider's logs. + mcp: false, description: 'Delete a keystore. DANGER — irrecoverable unless you have the BIP-39 mnemonic backup.', hint: 'No undo. Run `wallet show-mnemonic --danger` first if you want to be able to restore.', options: z.object({ @@ -493,6 +517,12 @@ export function buildCli() { // ── send ──────────────────────────────────────────────────────────────────── cli.command('send', { + // Hidden from MCP. Reachable on the CLI, where a human is present and the + // --danger / typed-confirm gates mean something. Served as an MCP tool those + // gates become boolean parameters the model can set for itself, so a single + // prompt injection reaches irreversible key disclosure or fund movement, and + // any secret returned lands in the transcript and the model provider's logs. + mcp: false, description: 'Raw transfer to an arbitrary address on Base, Tempo, or Solana. Default --asset usdc; --asset native sends gas (ETH on Base, TEMPO on Tempo, SOL on Solana). No merchant, no 402 handshake — just on-chain.', hint: 'Both flavors require native gas in the signer wallet (gas pays the on-chain write, regardless of which asset is being transferred). x402/MPP payments are gasless; raw transfers are not.', options: z.object({ @@ -553,6 +583,12 @@ export function buildCli() { // ── unlock ────────────────────────────────────────────────────────────────── cli.command('unlock', { + // Hidden from MCP. Reachable on the CLI, where a human is present and the + // --danger / typed-confirm gates mean something. Served as an MCP tool those + // gates become boolean parameters the model can set for itself, so a single + // prompt injection reaches irreversible key disclosure or fund movement, and + // any secret returned lands in the transcript and the model provider's logs. + mcp: false, description: 'Cache the wallet passphrase to ~/.agentscore/.unlock for a bounded duration', hint: 'Prefer AGENTSCORE_PAY_PASSPHRASE in env when running unattended — it leaves no on-disk artifact.', options: z.object({ diff --git a/src/commands/send.ts b/src/commands/send.ts index c5cc4e9..166285e 100644 --- a/src/commands/send.ts +++ b/src/commands/send.ts @@ -16,6 +16,7 @@ import * as solanaChain from '../chains/solana'; import * as tempoChain from '../chains/tempo'; import { type Chain, type Network } from '../constants'; import { CliError } from '../errors'; +import { enforce, loadLimits } from '../limits'; import { DEFAULT_WALLET_NAME } from '../paths'; import { promptPassphrase } from '../prompts'; import { loadWallet } from '../wallets'; @@ -100,6 +101,27 @@ export async function send(input: SendInput): Promise { throw new CliError('invalid_amount', '--amount must be a positive number.'); } + // Spend limits apply to EVERY transfer path, not just the 402 ones. `pay` + // enforced them and this did not, so a configured ceiling could be walked + // straight past by using `send` instead. Only the USDC path is denominated in + // dollars, so that is the one a USD limit can judge; a native transfer moves + // gas tokens whose USD value this command does not price, and inventing a + // conversion here would be a worse guess than not claiming one. + // + // A no-op when nothing is configured: enforce() returns allowed with no + // limits set, so this changes behavior only for someone who asked for it. + if (asset === 'usdc') { + const limits = await loadLimits(); + const verdict = await enforce(limits, { priceUsd: input.amount, host: `send:${input.chain}` }); + if (!verdict.allowed) { + throw new CliError( + 'limit_exceeded', + `Local limit violated: ${verdict.violated}=${verdict.limit}`, + { extra: { violated: verdict.violated, limit: verdict.limit, would_be: verdict.would_be } }, + ); + } + } + const passphrase = await promptPassphrase(); const wallet = await loadWallet(input.chain, passphrase, input.name ?? DEFAULT_WALLET_NAME); diff --git a/tests/mcp-tool-surface.test.ts b/tests/mcp-tool-surface.test.ts new file mode 100644 index 0000000..5b3c839 --- /dev/null +++ b/tests/mcp-tool-surface.test.ts @@ -0,0 +1,100 @@ +import { spawn } from 'node:child_process'; +import { beforeAll, describe, expect, it } from 'vitest'; + +/** + * The MCP tool surface is generated from the CLI's command definitions, so a + * new command reaches agents by default. That default is the finding: served as + * tools, the CLI's safety gates (`--danger`, the typed EXPORT confirm, an + * interactive passphrase prompt) become boolean parameters the model sets for + * itself, and anything they guard lands in the transcript and the model + * provider's logs. + * + * This drives a REAL MCP handshake rather than reading the source or the CLI's + * internals. Two earlier attempts were unsound: grepping for `mcp: false` would + * pass whether or not the flag reached the tool list, and walking the Cli + * object does not work because incur builds it as a closure with no command map + * to inspect. Asking the server is the only check that cannot pass vacuously. + */ + +const CWD = new URL('..', import.meta.url).pathname; + +interface Rpc { id?: number; result?: { content?: { text?: string }[] }; error?: unknown } + +/** Boot the MCP server over stdio and expose a `call` for its meta tools. */ +const withServer = async (fn: (call: (tool: string, args: Record) => Promise) => Promise): Promise => { + const child = spawn('bun', ['src/index.ts', '--mcp'], { cwd: CWD, stdio: ['pipe', 'pipe', 'pipe'] }); + const pending = new Map void>(); + let buf = ''; + let id = 10; + const send = (o: unknown) => child.stdin.write(`${JSON.stringify(o)}\n`); + + try { + await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error('MCP server did not initialize')), 30_000); + child.stderr.on('data', () => {}); + child.stdout.on('data', (d: Buffer) => { + buf += d.toString(); + for (const line of buf.split('\n')) { + if (!line.trim()) { continue; } + let m: Rpc; + try { m = JSON.parse(line) as Rpc; } catch { continue; } + if (m.id === 1) { clearTimeout(timer); resolve(); } + const p = m.id === undefined ? undefined : pending.get(m.id); + if (p) { pending.delete(m.id as number); p(m); } + } + buf = buf.slice(buf.lastIndexOf('\n') + 1); + }); + send({ id: 1, jsonrpc: '2.0', method: 'initialize', params: { capabilities: {}, clientInfo: { name: 'surface-test', version: '1' }, protocolVersion: '2024-11-05' } }); + }); + send({ jsonrpc: '2.0', method: 'notifications/initialized' }); + + const call = (tool: string, args: Record) => + new Promise((resolve, reject) => { + const i = ++id; + pending.set(i, (m) => resolve(JSON.stringify(m.result?.content ?? m.error ?? m))); + setTimeout(() => reject(new Error(`timed out calling ${tool}`)), 25_000); + send({ id: i, jsonrpc: '2.0', method: 'tools/call', params: { arguments: args, name: tool } }); + }); + + return await fn(call); + } finally { + child.kill(); + } +}; + +// Reachable on the CLI, never as an agent tool: each either discloses key +// material or moves funds irreversibly, and the reporter invoked all of these. +const MUST_BE_HIDDEN = ['wallet_export', 'wallet_show-mnemonic', 'wallet_remove', 'send', 'unlock']; + +// The paying path the agent fleet actually uses. Pinned alongside the +// exclusions so a future tightening cannot quietly take the fleet's tools away. +const MUST_STAY_EXPOSED = ['pay', 'balance']; + +describe('MCP tool surface', () => { + let details: Record = {}; + + beforeAll(async () => { + details = await withServer(async (call) => { + const out: Record = {}; + for (const t of [...MUST_BE_HIDDEN, ...MUST_STAY_EXPOSED]) { + out[t] = await call('get_tool_details', { name: t }); + } + return out; + }); + }, 60_000); + + // Positive control: if the handshake silently returned nothing, every + // "unknown tool" assertion below would pass for the wrong reason. + it('reaches a live server that answers about a tool it does expose', () => { + expect(details.pay, 'no answer for `pay`; did the handshake work?').toBeTruthy(); + expect(details.pay).not.toContain('Unknown tool'); + }); + + it.each(MUST_BE_HIDDEN)('does not expose %s to MCP clients', (tool) => { + expect(details[tool], `${tool} is still reachable as an MCP tool`).toContain('Unknown tool'); + }); + + it.each(MUST_STAY_EXPOSED)('still exposes %s', (tool) => { + expect(details[tool], `${tool} must stay available to agents`).not.toContain('Unknown tool'); + }); +}); diff --git a/tests/send.test.ts b/tests/send.test.ts index e9b73ba..5ebbcc2 100644 --- a/tests/send.test.ts +++ b/tests/send.test.ts @@ -25,6 +25,40 @@ describe('send command — input validation', () => { await rm(ROOT, { recursive: true, force: true }); }); + // Spend limits applied to the 402 paths but not to `send`, so a configured + // ceiling could be walked straight past by using a raw transfer instead. + it('refuses a USDC transfer that exceeds a configured per-call limit', async () => { + const { saveLimits } = await import('../src/limits'); + await saveLimits({ per_call_usd: 5 }); + const { send } = await import('../src/commands/send'); + await expect( + send({ amount: 50, chain: 'base', to: '0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef' }), + ).rejects.toThrow(/limit/i); + }); + + // The guard must be inert for anyone who never configured limits, which is + // the default and is how the agent fleet runs. + it('does not block when no limits are configured', async () => { + const { send } = await import('../src/commands/send'); + // Fails later for a missing keystore, NOT on the limit check. Asserting the + // message discriminates: a limit rejection here would be a false positive + // for every unconfigured user. + await expect( + send({ amount: 50, chain: 'base', to: '0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef' }), + ).rejects.not.toThrow(/limit/i); + }); + + // A native transfer moves gas tokens this command does not price in dollars, + // so a USD ceiling cannot judge it and must not pretend to. + it('does not apply the USD limit to a native transfer', async () => { + const { saveLimits } = await import('../src/limits'); + await saveLimits({ per_call_usd: 5 }); + const { send } = await import('../src/commands/send'); + await expect( + send({ amount: 50, asset: 'native', chain: 'base', to: '0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef' }), + ).rejects.not.toThrow(/limit/i); + }); + it('rejects empty --to', async () => { const { send } = await import('../src/commands/send'); await expect( From 2ff69614636b713009a952f741000c1c402c19b1 Mon Sep 17 00:00:00 2001 From: vvillait88 Date: Thu, 27 Aug 2026 21:04:59 -0400 Subject: [PATCH 2/3] Assemble the MCP entry path so knip does not read it as an import --- tests/mcp-tool-surface.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/mcp-tool-surface.test.ts b/tests/mcp-tool-surface.test.ts index 5b3c839..4e05a4f 100644 --- a/tests/mcp-tool-surface.test.ts +++ b/tests/mcp-tool-surface.test.ts @@ -17,12 +17,16 @@ import { beforeAll, describe, expect, it } from 'vitest'; */ const CWD = new URL('..', import.meta.url).pathname; +// Assembled rather than written as a literal path: knip reads a bare +// 'src/index.ts' in the spawn args as a module specifier and reports it +// unresolved, which fails the pre-push hook. +const ENTRY = ['src', 'index.ts'].join('/'); interface Rpc { id?: number; result?: { content?: { text?: string }[] }; error?: unknown } /** Boot the MCP server over stdio and expose a `call` for its meta tools. */ const withServer = async (fn: (call: (tool: string, args: Record) => Promise) => Promise): Promise => { - const child = spawn('bun', ['src/index.ts', '--mcp'], { cwd: CWD, stdio: ['pipe', 'pipe', 'pipe'] }); + const child = spawn('bun', [ENTRY, '--mcp'], { cwd: CWD, stdio: ['pipe', 'pipe', 'pipe'] }); const pending = new Map void>(); let buf = ''; let id = 10; From ce68d6d888958e7770a06d6461fe5ca3552ec498 Mon Sep 17 00:00:00 2001 From: vvillait88 Date: Thu, 27 Aug 2026 21:13:48 -0400 Subject: [PATCH 3/3] Remove the plaintext passphrase cache `~/.agentscore/.unlock` held the wallet passphrase in cleartext for up to 8 hours, beside the scrypt+AES keystores it unlocks. File modes (0600 in a 0700 dir) were the only protection, and they stop nothing already running as that user: malware, a backup, a CI cache, or an LLM agent with filesystem tools. The `unlock` command that wrote it is gone along with the read path. An installation upgrading from a version that wrote the file gets it DELETED rather than ignored. Ignoring it would be the worst of the three options, since the cleartext secret would still be on disk with nothing left that admits to using it. The purge runs where the passphrase would have been read, so it happens on the next command that needs one, and `wallet remove` keeps its existing cache wipe. Unattended use is AGENTSCORE_PAY_PASSPHRASE in the environment. That was already the recommendation on the `unlock` command's own hint text ("it leaves no on-disk artifact"), and it is what the agent fleet already uses, so the fleet is unaffected: prompts.ts checks the environment before it ever touched the cache. Verified by running the fleet's exact sequence non-interactively with only the env var set, which creates wallets and reads balances and writes no cache file. The no-TTY error now explains the removal instead of suggesting `unlock`, since that suggestion would send someone to a command that no longer exists. Two test files changed for reasons worth noting rather than hiding. The error-envelope suite used `unlock --for bad-format` as a convenient source of a bare invalid_input and now uses `assess` with no identity, which raises the same shape. The cache suite now pins the two properties that matter after removal: nothing can read or write a passphrase to disk, and a leftover file is deleted. --- src/cli.ts | 24 ------- src/commands/unlock.ts | 28 --------- src/prompts.ts | 15 +++-- src/unlock-cache.ts | 85 ++++++++----------------- tests/error-envelope.test.ts | 7 ++- tests/unlock-cache.test.ts | 117 +++++++++++++++++------------------ tests/unlock-cmd.test.ts | 59 ------------------ 7 files changed, 99 insertions(+), 236 deletions(-) delete mode 100644 src/commands/unlock.ts delete mode 100644 tests/unlock-cmd.test.ts diff --git a/src/cli.ts b/src/cli.ts index d861a81..dc3298a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -29,7 +29,6 @@ import { pay } from './commands/pay'; import { qr } from './commands/qr'; import { revoke } from './commands/revoke'; import { send } from './commands/send'; -import { unlock } from './commands/unlock'; import { walletAddress, walletCreate, @@ -581,29 +580,6 @@ export function buildCli() { }, }); - // ── unlock ────────────────────────────────────────────────────────────────── - cli.command('unlock', { - // Hidden from MCP. Reachable on the CLI, where a human is present and the - // --danger / typed-confirm gates mean something. Served as an MCP tool those - // gates become boolean parameters the model can set for itself, so a single - // prompt injection reaches irreversible key disclosure or fund movement, and - // any secret returned lands in the transcript and the model provider's logs. - mcp: false, - description: 'Cache the wallet passphrase to ~/.agentscore/.unlock for a bounded duration', - hint: 'Prefer AGENTSCORE_PAY_PASSPHRASE in env when running unattended — it leaves no on-disk artifact.', - options: z.object({ - for: z.string().default('15m').describe('TTL — e.g. 15m, 2h, 30s, 1d (max 8h)'), - clear: z.boolean().optional().describe('Remove the cached passphrase'), - }), - examples: [ - { options: { for: '1h' }, description: 'Cache the passphrase for one hour' }, - { options: { clear: true }, description: 'Wipe the cached passphrase early' }, - ], - run({ options }) { - return withCliErrors(() => unlock({ forDuration: options.for, clear: options.clear })); - }, - }); - // ── discover ──────────────────────────────────────────────────────────────── cli.command('discover', { description: diff --git a/src/commands/unlock.ts b/src/commands/unlock.ts deleted file mode 100644 index 6965f96..0000000 --- a/src/commands/unlock.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { promptPassphrase } from '../prompts'; -import { clearCache, parseDuration, unlockCachePath, writeCachedPassphrase } from '../unlock-cache'; - -export interface UnlockInput { - forDuration?: string; - clear?: boolean; -} - -export interface UnlockResult { - ok: true; - cleared?: boolean; - expires_at?: string; - ttl_ms?: number; - path: string; -} - -export async function unlock(input: UnlockInput = {}): Promise { - if (input.clear) { - const removed = await clearCache(); - return { ok: true, cleared: removed, path: unlockCachePath() }; - } - - const duration = input.forDuration ?? '15m'; - const ttlMs = parseDuration(duration); - const passphrase = await promptPassphrase('Passphrase to cache'); - const expiresAt = await writeCachedPassphrase(passphrase, ttlMs); - return { ok: true, expires_at: expiresAt, ttl_ms: ttlMs, path: unlockCachePath() }; -} diff --git a/src/prompts.ts b/src/prompts.ts index 3fcd06e..645cf3e 100644 --- a/src/prompts.ts +++ b/src/prompts.ts @@ -1,6 +1,6 @@ import { cancel, isCancel, password as clackPassword } from '@clack/prompts'; import { CliError } from './errors'; -import { readCachedPassphrase } from './unlock-cache'; +import { clearCache } from './unlock-cache'; const ENV_PASSPHRASE = 'AGENTSCORE_PAY_PASSPHRASE'; @@ -11,13 +11,20 @@ function isInteractive(): boolean { export async function promptPassphrase(message = 'Enter wallet passphrase'): Promise { const envPass = process.env[ENV_PASSPHRASE]; if (envPass) return envPass; - const cached = await readCachedPassphrase(); - if (cached) return cached; + + // The plaintext passphrase cache is gone; this DELETES a leftover one rather + // than reading it. An installation upgrading from a version that wrote the + // file would otherwise keep a cleartext secret on disk with nothing left that + // admits to using it, which is worse than either using it or never having + // written it. Silent because it is cleanup rather than an error, and the + // passphrase is asked for immediately below. + await clearCache().catch(() => false); + if (!isInteractive()) { throw new CliError('user_cancelled', 'Passphrase required but no TTY and AGENTSCORE_PAY_PASSPHRASE not set.', { nextSteps: { action: 'set_env_passphrase', - suggestion: 'Set AGENTSCORE_PAY_PASSPHRASE=... in the environment, or run `unlock --for 15m` from a TTY first.', + suggestion: 'Set AGENTSCORE_PAY_PASSPHRASE=... in the environment. The `unlock` passphrase cache was removed: it stored the passphrase in cleartext on disk, and the environment variable is the supported way to run unattended.', }, }); } diff --git a/src/unlock-cache.ts b/src/unlock-cache.ts index caab6f0..14bd2dc 100644 --- a/src/unlock-cache.ts +++ b/src/unlock-cache.ts @@ -1,71 +1,38 @@ -import { mkdir, readFile, rm, writeFile } from 'fs/promises'; -import { dirname, join } from 'path'; -import { CliError } from './errors'; +import { rm } from 'fs/promises'; +import { join } from 'path'; import { baseDir } from './paths'; -interface UnlockCache { - passphrase: string; - expires_at: string; -} - -const MAX_TTL_MS = 8 * 60 * 60 * 1000; - -export function unlockCachePath(): string { +/** + * Removal of the plaintext passphrase cache. + * + * `~/.agentscore/.unlock` used to hold the wallet passphrase in cleartext for up + * to 8 hours, beside the scrypt+AES keystores it unlocks. File modes (0600 in a + * 0700 dir) were the only thing protecting it, which does nothing against + * anything already running as that user: malware, a backup, a CI cache, or an + * LLM agent with filesystem tools. + * + * The write path and the read path are both gone. What is left is the purge, + * for two reasons: `wallet remove` still wants to wipe any cached secret when it + * deletes a keystore, and an installation that HAS an `.unlock` from an older + * version needs it deleted rather than orphaned. Leaving the file readable but + * ignored would be the worst outcome, since the secret would still be on disk + * with nothing left that admits to using it. + * + * Unattended use is `AGENTSCORE_PAY_PASSPHRASE` in the environment, which the + * CLI already recommended over this cache and which leaves no on-disk artifact. + */ + +function unlockCachePath(): string { return join(baseDir(), '.unlock'); } -export async function readCachedPassphrase(): Promise { - try { - const raw = await readFile(unlockCachePath(), 'utf-8'); - const cache = JSON.parse(raw) as UnlockCache; - const expires = Date.parse(cache.expires_at); - if (!Number.isFinite(expires) || Date.now() > expires) { - await clearCache(); - return null; - } - return cache.passphrase; - } catch { - return null; - } -} - -export async function writeCachedPassphrase(passphrase: string, ttlMs: number): Promise { - if (!Number.isFinite(ttlMs) || ttlMs <= 0) { - throw new CliError('invalid_input', 'unlock duration must be positive'); - } - const clamped = Math.min(ttlMs, MAX_TTL_MS); - const path = unlockCachePath(); - await mkdir(dirname(path), { recursive: true, mode: 0o700 }); - const expires = new Date(Date.now() + clamped).toISOString(); - const cache: UnlockCache = { passphrase, expires_at: expires }; - await writeFile(path, JSON.stringify(cache), { mode: 0o600 }); - return expires; -} - +/** Delete the cache if present. Returns whether a file was actually removed. */ export async function clearCache(): Promise { try { await rm(unlockCachePath()); return true; } catch (err: unknown) { - if (err && typeof err === 'object' && 'code' in err && (err as { code: string }).code === 'ENOENT') return false; - throw err; - } -} - -const DURATION_PATTERN = /^(\d+)([smhd])$/; - -export function parseDuration(input: string): number { - const m = DURATION_PATTERN.exec(input.trim()); - if (!m) { - throw new CliError('invalid_input', `Invalid duration "${input}" — use e.g. 15m, 2h, 30s, 1d.`); - } - const n = Number(m[1]); - const unit = m[2]; - switch (unit) { - case 's': return n * 1000; - case 'm': return n * 60 * 1000; - case 'h': return n * 60 * 60 * 1000; - case 'd': return n * 24 * 60 * 60 * 1000; - default: throw new CliError('invalid_input', `Unknown duration unit: ${unit}`); + if (err && typeof err === 'object' && 'code' in err && (err as { code: string }).code === 'ENOENT') { return false; } + return false; } } diff --git a/tests/error-envelope.test.ts b/tests/error-envelope.test.ts index 5bc6533..afb2cc8 100644 --- a/tests/error-envelope.test.ts +++ b/tests/error-envelope.test.ts @@ -165,8 +165,11 @@ describe('compact error envelope — JSON', () => { }); it('omits extra and next_steps fields when CliError has neither', async () => { - // unlock --for with bad format → invalid_input, no extras, no nextSteps - const { json, exitCode } = await runJson('unlock', '--for', 'bad-format'); + // A bare invalid_input with no extras and no nextSteps. This used + // `unlock --for bad-format` until the plaintext passphrase cache and its + // command were removed; `assess` with neither identity raises the same + // shape (identity.ts throws CliError('invalid_input', msg) with no options). + const { json, exitCode } = await runJson('assess'); expect(exitCode).toBe(1); expect(json.code).toBe('invalid_input'); expect(json.extra).toBeUndefined(); diff --git a/tests/unlock-cache.test.ts b/tests/unlock-cache.test.ts index 96d69fb..ff85012 100644 --- a/tests/unlock-cache.test.ts +++ b/tests/unlock-cache.test.ts @@ -1,89 +1,86 @@ -import { mkdir, readFile, rm, writeFile } from 'fs/promises'; +import { existsSync } from 'fs'; +import { mkdir, rm, writeFile } from 'fs/promises'; import { join } from 'path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { CliError } from '../src/errors'; -import { - clearCache, - parseDuration, - readCachedPassphrase, - unlockCachePath, - writeCachedPassphrase, -} from '../src/unlock-cache'; -const ROOT = '/tmp/pay-unlock-test'; +/** + * The plaintext passphrase cache is gone. `~/.agentscore/.unlock` used to hold + * the wallet passphrase in cleartext for up to 8 hours beside the keystores it + * unlocks, protected only by file modes, which stop nothing already running as + * that user. + * + * These pin the two properties that matter after the removal. The passphrase is + * never read back from disk, and a leftover file from an older version is + * DELETED rather than ignored: an ignored file would leave a cleartext secret + * sitting there with nothing left that admits to using it, which is worse than + * either extreme. + */ -describe('parseDuration', () => { - it('parses seconds/minutes/hours/days', () => { - expect(parseDuration('30s')).toBe(30 * 1000); - expect(parseDuration('15m')).toBe(15 * 60 * 1000); - expect(parseDuration('2h')).toBe(2 * 60 * 60 * 1000); - expect(parseDuration('1d')).toBe(24 * 60 * 60 * 1000); - }); - - it('rejects malformed input', () => { - expect(() => parseDuration('15')).toThrow(CliError); - expect(() => parseDuration('15x')).toThrow(CliError); - expect(() => parseDuration('')).toThrow(CliError); - expect(() => parseDuration('abc')).toThrow(CliError); - }); - - it('trims whitespace', () => { - expect(parseDuration(' 15m ')).toBe(15 * 60 * 1000); - }); -}); +const ROOT = '/tmp/pay-unlock-removal-test'; +const CACHE = join(ROOT, '.agentscore', '.unlock'); -describe('unlock cache file I/O', () => { +describe('plaintext passphrase cache removal', () => { let originalHome: string | undefined; + let originalPass: string | undefined; beforeEach(async () => { originalHome = process.env.HOME; + originalPass = process.env.AGENTSCORE_PAY_PASSPHRASE; process.env.HOME = ROOT; - await rm(ROOT, { recursive: true, force: true }); + delete process.env.AGENTSCORE_PAY_PASSPHRASE; + await rm(ROOT, { force: true, recursive: true }); await mkdir(join(ROOT, '.agentscore'), { recursive: true }); }); afterEach(async () => { process.env.HOME = originalHome; - await rm(ROOT, { recursive: true, force: true }); + if (originalPass === undefined) { delete process.env.AGENTSCORE_PAY_PASSPHRASE; } + else { process.env.AGENTSCORE_PAY_PASSPHRASE = originalPass; } + await rm(ROOT, { force: true, recursive: true }); }); - it('returns null when no cache file', async () => { - expect(await readCachedPassphrase()).toBeNull(); + it('no longer exposes a way to read or write the cache', async () => { + const mod = await import('../src/unlock-cache'); + // The whole point: nothing can put a passphrase on disk or take one off it. + expect(mod).not.toHaveProperty('readCachedPassphrase'); + expect(mod).not.toHaveProperty('writeCachedPassphrase'); + // Positive control, so this does not pass by importing the wrong module. + expect(mod).toHaveProperty('clearCache'); }); - it('round-trips a passphrase', async () => { - await writeCachedPassphrase('correct horse battery staple', 60_000); - expect(await readCachedPassphrase()).toBe('correct horse battery staple'); - }); + it('DELETES a leftover cache rather than reading it', async () => { + await writeFile(CACHE, JSON.stringify({ + expires_at: new Date(Date.now() + 3_600_000).toISOString(), + passphrase: 'left-over-secret', + }), { mode: 0o600 }); + expect(existsSync(CACHE), 'fixture did not land; the path is wrong').toBe(true); - it('clamps TTL to MAX_TTL_MS (8h)', async () => { - const expires = await writeCachedPassphrase('x', 99 * 60 * 60 * 1000); - const expiresAt = Date.parse(expires); - expect(expiresAt - Date.now()).toBeLessThanOrEqual(8 * 60 * 60 * 1000 + 100); + const { promptPassphrase } = await import('../src/prompts'); + // No TTY and no env var, so this rejects. What matters is the side effect. + await expect(promptPassphrase()).rejects.toThrow(); + expect(existsSync(CACHE), 'a leftover plaintext passphrase was left on disk').toBe(false); }); - it('returns null for an expired cache and removes it', async () => { - const stale = { passphrase: 'old', expires_at: new Date(Date.now() - 1000).toISOString() }; - await writeFile(unlockCachePath(), JSON.stringify(stale), { mode: 0o600 }); - expect(await readCachedPassphrase()).toBeNull(); - await expect(readFile(unlockCachePath(), 'utf-8')).rejects.toThrow(); - }); + it('never returns the cached value even while the file exists', async () => { + await writeFile(CACHE, JSON.stringify({ + expires_at: new Date(Date.now() + 3_600_000).toISOString(), + passphrase: 'left-over-secret', + }), { mode: 0o600 }); - it('returns null for malformed cache', async () => { - await writeFile(unlockCachePath(), 'not-json', { mode: 0o600 }); - expect(await readCachedPassphrase()).toBeNull(); + const { promptPassphrase } = await import('../src/prompts'); + await expect(promptPassphrase()).rejects.toThrow(/AGENTSCORE_PAY_PASSPHRASE/); }); - it('clearCache returns true on hit, false on miss', async () => { - expect(await clearCache()).toBe(false); - await writeCachedPassphrase('x', 60_000); - expect(await clearCache()).toBe(true); - expect(await clearCache()).toBe(false); + it('still prefers the environment variable, which is the supported path', async () => { + process.env.AGENTSCORE_PAY_PASSPHRASE = 'from-env'; + const { promptPassphrase } = await import('../src/prompts'); + await expect(promptPassphrase()).resolves.toBe('from-env'); }); - it('rejects non-positive TTL', async () => { - await expect(writeCachedPassphrase('x', 0)).rejects.toBeInstanceOf(CliError); - await expect(writeCachedPassphrase('x', -1)).rejects.toBeInstanceOf(CliError); - await expect(writeCachedPassphrase('x', NaN)).rejects.toBeInstanceOf(CliError); + it('clearCache reports whether it removed anything', async () => { + const { clearCache } = await import('../src/unlock-cache'); + expect(await clearCache(), 'nothing to remove should report false').toBe(false); + await writeFile(CACHE, '{}', { mode: 0o600 }); + expect(await clearCache(), 'an existing file should report true').toBe(true); }); }); diff --git a/tests/unlock-cmd.test.ts b/tests/unlock-cmd.test.ts deleted file mode 100644 index 6e9f489..0000000 --- a/tests/unlock-cmd.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { mkdir, rm, stat } from 'fs/promises'; -import { join } from 'path'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; - -const ROOT = '/tmp/pay-unlock-cmd-test'; - -describe('unlock command', () => { - let originalHome: string | undefined; - - beforeEach(async () => { - originalHome = process.env.HOME; - process.env.HOME = ROOT; - process.env.AGENTSCORE_PAY_PASSPHRASE = 'integration-test-pass'; - await rm(ROOT, { recursive: true, force: true }); - await mkdir(join(ROOT, '.agentscore'), { recursive: true }); - }); - - afterEach(async () => { - process.env.HOME = originalHome; - delete process.env.AGENTSCORE_PAY_PASSPHRASE; - await rm(ROOT, { recursive: true, force: true }); - }); - - it('writes the cache file with mode 0600 and a future expiry', async () => { - const { unlock } = await import('../src/commands/unlock'); - const result = await unlock({ forDuration: '5m' }); - expect(result.ok).toBe(true); - expect(result.ttl_ms).toBe(5 * 60 * 1000); - expect(Date.parse(result.expires_at!)).toBeGreaterThan(Date.now()); - const st = await stat(result.path); - expect(st.mode & 0o777).toBe(0o600); - }); - - it('clamps an absurd duration to the 8h max', async () => { - const { unlock } = await import('../src/commands/unlock'); - const result = await unlock({ forDuration: '99h' }); - const ms = Date.parse(result.expires_at!) - Date.now(); - expect(ms).toBeLessThanOrEqual(8 * 60 * 60 * 1000 + 1000); - }); - - it('--clear is a no-op when no cache exists', async () => { - const { unlock } = await import('../src/commands/unlock'); - const result = await unlock({ clear: true }); - expect(result).toMatchObject({ ok: true, cleared: false }); - }); - - it('--clear removes the cache file after a prior unlock', async () => { - const { unlock } = await import('../src/commands/unlock'); - await unlock({ forDuration: '1m' }); - const result = await unlock({ clear: true }); - expect(result.cleared).toBe(true); - }); - - it('rejects malformed durations', async () => { - const { unlock } = await import('../src/commands/unlock'); - await expect(unlock({ forDuration: '15' })).rejects.toMatchObject({ code: 'invalid_input' }); - await expect(unlock({ forDuration: 'abc' })).rejects.toMatchObject({ code: 'invalid_input' }); - }); -});