Close remaining guard bypasses: media-write hole + 6 convergence-verify single-command holes (3.29.12)

CHANGELOG.md

@@ -1,5 +1,29 @@
 # Changelog
 
+## Franklin Agent 3.29.12 — close the 6 single-command bypasses from the 3.29.11 convergence verify (round-7)
+
+An 8-agent adversarial convergence verify of the 3.29.11 guards confirmed **6 more single-command holes** — two critical. Each was traced through the real classifier and reproduced before fixing. The unifying principle this round: a bash segment is only auto-approved ("safe") when its **full effect is statically visible** — any runtime indirection the classifier can't see now forces a prompt.
+
+- **Env-var prefix exec/loader hijack (CRITICAL).** `BASH_ENV=./evilrc ls` auto-approved arbitrary code execution: the guard blindly stripped every `NAME=value` token, so the base command resolved to a benign `ls` while `BASH_ENV` (sourced by every non-interactive `bash -c`), `LD_PRELOAD`, `DYLD_INSERT_LIBRARIES`, `PERL5OPT`, `NODE_OPTIONS`, `PATH`, `IFS`, etc. ran attacker code. Now only a small allowlist of locale/display vars (`LANG`, `LC_*`, `TZ`, `TERM`, …) may prefix a safe command; any other leading assignment prompts.
+- **`$VAR`-rooted glob wallet read (CRITICAL).** `cat $HOME/.bl*/.s*` evaded the `~`/`.`/`/`-rooted glob guard because it starts with `$`, then expanded into `~/.blockrun/.session` — zero-prompt wallet-key exfiltration. Parameter expansion (`$VAR`, `${VAR}`) is now treated as opaque exactly like `$(...)`. A non-expansion `$` (a `grep 'foo$'` anchor, `$5`) stays safe.
+- **Redirect write on git/npm/cargo/bun (HIGH).** `git status > ~/.bashrc` (overwrite a shell rc → RCE on next shell) auto-approved because the output-redirect check only ran for `SAFE_COMMANDS`; git/npm/cargo resolve through their own branches and skipped it. The redirect check now runs for **every** segment.
+- **Host-credential reads (MEDIUM).** `cat ~/.ssh/id_rsa`, `~/.aws/credentials`, `~/.gnupg/*`, gcloud tokens, `.npmrc`/`.pgpass`/`.netrc`, docker creds auto-approved secrets into context. Now mirrors the Write tool's dangerous-path block and prompts.
+- **`git config`/`git remote` writes (MEDIUM).** `git config core.pager "node evil.js"` (plants an exec hook fired on the next git invocation) and `git remote add` auto-approved as read-only subcommands. `config` is now safe only in get/list form; `remote` only in read form.
+- **SSRF cloud-metadata hostnames (MEDIUM).** `metadata.google.internal` (and `*.internal`, `metadata.goog`, AWS `instance-data`, NAT64-embedded `169.254.169.254`) resolve to the metadata IP at the OS layer, after the literal guard — now denied by name.
+
+New regression tests pin all six classes plus the benign controls (locale prefixes, regex anchors, read-form git, public hosts). Verified: local suite 500/500.
+
+## Franklin Agent 3.29.11 — close the remaining guard bypasses + media-write hole (final sweep)
+
+A final comprehensive sweep of the 3.29.10 guards found 7 more single-command holes (and a new class the prior rounds missed). Fixed at the architectural level rather than by patching more strings:
+
+- **Media tools could overwrite the wallet key (HIGH, new class).** `ImageGen`/`VideoGen`/`MusicGen` (and BrowserX screenshots) write to a caller-controlled `output_path` with **no** wallet-key guard — `output_path: "~/.blockrun/.session"` would brick the wallet. They now apply the same `isWalletKeyPath` guard the Read/Write/Edit tools use.
+- **bash-guard: separators.** Newline, CR, and a lone `&` (background) were not segment separators, so `pwd\nnpm install evil` / `pwd & node evil.js` auto-approved the injected command. All bash separators now split (fd-dups like `2>&1` unaffected).
+- **bash-guard: substitution & path-globs (the wallet-key class, finally).** `$(...)`, backticks, and `<(...)` run an inner command the classifier can't see, and a glob in a `~`/`.`/`/`-rooted path (`cat ~/.b*/.s*`) expands into `~/.blockrun` after the guard runs. Both now force a prompt — "fail toward prompting" for anything the classifier can't statically resolve. Bare cwd globs (`*.md`, `src/*.ts`) stay auto-safe.
+- **SSRF:** also block the Alibaba (`100.100.100.200`) and Oracle (`192.0.0.192`) cloud-metadata literals and CGNAT (`100.64.0.0/10`).
+
+The file Read/Write/Edit/media tools' canonicalized `isWalletKeyPath` guard is the primary protection; the bash-guard is the best-effort shell net and now prompts on anything it can't reason about. New regression tests pin every bypass. Verified: local suite 494/494.
+
 ## Franklin Agent 3.29.10 — close the 3.29.9 guard bypasses (round-6: adversarial re-review)
 
 An adversarial bypass-hunt of the 3.29.9 security guards found **16 holes in the guards themselves** — the recurring root cause was matching shell text / exact strings instead of canonicalizing. All fixed.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@blockrun/franklin",
-  "version": "3.29.10",
+  "version": "3.29.12",
   "description": "Franklin Agent — The AI agent with a wallet. Spends USDC autonomously to get real work done. Pay per action, no subscriptions.",
   "type": "module",
   "exports": {

src/agent/bash-guard.ts

@@ -126,6 +126,14 @@ const SAFE_CARGO_SUBCOMMANDS = new Set([
   'fmt', 'tree', 'metadata', 'verify-project',
 ]);
 
+// Env vars that may be stripped as a benign command PREFIX (`LANG=C ls`). These
+// only affect locale/display — none change code loading, library injection, or
+// interpreter behavior. Anything NOT on this list (BASH_ENV, LD_PRELOAD,
+// DYLD_INSERT_LIBRARIES, PERL5OPT, NODE_OPTIONS, PATH, IFS, a custom var, …) is
+// treated as an execution-hijack risk and forces a prompt.
+const BENIGN_ENV_PREFIXES =
+  /^(?:LANG|LANGUAGE|LC_[A-Z]+|TZ|TERM|COLUMNS|LINES|NO_COLOR|FORCE_COLOR|CLICOLOR|CLICOLOR_FORCE|GREP_COLOR|GREP_COLORS)$/;
+
 // ─── Classifier ──────────────────────────────────────────────────────────
 
 export function classifyBashRisk(command: string): BashRiskResult {
@@ -136,8 +144,13 @@ export function classifyBashRisk(command: string): BashRiskResult {
     }
   }
 
-  // 2. Check if every segment is a known-safe command
-  const segments = command.split(/\s*(?:&&|\|\||[;|])\s*/);
+  // 2. Check if every segment is a known-safe command. Split on ALL bash
+  // command separators — &&, ||, ;, |, a lone & (background), and newline/CR —
+  // so an injected second command (`pwd\nnpm install evil`, `pwd & node x`) is
+  // classified on its own rather than hiding behind a benign first word. (`&&`
+  // is matched before the lone `&`; numeric fd dups like `2>&1` have no
+  // standalone `&` and are handled per-segment by the redirect check.)
+  const segments = command.split(/\s*(?:&&|\|\||[;|]|(?<![>&])&(?![&>])|[\n\r])\s*/);
   let allSafe = true;
 
   for (const segment of segments) {
@@ -173,9 +186,59 @@ function isSegmentSafe(segment: string): boolean {
   if (/(?<![\w-])(?:\.solana-session(?:-key2)?|\.session|[\w-]*wallet[\w-]*\.(?:json|key))(?![\w-])/i.test(segment)) {
     return false;
   }
-
-  // Parse: strip env vars, extract command and args
-  const words = segment.split(/\s+/).filter(w => !w.includes('='));
+  // Command/process substitution runs an arbitrary INNER command the classifier
+  // can't see (`echo $(node evil)`, `cat <(touch x)`) — never safe.
+  if (/\$\(|`|<\(|>\(/.test(segment)) {
+    return false;
+  }
+  // Parameter expansion (`$VAR`, `${VAR}`) expands to text the classifier also
+  // can't see, so a bare `$HOME` glob can reach the wallet store exactly like
+  // `$(...)` — `cat $HOME/.bl*/.s*` evaded the rooted-glob guard below because it
+  // starts with `$`, not `~`/`.`/`/`. Treat any `$NAME` / `${NAME}` as opaque.
+  // (`$` followed by a non-name char — e.g. a `grep 'foo$'` regex anchor, `$?`,
+  // `$5` — is left alone so common read commands still auto-approve.)
+  if (/\$\{?[A-Za-z_]/.test(segment)) {
+    return false;
+  }
+  // A glob/brace in an explicit PATH (a token rooted at ~, ., or /) expands AFTER
+  // this guard and can reach the wallet store (`cat ~/.b*/.s*`) or a sensitive
+  // file. Bare cwd globs (`*.md`, `src/*.ts`) have no such prefix and stay safe.
+  if (/(?:^|\s)(?:~|\.|\/)\S*[*?[{]/.test(segment)) {
+    return false;
+  }
+  // Output redirection to a FILE target is a write — block it for EVERY segment,
+  // not just SAFE_COMMANDS ones. git/npm/cargo/bun resolve through their own
+  // branches below and used to skip the redirect check, so `git status > ~/.bashrc`
+  // (overwrite a shell rc → RCE on next shell) and `npm test > attack.sh`
+  // auto-approved. Allows numeric fd dups (`2>&1`, `>&2` — a digit follows `>`).
+  if (/>[>|&]?\s*[^\s&|0-9]/.test(segment)) {
+    return false;
+  }
+  // Reading host credential stores should PROMPT — mirror the Write tool's
+  // dangerous-path block so `cat ~/.ssh/id_rsa`, `cat ~/.aws/credentials`,
+  // `cat ~/.gnupg/secring.gpg`, gcloud tokens, `.npmrc`/`.pgpass`/`.netrc`, and
+  // docker registry creds don't auto-approve secrets into model context.
+  if (/(?:^|[\s/~'"=])\.(?:ssh|aws|gnupg|kube)(?:\/|$|['"\s])/i.test(segment)) return false;
+  if (/\bid_(?:rsa|dsa|ecdsa|ed25519)\b/i.test(segment)) return false;
+  if (/(?:^|[\s/~'"=])\.(?:npmrc|pgpass|netrc)(?:$|['"\s])/i.test(segment)) return false;
+  if (/gcloud\/(?:credentials|access_tokens|application_default)|\.docker\/config/i.test(segment)) return false;
+
+  // Parse into words. An env-assignment PREFIX (`FOO=bar cmd …`) is a real
+  // assignment only in the LEADING run before the command word — a later `x=y`
+  // is just an argument (`grep x=y file`). Walk the leading run: reject the
+  // segment if any assignment names a code-loading / execution-hijack var, so a
+  // benign-looking base command can't smuggle one (`BASH_ENV=./rc ls`,
+  // `LD_PRELOAD=/x.so cat f`). Only locale/display vars strip silently.
+  const rawWords = segment.split(/\s+/).filter(Boolean);
+  let envPrefixCount = 0;
+  for (const w of rawWords) {
+    const eq = w.indexOf('=');
+    // Stop at the first token that isn't a `NAME=value` assignment — that's the command.
+    if (eq <= 0 || !/^[A-Za-z_][A-Za-z0-9_]*$/.test(w.slice(0, eq))) break;
+    if (!BENIGN_ENV_PREFIXES.test(w.slice(0, eq).toUpperCase())) return false;
+    envPrefixCount++;
+  }
+  const words = rawWords.slice(envPrefixCount);
   let idx = 0;
   let cmd = words[idx] || '';
 
@@ -193,7 +256,26 @@ function isSegmentSafe(segment: string): boolean {
 
   // git
   if (baseName === 'git') {
-    return SAFE_GIT_SUBCOMMANDS.has(subCmd);
+    if (!SAFE_GIT_SUBCOMMANDS.has(subCmd)) return false;
+    // `config` is read-only ONLY in get/list form. A bare `git config key value`
+    // WRITES — and `--global` escapes the repo to plant an exec hook via
+    // core.pager/core.editor/alias.x (`git config core.pager "node evil.js"`).
+    // Count positional args after `config`: 2+ (key + value) is a write; explicit
+    // write flags (--add/--unset/…) also force a prompt.
+    if (subCmd === 'config') {
+      const cfgPositionals = segment
+        .replace(/^[^]*?\bconfig\b/, '')
+        .split(/\s+/)
+        .filter((w) => w && !w.startsWith('-'));
+      const writeFlag = /(?:^|\s)--(?:add|unset(?:-all)?|replace-all|remove-section|rename-section|edit|set)\b/.test(segment);
+      if (cfgPositionals.length >= 2 || writeFlag) return false;
+    }
+    // `git remote add/set-url/remove/rename/…` mutate remotes (can point at an
+    // attacker repo). Only the read forms (`git remote`, `git remote -v`) are safe.
+    if (subCmd === 'remote' && /(?:^|\s)(?:add|set-url|set-head|set-branches|remove|rm|rename|prune|update)\b/.test(segment)) {
+      return false;
+    }
+    return true;
   }
 
   // npm / yarn / pnpm / bun / npx
@@ -222,9 +304,8 @@ function isSegmentSafe(segment: string): boolean {
   if (SAFE_COMMANDS.has(baseName)) {
     // sed -i is not read-only
     if (baseName === 'sed' && segment.includes(' -i')) return false;
-    // Output redirection means writing — not safe. Covers `>`, `>>`, `>|`, and
-    // `>&FILE` (a path target), while still allowing numeric fd dups (`2>&1`, `>&2`).
-    if (/>[>|&]?\s*[^\s&|0-9]/.test(segment)) return false;
+    // (Output redirection is now blocked for every segment near the top of this
+    // function, so the per-command redirect check that used to live here is gone.)
     // Coreutils with a hidden write mode the redirect check can't see:
     if ((baseName === 'sort' || baseName === 'uniq' || baseName === 'tree') && /(?:^|\s)(?:-o(?:[=\s]|$)|--output\b)/.test(segment)) return false;
     // `uniq IN OUT` overwrites the 2nd positional file.

src/tools/browsex.ts

@@ -19,6 +19,7 @@ import os from 'node:os';
 import type { CapabilityHandler, CapabilityResult, ExecutionScope } from '../agent/types.js';
 import { browserPool } from '../social/browser-pool.js';
 import { frameUntrusted } from './untrusted.js';
+import { isWalletKeyPath } from './sensitive-paths.js';
 
 type BrowserAction = 'open' | 'snapshot' | 'click' | 'scroll' | 'screenshot' | 'getUrl' | 'wait';
 
@@ -107,6 +108,9 @@ async function execute(
         const finalPath = outPath
           ? outPath
           : path.join(os.homedir(), '.blockrun', 'screenshots', `browsex-${Date.now()}.png`);
+        if (isWalletKeyPath(path.resolve(finalPath))) {
+          return { output: `Error: refusing to write to the wallet key store: ${finalPath}`, isError: true };
+        }
         try {
           const fs = await import('node:fs');
           fs.mkdirSync(path.dirname(finalPath), { recursive: true });

src/tools/imagegen.ts

@@ -24,6 +24,7 @@ import { recordUsage } from '../stats/tracker.js';
 import { findModel, estimateCostUsd, GATEWAY_MARGIN, type GatewayModel } from '../gateway-models.js';
 import { logger } from '../logger.js';
 import { ssrfSafeFetch } from './ssrf.js';
+import { isWalletKeyPath } from './sensitive-paths.js';
 
 interface ImageGenInput {
   prompt: string;
@@ -403,6 +404,11 @@ function buildExecute(deps: ImageGenDeps) {
   const outPath = output_path
     ? (path.isAbsolute(output_path) ? output_path : path.resolve(ctx.workingDir, output_path))
     : path.resolve(ctx.workingDir, `generated-${Date.now()}.png`);
+  // A caller-controlled output_path must not be allowed to overwrite the wallet
+  // key store (e.g. output_path="~/.blockrun/.session" would brick the wallet).
+  if (isWalletKeyPath(outPath)) {
+    return { output: `Error: refusing to write to the wallet key store: ${outPath}`, isError: true };
+  }
 
   const body = JSON.stringify(
     editMode

src/tools/musicgen.ts

@@ -30,6 +30,7 @@ import type { CapabilityHandler, CapabilityResult, ExecutionScope } from '../age
 import { loadChain, API_URLS, VERSION } from '../config.js';
 import { logger } from '../logger.js';
 import type { ContentLibrary } from '../content/library.js';
+import { isWalletKeyPath } from './sensitive-paths.js';
 import { findModel, estimateCostUsd, type GatewayModel } from '../gateway-models.js';
 import { recordUsage } from '../stats/tracker.js';
 
@@ -119,6 +120,10 @@ function buildExecute(deps: MusicGenDeps) {
     const outPath = output_path
       ? (path.isAbsolute(output_path) ? output_path : path.resolve(ctx.workingDir, output_path))
       : path.resolve(ctx.workingDir, `generated-${Date.now()}.mp3`);
+    // Don't let a caller-controlled output_path overwrite the wallet key store.
+    if (isWalletKeyPath(outPath)) {
+      return { output: `Error: refusing to write to the wallet key store: ${outPath}`, isError: true };
+    }
 
     const body = JSON.stringify({
       model: musicModel,

src/tools/ssrf.ts

@@ -7,18 +7,42 @@
  * it does NOT resolve DNS or re-validate each redirect hop, so it stops the
  * common direct-IP/localhost cases, not a DNS-rebinding or redirect attack.
  */
+// Named cloud-metadata endpoints that aren't `*.internal` (AWS legacy alias,
+// GCP short form). They resolve to 169.254.169.254 / a link-local address.
+const METADATA_HOSTS = new Set([
+  'metadata', 'metadata.goog', 'instance-data', 'instance-data.ec2.internal',
+]);
+
 export function isBlockedSsrfHost(hostname: string): boolean {
   // Normalize: lowercase, strip IPv6 brackets, strip a single trailing dot
   // (`localhost.` / `127.0.0.1.` resolve the same as without it).
   const h = hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '');
   if (!h) return true;
   if (h === 'localhost' || h.endsWith('.localhost') || h.endsWith('.local')) return true;
+  // Cloud-metadata hostnames resolve (at the OS/fetch layer, AFTER this literal
+  // check) to 169.254.169.254 / a NAT address, so blocking only the IP misses the
+  // named form. `.internal` is the ICANN-reserved private TLD (GCE
+  // metadata.google.internal, EC2 *.ec2.internal) — denied wholesale.
+  if (h.endsWith('.internal')) return true;
+  if (METADATA_HOSTS.has(h)) return true;
 
   // IPv6 literal (only IPv6 hosts contain a colon — so the fc/fd/fe80 ULA checks
   // can't false-positive on public DNS names like fda.gov / fcc.gov).
   if (h.includes(':')) {
     if (h === '::1' || h === '::') return true;                          // loopback / unspecified
     if (h.startsWith('fe80:') || h.startsWith('fc') || h.startsWith('fd')) return true; // link-local / ULA
+    // NAT64 (`64:ff9b::a.b.c.d` / `64:ff9b::hhhh:hhhh`) embeds an IPv4 that a NAT64
+    // gateway routes to — decode and recheck it like the IPv4-mapped form below.
+    const nat64 = h.match(/^64:ff9b::(.+)$/);
+    if (nat64) {
+      const t = nat64[1];
+      if (t.includes('.')) return isBlockedSsrfHost(t);
+      const seg = t.split(':');
+      if (seg.length === 2) {
+        const n = ((parseInt(seg[0], 16) << 16) | parseInt(seg[1], 16)) >>> 0;
+        return isBlockedSsrfHost(`${(n >>> 24) & 255}.${(n >>> 16) & 255}.${(n >>> 8) & 255}.${n & 255}`);
+      }
+    }
     // IPv4-mapped IPv6 (`::ffff:a.b.c.d` or `::ffff:hhhh:hhhh`) → recheck the embedded v4.
     const mapped = h.match(/::ffff:(.+)$/);
     if (mapped) {
@@ -36,11 +60,14 @@ export function isBlockedSsrfHost(hostname: string): boolean {
   // IPv4 literal
   const m = h.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/);
   if (m) {
-    const a = Number(m[1]), b = Number(m[2]);
+    const a = Number(m[1]), b = Number(m[2]), c = Number(m[3]);
     if (a === 127 || a === 0 || a === 10) return true;       // loopback / this-host / private
     if (a === 169 && b === 254) return true;                 // link-local + cloud metadata (169.254.169.254)
     if (a === 172 && b >= 16 && b <= 31) return true;        // private
     if (a === 192 && b === 168) return true;                 // private
+    if (a === 100 && b === 100) return true;                 // Alibaba Cloud metadata (100.100.100.200)
+    if (a === 192 && b === 0 && c === 0) return true;        // Oracle OCI metadata (192.0.0.192) / 192.0.0.0/24
+    if (a === 100 && b >= 64 && b <= 127) return true;       // CGNAT 100.64.0.0/10
   }
   return false;
 }

src/tools/videogen.ts

@@ -36,6 +36,7 @@ import { loadChain, API_URLS, VERSION } from '../config.js';
 import { logger } from '../logger.js';
 import type { ContentLibrary } from '../content/library.js';
 import { resolveReferenceImage } from './imagegen.js';
+import { isWalletKeyPath } from './sensitive-paths.js';
 import { recordUsage } from '../stats/tracker.js';
 import { findModel, estimateCostUsd, GATEWAY_MARGIN, type GatewayModel } from '../gateway-models.js';
 
@@ -211,6 +212,10 @@ function buildExecute(deps: VideoGenDeps) {
     const outPath = output_path
       ? (path.isAbsolute(output_path) ? output_path : path.resolve(ctx.workingDir, output_path))
       : path.resolve(ctx.workingDir, `generated-${Date.now()}.mp4`);
+    // Don't let a caller-controlled output_path overwrite the wallet key store.
+    if (isWalletKeyPath(outPath)) {
+      return { output: `Error: refusing to write to the wallet key store: ${outPath}`, isError: true };
+    }
 
     const body = JSON.stringify({
       model: videoModel,