fix: close money-movement gaps in subprocess env, withdraw, and trade plans

package.json

@@ -38,7 +38,7 @@
     "desktop:package:win": "npm run dist:win --workspace @blockrun/franklin-desktop",
     "dev": "tsc --watch",
     "start": "node dist/index.js",
-    "test": "npm run build && node --test --test-concurrency=4 --test-reporter=spec test/local.mjs test/api-key.local.mjs test/solana-migration.local.mjs test/serve-security.local.mjs test/skills.local.mjs test/repair.mjs test/model-resolver.mjs test/exa.local.mjs test/audit-batch.local.mjs test/polymarket.local.mjs test/market.local.mjs test/hooks.local.mjs test/scheduler.local.mjs test/trade-plan.local.mjs test/goal.local.mjs test/memory.local.mjs test/agent-host.local.mjs test/reservation.local.mjs test/model-catalog.local.mjs test/arc.local.mjs",
+    "test": "npm run build && node --test --test-concurrency=4 --test-reporter=spec test/local.mjs test/api-key.local.mjs test/solana-migration.local.mjs test/serve-security.local.mjs test/skills.local.mjs test/repair.mjs test/model-resolver.mjs test/exa.local.mjs test/audit-batch.local.mjs test/polymarket.local.mjs test/market.local.mjs test/hooks.local.mjs test/scheduler.local.mjs test/trade-plan.local.mjs test/goal.local.mjs test/memory.local.mjs test/agent-host.local.mjs test/reservation.local.mjs test/model-catalog.local.mjs test/arc.local.mjs test/subprocess-env.local.mjs test/withdraw.local.mjs test/orders.local.mjs",
     "test:e2e": "npm run build && node --test --test-reporter=spec test/e2e.mjs",
     "e2e:polymarket:readonly": "npm run build && node scripts/polymarket-e2e-readonly.mjs",
     "test:free-models": "npm run build && node --test --test-reporter=spec test/free-model-matrix.mjs",

src/agent/commands.ts

@@ -26,6 +26,7 @@ import {
 import type { Registry } from '../skills/registry.js';
 import { matchSkill } from '../skills/invoke.js';
 import type { ParsedSkill } from '../skills/types.js';
+import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js';
 
 type EventEmitter = (event: StreamEvent) => void;
 
@@ -51,6 +52,7 @@ interface CommandResult {
 
 function gitExec(cmd: string, cwd: string, timeout = 5000, maxBuffer?: number): string {
   return execSync(cmd, {
+    env: sanitizeSubprocessEnv(),
     cwd,
     encoding: 'utf-8',
     timeout,

src/agent/context.ts

@@ -12,6 +12,7 @@ import { Keypair } from '@solana/web3.js';
 import bs58 from 'bs58';
 import { loadLearnings, decayLearnings, saveLearnings, formatForPrompt } from '../learnings/store.js';
 import { isKeyMode, resolvePayMode } from '../payments/auth-mode.js';
+import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js';
 
 // ─── System Instructions Assembly ──────────────────────────────────────────
 // Composable prompt sections — each independently maintainable and conditionally includable.
@@ -109,7 +110,15 @@ Examples of risky actions that warrant user confirmation:
 
 When you encounter an obstacle, do not use destructive actions as a shortcut. Identify root causes and fix underlying issues rather than bypassing safety checks (e.g. --no-verify). If you discover unexpected state like unfamiliar files, branches, or configuration, investigate before deleting or overwriting — it may represent the user's in-progress work.
 
-A user approving an action once does NOT mean they approve it in all contexts. Match the scope of your actions to what was actually requested. When in doubt, ask before acting.`;
+A user approving an action once does NOT mean they approve it in all contexts. Match the scope of your actions to what was actually requested. When in doubt, ask before acting.
+
+## Ambiguous payment outcomes
+A payment, transfer, bet, or swap that was signed and sent may have settled even when the response says otherwise. A timeout, a 5xx (502 "upstream response lost" included), a dropped connection, or a missing receipt after a send means the outcome is UNKNOWN, not failed. This holds whichever path sent it: a built-in tool, a Bash script, or an MCP tool.
+- Never send a new payment for the same obligation while an earlier attempt's outcome is unknown. A new nonce or a new signature turns one obligation into two settlements.
+- Reconcile first: look up the earlier attempt by its transaction hash (receipt), the sender's nonce, and the sender/recipient balances or the venue's own status endpoint. If it settled, the obligation is paid. Do not pay again.
+- If it is still pending, wait and check again. If a retry is truly needed, re-broadcast the SAME signed transaction (same nonce), so at most one can ever land. Never sign a fresh one.
+- Only when the attempt has provably not settled (a reverted receipt, or a definite rejection of bytes the network never accepted) may a new payment be sent. If you cannot establish that, stop and tell the user exactly what is unknown.
+- In the final report, account for every attempt and every settlement. Never report one payment when two may have landed.`;
 }
 
 function getOutputEfficiencySection(): string {
@@ -766,7 +775,7 @@ function buildEnvironmentSection(workingDir: string): string {
 
   // Git repo detection
   try {
-    execSync('git rev-parse --is-inside-work-tree', { cwd: workingDir, timeout: 2000, stdio: ['pipe', 'pipe', 'pipe'] });
+    execSync('git rev-parse --is-inside-work-tree', { env: sanitizeSubprocessEnv(), cwd: workingDir, timeout: 2000, stdio: ['pipe', 'pipe', 'pipe'] });
     lines.push('- Is a git repository: true');
   } catch {
     lines.push('- Is a git repository: false');
@@ -874,6 +883,7 @@ const MAX_GIT_LOG_CHARS = 800;
 
 function getGitContext(workingDir: string): string | null {
   const gitCmd = (cmd: string) => execSync(cmd, {
+    env: sanitizeSubprocessEnv(),
     cwd: workingDir,
     encoding: 'utf-8',
     stdio: ['pipe', 'pipe', 'pipe'],

src/agent/tool-guard.ts

@@ -1,7 +1,7 @@
 import fs from 'node:fs';
 import path from 'node:path';
 import type { CapabilityInvocation, CapabilityResult, ExecutionScope } from './types.js';
-import { checkTradePlanGate, recordTradeExecution } from '../trading/trade-plan.js';
+import { checkTradePlanGate, recordTradeExecution, releaseTradeReservation } from '../trading/trade-plan.js';
 
 const MAX_WEBSEARCHES_PER_TURN = 8;
 const MAX_SIMILAR_SEARCHES_PER_TURN = 4;
@@ -484,6 +484,11 @@ export class SessionToolGuard {
   }
 
   cancelInvocation(invocationId: string): void {
+    // Denied before execution (permission refused, PreSpend veto): nothing
+    // was submitted, so a trade-plan reservation made by the gate goes back.
+    try {
+      releaseTradeReservation(invocationId);
+    } catch { /* budget accounting must never break tool flow */ }
     this.pendingSearches.delete(invocationId);
     this.pendingReads.delete(invocationId);
     this.pendingFetches.delete(invocationId);

src/agent/types.ts

@@ -86,6 +86,13 @@ export interface CapabilityResult {
    * getting a "Binary file" stub.
    */
   images?: Array<{ mediaType: string; base64: string }>;
+  /**
+   * Set by a money-moving tool only when the call provably never submitted
+   * anything (validation failure, user cancel, definite venue rejection). The
+   * trade-plan gate reserves budget before execution and releases it only on
+   * this signal — an error alone may hide an accepted, unacknowledged order.
+   */
+  notSubmitted?: boolean;
 }
 
 // ─── Execution Scope ───────────────────────────────────────────────────────

src/hooks/runner.ts

@@ -24,6 +24,7 @@ import {
   type HookInput,
   type LoadedHook,
 } from './types.js';
+import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js';
 
 const DEFAULT_TIMEOUT_SECS = 5;
 const MAX_STDOUT_BYTES = 64 * 1024;
@@ -94,6 +95,13 @@ export class HookEngine {
       if (fs.existsSync(resolved)) command = resolved;
     }
 
+    // Lifecycle hooks are user-authored and keep the full env. Tool and spend
+    // hooks receive model-controlled input on stdin and may execute it, so the
+    // inherited env drops wallet/account keys; the hook's own configured `env`
+    // is an explicit user grant and still applies.
+    const inherited = input.toolInput || input.spend ? sanitizeSubprocessEnv() : process.env;
+    const env = { ...inherited, ...hook.handler.env };
+
     return new Promise<HookDecision>(resolve => {
       const child = execFile(
         '/bin/sh',
@@ -102,8 +110,7 @@ export class HookEngine {
           timeout: timeoutMs,
           maxBuffer: MAX_STDOUT_BYTES,
           env: {
-            ...process.env,
-            ...hook.handler.env,
+            ...env,
             // Reserved vars always win over handler env.
             FRANKLIN_HOOK_EVENT: input.hookEventName,
             FRANKLIN_SESSION_ID: input.sessionId,

src/mcp/client.ts

@@ -24,6 +24,7 @@ import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js';
 import type { CapabilityHandler, CapabilityResult, ExecutionScope } from '../agent/types.js';
 import { logger } from '../logger.js';
 import { createOAuthProvider, type FranklinOAuthProvider } from './oauth.js';
+import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js';
 
 // ─── Types ────────────────────────────────────────────────────────────────
 
@@ -249,7 +250,9 @@ async function connectStdio(name: string, config: McpServerConfig): Promise<Conn
   const transport = new StdioClientTransport({
     command: config.command,
     args: config.args || [],
-    env: { ...process.env, ...(config.env || {}) } as Record<string, string>,
+    // Inherited env drops wallet/account keys; values the user wrote into this
+    // server's own `env` config are an explicit grant and pass through.
+    env: { ...sanitizeSubprocessEnv(), ...(config.env || {}) } as Record<string, string>,
     // Capture stderr so we can show it in `/mcp` rather than dumping to the
     // user's terminal. The previous `'ignore'` mode meant a misconfigured
     // server (missing env, OAuth failure, missing binary) showed up as a

src/mcp/codegraph.ts

@@ -22,6 +22,7 @@ import fs from 'node:fs';
 import path from 'node:path';
 import type { McpServerConfig } from './client.js';
 import { logger } from '../logger.js';
+import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js';
 
 const require = createRequire(import.meta.url);
 
@@ -101,6 +102,7 @@ export function ensureCodegraphIndex(workDir: string): void {
   try {
     const child = spawn(process.execPath, [shim, 'init', workDir, '-i'], {
       cwd: workDir,
+      env: sanitizeSubprocessEnv(),
       // Discard output: this is best-effort background indexing. Failures are
       // non-fatal — the agent simply keeps using grep/read until (and if) the
       // index appears. Surfacing a stack trace here would just be noise.

src/memory/store.ts

@@ -24,6 +24,7 @@ import path from 'node:path';
 import crypto from 'node:crypto';
 import { execFileSync } from 'node:child_process';
 import { BLOCKRUN_DIR } from '../config.js';
+import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js';
 
 export function memoryEnabled(): boolean {
   return process.env.FRANKLIN_MEMORY !== '0';
@@ -57,6 +58,7 @@ export function repoIdentity(workDir: string): string | null {
   let identity: string | null = null;
   try {
     const url = execFileSync('git', ['-C', workDir, 'remote', 'get-url', 'origin'], {
+      env: sanitizeSubprocessEnv(),
       encoding: 'utf-8',
       timeout: 2000,
       stdio: ['ignore', 'pipe', 'ignore'],

src/tasks/runner.ts

@@ -24,6 +24,7 @@ import fs from 'node:fs';
 import { readTaskMeta, applyEvent, writeTaskMeta } from './store.js';
 import { taskLogPath, ensureTaskDir } from './paths.js';
 import type { TaskStatus } from './types.js';
+import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js';
 
 const HEARTBEAT_MS = 5_000;
 const TAIL_BYTES = 500;
@@ -143,7 +144,7 @@ export async function runDetachedTask(runId: string): Promise<number> {
   const child = spawn('bash', ['-lc', meta.command], {
     cwd: meta.workingDir,
     stdio: ['ignore', logFd, logFd],
-    env: { ...process.env, FRANKLIN_TASK_RUN_ID: runId },
+    env: { ...sanitizeSubprocessEnv(), FRANKLIN_TASK_RUN_ID: runId },
   });
 
   // Cancel path: parent CLI sends SIGTERM (or user hits Ctrl-C). We must

src/tasks/spawn.ts

@@ -32,6 +32,7 @@ import { randomUUID } from 'node:crypto';
 import { writeTaskMeta } from './store.js';
 import { taskLogPath, ensureTaskDir } from './paths.js';
 import type { TaskRecord } from './types.js';
+import { sanitizeSubprocessEnv } from '../tools/subprocess-env.js';
 
 // Captured at module load so it survives later chdir / argv mutation.
 // `process.argv[1]` may be relative (`dist/index.js` in dev mode); we
@@ -95,7 +96,7 @@ export function startDetachedTask(input: StartDetachedTaskInput): string {
     cwd: input.workingDir,
     detached: true,
     stdio: ['ignore', logFd, logFd],
-    env: { ...process.env, FRANKLIN_TASK_RUN_ID: runId },
+    env: { ...sanitizeSubprocessEnv(), FRANKLIN_TASK_RUN_ID: runId },
   });
   child.unref();
 

src/tools/bash.ts

@@ -5,6 +5,7 @@
 import { spawn } from 'node:child_process';
 import fs from 'node:fs';
 import type { CapabilityHandler, CapabilityResult, ExecutionScope } from '../agent/types.js';
+import { sanitizeSubprocessEnv } from './subprocess-env.js';
 
 // ─── Smart Output Compression ─────────────────────────────────────────────
 // Learned from RTK (Rust Token Killer): strip noise before sending to LLM.
@@ -333,22 +334,22 @@ function executeCommand(command: string, timeoutMs: number, ctx: ExecutionScope)
     const shell = fs.existsSync('/bin/bash') ? '/bin/bash' : (process.env.SHELL || '/bin/sh');
     let child: ReturnType<typeof spawn>;
     try {
-      // The account bearer key is deliberately withheld from the subprocess.
-      // bash-guard auto-approves `printenv` and `echo` as safe, so
-      // `printenv BLOCKRUN_API_KEY` ran with no confirmation and returned the
-      // key as tool output — into the model prompt and the saved transcript.
-      // Wallet private keys are never in the environment at all; this gives
-      // the key the same treatment. Franklin's own gateway calls read
-      // process.env in-process and are unaffected, and a user who genuinely
-      // needs the key in a shell can export it there themselves.
+      // Credentials are withheld from the subprocess. bash-guard auto-approves
+      // `printenv` and `echo` as safe, so `printenv BLOCKRUN_API_KEY` once ran
+      // with no confirmation and returned the key as tool output. The same
+      // holds for the wallet keys the SDK reads from BLOCKRUN_WALLET_KEY /
+      // BASE_CHAIN_WALLET_KEY / SOLANA_WALLET_KEY: inherited, they let a
+      // model-written script sign transfers that no Franklin gate ever sees.
+      // Franklin's own gateway calls read process.env in-process and are
+      // unaffected, and a user who genuinely needs a key in a shell can
+      // export it there themselves.
       const childEnv: NodeJS.ProcessEnv = {
-        ...process.env,
+        ...sanitizeSubprocessEnv(),
         FRANKLIN: '1', // Let scripts detect they're running inside Franklin
         FRANKLIN_WORKDIR: ctx.workingDir,
         RUNCODE: '1', // Backwards compat
         RUNCODE_WORKDIR: ctx.workingDir,
       };
-      delete childEnv.BLOCKRUN_API_KEY;
 
       child = spawn(shell, ['-c', command], {
         cwd: ctx.workingDir,

src/tools/grep.ts

@@ -10,6 +10,7 @@ import { BLOCKRUN_DIR } from '../config.js';
 import {
   WALLET_KEY_PATHS, hostCredentialDirPaths, isHostCredentialPath, isWalletKeyPath, secretPathRefusal,
 } from './sensitive-paths.js';
+import { sanitizeSubprocessEnv } from './subprocess-env.js';
 
 interface GrepInput {
   pattern: string;
@@ -153,6 +154,7 @@ function runRipgrep(
 
   try {
     const result = execFileSync('rg', args, {
+      env: sanitizeSubprocessEnv(),
       encoding: 'utf-8',
       maxBuffer: 2 * 1024 * 1024,
       stdio: ['pipe', 'pipe', 'pipe'],
@@ -238,6 +240,7 @@ function runNativeGrep(
 
   try {
     const result = execFileSync('grep', args, {
+      env: sanitizeSubprocessEnv(),
       encoding: 'utf-8',
       maxBuffer: 2 * 1024 * 1024,
       stdio: ['pipe', 'pipe', 'pipe'],

src/tools/jupiter.ts

@@ -82,7 +82,7 @@ const TOKEN_DECIMALS: Record<string, number> = {
   EKpQGSJtjMFqKZ9KQanSqYXRcF8fBopzLHYxdM65zcjm: 6,
 };
 
-function resolveMint(input: string): string {
+export function resolveMint(input: string): string {
   const upper = input.trim().toUpperCase();
   if (SYMBOL_TO_MINT[upper]) return SYMBOL_TO_MINT[upper];
   return input.trim();

src/tools/polymarket-bet.ts

@@ -126,7 +126,7 @@ async function execute(raw: Record<string, unknown>, ctx: ExecutionScope): Promi
   try {
     await ensurePolymarketWallet();
   } catch (err) {
-    return { output: `Wallet unavailable: ${err instanceof Error ? err.message : String(err)}`, isError: true };
+    return { output: `Wallet unavailable: ${err instanceof Error ? err.message : String(err)}`, isError: true, notSubmitted: true };
   }
 
   const wantsConfirm = input.confirm === true;
@@ -143,10 +143,10 @@ async function execute(raw: Record<string, unknown>, ctx: ExecutionScope): Promi
       if (preview.isError) {
         // The dry-run itself failed (bad params, insufficient balance, not set
         // up) — surface that instead of prompting to sign a doomed action.
-        return { output: preview.text, isError: true, fullOutput: preview.text };
+        return { output: preview.text, isError: true, fullOutput: preview.text, notSubmitted: true };
       }
     } catch (err) {
-      return { output: `Preview failed: ${err instanceof Error ? err.message : String(err)}`, isError: true };
+      return { output: `Preview failed: ${err instanceof Error ? err.message : String(err)}`, isError: true, notSubmitted: true };
     }
 
     const answer = await ctx.onAskUser(
@@ -158,7 +158,7 @@ async function execute(raw: Record<string, unknown>, ctx: ExecutionScope): Promi
       ['Confirm', 'Cancel'],
     );
     if (answer.toLowerCase() !== 'confirm') {
-      return { output: 'Polymarket action cancelled by user.' };
+      return { output: 'Polymarket action cancelled by user.', notSubmitted: true };
     }
   }
 
@@ -183,7 +183,7 @@ async function execute(raw: Record<string, unknown>, ctx: ExecutionScope): Promi
   }
 
   if (result.isError) {
-    return { output: result.text, isError: true, fullOutput: result.text };
+    return { output: result.text, isError: true, fullOutput: result.text, ...(result.notSubmitted ? { notSubmitted: true } : {}) };
   }
   return { output: result.text, fullOutput: result.text };
 }
@@ -221,7 +221,7 @@ export const polymarketBetCapability: CapabilityHandler = {
         post_only: { type: 'boolean', description: 'Maker-only limit order (rejected if it would cross the book).' },
         order_id: { type: 'string', description: 'Order ID to cancel.' },
         all: { type: 'boolean', description: 'cancel: cancel ALL open orders.' },
-        to_address: { type: 'string', description: 'withdraw: destination address on Base (default: your agent wallet).' },
+        to_address: { type: 'string', description: 'withdraw: your agent wallet on Base only (default). Other recipients are refused; withdraw to your agent wallet first.' },
         confirm: { type: 'boolean', description: 'Must be true to place orders / sign approvals / redeem. Omit for a dry-run preview.' },
         auto_approve: { type: 'boolean', description: 'Skip the interactive user confirmation prompt (headless use). Caps still apply.' },
         agent_id: { type: 'string', description: 'Tag for the session betting ledger (bets do NOT draw from the x402 API budget).' },