From 50d2f1c7a083d6740ef72293b2111d6205a0c3a1 Mon Sep 17 00:00:00 2001 From: Georges Garnier Date: Mon, 27 Apr 2026 22:18:13 +0200 Subject: [PATCH 1/9] feat(p5.1): hardened Docker sandbox driven by AGENT.md Sandbox isolation is no longer "whatever docker run defaults to". Every container launched by Agent Forge now runs under a strict hardening profile, with relaxations declared explicitly in AGENT.md and surfaced to the user at confirm time. Schema (packages/core/src/types/agent-md.ts) : sandbox: image: agent-forge/base:latest timeout: 60s network: none|bridge (default none) readOnlyRoot: true|false (default true) user: agent (default agent) resources: memory: 512m (default 512m) cpus: 1 (default 1) pidsLimit: 128 (default 128) Existing AGENT.md files keep parsing : every new field is optional and applySandboxDefaults() fills the gaps. SANDBOX_DEFAULTS is the single source of truth shared between DockerLaunch and the ConfirmAction dialog. DockerLaunch (packages/tools-core/src/docker-launch.ts) now : - reads + parses the agent's AGENT.md before spawning, so a malformed file fails fast with a clear error instead of a cryptic docker error - translates the resolved config into flags via hardeningFlags() (exported for testing) : --cap-drop=ALL --security-opt=no-new-privileges --network={none|bridge} --user= --memory= --cpus= --pids-limit= --read-only --tmpfs=/tmp:rw,size=64m,mode=1777 (only when readOnlyRoot stays true ; tmpfs covers package installers and shell utilities that scribble in /tmp) - drops the FORGE_AGENT_IMAGE env override : the image now comes from AGENT.md.sandbox.image, where the agent author already has to declare it Permission dialog (packages/cli/src/components/ConfirmAction.tsx) : for write actions producing an AGENT.md and for run actions pointing to an existing one, parse the sandbox section and diff it against the strict defaults. Any relaxation (network=bridge, readOnlyRoot=false, larger memory / cpus / pids, custom user) appears as a red warning between the metadata and the preview, in the user's language. Dockerfile : header comments updated to reflect the new hardening contract (non-root agent user, /workspace and /tmp writable, --read-only outside, no caps, no network by default). Tests : - packages/core/tests/agent-md.test.ts : the new sandbox fields parse, invalid network value rejected, malformed memory rejected, applySandboxDefaults fills missing fields, keeps overrides intact. - packages/tools-core/tests/hardening-flags.test.ts : every strict default lands in the docker run args, every relaxation is honoured, and --cap-drop=ALL + --security-opt=no-new-privileges stay on under any AGENT.md (invariants). Real container escape tests are intentionally NOT in this commit : they require a built image and a docker daemon, so they belong in a separate suite gated on CI infrastructure. Roadmap (README EN/FR) updated to track P5.1 as done, P5.2 (artifact extraction) and P5.3 (persistent agents) as the next steps. --- README.fr.md | 4 +- README.md | 4 +- docker/base.Dockerfile | 12 +- packages/cli/src/components/ConfirmAction.tsx | 132 ++++++++++++++++++ packages/core/README.md | 2 +- packages/core/src/types/agent-md.ts | 75 ++++++++++ packages/core/src/types/index.ts | 4 + packages/core/tests/agent-md.test.ts | 75 +++++++++- packages/tools-core/src/docker-launch.ts | 83 ++++++++++- packages/tools-core/src/index.ts | 2 + .../tools-core/tests/hardening-flags.test.ts | 107 ++++++++++++++ 11 files changed, 492 insertions(+), 8 deletions(-) create mode 100644 packages/tools-core/tests/hardening-flags.test.ts diff --git a/README.fr.md b/README.fr.md index 64c8456..9292852 100644 --- a/README.fr.md +++ b/README.fr.md @@ -37,7 +37,9 @@ Le builder est la seule surface conversationnelle. Les sous-agents sont créés | **P3** | Le builder écrit l'`AGENT.md`, demande la permission, lance l'agent dans un container neuf, streame la sortie | ✅ fait | | **P4** | Six tools natifs sandboxés sous `/workspace` : Bash, FileWrite, FileRead, FileEdit, Grep, Glob ; tool-loop runtime avec `maxTurns` | ✅ fait | | **P6** | Couche skills : format `SKILL.md`, catalogue (built-in + `~/.agent-forge/skills/`), matching des triggers côté serveur, runner à 2 appels (un pour AGENT.md, un pour le run prompt) | ✅ fait | -| P5 | Sandbox durci + agents persistants (`docker exec`) + extraction d'artefacts vers le host | suivant | +| **P5.1** | Sandbox Docker durci : user non-root, racine read-only + tmpfs `/tmp`, `--cap-drop=ALL`, `--security-opt=no-new-privileges`, `--network=none` par défaut, caps ressources (mémoire / cpus / pids). Le dialog de permission signale toute relaxation déclarée dans l'AGENT.md. | ✅ fait | +| P5.2 | Extraction d'artefacts vers le host (`~/.agent-forge/artifacts///`) | suivant | +| P5.3 | Agents persistants via `docker exec`, slash commands de cycle de vie | | | P7 | `TEAM.md` — exécutions multi-agents coordonnées | | | P8 | Dashboard pixel art (activité agents en direct) | | | P9 | ★ POC validé : démo Next.js + Laravel + QA de bout en bout | | diff --git a/README.md b/README.md index ab7c624..b297d9f 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,9 @@ The builder is the only conversational surface. Sub-agents are spawned on demand | **P3** | Builder writes `AGENT.md`, asks for permission, launches the agent in a fresh container, streams its output | ✅ done | | **P4** | Six native tools sandboxed under `/workspace` : Bash, FileWrite, FileRead, FileEdit, Grep, Glob ; runtime tool-loop with `maxTurns` | ✅ done | | **P6** | Skill layer : `SKILL.md` format, catalog (built-in + `~/.agent-forge/skills/`), server-side trigger matching, two-call runner (one for AGENT.md, one for the run prompt) | ✅ done | -| P5 | Hardened sandbox + persistent agents (`docker exec`) + artifact extraction back to host | next | +| **P5.1** | Hardened Docker sandbox : non-root user, read-only root + tmpfs `/tmp`, `--cap-drop=ALL`, `--security-opt=no-new-privileges`, `--network=none` by default, resource caps (memory / cpus / pids). Permission dialog flags any AGENT.md relaxation. | ✅ done | +| P5.2 | Artifact extraction back to host (`~/.agent-forge/artifacts///`) | next | +| P5.3 | Persistent agents via `docker exec`, lifecycle slash commands | | | P7 | `TEAM.md` — coordinated multi-agent runs | | | P8 | Pixel-art dashboard (live agent activity) | | | P9 | ★ POC validated : Next.js + Laravel + QA demo end-to-end | | diff --git a/docker/base.Dockerfile b/docker/base.Dockerfile index a0e0c6f..f2a45c7 100644 --- a/docker/base.Dockerfile +++ b/docker/base.Dockerfile @@ -1,7 +1,17 @@ # Agent Forge — base image # Minimal sandbox for simple agents (read, edit, run shell). # -# Status : POC, not built yet. This file is a sketch of the target. +# Hardening (P5) : +# - non-root user `agent` (uid 1000) is the default at runtime +# - /workspace is the only writable dir owned by `agent` +# - DockerLaunch passes --read-only on the root FS, plus a tmpfs +# mount on /tmp so package installers and test runners that +# write under /tmp keep working without granting write to the +# image +# - --cap-drop=ALL --security-opt=no-new-privileges +# - --network=none by default ; agents that need network must +# declare `network: bridge` in AGENT.md.sandbox and the user +# gets a warning at confirm time FROM debian:bookworm-slim diff --git a/packages/cli/src/components/ConfirmAction.tsx b/packages/cli/src/components/ConfirmAction.tsx index 5efef0b..9a6f8c7 100644 --- a/packages/cli/src/components/ConfirmAction.tsx +++ b/packages/cli/src/components/ConfirmAction.tsx @@ -5,15 +5,120 @@ // Style : framed (double orange border), distinct from the chat flow, with // an explicit question, a metadata block, a preview, and three "button- // like" choices. +// +// P5 : when a write action concerns an AGENT.md whose sandbox section +// relaxes the strict defaults (network=bridge, readOnlyRoot=false, +// elevated resources), we surface a list of warnings between the +// metadata and the preview so the user notices before approving. +import { existsSync, readFileSync } from 'node:fs' +import { homedir } from 'node:os' +import { join } from 'node:path' import { Box, Text, useInput, useStdin } from 'ink' import React, { useState } from 'react' +import { + type AppliedSandboxConfig, + SANDBOX_DEFAULTS, + applySandboxDefaults, + parseAgentMd, +} from '@agent-forge/core/types' import type { Action } from '../actions/types.ts' import { useLanguage } from '../i18n/LanguageContext.tsx' import { C } from '../theme/colors.ts' const PREVIEW_LINES = 6 +type SandboxWarning = { + field: string + detail: string +} + +function diffAgainstDefaults( + cfg: AppliedSandboxConfig, + lang: 'en' | 'fr', +): SandboxWarning[] { + const t = lang === 'fr' + const out: SandboxWarning[] = [] + if (cfg.network !== SANDBOX_DEFAULTS.network) { + out.push({ + field: 'network', + detail: t + ? `réseau ouvert (${cfg.network}) — l'agent pourra accéder à internet` + : `network open (${cfg.network}) — agent will have internet access`, + }) + } + if (cfg.readOnlyRoot !== SANDBOX_DEFAULTS.readOnlyRoot) { + out.push({ + field: 'readOnlyRoot', + detail: t + ? "racine en écriture — l'agent peut modifier le système de fichiers de l'image" + : 'writable root — agent can mutate the image filesystem', + }) + } + if (cfg.user !== SANDBOX_DEFAULTS.user) { + out.push({ + field: 'user', + detail: t + ? `utilisateur "${cfg.user}" au lieu de "${SANDBOX_DEFAULTS.user}"` + : `user "${cfg.user}" instead of "${SANDBOX_DEFAULTS.user}"`, + }) + } + if (cfg.memory !== SANDBOX_DEFAULTS.memory) { + out.push({ + field: 'memory', + detail: t + ? `mémoire ${cfg.memory} (défaut ${SANDBOX_DEFAULTS.memory})` + : `memory ${cfg.memory} (default ${SANDBOX_DEFAULTS.memory})`, + }) + } + if (cfg.cpus !== SANDBOX_DEFAULTS.cpus) { + out.push({ + field: 'cpus', + detail: t + ? `CPU ${cfg.cpus} (défaut ${SANDBOX_DEFAULTS.cpus.toString()})` + : `cpus ${cfg.cpus.toString()} (default ${SANDBOX_DEFAULTS.cpus.toString()})`, + }) + } + if (cfg.pidsLimit !== SANDBOX_DEFAULTS.pidsLimit) { + out.push({ + field: 'pidsLimit', + detail: t + ? `pids ${cfg.pidsLimit.toString()} (défaut ${SANDBOX_DEFAULTS.pidsLimit.toString()})` + : `pids ${cfg.pidsLimit.toString()} (default ${SANDBOX_DEFAULTS.pidsLimit.toString()})`, + }) + } + return out +} + +// Resolve the sandbox config from whichever source the action carries : +// - write action targeting an AGENT.md → parse the proposed content +// directly (the file may not exist on disk yet) +// - run action → read the persisted AGENT.md from ~/.agent-forge +// Returns null if no AGENT.md is involved or parsing fails. +function sandboxFor(action: Action): AppliedSandboxConfig | null { + try { + if (action.kind === 'write' && action.path.endsWith('AGENT.md')) { + const parsed = parseAgentMd(action.content) + return applySandboxDefaults(parsed.meta.sandbox) + } + if (action.kind === 'run') { + const path = join( + homedir(), + '.agent-forge', + 'agents', + action.agent, + 'AGENT.md', + ) + if (!existsSync(path)) return null + const parsed = parseAgentMd(readFileSync(path, 'utf8')) + return applySandboxDefaults(parsed.meta.sandbox) + } + } catch { + return null + } + return null +} + type Strings = { title: string questionWrite: string @@ -29,6 +134,7 @@ type Strings = { collapse: string actionWrite: string actionRun: string + warningHeader: string } const STRINGS: Record<'en' | 'fr', Strings> = { @@ -47,6 +153,7 @@ const STRINGS: Record<'en' | 'fr', Strings> = { collapse: 'Collapse preview', actionWrite: 'create file', actionRun: 'launch agent', + warningHeader: 'Sandbox relaxations applied to this agent :', }, fr: { title: 'AUTORISATION REQUISE', @@ -63,6 +170,7 @@ const STRINGS: Record<'en' | 'fr', Strings> = { collapse: "Réduire l’aperçu", actionWrite: 'créer un fichier', actionRun: 'lancer un agent', + warningHeader: 'Relaxations sandbox appliquées à cet agent :', }, } @@ -186,6 +294,30 @@ export function ConfirmAction({ )} + {/* Sandbox warnings : surface every relaxation vs the strict + defaults so the user can spot e.g. network=bridge before + approving. Only renders when at least one relaxation is + declared in the AGENT.md sandbox section. */} + {(() => { + const cfg = sandboxFor(action) + if (!cfg) return null + const warnings = diffAgainstDefaults(cfg, lang ?? 'en') + if (warnings.length === 0) return null + return ( + + + {` ▲ ${s.warningHeader}`} + + {warnings.map((w) => ( + + {` · ${w.field}`} + {` — ${w.detail}`} + + ))} + + ) + })()} + {/* Preview */} diff --git a/packages/core/README.md b/packages/core/README.md index 4562ada..e88cc01 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -12,7 +12,7 @@ Primitives de base d'Agent Forge. - **`skill-matcher.ts`** — match côté serveur des triggers (sous-chaîne insensible à la casse) - **`skill-runner.ts`** — orchestration de `scaffold-and-run` (deux appels `generateText` ciblés, un pour AGENT.md, un pour le run prompt) - **`skills/scaffold-and-run.md`** — première skill built-in -- **`types/agent-md.ts`** — `parseAgentMd(text)` : sépare frontmatter / body, valide via Zod (name kebab-case, description non vide, sandbox.image, sandbox.timeout, maxTurns) +- **`types/agent-md.ts`** — `parseAgentMd(text)` : sépare frontmatter / body, valide via Zod (name kebab-case, description non vide, sandbox.image, sandbox.timeout, maxTurns). P5 : champs `sandbox.network` (`none` / `bridge`), `sandbox.readOnlyRoot`, `sandbox.user`, `sandbox.resources.{memory, cpus, pidsLimit}`. Helper `applySandboxDefaults()` qui applique les valeurs strictes (`network: none`, `readOnlyRoot: true`, `user: agent`, `512m / 1 cpu / 128 pids`) à tout champ omis. - **`types/skill-md.ts`** — `parseSkillMd(text)` : même pattern pour les skills (name, description, triggers, actions) ## À venir diff --git a/packages/core/src/types/agent-md.ts b/packages/core/src/types/agent-md.ts index 73c405f..7e57cda 100644 --- a/packages/core/src/types/agent-md.ts +++ b/packages/core/src/types/agent-md.ts @@ -26,12 +26,45 @@ import { z } from 'zod' const FRONTMATTER_RE = /^---\s*\n([\s\S]*?)\n---\s*\n?([\s\S]*)$/ +// Resource caps applied via `docker run --memory`, `--cpus`, `--pids-limit`. +// Strict defaults : an agent should never need more than this for the +// kind of small task we run in P5. Authors can relax via AGENT.md when +// a specific job needs it ; the permission dialog surfaces the change. +export const AgentSandboxResourcesSchema = z.object({ + memory: z + .string() + .regex(/^\d+[mg]$/i, 'memory must look like 512m, 2g') + .optional(), + cpus: z.number().positive().max(8).optional(), + pidsLimit: z.number().int().positive().max(4096).optional(), +}) + export const AgentSandboxSchema = z.object({ image: z.string().min(1), timeout: z .string() .regex(/^\d+[smh]$/, 'timeout must look like 30s, 5m, 1h') .optional(), + // P5 hardening : isolation knobs the builder can declare per agent. + // All optional with strict defaults applied at launch time. + network: z + .enum(['none', 'bridge']) + .optional() + .describe( + "Network policy. 'none' (default) : no internet. 'bridge' : Docker bridge network, agent can reach the internet — surfaced in the permission dialog as risky.", + ), + readOnlyRoot: z + .boolean() + .optional() + .describe( + 'Read-only root filesystem. Default true. /workspace and /tmp stay writable. Disable only for agents that need to install packages on top of the base image (rare).', + ), + user: z + .string() + .regex(/^[a-z][a-z0-9-]*$/i, 'user must be a valid Unix username') + .optional() + .describe('Linux user the runtime runs as inside the container. Default "agent" (uid 1000, non-root).'), + resources: AgentSandboxResourcesSchema.optional(), }) export const AgentMdSchema = z.object({ @@ -45,6 +78,48 @@ export const AgentMdSchema = z.object({ maxTurns: z.number().int().positive().default(1), }) +// Strict defaults applied at launch time when AGENT.md leaves the +// hardening fields off. Centralised here so DockerLaunch and the +// permission dialog read the same source of truth. +export const SANDBOX_DEFAULTS = { + network: 'none' as const, + readOnlyRoot: true, + user: 'agent', + memory: '512m', + cpus: 1, + pidsLimit: 128, +} as const + +export type AppliedSandboxConfig = { + image: string + network: 'none' | 'bridge' + readOnlyRoot: boolean + user: string + memory: string + cpus: number + pidsLimit: number +} + +/** + * Resolve an AGENT.md sandbox block against the strict defaults. + * Anything not specified by the agent author falls back to the + * defaults — DockerLaunch can then translate the result directly + * into `docker run` flags without re-reading the schema. + */ +export function applySandboxDefaults( + sandbox: AgentMd['sandbox'], +): AppliedSandboxConfig { + return { + image: sandbox.image, + network: sandbox.network ?? SANDBOX_DEFAULTS.network, + readOnlyRoot: sandbox.readOnlyRoot ?? SANDBOX_DEFAULTS.readOnlyRoot, + user: sandbox.user ?? SANDBOX_DEFAULTS.user, + memory: sandbox.resources?.memory ?? SANDBOX_DEFAULTS.memory, + cpus: sandbox.resources?.cpus ?? SANDBOX_DEFAULTS.cpus, + pidsLimit: sandbox.resources?.pidsLimit ?? SANDBOX_DEFAULTS.pidsLimit, + } +} + export type AgentMd = z.infer export type ParsedAgentMd = { diff --git a/packages/core/src/types/index.ts b/packages/core/src/types/index.ts index 505de0e..4d65f68 100644 --- a/packages/core/src/types/index.ts +++ b/packages/core/src/types/index.ts @@ -1,9 +1,13 @@ export { AgentMdError, AgentMdSchema, + AgentSandboxResourcesSchema, AgentSandboxSchema, + SANDBOX_DEFAULTS, + applySandboxDefaults, parseAgentMd, type AgentMd, + type AppliedSandboxConfig, type ParsedAgentMd, } from './agent-md.ts' diff --git a/packages/core/tests/agent-md.test.ts b/packages/core/tests/agent-md.test.ts index 0421f53..0c720aa 100644 --- a/packages/core/tests/agent-md.test.ts +++ b/packages/core/tests/agent-md.test.ts @@ -1,7 +1,12 @@ // Schema and parser tests for AGENT.md. import { describe, expect, test } from 'bun:test' -import { AgentMdError, parseAgentMd } from '../src/types/agent-md.ts' +import { + AgentMdError, + SANDBOX_DEFAULTS, + applySandboxDefaults, + parseAgentMd, +} from '../src/types/agent-md.ts' const valid = `--- name: haiku-writer @@ -55,4 +60,72 @@ describe('parseAgentMd', () => { const result = parseAgentMd(noTurns) expect(result.meta.maxTurns).toBe(1) }) + + test('parses the new hardening fields when present', () => { + const hardened = valid.replace( + 'sandbox:\n image: agent-forge/base:latest\n timeout: 60s', + [ + 'sandbox:', + ' image: agent-forge/base:latest', + ' timeout: 60s', + ' network: bridge', + ' readOnlyRoot: false', + ' user: agent', + ' resources:', + ' memory: 1g', + ' cpus: 2', + ' pidsLimit: 256', + ].join('\n'), + ) + const result = parseAgentMd(hardened) + expect(result.meta.sandbox.network).toBe('bridge') + expect(result.meta.sandbox.readOnlyRoot).toBe(false) + expect(result.meta.sandbox.user).toBe('agent') + expect(result.meta.sandbox.resources?.memory).toBe('1g') + expect(result.meta.sandbox.resources?.cpus).toBe(2) + expect(result.meta.sandbox.resources?.pidsLimit).toBe(256) + }) + + test('rejects an invalid network value', () => { + const bad = valid.replace( + 'timeout: 60s', + 'timeout: 60s\n network: open', + ) + expect(() => parseAgentMd(bad)).toThrow(/network/) + }) + + test('rejects malformed memory string', () => { + const bad = valid.replace( + 'timeout: 60s', + 'timeout: 60s\n resources:\n memory: lots', + ) + expect(() => parseAgentMd(bad)).toThrow(/memory/) + }) +}) + +describe('applySandboxDefaults', () => { + test('fills every missing field with the strict default', () => { + const result = applySandboxDefaults({ image: 'agent-forge/base:latest' }) + expect(result.image).toBe('agent-forge/base:latest') + expect(result.network).toBe(SANDBOX_DEFAULTS.network) + expect(result.readOnlyRoot).toBe(SANDBOX_DEFAULTS.readOnlyRoot) + expect(result.user).toBe(SANDBOX_DEFAULTS.user) + expect(result.memory).toBe(SANDBOX_DEFAULTS.memory) + expect(result.cpus).toBe(SANDBOX_DEFAULTS.cpus) + expect(result.pidsLimit).toBe(SANDBOX_DEFAULTS.pidsLimit) + }) + + test('keeps caller overrides intact', () => { + const result = applySandboxDefaults({ + image: 'custom:latest', + network: 'bridge', + readOnlyRoot: false, + resources: { memory: '2g', cpus: 4 }, + }) + expect(result.network).toBe('bridge') + expect(result.readOnlyRoot).toBe(false) + expect(result.memory).toBe('2g') + expect(result.cpus).toBe(4) + expect(result.pidsLimit).toBe(SANDBOX_DEFAULTS.pidsLimit) + }) }) diff --git a/packages/tools-core/src/docker-launch.ts b/packages/tools-core/src/docker-launch.ts index 5544e00..ebdfaa0 100644 --- a/packages/tools-core/src/docker-launch.ts +++ b/packages/tools-core/src/docker-launch.ts @@ -5,13 +5,25 @@ // async generator. The container is named so we can force-remove it on // crash, signal, or timeout. // +// P5 hardening : sandbox flags (--read-only, --cap-drop=ALL, +// --security-opt=no-new-privileges, --network=none, --memory, --cpus, +// --pids-limit, --user) are derived from the agent's AGENT.md sandbox +// section through applySandboxDefaults. Defaults are strict — agents +// must opt in to relax (network: bridge, readOnlyRoot: false, …) and +// the permission dialog flags any non-default choice. +// // Multi-instance ready : each call uses a unique container name so several // agents can run in parallel without collision. import { spawn, spawnSync } from 'node:child_process' -import { existsSync, mkdirSync } from 'node:fs' +import { existsSync, mkdirSync, readFileSync } from 'node:fs' import { join } from 'node:path' import { z } from 'zod' +import { + type AppliedSandboxConfig, + applySandboxDefaults, + parseAgentMd, +} from '@agent-forge/core/types' import { FORGE_HOME } from './file-write.ts' export const DockerLaunchInputSchema = z.object({ @@ -30,7 +42,6 @@ export type DockerLaunchEvent = | { type: 'done'; exitCode: number } | { type: 'error'; error: string } -const IMAGE = process.env.FORGE_AGENT_IMAGE ?? 'agent-forge/base:latest' const TIMEOUT_MS = Number(process.env.FORGE_RUN_TIMEOUT_MS ?? '120000') // Resolved at runtime (host filesystem) — same path the CLI uses to mount @@ -57,12 +68,59 @@ function inheritEnv(): string[] { return out } +// Build the docker run flags that enforce the hardening profile +// declared in AGENT.md. These come BEFORE the image name in the +// args array, after the standard --name / -i / -v block. +// +// Exported (alongside resolveSandboxFromAgentMd) so tests can verify +// the translation without spawning an actual container. +export function hardeningFlags(cfg: AppliedSandboxConfig): string[] { + const out: string[] = [ + '--cap-drop=ALL', + '--security-opt=no-new-privileges', + `--network=${cfg.network}`, + `--user=${cfg.user}`, + `--memory=${cfg.memory}`, + `--cpus=${cfg.cpus.toString()}`, + `--pids-limit=${cfg.pidsLimit.toString()}`, + ] + if (cfg.readOnlyRoot) { + // Read-only root FS, with a tmpfs over /tmp so package + // installers, test runners and shell utilities that scribble + // there keep working without granting write to the image. + // /workspace is bind-mounted RW just below, so it's not + // affected by --read-only. + out.push('--read-only', '--tmpfs=/tmp:rw,size=64m,mode=1777') + } + return out +} + export type LaunchHandle = { containerName: string events: AsyncGenerator abort: () => void } +/** + * Resolve the AGENT.md frontmatter to its applied sandbox config — + * same routine used internally before launching, exposed here so the + * permission dialog can show the user what hardening profile this + * agent will run with (and warn on relaxations like network=bridge). + * + * Returns null if the AGENT.md cannot be parsed ; the caller should + * surface a parse error in the UI instead. + */ +export function resolveSandboxFromAgentMd( + agentMdContent: string, +): AppliedSandboxConfig | null { + try { + const parsed = parseAgentMd(agentMdContent) + return applySandboxDefaults(parsed.meta.sandbox) + } catch { + return null + } +} + /** * Launch a sub-agent and stream its output. The returned events generator * yields chunk events as they arrive, then a final `done` (or `error`). @@ -100,6 +158,22 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { return } + // Read the AGENT.md to know which image and which hardening + // profile to apply. Parsing here means malformed AGENT.md + // surfaces as a clean error before docker is even invoked. + let sandboxCfg: AppliedSandboxConfig + try { + const raw = readFileSync(agentMdPath, 'utf8') + const parsed = parseAgentMd(raw) + sandboxCfg = applySandboxDefaults(parsed.meta.sandbox) + } catch (err) { + yield { + type: 'error', + error: `cannot read AGENT.md : ${err instanceof Error ? err.message : String(err)}`, + } + return + } + mkdirSync(workspaceHostDir, { recursive: true }) const args = [ @@ -108,6 +182,8 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { '-i', '--name', containerName, + // Volumes BEFORE hardening flags so --read-only doesn't reject + // them. '-v', `${agentMdPath}:/agent/AGENT.md:ro`, '-v', @@ -116,8 +192,9 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { `${workspaceHostDir}:/workspace`, '-w', '/workspace', + ...hardeningFlags(sandboxCfg), ...inheritEnv(), - IMAGE, + sandboxCfg.image, 'node', '/runtime/runtime.mjs', ] diff --git a/packages/tools-core/src/index.ts b/packages/tools-core/src/index.ts index c38bac8..d787169 100644 --- a/packages/tools-core/src/index.ts +++ b/packages/tools-core/src/index.ts @@ -16,7 +16,9 @@ export { export { DockerLaunchInputSchema, + hardeningFlags, launchAgent, + resolveSandboxFromAgentMd, type DockerLaunchEvent, type DockerLaunchInput, type LaunchHandle, diff --git a/packages/tools-core/tests/hardening-flags.test.ts b/packages/tools-core/tests/hardening-flags.test.ts new file mode 100644 index 0000000..78cd752 --- /dev/null +++ b/packages/tools-core/tests/hardening-flags.test.ts @@ -0,0 +1,107 @@ +// Verify that the AGENT.md sandbox section is correctly translated +// into docker run flags. Pure unit test : no container is spawned. +// +// We deliberately avoid running real escape attempts (write outside +// /workspace, fork bomb, apt install) here — those are integration +// tests that require Docker on the CI host and a built image. They +// belong in a separate suite gated on the docker daemon. + +import { describe, expect, test } from 'bun:test' +import { applySandboxDefaults } from '../../core/src/types/agent-md.ts' +import { hardeningFlags } from '../src/docker-launch.ts' + +describe('hardeningFlags — strict defaults', () => { + const cfg = applySandboxDefaults({ image: 'agent-forge/base:latest' }) + const flags = hardeningFlags(cfg) + + test('drops every Linux capability', () => { + expect(flags).toContain('--cap-drop=ALL') + }) + + test('forbids privilege escalation', () => { + expect(flags).toContain('--security-opt=no-new-privileges') + }) + + test('disables network', () => { + expect(flags).toContain('--network=none') + }) + + test('runs as the non-root agent user', () => { + expect(flags).toContain('--user=agent') + }) + + test('caps memory at 512m', () => { + expect(flags).toContain('--memory=512m') + }) + + test('caps cpu at 1', () => { + expect(flags).toContain('--cpus=1') + }) + + test('caps pids at 128', () => { + expect(flags).toContain('--pids-limit=128') + }) + + test('mounts root FS read-only with a tmpfs over /tmp', () => { + expect(flags).toContain('--read-only') + const tmpfs = flags.find((f) => f.startsWith('--tmpfs=/tmp')) + expect(tmpfs).toBeDefined() + }) +}) + +describe('hardeningFlags — relaxations from AGENT.md', () => { + test('network=bridge is reflected in the flag', () => { + const cfg = applySandboxDefaults({ + image: 'agent-forge/base:latest', + network: 'bridge', + }) + expect(hardeningFlags(cfg)).toContain('--network=bridge') + expect(hardeningFlags(cfg)).not.toContain('--network=none') + }) + + test('readOnlyRoot=false drops --read-only entirely', () => { + const cfg = applySandboxDefaults({ + image: 'agent-forge/base:latest', + readOnlyRoot: false, + }) + const flags = hardeningFlags(cfg) + expect(flags).not.toContain('--read-only') + expect(flags.some((f) => f.startsWith('--tmpfs=/tmp'))).toBe(false) + }) + + test('custom resources override the defaults', () => { + const cfg = applySandboxDefaults({ + image: 'agent-forge/base:latest', + resources: { memory: '2g', cpus: 4, pidsLimit: 512 }, + }) + const flags = hardeningFlags(cfg) + expect(flags).toContain('--memory=2g') + expect(flags).toContain('--cpus=4') + expect(flags).toContain('--pids-limit=512') + }) + + test('custom user is forwarded', () => { + const cfg = applySandboxDefaults({ + image: 'agent-forge/base:latest', + user: 'root', + }) + expect(hardeningFlags(cfg)).toContain('--user=root') + }) +}) + +describe('hardeningFlags — invariants', () => { + test('cap-drop and no-new-privileges are ALWAYS present', () => { + // Even with the most permissive AGENT.md, these two stay on : we + // never want an agent to gain caps or escalate privileges. + const cfg = applySandboxDefaults({ + image: 'agent-forge/base:latest', + network: 'bridge', + readOnlyRoot: false, + user: 'root', + resources: { memory: '8g', cpus: 4, pidsLimit: 1024 }, + }) + const flags = hardeningFlags(cfg) + expect(flags).toContain('--cap-drop=ALL') + expect(flags).toContain('--security-opt=no-new-privileges') + }) +}) From 6bf057c9283b71d5da71a15eced629dc160426f8 Mon Sep 17 00:00:00 2001 From: Georges Garnier Date: Tue, 28 Apr 2026 00:40:26 +0200 Subject: [PATCH 2/9] feat(core,cli): structured file logger + FORGE_DEBUG / FORGE_LOG_FILE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Ink TUI owns stdout, so the moment something goes wrong inside a long pipeline (skill runner, docker launch, agent run) the user has nothing to grep. Add a real logger. Module : packages/core/src/log/index.ts. Tiny dependency-free JSON-lines logger, off by default, opt-in via env : FORGE_DEBUG=1 → debug threshold FORGE_DEBUG=trace|info|warn → explicit threshold FORGE_LOG_FILE=/path → override default file default file : ~/.agent-forge/logs/forge--.log When neither variable is set the logger is a no-op (zero IO). Failures inside the logger never throw — we'd rather drop a line than crash the host. Instrumented hot points : - useChat.send : prompt in, skill match outcome, parsed action blocks count + kinds. - streamBuilder : start (lang, skillCount, lastMessage), full reply at debug, system prompt at trace. - runScaffoldAndRun : both LLM calls (raw + stripped output), abort reasons. - launchAgent : applied sandbox config, full docker spawn args at debug, stderr / exit / errors. CLI surface : - /log slash command : prints current log path, or tells the user to set FORGE_DEBUG=1 if disabled. - /help lists /log. Tests : level filtering, JSON-lines format, circular-data safety, no-op when disabled. Docs : new "Debugging" section in both READMEs (EN + FR), with the env vars, the file location, and useful jq / grep one-liners. --- README.fr.md | 24 +++ README.md | 24 +++ packages/cli/src/commands.ts | 24 +++ packages/cli/src/hooks/useChat.ts | 12 ++ packages/core/package.json | 1 + packages/core/src/builder/skill-runner.ts | 33 +++- packages/core/src/builder/stream.ts | 18 ++- packages/core/src/log/index.ts | 177 ++++++++++++++++++++++ packages/core/tests/log.test.ts | 121 +++++++++++++++ packages/tools-core/src/docker-launch.ts | 30 ++-- 10 files changed, 452 insertions(+), 12 deletions(-) create mode 100644 packages/core/src/log/index.ts create mode 100644 packages/core/tests/log.test.ts diff --git a/README.fr.md b/README.fr.md index 9292852..d9f4672 100644 --- a/README.fr.md +++ b/README.fr.md @@ -203,6 +203,30 @@ La skill `scaffold-and-run` est livrée par défaut : elle se déclenche sur des - `↑↓ / PgUp / PgDn / g / G` — scroll dans la vue détail - `Ctrl+E` — retour live dans le transcript +## Debug + +La TUI possède stdout, donc pas de `console.log` possible — Forge écrit dans un fichier de log structuré. + +```bash +# Désactivé par défaut. Active via une de ces variables : +FORGE_DEBUG=1 bun run forge # niveau debug → ~/.agent-forge/logs/forge--.log +FORGE_DEBUG=trace bun run forge # plus fin (system prompts, réponses LLM complètes) +FORGE_LOG_FILE=/tmp/forge.log bun run forge # chemin explicite + +# Dans le REPL : +/log # affiche le chemin du log courant +``` + +Le log est en JSON-lines, une entrée par ligne : + +```json +{"t":"2026-04-27T22:30:00.000Z","level":"info","source":"useChat","msg":"send","data":{"prompt":"…"}} +{"t":"2026-04-27T22:30:01.523Z","level":"info","source":"skillRunner","msg":"runScaffoldAndRun start"} +{"t":"2026-04-27T22:30:04.812Z","level":"info","source":"dockerLaunch","msg":"launching","data":{"agent":"…","sandboxCfg":{…}}} +``` + +Greps utiles : `jq -r 'select(.level=="error")' forge-*.log`, ou `grep '"source":"dockerLaunch"' forge-*.log | jq`. + ## Architecture ``` diff --git a/README.md b/README.md index b297d9f..3666e13 100644 --- a/README.md +++ b/README.md @@ -203,6 +203,30 @@ Built-in `scaffold-and-run` ships today : it triggers on words like `audite`, `t - `↑↓ / PgUp / PgDn / g / G` — scroll inside the detail view - `Ctrl+E` — return the chat transcript to live mode +## Debugging + +The TUI owns stdout, so we never `console.log` — instead Forge ships a structured file logger. + +```bash +# Off by default. Either flag turns it on : +FORGE_DEBUG=1 bun run forge # debug level → ~/.agent-forge/logs/forge--.log +FORGE_DEBUG=trace bun run forge # finer (system prompts, full LLM replies) +FORGE_LOG_FILE=/tmp/forge.log bun run forge # explicit path + +# Inside the REPL : +/log # prints the current log path +``` + +The log is JSON-lines, one entry per line : + +```json +{"t":"2026-04-27T22:30:00.000Z","level":"info","source":"useChat","msg":"send","data":{"prompt":"…"}} +{"t":"2026-04-27T22:30:01.523Z","level":"info","source":"skillRunner","msg":"runScaffoldAndRun start"} +{"t":"2026-04-27T22:30:04.812Z","level":"info","source":"dockerLaunch","msg":"launching","data":{"agent":"…","sandboxCfg":{…}}} +``` + +Useful greps : `jq -r 'select(.level=="error")' forge-*.log`, or `grep '"source":"dockerLaunch"' forge-*.log | jq`. + ## Architecture ``` diff --git a/packages/cli/src/commands.ts b/packages/cli/src/commands.ts index ef214ad..ff49d02 100644 --- a/packages/cli/src/commands.ts +++ b/packages/cli/src/commands.ts @@ -8,6 +8,7 @@ import { loadSkillCatalog, setProviderOverride, } from '@agent-forge/core/builder' +import { currentLogPath, isLoggingEnabled } from '@agent-forge/core/log' import { type ForgeConfig, type Lang, @@ -61,6 +62,11 @@ function helpLines(lang: Lang): string[] { ? 'liste les skills disponibles' : 'list available skills' }`, + ` /log ${ + lang === 'fr' + ? "affiche le chemin du fichier de log courant (FORGE_DEBUG=1 pour activer)" + : 'show the current log file path (FORGE_DEBUG=1 to enable)' + }`, ] } @@ -187,6 +193,24 @@ export function runCommand( return { lines } } + case '/log': { + if (!isLoggingEnabled()) { + return { + lines: [ + lang === 'fr' + ? 'logging désactivé — relance avec FORGE_DEBUG=1 (ou FORGE_LOG_FILE=/path)' + : 'logging disabled — restart with FORGE_DEBUG=1 (or FORGE_LOG_FILE=/path)', + ], + } + } + const path = currentLogPath() + return { + lines: [ + lang === 'fr' ? `log courant : ${path ?? '?'}` : `current log : ${path ?? '?'}`, + ], + } + } + case '/skills': { const catalog = loadSkillCatalog() if (catalog.skills.length === 0) { diff --git a/packages/cli/src/hooks/useChat.ts b/packages/cli/src/hooks/useChat.ts index 83b42c9..32f8817 100644 --- a/packages/cli/src/hooks/useChat.ts +++ b/packages/cli/src/hooks/useChat.ts @@ -17,6 +17,7 @@ import { runScaffoldAndRun, streamBuilder, } from '@agent-forge/core/builder' +import { getLogger } from '@agent-forge/core/log' import { launchAgent } from '@agent-forge/tools-core' import { useCallback, useMemo, useRef, useState } from 'react' import { @@ -35,6 +36,8 @@ import { import type { Lang } from '../config/store.ts' import { getCurrentSession } from '../session/store.ts' +const log = getLogger('useChat') + export type TurnRole = 'user' | 'assistant' | 'system' export type ChatTurn = { @@ -320,6 +323,7 @@ export function useChat(lang: Lang): { const send = useCallback( async (prompt: string): Promise => { + log.info('send', { prompt, lang }) const userTurn: ChatTurn = { id: nextId(), role: 'user', content: prompt } const assistantTurn: ChatTurn = { id: nextId(), @@ -343,6 +347,8 @@ export function useChat(lang: Lang): { // (one per artefact) so small models keep the AGENT.md and the // run prompt cleanly separated. const matched = matchSkillForMessage(prompt, skillCatalog.skills) + if (matched) log.info('skill matched', { skill: matched.name }) + else log.debug('no skill matched') if (matched && matched.name === 'scaffold-and-run') { const skillCard: SkillAction = { id: nextActionId(), @@ -459,6 +465,12 @@ export function useChat(lang: Lang): { // Extract any forge:* blocks BEFORE persisting the assistant text. const blocks = findActionBlocks(acc) + log.debug('parsed action blocks', { + count: blocks.length, + blocks: blocks.map((b) => + b.ok ? { ok: true, kind: b.action.kind } : { ok: false, error: b.error }, + ), + }) const parseErrors: ChatTurn[] = [] const newActions: Action[] = [] // Skill bodies executed inline get appended to the assistant turn diff --git a/packages/core/package.json b/packages/core/package.json index 84419fe..7fc7da0 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -10,6 +10,7 @@ ".": "./src/index.ts", "./builder": "./src/builder/index.ts", "./docker": "./src/docker/index.ts", + "./log": "./src/log/index.ts", "./tools": "./src/tools/index.ts", "./types": "./src/types/index.ts" }, diff --git a/packages/core/src/builder/skill-runner.ts b/packages/core/src/builder/skill-runner.ts index ccf3bb9..9cdbc41 100644 --- a/packages/core/src/builder/skill-runner.ts +++ b/packages/core/src/builder/skill-runner.ts @@ -14,9 +14,12 @@ // work into two narrow calls forces the model to keep them apart. import { generateText } from 'ai' +import { getLogger } from '../log/index.ts' import { getBuilderModel } from './provider.ts' import type { BuilderLang } from './system-prompt.ts' +const log = getLogger('skillRunner') + export type ScaffoldAndRunResult = { agentName: string agentMdContent: string // full AGENT.md (frontmatter + body), no fences @@ -137,6 +140,11 @@ export async function runScaffoldAndRun(args: { const agentMdInstruction = buildAgentMdInstruction(args.lang) const runPromptInstruction = buildRunPromptInstruction(args.lang) + log.info('runScaffoldAndRun start', { + lang: args.lang, + userMessage: args.userMessage, + }) + // Call 1 : produce the AGENT.md. const agentMdResp = await generateText({ model, @@ -146,7 +154,18 @@ export async function runScaffoldAndRun(args: { }) const agentMdContent = stripFences(agentMdResp.text) const agentName = extractAgentName(agentMdContent) - if (!agentName) return null + log.debug('call 1 — AGENT.md', { + agentName, + agentMdRaw: agentMdResp.text, + agentMdStripped: agentMdContent, + }) + if (!agentName) { + log.warn('runScaffoldAndRun aborted', { + reason: 'no name extracted from AGENT.md', + content: agentMdContent, + }) + return null + } // Call 2 : produce the run prompt. const runResp = await generateText({ @@ -156,7 +175,17 @@ export async function runScaffoldAndRun(args: { maxTokens: 400, }) const runPrompt = stripFences(runResp.text) - if (runPrompt.length === 0) return null + log.debug('call 2 — run prompt', { + runPromptRaw: runResp.text, + runPromptStripped: runPrompt, + }) + if (runPrompt.length === 0) { + log.warn('runScaffoldAndRun aborted', { + reason: 'empty run prompt', + }) + return null + } + log.info('runScaffoldAndRun done', { agentName }) return { agentName, agentMdContent, runPrompt } } diff --git a/packages/core/src/builder/stream.ts b/packages/core/src/builder/stream.ts index dafec65..abd6dc1 100644 --- a/packages/core/src/builder/stream.ts +++ b/packages/core/src/builder/stream.ts @@ -8,6 +8,7 @@ // packages/cli/src/builder-actions.ts. import { streamText, type CoreMessage } from 'ai' +import { getLogger } from '../log/index.ts' import { getBuilderModel } from './provider.ts' import { type BuilderLang, @@ -15,6 +16,8 @@ import { getBuilderSystemPrompt, } from './system-prompt.ts' +const log = getLogger('streamBuilder') + export type ChatRole = 'user' | 'assistant' | 'system' export type ChatMessage = { @@ -36,16 +39,29 @@ export async function* streamBuilder({ lang, skills, }: StreamBuilderArgs): AsyncGenerator { + const system = getBuilderSystemPrompt(lang, { skills }) + log.info('streamBuilder start', { + lang, + skillCount: skills?.length ?? 0, + messageCount: messages.length, + lastMessage: messages[messages.length - 1], + }) + log.trace('streamBuilder system prompt', { system }) + const result = streamText({ model: getBuilderModel(), - system: getBuilderSystemPrompt(lang, { skills }), + system, messages: messages as CoreMessage[], // 512 leaves room for a full forge:write block (~300 tokens) plus a // short intro sentence. Override via FORGE_MAX_TOKENS if needed. maxTokens: Number(process.env.FORGE_MAX_TOKENS ?? '512'), }) + let acc = '' for await (const chunk of result.textStream) { + acc += chunk yield chunk } + log.info('streamBuilder done', { length: acc.length }) + log.debug('streamBuilder full reply', { reply: acc }) } diff --git a/packages/core/src/log/index.ts b/packages/core/src/log/index.ts new file mode 100644 index 0000000..ef00cb9 --- /dev/null +++ b/packages/core/src/log/index.ts @@ -0,0 +1,177 @@ +// Structured file logger. +// +// Why a custom mini-logger and not pino/winston : +// - We MUST never write to stdout : the Ink TUI owns it. A stray +// console.log corrupts the rendering. +// - We need it to work the same way in three places : the CLI host +// process, the runtime inside the container, and the tools-core +// helpers — which means no React-y dependency. +// - JSON lines so the log is grep-able and machine-readable later. +// +// Activation : +// - FORGE_DEBUG=1 → logs at level >= debug +// - FORGE_DEBUG=info|debug|trace → explicit threshold +// - FORGE_LOG_FILE=/path/to/log → override the default file +// - default file : ~/.agent-forge/logs/forge--.log +// +// When neither FORGE_DEBUG nor FORGE_LOG_FILE is set, the logger is a +// no-op : zero file IO, zero overhead. + +import { appendFileSync, existsSync, mkdirSync } from 'node:fs' +import { homedir } from 'node:os' +import { dirname, join } from 'node:path' + +export type LogLevel = 'trace' | 'debug' | 'info' | 'warn' | 'error' + +const LEVEL_ORDER: Record = { + trace: 10, + debug: 20, + info: 30, + warn: 40, + error: 50, +} + +type LoggerState = { + enabled: boolean + threshold: number + filePath: string | null + // Resolved at first use, then memoised. Null while disabled. +} + +let state: LoggerState | null = null + +function resolveState(): LoggerState { + if (state !== null) return state + + const envDebug = process.env.FORGE_DEBUG ?? '' + const envFile = process.env.FORGE_LOG_FILE ?? '' + const enabled = envDebug.length > 0 || envFile.length > 0 + + if (!enabled) { + state = { enabled: false, threshold: Number.MAX_SAFE_INTEGER, filePath: null } + return state + } + + let threshold = LEVEL_ORDER.debug + const lower = envDebug.toLowerCase() + if (lower in LEVEL_ORDER) { + threshold = LEVEL_ORDER[lower as LogLevel] + } else if (envDebug === '0' || envDebug === 'false') { + // Explicit off : someone set FORGE_DEBUG=0 to override an upstream + // setting. Honour it unless FORGE_LOG_FILE is also set. + if (envFile.length === 0) { + state = { + enabled: false, + threshold: Number.MAX_SAFE_INTEGER, + filePath: null, + } + return state + } + } + + let filePath: string + if (envFile.length > 0) { + filePath = envFile + } else { + const dir = join(homedir(), '.agent-forge', 'logs') + const ts = new Date().toISOString().replace(/[:.]/g, '-') + filePath = join(dir, `forge-${process.pid.toString()}-${ts}.log`) + } + + try { + mkdirSync(dirname(filePath), { recursive: true }) + } catch { + // Directory creation failed ; disable logging silently rather + // than crash the host process. + state = { enabled: false, threshold: Number.MAX_SAFE_INTEGER, filePath: null } + return state + } + + state = { enabled: true, threshold, filePath } + return state +} + +function write(level: LogLevel, source: string, msg: string, data?: unknown): void { + const s = resolveState() + if (!s.enabled || s.filePath === null) return + if (LEVEL_ORDER[level] < s.threshold) return + + const entry = { + t: new Date().toISOString(), + level, + source, + msg, + ...(data !== undefined ? { data: safeStringify(data) } : {}), + pid: process.pid, + } + try { + appendFileSync(s.filePath, `${JSON.stringify(entry)}\n`, 'utf8') + } catch { + // Never throw out of the logger — we'd rather lose a line than + // crash the host. + } +} + +// Stringify with a circular guard so passing complex objects (like +// docker spawn args, LLM responses) doesn't blow up. +function safeStringify(value: unknown): unknown { + try { + JSON.stringify(value) + return value + } catch { + const seen = new WeakSet() + return JSON.parse( + JSON.stringify(value, (_k, v) => { + if (typeof v === 'object' && v !== null) { + if (seen.has(v)) return '[circular]' + seen.add(v) + } + return v + }), + ) + } +} + +/** + * Get a logger bound to a source label. Use one per module so log + * lines self-identify (`useChat`, `dockerLaunch`, `skillRunner`, + * `runtime`, …). + */ +export function getLogger(source: string): { + trace: (msg: string, data?: unknown) => void + debug: (msg: string, data?: unknown) => void + info: (msg: string, data?: unknown) => void + warn: (msg: string, data?: unknown) => void + error: (msg: string, data?: unknown) => void +} { + return { + trace: (msg, data) => write('trace', source, msg, data), + debug: (msg, data) => write('debug', source, msg, data), + info: (msg, data) => write('info', source, msg, data), + warn: (msg, data) => write('warn', source, msg, data), + error: (msg, data) => write('error', source, msg, data), + } +} + +/** + * Path of the currently active log file, or null if logging is off. + * Used by the `/log` slash command to tell the user where to look. + */ +export function currentLogPath(): string | null { + const s = resolveState() + return s.enabled ? s.filePath : null +} + +/** + * True when at least one logging trigger is active. Cheap to call. + */ +export function isLoggingEnabled(): boolean { + return resolveState().enabled +} + +/** + * Test-only : reset the memoised state. Don't call from production. + */ +export function _resetLoggerStateForTests(): void { + state = null +} diff --git a/packages/core/tests/log.test.ts b/packages/core/tests/log.test.ts new file mode 100644 index 0000000..d14945b --- /dev/null +++ b/packages/core/tests/log.test.ts @@ -0,0 +1,121 @@ +// Logger tests : enabled / disabled by env, JSON-lines format, level +// filtering, never throws on bad input. + +import { afterEach, beforeEach, describe, expect, test } from 'bun:test' +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { + _resetLoggerStateForTests, + currentLogPath, + getLogger, + isLoggingEnabled, +} from '../src/log/index.ts' + +let TMP_DIR: string +const ORIGINAL_DEBUG = process.env.FORGE_DEBUG +const ORIGINAL_FILE = process.env.FORGE_LOG_FILE + +beforeEach(() => { + TMP_DIR = mkdtempSync(join(tmpdir(), 'forge-log-')) + delete process.env.FORGE_DEBUG + delete process.env.FORGE_LOG_FILE + _resetLoggerStateForTests() +}) + +afterEach(() => { + if (ORIGINAL_DEBUG === undefined) delete process.env.FORGE_DEBUG + else process.env.FORGE_DEBUG = ORIGINAL_DEBUG + if (ORIGINAL_FILE === undefined) delete process.env.FORGE_LOG_FILE + else process.env.FORGE_LOG_FILE = ORIGINAL_FILE + _resetLoggerStateForTests() + rmSync(TMP_DIR, { recursive: true, force: true }) +}) + +describe('logger — disabled by default', () => { + test('isLoggingEnabled is false when no env var set', () => { + expect(isLoggingEnabled()).toBe(false) + }) + + test('currentLogPath returns null', () => { + expect(currentLogPath()).toBeNull() + }) + + test('writing while disabled is a no-op', () => { + const log = getLogger('test') + log.info('this goes nowhere') + log.error('still nowhere', { code: 1 }) + // No file should have been created anywhere — we don't have a + // path to check, but the call must not have thrown. + expect(true).toBe(true) + }) +}) + +describe('logger — enabled via FORGE_LOG_FILE', () => { + test('writes JSON-lines to the configured path', () => { + const file = join(TMP_DIR, 'out.log') + process.env.FORGE_LOG_FILE = file + _resetLoggerStateForTests() + + const log = getLogger('myMod') + log.info('hello', { n: 1 }) + log.warn('careful', { reason: 'xx' }) + + expect(existsSync(file)).toBe(true) + const lines = readFileSync(file, 'utf8').trim().split('\n') + expect(lines.length).toBe(2) + const first = JSON.parse(lines[0] as string) + expect(first.level).toBe('info') + expect(first.source).toBe('myMod') + expect(first.msg).toBe('hello') + expect(first.data).toEqual({ n: 1 }) + expect(typeof first.t).toBe('string') + }) +}) + +describe('logger — level threshold via FORGE_DEBUG', () => { + test('FORGE_DEBUG=info skips trace and debug', () => { + const file = join(TMP_DIR, 'level.log') + process.env.FORGE_DEBUG = 'info' + process.env.FORGE_LOG_FILE = file + _resetLoggerStateForTests() + + const log = getLogger('lv') + log.trace('trace skipped') + log.debug('debug skipped') + log.info('info kept') + log.error('error kept') + + const lines = readFileSync(file, 'utf8').trim().split('\n') + expect(lines.length).toBe(2) + expect(JSON.parse(lines[0] as string).level).toBe('info') + expect(JSON.parse(lines[1] as string).level).toBe('error') + }) + + test('FORGE_DEBUG=1 defaults to debug threshold', () => { + const file = join(TMP_DIR, 'one.log') + process.env.FORGE_DEBUG = '1' + process.env.FORGE_LOG_FILE = file + _resetLoggerStateForTests() + + const log = getLogger('one') + log.trace('skipped') + log.debug('kept') + const lines = readFileSync(file, 'utf8').trim().split('\n') + expect(lines.length).toBe(1) + expect(JSON.parse(lines[0] as string).level).toBe('debug') + }) +}) + +describe('logger — robustness', () => { + test('does not throw on circular data', () => { + const file = join(TMP_DIR, 'circ.log') + process.env.FORGE_LOG_FILE = file + _resetLoggerStateForTests() + + const log = getLogger('safe') + const a: { self?: unknown } = {} + a.self = a + expect(() => log.info('circular', a)).not.toThrow() + }) +}) diff --git a/packages/tools-core/src/docker-launch.ts b/packages/tools-core/src/docker-launch.ts index ebdfaa0..7ae0634 100644 --- a/packages/tools-core/src/docker-launch.ts +++ b/packages/tools-core/src/docker-launch.ts @@ -19,6 +19,7 @@ import { spawn, spawnSync } from 'node:child_process' import { existsSync, mkdirSync, readFileSync } from 'node:fs' import { join } from 'node:path' import { z } from 'zod' +import { getLogger } from '@agent-forge/core/log' import { type AppliedSandboxConfig, applySandboxDefaults, @@ -26,6 +27,8 @@ import { } from '@agent-forge/core/types' import { FORGE_HOME } from './file-write.ts' +const log = getLogger('dockerLaunch') + export const DockerLaunchInputSchema = z.object({ agent: z .string() @@ -167,14 +170,19 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { const parsed = parseAgentMd(raw) sandboxCfg = applySandboxDefaults(parsed.meta.sandbox) } catch (err) { - yield { - type: 'error', - error: `cannot read AGENT.md : ${err instanceof Error ? err.message : String(err)}`, - } + const msg = err instanceof Error ? err.message : String(err) + log.error('agent.md parse failed', { agent: input.agent, error: msg }) + yield { type: 'error', error: `cannot read AGENT.md : ${msg}` } return } mkdirSync(workspaceHostDir, { recursive: true }) + log.info('launching', { + agent: input.agent, + containerName, + workspaceHostDir, + sandboxCfg, + }) const args = [ 'run', @@ -199,6 +207,7 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { '/runtime/runtime.mjs', ] + log.debug('docker spawn args', { containerName, args }) const child = spawn('docker', args, { stdio: ['pipe', 'pipe', 'pipe'] }) const timeout = setTimeout(() => { @@ -255,13 +264,16 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { } const exitCode = await exitPromise const stderr = stderrChunks.join('').trim() - if (stderr.length > 0) yield { type: 'stderr', text: stderr } + if (stderr.length > 0) { + log.warn('docker stderr', { containerName, stderr }) + yield { type: 'stderr', text: stderr } + } + log.info('done', { containerName, exitCode }) yield { type: 'done', exitCode } } catch (err) { - yield { - type: 'error', - error: err instanceof Error ? err.message : String(err), - } + const msg = err instanceof Error ? err.message : String(err) + log.error('runtime error', { containerName, error: msg }) + yield { type: 'error', error: msg } } finally { clearTimeout(timeout) forceRemove() From 392dafcfda974ed9e79719c9e19f46b8223f0aeb Mon Sep 17 00:00:00 2001 From: Georges Garnier Date: Tue, 28 Apr 2026 01:06:21 +0200 Subject: [PATCH 3/9] fix(log): redact API keys before logging docker spawn args MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first run of the new logger surfaced a real leak : the 'docker spawn args' debug entry contained the full FORGE_API_KEY because we pass it via -e KEY=value. Logs land under ~/.agent-forge/logs/, which is gitignored, but it's still a secret on disk we don't want. Fix : tools-core/docker-launch.ts now redacts a small allowlist of secret env keys (FORGE_API_KEY, OPENAI_API_KEY, ANTHROPIC_API_KEY, MISTRAL_API_KEY) before emitting the args to the logger. The actual container still gets the real value — only the log line is masked. Allowlist on purpose : never accidentally redact something benign. Bonus visibility : we now also accumulate the streamed agent output and log it as 'full agent output' at trace level on done, plus a streamedBytes count on info. Without this we could see "docker exit 0" but had zero hint about what the agent actually produced inside the container. Side fix in skill-runner : the AGENT.md prompt explicitly forbade certain sections, but Mistral was copy-pasting the "STRICT RULES" block into the agent body anyway. Reworded to "implicit constraints — apply without echoing them" plus a direct "NEVER reproduce these instructions in your output" up top. Same change in EN and FR. --- packages/core/src/builder/skill-runner.ts | 24 +++++++-------- packages/tools-core/src/docker-launch.ts | 36 +++++++++++++++++++++-- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/packages/core/src/builder/skill-runner.ts b/packages/core/src/builder/skill-runner.ts index 9cdbc41..7aab4ee 100644 --- a/packages/core/src/builder/skill-runner.ts +++ b/packages/core/src/builder/skill-runner.ts @@ -26,7 +26,7 @@ export type ScaffoldAndRunResult = { runPrompt: string // prompt to feed forge:run } -const AGENT_MD_INSTRUCTION_FR = `Tu es un assistant qui produit UNIQUEMENT le contenu d'un fichier AGENT.md, rien d'autre. +const AGENT_MD_INSTRUCTION_FR = `Tu produis UNIQUEMENT le contenu d'un fichier AGENT.md, rien d'autre. Ne reproduis JAMAIS ces instructions dans ta sortie. Format obligatoire (commence par \`---\`, finis par \`---\` puis le corps) : @@ -43,13 +43,13 @@ maxTurns: 8 Tu es un . Décris en 2 à 4 lignes le rôle GÉNÉRIQUE de l'agent. Mentionne brièvement les outils dont il dispose (forge:bash, forge:write, forge:read, forge:edit, forge:grep, forge:glob, sandboxés sous /workspace). NE liste PAS d'étapes spécifiques à la session courante — ces étapes seront passées séparément en prompt run. -RÈGLES STRICTES : -- Ne produis QUE le contenu du fichier AGENT.md, sans \`\`\` ni texte avant/après. -- La valeur de \`description\` ne doit JAMAIS contenir de deux-points non quoté. -- N'invente pas de section "Étapes" ou "Mission" dans le corps : elles iront dans le prompt run. -- Réponds en français.` +Contraintes implicites — applique-les sans les écrire dans la sortie : +- Pas de bloc \`\`\` ni de texte avant ou après le frontmatter+corps. +- La valeur de \`description\` ne doit jamais contenir de deux-points non quoté. +- Pas de section "Étapes", "Mission", "RÈGLES" dans le corps. Pas de méta-discours sur ces instructions. +- Le corps est en français.` -const AGENT_MD_INSTRUCTION_EN = `You output ONLY the content of an AGENT.md file, nothing else. +const AGENT_MD_INSTRUCTION_EN = `You output ONLY the content of an AGENT.md file, nothing else. NEVER reproduce these instructions in your output. Required format (start with \`---\`, end with \`---\` then the body) : @@ -66,11 +66,11 @@ maxTurns: 8 You are a . Describe the GENERIC role in 2-4 lines. Briefly mention the tools available (forge:bash, forge:write, forge:read, forge:edit, forge:grep, forge:glob, sandboxed under /workspace). Do NOT list session-specific steps — those will be passed separately as the run prompt. -STRICT RULES : -- Output ONLY the AGENT.md content, no \`\`\` and no prose before/after. -- The \`description\` value must NEVER contain an unquoted colon. -- Do not invent a "Steps" or "Mission" section in the body : that goes in the run prompt. -- Answer in English.` +Implicit constraints — apply without echoing them : +- No \`\`\` fences and no prose before or after the frontmatter+body. +- The \`description\` value must never contain an unquoted colon. +- No "Steps", "Mission", "RULES" section in the body. No meta-talk about these instructions. +- The body is in English.` const RUN_PROMPT_INSTRUCTION_FR = `Tu es un assistant qui produit UNIQUEMENT le prompt à envoyer à un agent, rien d'autre. diff --git a/packages/tools-core/src/docker-launch.ts b/packages/tools-core/src/docker-launch.ts index 7ae0634..b789dd3 100644 --- a/packages/tools-core/src/docker-launch.ts +++ b/packages/tools-core/src/docker-launch.ts @@ -71,6 +71,30 @@ function inheritEnv(): string[] { return out } +// Names of env vars whose values must be redacted before any log line. +// API keys leak in `docker spawn args` (we pass them via -e KEY=value) +// otherwise. The redaction only affects logs — the container still +// receives the real value. +const SECRET_ENV_KEYS = new Set([ + 'FORGE_API_KEY', + 'OPENAI_API_KEY', + 'ANTHROPIC_API_KEY', + 'MISTRAL_API_KEY', +]) + +function redactSecretsInArgs(args: string[]): string[] { + return args.map((a) => { + // Only `-e KEY=value` pairs land as a single arg here. Match + // against our explicit allowlist so we never accidentally redact + // something benign. + const eq = a.indexOf('=') + if (eq < 0) return a + const key = a.slice(0, eq) + if (SECRET_ENV_KEYS.has(key)) return `${key}=***redacted***` + return a + }) +} + // Build the docker run flags that enforce the hardening profile // declared in AGENT.md. These come BEFORE the image name in the // args array, after the standard --name / -i / -v block. @@ -207,7 +231,10 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { '/runtime/runtime.mjs', ] - log.debug('docker spawn args', { containerName, args }) + log.debug('docker spawn args', { + containerName, + args: redactSecretsInArgs(args), + }) const child = spawn('docker', args, { stdio: ['pipe', 'pipe', 'pipe'] }) const timeout = setTimeout(() => { @@ -256,10 +283,14 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { child.stdin.write(input.prompt) child.stdin.end() + let streamedTotal = 0 + let streamedAcc = '' try { while (true) { const evt = await next() if (evt.kind === 'end') break + streamedTotal += evt.text.length + streamedAcc += evt.text yield { type: 'chunk', text: evt.text } } const exitCode = await exitPromise @@ -268,7 +299,8 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { log.warn('docker stderr', { containerName, stderr }) yield { type: 'stderr', text: stderr } } - log.info('done', { containerName, exitCode }) + log.info('done', { containerName, exitCode, streamedBytes: streamedTotal }) + log.trace('full agent output', { containerName, output: streamedAcc }) yield { type: 'done', exitCode } } catch (err) { const msg = err instanceof Error ? err.message : String(err) From 41b964637cde1a8d8ca1241c07cffac6126411fa Mon Sep 17 00:00:00 2001 From: Georges Garnier Date: Tue, 28 Apr 2026 01:30:06 +0200 Subject: [PATCH 4/9] =?UTF-8?q?feat(p5.1.2):=20per-run=20LLM=20proxy=20on?= =?UTF-8?q?=20Unix=20socket=20=E2=80=94=20keeps=20network=3Dnone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scenario A surfaced the real cost of the strict sandbox : with --network=none, the in-container runtime cannot reach api.mistral.ai (or any LLM upstream), and streamText() fails silently. The container exited 0 with a single-byte stdout (just the trailing newline). Fix : a per-run LLM proxy on the host, exposed to the container through a Unix domain socket bind-mounted from the host. The container keeps --network=none ; the runtime points its OpenAI client at unix:///run/forge/llm.sock/v1, the proxy injects the host's API key and forwards to the real upstream. The container never sees the URL, never sees the credential. New module : packages/tools-core/src/llm-proxy.ts - startLlmProxy({ socketPath, upstreamBaseUrl, apiKey }) starts an http server on a Unix socket - allowlist : only POST /v1/chat/completions ; everything else returns 403 (no /v1/models, no /admin, no GET /) - the host's API key is set as Authorization, regardless of any Authorization header the client sent (an agent cannot impersonate by injecting its own) - streaming is preserved : the upstream IncomingMessage is wrapped in a ReadableStream so SSE chunks reach the SDK as they arrive - http vs https client picked from the upstream protocol so we support both Mistral cloud and local MLX DockerLaunch : - starts the proxy BEFORE docker run, stops it in the same try/finally that removes the container - bind-mounts the host socket dir at /run/forge inside the container - new containerEnv() : drops FORGE_API_KEY and the real FORGE_BASE_URL ; sends FORGE_BASE_URL=unix:///run/forge/llm.sock/v1 and a sentinel FORGE_API_KEY=via-proxy (the SDK requires a non-empty value) Runtime : - detects unix:// in FORGE_BASE_URL and builds a custom fetch that routes through an http.Agent bound to the socket - the SDK gets a synthetic http://localhost URL for path computation ; our fetch ignores the host Dockerfile : - mkdir /run/forge owned by agent so the bind-mount lands on a directory the user can read/write - hardening header updated to reflect that --network=none is now the default ALWAYS, not the default with bridge fallback Tests : tools-core/tests/llm-proxy.test.ts spins up a fake TCP upstream, drives the proxy through its socket, and checks : - allowed POST /v1/chat/completions forwards body and gets 200 - the host API key is injected as Authorization - a client-supplied Authorization is overridden - GET on the allowed path returns 405 - POST /v1/models, POST /admin, GET / all return 403 Out of scope for this PR : - ResponsesAPI / completions / embeddings — only chat completions is wired today, others can be added when needed - Per-agent token quotas — the proxy could enforce them, but that's a separate feature --- docker/base.Dockerfile | 11 +- packages/runtime/src/index.ts | 109 +++++++++- packages/tools-core/src/docker-launch.ts | 83 +++++-- packages/tools-core/src/index.ts | 6 + packages/tools-core/src/llm-proxy.ts | 228 ++++++++++++++++++++ packages/tools-core/tests/llm-proxy.test.ts | 167 ++++++++++++++ 6 files changed, 585 insertions(+), 19 deletions(-) create mode 100644 packages/tools-core/src/llm-proxy.ts create mode 100644 packages/tools-core/tests/llm-proxy.test.ts diff --git a/docker/base.Dockerfile b/docker/base.Dockerfile index f2a45c7..4f87520 100644 --- a/docker/base.Dockerfile +++ b/docker/base.Dockerfile @@ -9,9 +9,10 @@ # write under /tmp keep working without granting write to the # image # - --cap-drop=ALL --security-opt=no-new-privileges -# - --network=none by default ; agents that need network must -# declare `network: bridge` in AGENT.md.sandbox and the user -# gets a warning at confirm time +# - --network=none always — even agents that need an LLM call go +# through the host's per-run LLM proxy, bind-mounted as a Unix +# socket at /run/forge/llm.sock. The host injects the API key +# and forwards only /v1/chat/completions to the real upstream. FROM debian:bookworm-slim @@ -34,8 +35,8 @@ RUN curl -fsSL https://deb.nodesource.com/setup_${NODE_VERSION}.x | bash - \ # ─── Non-root user ─────────────────────────────────────────────── RUN useradd -m -s /bin/bash agent \ - && mkdir -p /workspace \ - && chown agent:agent /workspace + && mkdir -p /workspace /run/forge \ + && chown agent:agent /workspace /run/forge USER agent WORKDIR /workspace diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index a473609..f59289a 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -17,6 +17,7 @@ // inside [forge:tool] markers so the host can show them in Mission // Control without re-running the parser. +import { Agent as HttpAgent, request as httpRequest } from 'node:http' import { readFileSync } from 'node:fs' import { createOpenAI } from '@ai-sdk/openai' import { parseAgentMd } from '@agent-forge/core/types' @@ -82,6 +83,108 @@ function loadAgentConfig(): AgentConfig { return config } +// When FORGE_BASE_URL points at a Unix socket (unix:///path/to/sock/v1), +// we can't hand it directly to the OpenAI SDK : its fetch implementation +// only knows about TCP. Two helpers solve it : +// - normaliseBaseUrl returns a synthetic http://localhost URL with +// the same path, used purely as a key by the SDK +// - makeFetchFor returns a custom fetch that routes every request +// through an http.Agent bound to the socket +// +// The host's LLM proxy listens on that socket, injects the real +// API key, and forwards to the upstream. The container therefore +// runs with --network=none and never sees a credential. + +function isUnixBaseUrl(baseUrl: string): boolean { + return baseUrl.startsWith('unix://') +} + +function unixSocketPath(baseUrl: string): string { + // unix:///run/forge/llm.sock/v1 → /run/forge/llm.sock + // We split on the next path segment after the socket file. The + // proxy's allowlist works at /v1/chat/completions, so by + // convention we use a path suffix of /v1. + const stripped = baseUrl.slice('unix://'.length) + const v1Idx = stripped.lastIndexOf('/v1') + return v1Idx > 0 ? stripped.slice(0, v1Idx) : stripped +} + +function urlPathSuffix(baseUrl: string): string { + const stripped = baseUrl.slice('unix://'.length) + const v1Idx = stripped.lastIndexOf('/v1') + return v1Idx > 0 ? stripped.slice(v1Idx) : '/v1' +} + +function normaliseBaseUrl(baseUrl: string): string { + if (!isUnixBaseUrl(baseUrl)) return baseUrl + // The SDK requires a real-looking URL to compute the request path. + // The host part is bogus — our custom fetch ignores it and uses + // the Unix socket instead. + return `http://localhost${urlPathSuffix(baseUrl)}` +} + +function makeFetchFor(baseUrl: string): typeof fetch | undefined { + if (!isUnixBaseUrl(baseUrl)) return undefined + const socketPath = unixSocketPath(baseUrl) + const agent = new HttpAgent({ socketPath } as unknown as ConstructorParameters[0]) + return async (input: RequestInfo | URL, init?: RequestInit): Promise => { + const url = typeof input === 'string' ? input : input instanceof URL ? input.toString() : input.url + const parsed = new URL(url) + const method = init?.method ?? 'GET' + const headers: Record = {} + if (init?.headers) { + const h = new Headers(init.headers) + h.forEach((v, k) => { + headers[k] = v + }) + } + return await new Promise((resolve, reject) => { + const req = httpRequest( + { + method, + path: parsed.pathname + parsed.search, + headers, + agent, + }, + (res) => { + const respHeaders = new Headers() + for (const [k, v] of Object.entries(res.headers)) { + if (typeof v === 'string') respHeaders.set(k, v) + else if (Array.isArray(v)) respHeaders.set(k, v.join(', ')) + } + // Wrap the IncomingMessage in a ReadableStream so the + // Vercel AI SDK gets the SSE chunks as they arrive, + // instead of after the upstream closes the connection. + const body = new ReadableStream({ + start(controller) { + res.on('data', (b: Buffer) => controller.enqueue(new Uint8Array(b))) + res.on('end', () => controller.close()) + res.on('error', (err) => controller.error(err)) + }, + cancel() { + res.destroy() + }, + }) + resolve( + new Response(body, { + status: res.statusCode ?? 502, + headers: respHeaders, + }), + ) + }, + ) + req.on('error', reject) + if (init?.body) { + if (typeof init.body === 'string') req.end(init.body) + else if (init.body instanceof Uint8Array) req.end(Buffer.from(init.body)) + else req.end(String(init.body)) + } else { + req.end() + } + }) + } +} + async function readStdin(): Promise { const chunks: Uint8Array[] = [] for await (const chunk of process.stdin) { @@ -215,7 +318,11 @@ async function main(): Promise { process.exit(1) } - const provider = createOpenAI({ baseURL: BASE_URL, apiKey: API_KEY }) + const provider = createOpenAI({ + baseURL: normaliseBaseUrl(BASE_URL), + apiKey: API_KEY, + fetch: makeFetchFor(BASE_URL), + }) const hasTools = config.maxTurns > 1 const system = buildSystem(config, hasTools) diff --git a/packages/tools-core/src/docker-launch.ts b/packages/tools-core/src/docker-launch.ts index b789dd3..61b15fb 100644 --- a/packages/tools-core/src/docker-launch.ts +++ b/packages/tools-core/src/docker-launch.ts @@ -12,12 +12,21 @@ // must opt in to relax (network: bridge, readOnlyRoot: false, …) and // the permission dialog flags any non-default choice. // -// Multi-instance ready : each call uses a unique container name so several -// agents can run in parallel without collision. +// LLM proxy : because --network=none cuts the container off from the +// internet, an LLM proxy server is started on the HOST before docker +// run, listening on a Unix socket bind-mounted into the container at +// /run/forge/llm.sock. The runtime points its OpenAI client at that +// socket. The proxy forwards only /v1/chat/completions to the real +// FORGE_BASE_URL with FORGE_API_KEY injected. Container env carries +// neither the real URL nor the key. +// +// Multi-instance ready : each call uses a unique container name + a +// unique socket path, so several agents can run in parallel without +// collision. import { spawn, spawnSync } from 'node:child_process' import { existsSync, mkdirSync, readFileSync } from 'node:fs' -import { join } from 'node:path' +import { dirname, join } from 'node:path' import { z } from 'zod' import { getLogger } from '@agent-forge/core/log' import { @@ -26,9 +35,16 @@ import { parseAgentMd, } from '@agent-forge/core/types' import { FORGE_HOME } from './file-write.ts' +import { type LlmProxyHandle, startLlmProxy } from './llm-proxy.ts' const log = getLogger('dockerLaunch') +// Where the proxy socket appears INSIDE the container. The runtime's +// OpenAI client reads FORGE_BASE_URL and connects to this socket via +// a custom http agent. +const CONTAINER_SOCKET_PATH = '/run/forge/llm.sock' +const CONTAINER_BASE_URL = `unix://${CONTAINER_SOCKET_PATH}/v1` + export const DockerLaunchInputSchema = z.object({ agent: z .string() @@ -59,15 +75,19 @@ function uniqueContainerName(agent: string): string { return `agent-forge-run-${agent}-${id}` } -function inheritEnv(): string[] { - // Forward LLM credentials and overrides into the container so the runtime - // can call the same provider as the builder. - const passthrough = ['FORGE_BASE_URL', 'FORGE_API_KEY', 'FORGE_MODEL'] - const out: string[] = [] - for (const k of passthrough) { - const v = process.env[k] - if (v) out.push('-e', `${k}=${v}`) - } +function containerEnv(): string[] { + // The container does NOT receive the real LLM endpoint nor the API + // key. Instead it gets the in-container Unix socket URL ; the host + // proxy injects credentials and forwards to the real upstream. + const out: string[] = ['-e', `FORGE_BASE_URL=${CONTAINER_BASE_URL}`] + // Model name is fine to share — it's not a secret and the runtime + // needs it to ask for the right model. + const model = process.env.FORGE_MODEL + if (model) out.push('-e', `FORGE_MODEL=${model}`) + // The OpenAI SDK requires a non-empty API key field even when the + // upstream doesn't authenticate. Use a sentinel that's obviously + // not a credential. + out.push('-e', 'FORGE_API_KEY=via-proxy') return out } @@ -201,10 +221,33 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { } mkdirSync(workspaceHostDir, { recursive: true }) + + // Start the per-run LLM proxy on a Unix socket. The socket lives + // on the host under ~/.agent-forge/run//llm.sock and + // is bind-mounted at /run/forge/llm.sock inside the container. + const socketHostDir = join(FORGE_HOME, 'run', containerName) + const socketHostPath = join(socketHostDir, 'llm.sock') + const upstreamBase = process.env.FORGE_BASE_URL ?? 'https://api.mistral.ai/v1' + const upstreamKey = process.env.FORGE_API_KEY ?? '' + let proxy: LlmProxyHandle | null = null + try { + proxy = await startLlmProxy({ + socketPath: socketHostPath, + upstreamBaseUrl: upstreamBase, + apiKey: upstreamKey, + }) + } catch (err) { + const msg = err instanceof Error ? err.message : String(err) + log.error('proxy start failed', { error: msg }) + yield { type: 'error', error: `cannot start LLM proxy : ${msg}` } + return + } + log.info('launching', { agent: input.agent, containerName, workspaceHostDir, + socketHostPath, sandboxCfg, }) @@ -222,10 +265,15 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { `${RUNTIME_DIST_FROM_TOOLS}:/runtime:ro`, '-v', `${workspaceHostDir}:/workspace`, + // The LLM proxy socket : the host file is bind-mounted at the + // container path the runtime expects. The directory is mounted + // (not the file) so the socket inode resolves correctly. + '-v', + `${socketHostDir}:${dirname(CONTAINER_SOCKET_PATH)}`, '-w', '/workspace', ...hardeningFlags(sandboxCfg), - ...inheritEnv(), + ...containerEnv(), sandboxCfg.image, 'node', '/runtime/runtime.mjs', @@ -309,6 +357,15 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { } finally { clearTimeout(timeout) forceRemove() + if (proxy) { + try { + proxy.stop() + } catch (err) { + log.warn('proxy stop failed', { + error: err instanceof Error ? err.message : String(err), + }) + } + } } } diff --git a/packages/tools-core/src/index.ts b/packages/tools-core/src/index.ts index d787169..d3ad16a 100644 --- a/packages/tools-core/src/index.ts +++ b/packages/tools-core/src/index.ts @@ -24,6 +24,12 @@ export { type LaunchHandle, } from './docker-launch.ts' +export { + startLlmProxy, + type LlmProxyHandle, + type LlmProxyOptions, +} from './llm-proxy.ts' + // Runtime-side tools — used INSIDE the agent's container, sandboxed to // /workspace. Distinct from the host-side FileWrite above. export { diff --git a/packages/tools-core/src/llm-proxy.ts b/packages/tools-core/src/llm-proxy.ts new file mode 100644 index 0000000..30e36ba --- /dev/null +++ b/packages/tools-core/src/llm-proxy.ts @@ -0,0 +1,228 @@ +// LLM proxy — minimal HTTP server on a Unix domain socket that +// forwards a SINGLE OpenAI-compatible route to the real provider, +// with the host's API key injected at the boundary. +// +// Why : agent containers run with --network=none. They can't reach +// api.mistral.ai (or any other LLM endpoint) directly. The proxy +// gives them a controlled hole : a Unix socket bind-mounted into +// the container, exposing only the chat-completions route. The +// API key never enters the container — it's added to the outgoing +// request server-side. +// +// Lifecycle : +// - one proxy per run, started by DockerLaunch BEFORE `docker run`, +// stopped in the same try/finally that removes the container +// - the socket file lives under ~/.agent-forge/run// +// and is bind-mounted at /run/forge/llm.sock inside the container + +import { + type IncomingMessage, + type ServerResponse, + createServer, + request as httpRequest, +} from 'node:http' +import { request as httpsRequest } from 'node:https' +import { dirname } from 'node:path' +import { existsSync, mkdirSync, unlinkSync } from 'node:fs' +import { URL } from 'node:url' +import { getLogger } from '@agent-forge/core/log' + +const log = getLogger('llmProxy') + +// Routes the proxy is willing to forward. Anything else gets a 403 +// — an agent must not be able to list models, hit billing, or +// interact with anything beyond what it's supposed to talk to. +const ALLOWED_PATHS = new Set(['/v1/chat/completions']) + +export type LlmProxyHandle = { + socketPath: string + stop: () => void +} + +export type LlmProxyOptions = { + socketPath: string + // Real upstream — typically https://api.mistral.ai/v1, taken from + // the host's FORGE_BASE_URL. + upstreamBaseUrl: string + // The host's API key — injected into the forwarded request. + // Never leaves this process. + apiKey: string +} + +function stripTrailingSlash(s: string): string { + return s.endsWith('/') ? s.slice(0, -1) : s +} + +/** + * Start an LLM proxy on a Unix socket. Caller is responsible for + * calling stop() when the run ends ; we also unlink the socket file + * on stop, so a leftover from a previous crash doesn't block the + * next start (we unlink on start too). + * + * Returns once the server is actually listening, so DockerLaunch can + * safely bind-mount the socket immediately after. + */ +export async function startLlmProxy( + options: LlmProxyOptions, +): Promise { + // Ensure the parent dir exists and there's no stale socket file. + mkdirSync(dirname(options.socketPath), { recursive: true }) + if (existsSync(options.socketPath)) { + try { + unlinkSync(options.socketPath) + } catch { + // best effort + } + } + + const upstreamBase = stripTrailingSlash(options.upstreamBaseUrl) + const upstream = new URL(upstreamBase) + + const server = createServer((req: IncomingMessage, res: ServerResponse) => { + handleRequest(req, res, upstream, options.apiKey).catch((err) => { + log.error('proxy handler crash', { error: errString(err) }) + if (!res.headersSent) { + res.writeHead(502, { 'content-type': 'text/plain' }) + } + try { + res.end('proxy error') + } catch { + // ignore + } + }) + }) + + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(options.socketPath, () => resolve()) + }) + + log.info('proxy listening', { + socketPath: options.socketPath, + upstreamBase, + }) + + return { + socketPath: options.socketPath, + stop: () => { + server.close(() => { + if (existsSync(options.socketPath)) { + try { + unlinkSync(options.socketPath) + } catch { + // ignore + } + } + log.info('proxy stopped', { socketPath: options.socketPath }) + }) + }, + } +} + +async function handleRequest( + req: IncomingMessage, + res: ServerResponse, + upstream: URL, + apiKey: string, +): Promise { + const path = (req.url ?? '/').split('?')[0] ?? '/' + // Allow a base path under which the upstream lives (e.g. /v1) by + // checking the suffix of the incoming path against the allowlist. + // The container client uses the SAME paths as a regular OpenAI + // endpoint — `/v1/chat/completions` — and we forward it to the + // upstream's host preserving the same path. + if (!ALLOWED_PATHS.has(path)) { + log.warn('proxy reject', { method: req.method, path }) + res.writeHead(403, { 'content-type': 'application/json' }) + res.end(JSON.stringify({ error: { type: 'forbidden', path } })) + return + } + if (req.method !== 'POST') { + res.writeHead(405, { 'content-type': 'application/json' }) + res.end(JSON.stringify({ error: { type: 'method_not_allowed' } })) + return + } + + // Build the upstream URL : keep the upstream's host/port, replace + // the path with the incoming path. The upstream base is something + // like https://api.mistral.ai/v1 — we want POST https://api.mistral.ai/v1/chat/completions. + const upstreamPath = `${stripTrailingSlash(upstream.pathname)}${stripPrefix( + path, + stripTrailingSlash(upstream.pathname), + )}` + const targetUrl = new URL(upstreamPath, `${upstream.protocol}//${upstream.host}`) + + log.debug('proxy forward', { + incomingPath: path, + upstreamUrl: targetUrl.toString(), + }) + + // Sanitise headers : drop anything that could leak the agent's + // identity to the upstream beyond what we control. We keep + // content-type, accept, accept-encoding ; we set authorization + // ourselves. We forward `accept: text/event-stream` so streaming + // works. + const outHeaders: Record = { + authorization: `Bearer ${apiKey}`, + } + for (const [k, v] of Object.entries(req.headers)) { + const key = k.toLowerCase() + if (key === 'content-type' || key === 'accept' || key === 'accept-encoding') { + if (typeof v === 'string') outHeaders[key] = v + } + } + + // Pick http vs https client based on the upstream protocol so we + // support both Mistral cloud (https) and a local MLX server (http). + const requestFn = targetUrl.protocol === 'https:' ? httpsRequest : httpRequest + const upstreamReq = requestFn( + { + method: 'POST', + hostname: targetUrl.hostname, + port: targetUrl.port || (targetUrl.protocol === 'https:' ? 443 : 80), + path: targetUrl.pathname + targetUrl.search, + headers: outHeaders, + }, + (upstreamRes) => { + // Pipe the upstream response back. Forward status + a + // sanitised set of headers (content-type, transfer-encoding + // for streaming, cache-control). We do NOT pass through + // server / set-cookie / x-* — they have no use inside the + // sandboxed runtime. + const passthrough: Record = {} + for (const [k, v] of Object.entries(upstreamRes.headers)) { + const key = k.toLowerCase() + if ( + key === 'content-type' || + key === 'transfer-encoding' || + key === 'cache-control' + ) { + if (typeof v === 'string') passthrough[key] = v + } + } + res.writeHead(upstreamRes.statusCode ?? 502, passthrough) + upstreamRes.pipe(res) + }, + ) + + upstreamReq.on('error', (err) => { + log.error('upstream error', { error: errString(err) }) + if (!res.headersSent) { + res.writeHead(502, { 'content-type': 'application/json' }) + } + res.end( + JSON.stringify({ error: { type: 'upstream_error', message: errString(err) } }), + ) + }) + + // Pipe the request body straight through. + req.pipe(upstreamReq) +} + +function stripPrefix(s: string, prefix: string): string { + return s.startsWith(prefix) ? s.slice(prefix.length) : s +} + +function errString(err: unknown): string { + return err instanceof Error ? err.message : String(err) +} diff --git a/packages/tools-core/tests/llm-proxy.test.ts b/packages/tools-core/tests/llm-proxy.test.ts new file mode 100644 index 0000000..bd60f3c --- /dev/null +++ b/packages/tools-core/tests/llm-proxy.test.ts @@ -0,0 +1,167 @@ +// Tests for the LLM proxy. We spin up a fake upstream on an +// ephemeral TCP port, point the proxy at it, then issue HTTP +// requests through the Unix socket. Verifies : +// - allowed route is forwarded +// - other routes return 403 +// - non-POST returns 405 +// - the host's API key is injected as Authorization, regardless of +// what the client sent +// - the upstream Authorization is replaced (i.e. the agent can't +// impersonate someone else by setting it itself) + +import { afterEach, beforeEach, describe, expect, test } from 'bun:test' +import { type AddressInfo, createServer } from 'node:net' +import { type IncomingMessage, type Server, createServer as createHttpServer } from 'node:http' +import { request as httpRequest } from 'node:http' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { startLlmProxy, type LlmProxyHandle } from '../src/llm-proxy.ts' + +type FakeUpstream = { + port: number + receivedAuth: string | null + receivedPath: string | null + receivedBody: string + stop: () => Promise +} + +async function startFakeUpstream(): Promise { + const state: FakeUpstream = { + port: 0, + receivedAuth: null, + receivedPath: null, + receivedBody: '', + stop: async () => {}, + } + const server: Server = createHttpServer((req: IncomingMessage, res) => { + state.receivedAuth = (req.headers.authorization as string | undefined) ?? null + state.receivedPath = req.url ?? null + const chunks: Buffer[] = [] + req.on('data', (b: Buffer) => chunks.push(b)) + req.on('end', () => { + state.receivedBody = Buffer.concat(chunks).toString('utf8') + res.writeHead(200, { 'content-type': 'application/json' }) + res.end(JSON.stringify({ ok: true })) + }) + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + state.port = (server.address() as AddressInfo).port + state.stop = () => + new Promise((resolve) => server.close(() => resolve())) + return state +} + +let TMP_DIR: string +let upstream: FakeUpstream +let proxy: LlmProxyHandle +const ORIGINAL_DEBUG = process.env.FORGE_DEBUG + +beforeEach(async () => { + delete process.env.FORGE_DEBUG // keep test output clean + TMP_DIR = mkdtempSync(join(tmpdir(), 'forge-llmproxy-')) + upstream = await startFakeUpstream() + proxy = await startLlmProxy({ + socketPath: join(TMP_DIR, 'llm.sock'), + upstreamBaseUrl: `http://127.0.0.1:${upstream.port.toString()}/v1`, + apiKey: 'host-secret-key', + }) +}) + +afterEach(async () => { + proxy.stop() + await upstream.stop() + rmSync(TMP_DIR, { recursive: true, force: true }) + if (ORIGINAL_DEBUG === undefined) delete process.env.FORGE_DEBUG + else process.env.FORGE_DEBUG = ORIGINAL_DEBUG +}) + +type ProxyResponse = { + status: number + body: string +} + +function callProxy( + method: string, + path: string, + options: { body?: string; headers?: Record } = {}, +): Promise { + return new Promise((resolve, reject) => { + const req = httpRequest( + { + method, + path, + socketPath: proxy.socketPath, + headers: options.headers ?? {}, + }, + (res) => { + const chunks: Buffer[] = [] + res.on('data', (b: Buffer) => chunks.push(b)) + res.on('end', () => + resolve({ + status: res.statusCode ?? 0, + body: Buffer.concat(chunks).toString('utf8'), + }), + ) + }, + ) + req.on('error', reject) + if (options.body) req.end(options.body) + else req.end() + }) +} + +describe('llm-proxy — allowed route', () => { + test('forwards POST /v1/chat/completions to upstream', async () => { + const r = await callProxy('POST', '/v1/chat/completions', { + body: JSON.stringify({ model: 'x', messages: [] }), + headers: { 'content-type': 'application/json' }, + }) + expect(r.status).toBe(200) + expect(JSON.parse(r.body)).toEqual({ ok: true }) + expect(upstream.receivedPath).toBe('/v1/chat/completions') + expect(upstream.receivedBody).toContain('"model":"x"') + }) + + test('injects host API key as Authorization', async () => { + await callProxy('POST', '/v1/chat/completions', { + body: '{}', + headers: { 'content-type': 'application/json' }, + }) + expect(upstream.receivedAuth).toBe('Bearer host-secret-key') + }) + + test('client-supplied Authorization is overridden', async () => { + await callProxy('POST', '/v1/chat/completions', { + body: '{}', + headers: { + 'content-type': 'application/json', + authorization: 'Bearer agent-attempt-to-impersonate', + }, + }) + expect(upstream.receivedAuth).toBe('Bearer host-secret-key') + }) +}) + +describe('llm-proxy — denied routes', () => { + test('GET /v1/chat/completions → 405', async () => { + const r = await callProxy('GET', '/v1/chat/completions') + expect(r.status).toBe(405) + }) + + test('POST /v1/models → 403 (not in allowlist)', async () => { + const r = await callProxy('POST', '/v1/models', { body: '{}' }) + expect(r.status).toBe(403) + expect(upstream.receivedPath).toBeNull() + }) + + test('POST /admin → 403', async () => { + const r = await callProxy('POST', '/admin', { body: '{}' }) + expect(r.status).toBe(403) + }) + + test('GET / → 403', async () => { + const r = await callProxy('GET', '/') + expect(r.status).toBe(403) + }) +}) From d0872bf51afbd0d1bde026214786dea76da46e5f Mon Sep 17 00:00:00 2001 From: Georges Garnier Date: Tue, 28 Apr 2026 01:34:49 +0200 Subject: [PATCH 5/9] fix(runtime): surface LLM upstream errors instead of silently exiting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scenario A on the hardened sandbox kept producing 'streamedBytes: 1' (the trailing newline) with exit 0 and zero stderr. Direct repro in the container with a bogus FORGE_BASE_URL also exited 0 silently — the bug is in the runtime, not the proxy. Vercel AI SDK v4 quirk : `for await (const chunk of result.textStream)` silently completes when the upstream errors out. Errors land in `result.fullStream` as a part of type 'error', not as a thrown exception on the textStream iterator. Fix : iterate `fullStream`, pick text-delta parts as usual, and throw a real Error on any 'error' part. main().catch then logs '✗ runtime error: …' on stderr, dockerLaunch picks it up via the existing 'docker stderr' warn, and the user finally sees what went wrong instead of a green DONE that did nothing. --- packages/runtime/src/index.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index f59289a..cfd86f8 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -271,9 +271,19 @@ async function streamOneTurn( maxTokens: MAX_TOKENS, }) let acc = '' - for await (const chunk of result.textStream) { - process.stdout.write(chunk) - acc += chunk + // textStream silently completes on upstream errors in Vercel AI SDK + // v4 ; we walk fullStream instead so 'error' parts surface as a + // real throw the host can see in stderr (and the dockerLaunch + // logger will pick up). + for await (const part of result.fullStream) { + if (part.type === 'text-delta') { + process.stdout.write(part.textDelta) + acc += part.textDelta + } else if (part.type === 'error') { + const err = part.error + const msg = err instanceof Error ? err.message : String(err) + throw new Error(`LLM upstream error : ${msg}`) + } } return acc } From ad834d1f1503b1bfdf81e421c0a17e65d0d25c59 Mon Sep 17 00:00:00 2001 From: Georges Garnier Date: Tue, 28 Apr 2026 01:45:02 +0200 Subject: [PATCH 6/9] fix(runtime): use http.request socketPath option, drop http.Agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test on the hardened sandbox surfaced : ✗ runtime error: LLM upstream error : connect ENOTSUP /run/forge/llm.sock The previous attempt wired the Unix socket through a custom http.Agent ({ socketPath } as constructor opt). That pattern isn't supported by the default Node Agent — it tries TCP, the socket isn't a TCP endpoint, ENOTSUP fires. Right way : socketPath is a top-level option on http.request (and https.request) itself. Pass it directly, drop the Agent. Same effect, no constructor cast hack. The custom fetch is otherwise unchanged : it still wraps the IncomingMessage in a ReadableStream so SSE chunks reach the SDK as they arrive, and the SDK's URL parsing is satisfied by the synthetic http://localhost URL produced by normaliseBaseUrl. --- packages/runtime/src/index.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index cfd86f8..dde3411 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -17,7 +17,7 @@ // inside [forge:tool] markers so the host can show them in Mission // Control without re-running the parser. -import { Agent as HttpAgent, request as httpRequest } from 'node:http' +import { request as httpRequest } from 'node:http' import { readFileSync } from 'node:fs' import { createOpenAI } from '@ai-sdk/openai' import { parseAgentMd } from '@agent-forge/core/types' @@ -88,8 +88,9 @@ function loadAgentConfig(): AgentConfig { // only knows about TCP. Two helpers solve it : // - normaliseBaseUrl returns a synthetic http://localhost URL with // the same path, used purely as a key by the SDK -// - makeFetchFor returns a custom fetch that routes every request -// through an http.Agent bound to the socket +// - makeFetchFor returns a custom fetch that pipes every request +// through http.request with a top-level socketPath option (the +// standard Node.js way to do HTTP-over-UDS — no http.Agent) // // The host's LLM proxy listens on that socket, injects the real // API key, and forwards to the upstream. The container therefore @@ -126,7 +127,6 @@ function normaliseBaseUrl(baseUrl: string): string { function makeFetchFor(baseUrl: string): typeof fetch | undefined { if (!isUnixBaseUrl(baseUrl)) return undefined const socketPath = unixSocketPath(baseUrl) - const agent = new HttpAgent({ socketPath } as unknown as ConstructorParameters[0]) return async (input: RequestInfo | URL, init?: RequestInit): Promise => { const url = typeof input === 'string' ? input : input instanceof URL ? input.toString() : input.url const parsed = new URL(url) @@ -139,12 +139,15 @@ function makeFetchFor(baseUrl: string): typeof fetch | undefined { }) } return await new Promise((resolve, reject) => { + // socketPath is a top-level option on http.request — passing + // it via Agent throws ENOTSUP because the default Agent does + // TCP connect. Direct option = direct Unix connect. const req = httpRequest( { method, + socketPath, path: parsed.pathname + parsed.search, headers, - agent, }, (res) => { const respHeaders = new Headers() From 8d9396005c901f5778c002131a722d088f7366a8 Mon Sep 17 00:00:00 2001 From: Georges Garnier Date: Tue, 28 Apr 2026 03:21:37 +0200 Subject: [PATCH 7/9] fix(p5.1.3): auto-fallback to bridge when UDS bind-mount unsupported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docker Desktop on macOS bind-mounts host directories through a virtual filesystem (gRPC-FUSE / VirtioFS) that does NOT support Unix domain socket files as a recognised type. A connect() from inside the container fails with ENOTSUP, no matter how the client side wires socketPath. Reproduced in isolation with a minimal node http server on the host socket and a one-line http.request() inside the container — host can connect, container cannot, regardless of the agent construction or the option style. Pragmatic compromise : detect the limitation at startup and pick between two profiles. proxy : --network=none + per-run UDS proxy (the secure path we built in P5.1.2). Container has no network, no credentials. bridge : --network=bridge, FORGE_BASE_URL and FORGE_API_KEY forwarded to the container. Less ideal but works everywhere. New module : packages/tools-core/src/sandbox-network.ts - detectSandboxNetworkProfile() runs a tiny throwaway docker container that probes whether it can connect() to a host socket bind-mounted at /run/forge. Result is memoised for the session. - FORGE_SANDBOX_NETWORK=proxy|bridge overrides the probe (CI, tests, reproducing prod profile on a Mac dev machine). - On any probe failure (no docker, no image, timeout) we conservatively pick 'bridge' so the user gets the working flow ; the log explains why. DockerLaunch : - awaits the profile before composing args - in proxy mode : starts the proxy, mounts the socket, container env carries the unix:// URL - in bridge mode : skips the proxy, container env carries the real FORGE_BASE_URL and FORGE_API_KEY (the redaction layer in the logger keeps them out of every log line) hardeningFlags() now takes the profile as a 2nd argument : network=none in AGENT.md is upgraded to bridge under the bridge profile ; everything else (cap-drop, no-new-privileges, user, read-only, resource caps) stays on regardless. Tests : new "bridge fallback profile" suite verifies the network field upgrade, while the existing strict-defaults suite still passes (proxy is the default 2nd-arg). Docs : new "Sandbox networking" section in both READMEs. --- README.fr.md | 11 ++ README.md | 11 ++ packages/tools-core/src/docker-launch.ts | 114 ++++++++----- packages/tools-core/src/index.ts | 5 + packages/tools-core/src/sandbox-network.ts | 153 ++++++++++++++++++ .../tools-core/tests/hardening-flags.test.ts | 31 ++++ 6 files changed, 288 insertions(+), 37 deletions(-) create mode 100644 packages/tools-core/src/sandbox-network.ts diff --git a/README.fr.md b/README.fr.md index d9f4672..6a2ccbf 100644 --- a/README.fr.md +++ b/README.fr.md @@ -203,6 +203,17 @@ La skill `scaffold-and-run` est livrée par défaut : elle se déclenche sur des - `↑↓ / PgUp / PgDn / g / G` — scroll dans la vue détail - `Ctrl+E` — retour live dans le transcript +## Réseau de la sandbox + +Deux profils, choisis automatiquement au premier run : + +- **proxy** — `--network=none` dans le container ; le host fait tourner un proxy LLM par run sur une socket Unix bind-mountée à `/run/forge/llm.sock`. La clé API n'entre jamais dans le container. **C'est le profil strict que l'on veut.** +- **bridge** — `--network=bridge` ; le runtime parle directement à l'upstream. La clé API doit être passée en env du container. Moins idéal, mais c'est la seule chose qui marche sous Docker Desktop sur macOS (la couche FUSE des bind-mounts ne supporte pas les sockets Unix). + +Le détecteur fait un test au premier run avec un container jetable. Sous Linux, profil `proxy` ; sous Docker Desktop Mac, profil `bridge`. Override possible avec `FORGE_SANDBOX_NETWORK=proxy|bridge`. + +Les autres flags de hardening restent actifs quel que soit le profil : `--cap-drop=ALL`, `--security-opt=no-new-privileges`, `--read-only`, `--user=agent`, caps mémoire / cpus / pids. + ## Debug La TUI possède stdout, donc pas de `console.log` possible — Forge écrit dans un fichier de log structuré. diff --git a/README.md b/README.md index 3666e13..13e3a24 100644 --- a/README.md +++ b/README.md @@ -203,6 +203,17 @@ Built-in `scaffold-and-run` ships today : it triggers on words like `audite`, `t - `↑↓ / PgUp / PgDn / g / G` — scroll inside the detail view - `Ctrl+E` — return the chat transcript to live mode +## Sandbox networking + +Two profiles, picked automatically at first run : + +- **proxy** — `--network=none` inside the container ; the host runs a per-run LLM proxy on a Unix socket bind-mounted at `/run/forge/llm.sock`. The container never sees the API key. **This is the strict, secure profile we want.** +- **bridge** — `--network=bridge` ; the runtime talks to the upstream directly. The API key has to be forwarded into the container env. Less ideal, but it's the only thing that works under Docker Desktop on macOS (the FUSE bind-mount layer doesn't support Unix sockets). + +The detector probes once at startup with a tiny throwaway container. Runs on Linux pick `proxy` ; runs on Docker Desktop Mac pick `bridge`. Override with `FORGE_SANDBOX_NETWORK=proxy|bridge`. + +The other hardening flags stay on regardless of profile : `--cap-drop=ALL`, `--security-opt=no-new-privileges`, `--read-only`, `--user=agent`, memory / cpus / pids caps. + ## Debugging The TUI owns stdout, so we never `console.log` — instead Forge ships a structured file logger. diff --git a/packages/tools-core/src/docker-launch.ts b/packages/tools-core/src/docker-launch.ts index 61b15fb..9674848 100644 --- a/packages/tools-core/src/docker-launch.ts +++ b/packages/tools-core/src/docker-launch.ts @@ -36,6 +36,10 @@ import { } from '@agent-forge/core/types' import { FORGE_HOME } from './file-write.ts' import { type LlmProxyHandle, startLlmProxy } from './llm-proxy.ts' +import { + type SandboxNetworkProfile, + detectSandboxNetworkProfile, +} from './sandbox-network.ts' const log = getLogger('dockerLaunch') @@ -75,19 +79,29 @@ function uniqueContainerName(agent: string): string { return `agent-forge-run-${agent}-${id}` } -function containerEnv(): string[] { - // The container does NOT receive the real LLM endpoint nor the API - // key. Instead it gets the in-container Unix socket URL ; the host - // proxy injects credentials and forwards to the real upstream. - const out: string[] = ['-e', `FORGE_BASE_URL=${CONTAINER_BASE_URL}`] - // Model name is fine to share — it's not a secret and the runtime - // needs it to ask for the right model. +function containerEnv(profile: SandboxNetworkProfile): string[] { + const out: string[] = [] const model = process.env.FORGE_MODEL if (model) out.push('-e', `FORGE_MODEL=${model}`) - // The OpenAI SDK requires a non-empty API key field even when the - // upstream doesn't authenticate. Use a sentinel that's obviously - // not a credential. - out.push('-e', 'FORGE_API_KEY=via-proxy') + + if (profile === 'proxy') { + // The container does NOT receive the real LLM endpoint nor the + // API key. Instead it gets the in-container Unix socket URL ; + // the host proxy injects credentials and forwards. + out.push('-e', `FORGE_BASE_URL=${CONTAINER_BASE_URL}`) + // The OpenAI SDK requires a non-empty API key. Sentinel that's + // obviously not a credential. + out.push('-e', 'FORGE_API_KEY=via-proxy') + } else { + // Bridge fallback : container talks to the upstream directly. + // We HAVE to forward the real key here — there's no proxy in + // the path. The redaction layer in the logger keeps it out of + // every log line. + const upstreamBase = process.env.FORGE_BASE_URL + const upstreamKey = process.env.FORGE_API_KEY + if (upstreamBase) out.push('-e', `FORGE_BASE_URL=${upstreamBase}`) + if (upstreamKey) out.push('-e', `FORGE_API_KEY=${upstreamKey}`) + } return out } @@ -119,13 +133,29 @@ function redactSecretsInArgs(args: string[]): string[] { // declared in AGENT.md. These come BEFORE the image name in the // args array, after the standard --name / -i / -v block. // +// `networkProfile` overrides AGENT.md's `network` field when the host +// can't ship a Unix socket through a bind-mount (Docker Desktop on +// macOS — the FUSE layer doesn't pass UDS through). On those hosts +// we fall back to network=bridge so the runtime can talk to the +// upstream directly. Other hardening (cap-drop, no-new-privileges, +// non-root user, read-only root, resource caps) stays on. +// // Exported (alongside resolveSandboxFromAgentMd) so tests can verify // the translation without spawning an actual container. -export function hardeningFlags(cfg: AppliedSandboxConfig): string[] { +export function hardeningFlags( + cfg: AppliedSandboxConfig, + networkProfile: SandboxNetworkProfile = 'proxy', +): string[] { + // Resolve the actual --network flag : when AGENT.md asks for none + // but we need a fallback bridge to reach the LLM, the profile wins. + const effectiveNetwork = + cfg.network === 'none' && networkProfile === 'bridge' + ? 'bridge' + : cfg.network const out: string[] = [ '--cap-drop=ALL', '--security-opt=no-new-privileges', - `--network=${cfg.network}`, + `--network=${effectiveNetwork}`, `--user=${cfg.user}`, `--memory=${cfg.memory}`, `--cpus=${cfg.cpus.toString()}`, @@ -222,32 +252,41 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { mkdirSync(workspaceHostDir, { recursive: true }) - // Start the per-run LLM proxy on a Unix socket. The socket lives - // on the host under ~/.agent-forge/run//llm.sock and - // is bind-mounted at /run/forge/llm.sock inside the container. + // Pick the network profile : 'proxy' (network=none + UDS proxy) + // when supported, 'bridge' when the host can't ship a Unix + // socket through a bind-mount (Docker Desktop on macOS). + const networkProfile: SandboxNetworkProfile = + await detectSandboxNetworkProfile() + + // Proxy profile : start the per-run LLM proxy on a Unix socket + // bind-mounted into the container. Bridge profile : skip the + // proxy, the runtime talks to the upstream directly. const socketHostDir = join(FORGE_HOME, 'run', containerName) const socketHostPath = join(socketHostDir, 'llm.sock') - const upstreamBase = process.env.FORGE_BASE_URL ?? 'https://api.mistral.ai/v1' - const upstreamKey = process.env.FORGE_API_KEY ?? '' let proxy: LlmProxyHandle | null = null - try { - proxy = await startLlmProxy({ - socketPath: socketHostPath, - upstreamBaseUrl: upstreamBase, - apiKey: upstreamKey, - }) - } catch (err) { - const msg = err instanceof Error ? err.message : String(err) - log.error('proxy start failed', { error: msg }) - yield { type: 'error', error: `cannot start LLM proxy : ${msg}` } - return + if (networkProfile === 'proxy') { + const upstreamBase = process.env.FORGE_BASE_URL ?? 'https://api.mistral.ai/v1' + const upstreamKey = process.env.FORGE_API_KEY ?? '' + try { + proxy = await startLlmProxy({ + socketPath: socketHostPath, + upstreamBaseUrl: upstreamBase, + apiKey: upstreamKey, + }) + } catch (err) { + const msg = err instanceof Error ? err.message : String(err) + log.error('proxy start failed', { error: msg }) + yield { type: 'error', error: `cannot start LLM proxy : ${msg}` } + return + } } log.info('launching', { agent: input.agent, containerName, workspaceHostDir, - socketHostPath, + networkProfile, + socketHostPath: networkProfile === 'proxy' ? socketHostPath : null, sandboxCfg, }) @@ -265,15 +304,16 @@ export function launchAgent(input: DockerLaunchInput): LaunchHandle { `${RUNTIME_DIST_FROM_TOOLS}:/runtime:ro`, '-v', `${workspaceHostDir}:/workspace`, - // The LLM proxy socket : the host file is bind-mounted at the - // container path the runtime expects. The directory is mounted - // (not the file) so the socket inode resolves correctly. - '-v', - `${socketHostDir}:${dirname(CONTAINER_SOCKET_PATH)}`, + // The LLM proxy socket : only mounted in proxy profile. We + // mount the directory (not the file) so the socket inode + // resolves correctly inside the container. + ...(networkProfile === 'proxy' + ? ['-v', `${socketHostDir}:${dirname(CONTAINER_SOCKET_PATH)}`] + : []), '-w', '/workspace', - ...hardeningFlags(sandboxCfg), - ...containerEnv(), + ...hardeningFlags(sandboxCfg, networkProfile), + ...containerEnv(networkProfile), sandboxCfg.image, 'node', '/runtime/runtime.mjs', diff --git a/packages/tools-core/src/index.ts b/packages/tools-core/src/index.ts index d3ad16a..e442763 100644 --- a/packages/tools-core/src/index.ts +++ b/packages/tools-core/src/index.ts @@ -30,6 +30,11 @@ export { type LlmProxyOptions, } from './llm-proxy.ts' +export { + detectSandboxNetworkProfile, + type SandboxNetworkProfile, +} from './sandbox-network.ts' + // Runtime-side tools — used INSIDE the agent's container, sandboxed to // /workspace. Distinct from the host-side FileWrite above. export { diff --git a/packages/tools-core/src/sandbox-network.ts b/packages/tools-core/src/sandbox-network.ts new file mode 100644 index 0000000..191a86f --- /dev/null +++ b/packages/tools-core/src/sandbox-network.ts @@ -0,0 +1,153 @@ +// Sandbox networking profile picker. +// +// The strict profile (network=none + per-run LLM proxy on a Unix +// socket bind-mounted into the container) is what we want for +// security : the container cannot make outbound calls beyond the +// allowlisted /v1/chat/completions, the API key never enters the +// container, etc. +// +// But Docker Desktop on macOS bind-mounts host directories through +// a virtual filesystem (gRPC-FUSE / VirtioFS) that does NOT support +// Unix domain socket files as a recognised type. A connect() from +// inside the container fails with ENOTSUP. +// +// The pragmatic compromise : detect that limitation once at startup +// and fall back to network=bridge for that host. The runtime then +// talks to the upstream directly. Less ideal — we leak the API key +// into the container env — but it's the only thing that works under +// Docker Desktop until they fix the FUSE layer. +// +// On a real Linux host (or Linux running inside a real VM), the +// detector returns 'proxy' and we keep the strict profile. +// +// Env override : FORGE_SANDBOX_NETWORK=proxy|bridge to force one or +// the other regardless of the probe (useful for tests, CI, or to +// reproduce the prod profile on a Mac dev machine). + +import { spawnSync } from 'node:child_process' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { getLogger } from '@agent-forge/core/log' +import { startLlmProxy } from './llm-proxy.ts' + +const log = getLogger('sandboxNetwork') + +export type SandboxNetworkProfile = 'proxy' | 'bridge' + +let cachedProfile: SandboxNetworkProfile | null = null + +function envOverride(): SandboxNetworkProfile | null { + const v = (process.env.FORGE_SANDBOX_NETWORK ?? '').toLowerCase() + if (v === 'proxy' || v === 'bridge') return v + return null +} + +/** + * Probe whether this host can expose a Unix domain socket to a + * container through a bind-mount. Spawns a tiny container, points + * it at a socket served by a one-shot Node http server on the host, + * and checks whether `node http.request({ socketPath })` connects. + * + * The result is memoised : the probe runs at most once per process. + * + * On any error (docker missing, image missing, probe timeout) we + * conservatively assume the strict profile is unsafe and pick + * 'bridge'. The user gets the working flow ; the log explains why. + */ +export async function detectSandboxNetworkProfile(): Promise { + if (cachedProfile !== null) return cachedProfile + const forced = envOverride() + if (forced !== null) { + cachedProfile = forced + log.info('profile forced by env', { profile: forced }) + return forced + } + + const tmp = mkdtempSync(join(tmpdir(), 'forge-uds-probe-')) + const socketPath = join(tmp, 'probe.sock') + let proxy: Awaited> | null = null + + try { + // Spin up an LLM proxy on the socket — it doesn't matter that + // there's no real upstream behind, we only check whether the + // container can reach the socket file. + proxy = await startLlmProxy({ + socketPath, + upstreamBaseUrl: 'http://127.0.0.1:1/v1', // unreachable, fine for probe + apiKey: 'probe', + }) + + const result = spawnSync( + 'docker', + [ + 'run', + '--rm', + '--user=agent', + '--network=none', + '-v', + `${tmp}:/run/forge`, + 'agent-forge/base:latest', + 'node', + '-e', + // Connect to the socket, send a POST to a route the proxy + // refuses anyway (we only care about connect/EBADF/ENOTSUP). + // Print 'OK' on connect or the error code otherwise. + `const http=require("node:http");const req=http.request({socketPath:"/run/forge/probe.sock",path:"/v1/chat/completions",method:"POST",headers:{"content-type":"application/json"}},r=>{console.log("OK",r.statusCode)});req.on("error",e=>console.log("ERR",e.code||e.message));req.end("{}")`, + ], + { encoding: 'utf8', timeout: 8000 }, + ) + const stdout = (result.stdout ?? '').trim() + const stderr = (result.stderr ?? '').trim() + log.debug('probe result', { + exit: result.status, + stdout, + stderr: stderr.slice(0, 500), + }) + + // Anything starting with OK = the connect succeeded ; the + // socket is reachable from inside the container. + if (stdout.startsWith('OK')) { + cachedProfile = 'proxy' + log.info('profile picked', { + profile: 'proxy', + reason: 'socket bind-mount supported', + }) + } else { + cachedProfile = 'bridge' + log.warn('profile picked', { + profile: 'bridge', + reason: 'socket bind-mount unsupported (likely Docker Desktop on macOS)', + probeStdout: stdout, + probeStderr: stderr.slice(0, 200), + }) + } + } catch (err) { + cachedProfile = 'bridge' + log.warn('profile picked', { + profile: 'bridge', + reason: 'probe crashed', + error: err instanceof Error ? err.message : String(err), + }) + } finally { + if (proxy) { + try { + proxy.stop() + } catch { + // ignore + } + } + try { + rmSync(tmp, { recursive: true, force: true }) + } catch { + // ignore + } + } + + return cachedProfile +} + +/** Test-only : reset the memoised result. */ +export function _resetProfileCacheForTests(): void { + cachedProfile = null +} diff --git a/packages/tools-core/tests/hardening-flags.test.ts b/packages/tools-core/tests/hardening-flags.test.ts index 78cd752..911ff57 100644 --- a/packages/tools-core/tests/hardening-flags.test.ts +++ b/packages/tools-core/tests/hardening-flags.test.ts @@ -89,6 +89,37 @@ describe('hardeningFlags — relaxations from AGENT.md', () => { }) }) +describe('hardeningFlags — bridge fallback profile', () => { + test('upgrades network=none to bridge when profile=bridge', () => { + // AGENT.md asks for none (the strict default), but the host + // can't expose a UDS bind-mount → fallback bridge is applied. + const cfg = applySandboxDefaults({ image: 'agent-forge/base:latest' }) + const flags = hardeningFlags(cfg, 'bridge') + expect(flags).toContain('--network=bridge') + expect(flags).not.toContain('--network=none') + }) + + test('keeps explicit bridge under bridge profile', () => { + const cfg = applySandboxDefaults({ + image: 'agent-forge/base:latest', + network: 'bridge', + }) + const flags = hardeningFlags(cfg, 'bridge') + expect(flags).toContain('--network=bridge') + }) + + test('keeps explicit bridge under proxy profile too', () => { + // The author has explicitly opted into bridge ; we don't + // downgrade to none just because the proxy is available. + const cfg = applySandboxDefaults({ + image: 'agent-forge/base:latest', + network: 'bridge', + }) + const flags = hardeningFlags(cfg, 'proxy') + expect(flags).toContain('--network=bridge') + }) +}) + describe('hardeningFlags — invariants', () => { test('cap-drop and no-new-privileges are ALWAYS present', () => { // Even with the most permissive AGENT.md, these two stay on : we From f37882aa759e8576c64662c7e496ed3d461624b4 Mon Sep 17 00:00:00 2001 From: Georges Garnier Date: Tue, 28 Apr 2026 04:02:04 +0200 Subject: [PATCH 8/9] fix(p5.1.4): runtime executes ALL tool blocks per turn, not just the first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scenario A on the working sandbox revealed the next bug : the agent emitted SIX forge:* blocks in a single reply (write, write, grep, edit, edit, bash) and only ONE got executed. Cause : parseFirstToolBlock returned just the first match. The runtime acted on it, then re-invoked the LLM with a context that showed the agent had emitted 6 calls but received 1 result. The LLM tried to recover ("let me run npm test instead", "let me try ts-node", etc.) and the source files on disk stayed broken — the forge:edit blocks that should have applied the fixes never ran. Fix : - new parseAllToolBlocks(stream) walks every fenced forge:* block in source order, returning a ParseOutcome[] with the prose chunk that precedes each block. Invalid blocks are kept in the array (rather than silently dropped) so the model can see the 'forge:tool error' feedback. - runtime tool loop iterates the array : execute each block, write its [forge:tool] result to stdout, append it as a user message to the conversation, then fall through to the next block. Only after every block of a turn is processed do we re-invoke the LLM. This way the model sees a clean (call, result, call, result) sequence in its history, exactly the way OpenAI tool-calls would have arrived. - parseFirstToolBlock kept (still used by tests) but reduced to a thin wrapper around the shared buildOutcomeFromMatch. Tests : new parseAllToolBlocks suite covers ordering, invalid blocks not silently dropped, prose attribution per block. --- packages/runtime/src/index.ts | 40 +++++++------ packages/runtime/src/tool-protocol.ts | 57 ++++++++++++++----- packages/runtime/tests/tool-protocol.test.ts | 60 +++++++++++++++++++- 3 files changed, 124 insertions(+), 33 deletions(-) diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index dde3411..8473fe7 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -31,7 +31,7 @@ import { } from '@agent-forge/tools-core' import { type CoreMessage, streamText } from 'ai' import { - parseFirstToolBlock, + parseAllToolBlocks, renderBashResult, renderEditResult, renderGlobResult, @@ -292,7 +292,7 @@ async function streamOneTurn( } async function executeToolBlock( - parsed: Extract, { kind: 'tool' }>, + parsed: Extract[number], { kind: 'tool' }>, ): Promise { const tool = parsed.tool switch (tool.kind) { @@ -347,25 +347,31 @@ async function main(): Promise { if (!hasTools) break - const parsed = parseFirstToolBlock(reply) - if (parsed.kind === 'none') break + const parsed = parseAllToolBlocks(reply) + if (parsed.length === 0) break - // Record what the LLM just said (text + raw block) so the next turn - // sees it as a real assistant message. + // Record what the LLM just said (text + every raw block) so the + // next turn sees it as a real assistant message. messages.push({ role: 'assistant', content: reply }) - let toolReply: string - if (parsed.kind === 'invalid') { - toolReply = renderInvalid(parsed.error) - } else { - toolReply = await executeToolBlock(parsed) + // Execute each block in source order. Each result is appended + // back as its own user message so the LLM gets the full + // sequence of (call, result, call, result, …) instead of a + // fused blob — that ordering matters for the model to + // attribute each result to its corresponding call. + for (const block of parsed) { + let toolReply: string + if (block.kind === 'invalid') { + toolReply = renderInvalid(block.error) + } else { + toolReply = await executeToolBlock(block) + } + // Mark tool output for the host TUI so it can render it + // inside the Mission Control card instead of mixing it + // with prose. + process.stdout.write(`\n[forge:tool]\n${toolReply}\n[/forge:tool]\n`) + messages.push({ role: 'user', content: toolReply }) } - - // Mark tool output for the host TUI so it can render it inside the - // Mission Control card instead of mixing it with prose. - process.stdout.write(`\n[forge:tool]\n${toolReply}\n[/forge:tool]\n`) - - messages.push({ role: 'user', content: toolReply }) } } diff --git a/packages/runtime/src/tool-protocol.ts b/packages/runtime/src/tool-protocol.ts index b9ea1a0..e7b9c86 100644 --- a/packages/runtime/src/tool-protocol.ts +++ b/packages/runtime/src/tool-protocol.ts @@ -33,10 +33,13 @@ // { "pattern": "src/**/*.ts" } // ``` // -// Only ONE block is parsed per turn (the first encountered). Everything -// before the block is treated as the agent's "thinking out loud" text -// and streamed to the host. Everything after the block is dropped — the -// agent will see the tool result on the next turn and continue from there. +// All blocks emitted in a single LLM reply are parsed and executed in +// order via parseAllToolBlocks. Small models (Mistral Small) routinely +// emit several blocks in one turn ; processing only the first one +// (the historical behaviour, kept as parseFirstToolBlock for tests +// and edge cases) led to lost work and the agent visibly panicking +// in the next turn ("I wrote utils.ts but forgot the edits, let me +// run npm test instead"). import { z } from 'zod' import { @@ -86,15 +89,16 @@ const SCHEMAS: Record = { } const FENCE_RE = /```forge:(bash|write|read|edit|grep|glob)\s*\n([\s\S]*?)```/ - -export function parseFirstToolBlock(stream: string): ParseOutcome { - const m = FENCE_RE.exec(stream) - if (!m) return { kind: 'none', text: stream } - +// Same shape as FENCE_RE but with the global flag so matchAll can walk +// every block in the reply, in order. +const FENCE_RE_GLOBAL = /```forge:(bash|write|read|edit|grep|glob)\s*\n([\s\S]*?)```/g + +function buildOutcomeFromMatch( + m: RegExpMatchArray, + before: string, +): ParseOutcome { const tag = m[1] as ToolKind const body = m[2] ?? '' - const before = stream.slice(0, m.index) - let parsed: unknown try { parsed = JSON.parse(body) @@ -108,7 +112,6 @@ export function parseFirstToolBlock(stream: string): ParseOutcome { raw: m[0], } } - const schema = SCHEMAS[tag] const result = schema.safeParse(parsed) if (!result.success) { @@ -119,9 +122,6 @@ export function parseFirstToolBlock(stream: string): ParseOutcome { raw: m[0], } } - - // Narrow to the right ParsedTool variant by tag — the schema guarantees - // the data shape matches. return { kind: 'tool', text: before, @@ -129,6 +129,33 @@ export function parseFirstToolBlock(stream: string): ParseOutcome { } } +/** + * Parse every forge:* block in the reply, in source order. Each entry + * carries the prose chunk that PRECEDES it (so the runtime can surface + * the agent's narration before each tool call). Returns an empty array + * if the reply has no blocks at all. + */ +export function parseAllToolBlocks(stream: string): ParseOutcome[] { + const matches = [...stream.matchAll(FENCE_RE_GLOBAL)] + if (matches.length === 0) return [] + const outcomes: ParseOutcome[] = [] + let cursor = 0 + for (const m of matches) { + const idx = m.index ?? 0 + const before = stream.slice(cursor, idx) + outcomes.push(buildOutcomeFromMatch(m, before)) + cursor = idx + m[0].length + } + return outcomes +} + +export function parseFirstToolBlock(stream: string): ParseOutcome { + const m = FENCE_RE.exec(stream) + if (!m) return { kind: 'none', text: stream } + const before = stream.slice(0, m.index) + return buildOutcomeFromMatch(m, before) +} + function formatZodError(err: z.ZodError): string { return err.errors .map((e) => `${e.path.join('.') || '(root)'}: ${e.message}`) diff --git a/packages/runtime/tests/tool-protocol.test.ts b/packages/runtime/tests/tool-protocol.test.ts index b05cae1..213b313 100644 --- a/packages/runtime/tests/tool-protocol.test.ts +++ b/packages/runtime/tests/tool-protocol.test.ts @@ -1,7 +1,7 @@ // Tests for the agent-side tool block parser. Pure : no FS, no spawn. import { describe, expect, test } from 'bun:test' -import { parseFirstToolBlock } from '../src/tool-protocol.ts' +import { parseAllToolBlocks, parseFirstToolBlock } from '../src/tool-protocol.ts' describe('parseFirstToolBlock', () => { test('returns kind=none on plain text', () => { @@ -123,3 +123,61 @@ describe('parseFirstToolBlock', () => { expect(r.kind).toBe('invalid') }) }) + +describe('parseAllToolBlocks', () => { + test('returns [] on plain text', () => { + expect(parseAllToolBlocks('hello world')).toEqual([]) + }) + + test('parses every block in source order', () => { + const stream = [ + 'Step 1.', + '```forge:write', + '{ "path": "a.ts", "content": "1" }', + '```', + 'Step 2.', + '```forge:bash', + '{ "command": "ls" }', + '```', + 'Step 3.', + '```forge:read', + '{ "path": "a.ts" }', + '```', + ].join('\n') + const r = parseAllToolBlocks(stream) + expect(r.length).toBe(3) + expect(r[0]?.kind).toBe('tool') + expect(r[1]?.kind).toBe('tool') + expect(r[2]?.kind).toBe('tool') + if (r[0]?.kind === 'tool') expect(r[0].tool.kind).toBe('write') + if (r[1]?.kind === 'tool') expect(r[1].tool.kind).toBe('bash') + if (r[2]?.kind === 'tool') expect(r[2].tool.kind).toBe('read') + }) + + test('keeps invalid blocks visible instead of dropping them', () => { + const stream = [ + '```forge:write', + '{ "path": "ok.ts", "content": "1" }', + '```', + '```forge:bash', + '{ not json }', + '```', + '```forge:bash', + '{ "command": "echo done" }', + '```', + ].join('\n') + const r = parseAllToolBlocks(stream) + expect(r.length).toBe(3) + expect(r[0]?.kind).toBe('tool') + expect(r[1]?.kind).toBe('invalid') + expect(r[2]?.kind).toBe('tool') + }) + + test('text before each block follows source order', () => { + const stream = ['intro', '```forge:bash', '{ "command": "a" }', '```', 'middle', '```forge:bash', '{ "command": "b" }', '```'].join('\n') + const r = parseAllToolBlocks(stream) + expect(r.length).toBe(2) + expect(r[0]?.text.trim()).toBe('intro') + expect(r[1]?.text.trim()).toBe('middle') + }) +}) From 9fb990ac0434595ece517504ebe88ffc1b552c59 Mon Sep 17 00:00:00 2001 From: Georges Garnier Date: Tue, 28 Apr 2026 13:31:20 +0200 Subject: [PATCH 9/9] feat(cli): expand \\n in forge:* fence bodies in the run detail view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lines 37, 41, 64 of the run detail screenshot showed the JSON the agent emits inside forge:write / forge:edit / forge:bash fences with every embedded newline as the literal two-character "\\n". A 50-line file written via forge:write rendered as one giant unreadable line. highlightAgentRun now buffers the body of each forge:* fence and on close tries JSON.parse it. On success it pretty-prints the object : single-line entries stay inline, but any string value that contains "\\n" (typically content / oldString / newString / command / stderr) is rendered across multiple terminal lines with indentation. On failure the previous per-line colouring is kept, so a malformed block still shows up. Other fence types (yaml, plain, unknown) keep their existing per-line rendering — only the forge:* family triggers the JSON pretty-print. --- packages/cli/src/components/syntax.ts | 95 +++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/components/syntax.ts b/packages/cli/src/components/syntax.ts index bd3a911..42caf5e 100644 --- a/packages/cli/src/components/syntax.ts +++ b/packages/cli/src/components/syntax.ts @@ -275,26 +275,107 @@ export function highlightMarkdown(text: string): HighlightedLine[] { // We treat the markers like another fence type : everything between // [forge:tool] and [/forge:tool] is rendered with a dim, distinct // colour so the user can tell tool output from the agent's narration. +// +// forge:* fence bodies are JSON the agent built. Reading raw JSON +// where every newline shows up as the literal two-character "\n" +// sequence is unreadable for files of any real length. Below we +// buffer the body, then on close try to JSON.parse it ; on success +// we pretty-print it line by line with embedded "\n" turned into +// real line breaks. On failure we fall back to colouring the raw +// JSON as before (it's still readable, just less pretty). const TOOL_OPEN_RE = /^\[forge:tool\]\s*$/ const TOOL_CLOSE_RE = /^\[\/forge:tool\]\s*$/ +// Pretty-print one JSON object the agent emitted inside a forge:* +// fence. The keys we care about — `content`, `oldString`, `newString`, +// `command`, `stderr`, `stdout` — all carry strings that the LLM +// builds with embedded "\n". For each such key we render the value +// across multiple lines. Other keys go on one line. +function expandFenceJsonLines(value: unknown): HighlightedLine[] { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + // Fallback : single-line JSON dump. + return [highlightJsonLine(JSON.stringify(value))] + } + const obj = value as Record + const out: HighlightedLine[] = [] + out.push([{ text: '{', color: C.grey }]) + const entries = Object.entries(obj) + for (let i = 0; i < entries.length; i += 1) { + const [key, val] = entries[i] as [string, unknown] + const trailing = i < entries.length - 1 ? ',' : '' + if (typeof val === 'string' && val.includes('\n')) { + // Multi-line string : open on its own line, body indented, + // close + comma on its own line. + out.push([ + { text: ' ' }, + { text: `"${key}"`, color: C.orange, bold: true }, + { text: ': ', color: C.grey }, + { text: '"', color: C.greyLight }, + ]) + const lines = val.split('\n') + for (const line of lines) { + out.push([ + { text: ' ' }, + { text: line.length > 0 ? line : ' ', color: C.greyLight }, + ]) + } + out.push([ + { text: ' ' }, + { text: `"${trailing}`, color: C.greyLight }, + ]) + } else { + // Single-line entry, JSON-coloured. + const rendered = ` ${JSON.stringify(key)}: ${JSON.stringify(val)}${trailing}` + out.push(highlightJsonLine(rendered)) + } + } + out.push([{ text: '}', color: C.grey }]) + return out +} + export function highlightAgentRun(text: string): HighlightedLine[] { const out: HighlightedLine[] = [] const lines = text.split('\n') let inFence = false + let fenceTag = '' let fenceLine: ((line: string) => HighlightedLine) | null = null + let fenceBuffer: string[] = [] let inTool = false + const flushFenceBuffer = (closeLine: string): void => { + const joined = fenceBuffer.join('\n').trim() + let rendered: HighlightedLine[] | null = null + // Only the forge:* fences carry JSON we want to expand. Other + // fenced languages (yaml, plain) keep their per-line rendering. + if (fenceTag.startsWith('forge:') && joined.length > 0) { + try { + const parsed: unknown = JSON.parse(joined) + rendered = expandFenceJsonLines(parsed) + } catch { + rendered = null + } + } + if (rendered) { + out.push(...rendered) + } else { + const lineFn = fenceLine ?? highlightYamlLine + for (const b of fenceBuffer) out.push(lineFn(b)) + } + out.push([{ text: closeLine, color: C.grey, dim: true }]) + inFence = false + fenceTag = '' + fenceLine = null + fenceBuffer = [] + } + for (const raw of lines) { if (inFence) { if (FENCE_CLOSE_RE.test(raw)) { - out.push([{ text: raw, color: C.grey, dim: true }]) - inFence = false - fenceLine = null + flushFenceBuffer(raw) continue } - out.push((fenceLine ?? highlightYamlLine)(raw)) + fenceBuffer.push(raw) continue } if (inTool) { @@ -317,7 +398,9 @@ export function highlightAgentRun(text: string): HighlightedLine[] { const fenceOpen = raw.match(FENCE_OPEN_RE) if (fenceOpen) { inFence = true - fenceLine = languageHighlighter(fenceOpen[1] ?? '') + fenceTag = fenceOpen[1] ?? '' + fenceLine = languageHighlighter(fenceTag) + fenceBuffer = [] out.push([{ text: raw, color: C.orange, bold: true }]) continue } @@ -327,5 +410,7 @@ export function highlightAgentRun(text: string): HighlightedLine[] { } out.push(highlightInlineMarkdown(raw)) } + // EOF inside an unclosed fence : flush what we have. + if (inFence) flushFenceBuffer('') return out }