Files
fusion/plugins/fusion-plugin-acp-runtime/src/process-manager.ts
gsxdsm 590383bd3a fix(acp): address code-review findings (security, correctness, reliability)
Tier-2 code review fixes:
- P1 correctness: EventBridge per-turn state never reset — once the per-turn
  output cap tripped, all later turns were silently suppressed and tool/accum
  state bled across turns. Surface resetTurn() and call it per prompt turn.
- P1: plan_update read a non-existent .entries field (wrong SDK shape) and
  wiped the displayed plan — now a documented no-op (full 'plan' is source of
  truth).
- P1 security: write-path TOCTOU — open without O_TRUNC, re-validate realpath,
  then truncate, so an intermediate-symlink-swapped escaped target is never
  truncated before rejection.
- DoS: fs read stat-gates and bounded-reads oversized files instead of loading
  them fully before the ceiling.
- Security: stderr redaction now spans chunk boundaries; secret deny-list adds
  .git-credentials/*.p12/*.pfx/*.keystore/.pgpass/.htpasswd/etc.
- Reliability: cancelAcpSession bounded by a timeout so a blocked stdin can't
  delay the registry SIGKILL.
- Maintainability: drop dead ACP_NOT_IMPLEMENTED export; type agentCapabilities
  via the SDK AgentCapabilities; strengthen the S1 write-denial assertion.

+4 tests (181 total); typecheck + eslint clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-03 09:44:36 -07:00

161 lines
5.7 KiB
TypeScript

// Subprocess lifecycle for the ACP runtime.
//
// Mirrors the hardening conventions in
// `plugins/fusion-plugin-droid-runtime/src/process-manager.ts`: a self-cleaning
// process registry, SIGKILL teardown scoped to agent subprocesses only (never
// the dashboard/port-4040 — KTD4), bounded stderr capture with secret redaction
// (Risk S8), and a high inactivity ceiling (the engine's StuckTaskDetector is
// the authoritative aborter — KTD4).
//
// The ACP agent is UNTRUSTED. The spawn env is built from an explicit allow-list
// (KTD6b), never inherited `process.env`, so secret-bearing vars are not handed
// to the agent.
import { spawn, type ChildProcess } from "node:child_process";
function debugLog(message: string): void {
if (process.env.PI_ACP_DEBUG !== "1") return;
console.error(`[acp-runtime] ${message}`);
}
/** Registry of active agent subprocesses for teardown. Self-cleans on exit. */
const activeProcesses = new Set<ChildProcess>();
/**
* Register a subprocess in the agent process registry.
* Auto-removed from the registry when it exits.
*/
export function registerProcess(child: ChildProcess): void {
activeProcesses.add(child);
child.on("exit", () => activeProcesses.delete(child));
}
/** Remove a subprocess from the registry (idempotent). */
export function unregisterProcess(child: ChildProcess): void {
activeProcesses.delete(child);
}
/** Number of registered (presumed-live) agent subprocesses — for diagnostics/tests. */
export function activeProcessCount(): number {
return activeProcesses.size;
}
/**
* Force-kill a subprocess via SIGKILL. No-op if already dead (killed or exited).
* Cross-platform safe: Node treats SIGKILL as forceful termination on Windows.
*/
export function forceKill(child: ChildProcess): void {
if (child.killed || child.exitCode !== null) return;
try {
child.kill("SIGKILL");
} catch {
// already gone
}
}
/**
* Force-kill every registered agent subprocess and clear the registry.
*
* Scoped to agent subprocesses tracked here only — never the dashboard / port
* 4040 / any other process (KTD4 / kill-guard conventions). Safe to call
* repeatedly; no-ops on already-dead processes.
*/
export function killAllProcesses(): void {
for (const child of activeProcesses) {
forceKill(child);
}
activeProcesses.clear();
}
/**
* Build the subprocess environment from an explicit allow-list (KTD6b).
*
* Returns ONLY allow-listed vars copied from `process.env`. The full env is
* never inherited — the agent is untrusted and must not receive secret-bearing
* vars. Returns an empty env by default (empty allow-list).
*/
export function buildSpawnEnv(allowList: string[]): NodeJS.ProcessEnv {
const env: NodeJS.ProcessEnv = {};
for (const key of allowList) {
const value = process.env[key];
if (typeof value === "string") env[key] = value;
}
return env;
}
export interface SpawnAgentOptions {
binaryPath: string;
args: string[];
cwd: string;
env: NodeJS.ProcessEnv;
}
/**
* Spawn the ACP agent subprocess with piped stdio.
*
* Registers the child on spawn and unregisters it on exit. The caller wraps
* stdin/stdout into a web stream for `ndJsonStream`.
*/
export function spawnAgent(options: SpawnAgentOptions): ChildProcess {
const child = spawn(options.binaryPath, options.args, {
stdio: ["pipe", "pipe", "pipe"],
cwd: options.cwd,
env: options.env,
});
registerProcess(child);
debugLog(`spawnAgent: pid=${child.pid} binary=${options.binaryPath}`);
return child;
}
// --- stderr capture + secret redaction (Risk S8) --------------------------
/** Maximum stderr bytes retained; older output is dropped to bound memory. */
const STDERR_BUFFER_CEILING = 64 * 1024;
/**
* Redact token-like / auth patterns from text so auth errors don't leak
* verbatim into the stderr buffer or logs (Risk S8). Best-effort: covers
* bearer tokens, `Authorization:` header values, `key=`/`token=`/`secret=`
* assignments, and long base64/hex secrets.
*/
export function redactSecrets(text: string): string {
return (
text
// Authorization: Bearer <token> / Authorization: <token>
.replace(/(authorization\s*[:=]\s*)(bearer\s+)?[^\s,;"']+/gi, "$1$2[REDACTED]")
// Bearer <token>
.replace(/\b(bearer)\s+[A-Za-z0-9._\-+/=]+/gi, "$1 [REDACTED]")
// key=... token=... secret=... password=... apikey=... (quoted or bare)
.replace(
/\b((?:api[_-]?key|key|token|secret|password|passwd|pwd|access[_-]?token|refresh[_-]?token|client[_-]?secret)\s*[:=]\s*)("?)[^\s,;"']+\2/gi,
"$1$2[REDACTED]$2",
)
// sk-/ghp_/github_pat_/xoxb-/AKIA-style long opaque tokens
.replace(/\b(sk-|ghp_|gho_|github_pat_|xox[abpr]-|AKIA)[A-Za-z0-9_\-]{8,}/g, "[REDACTED]")
// standalone long base64/hex secrets (>=32 chars)
.replace(/\b[A-Za-z0-9+/]{40,}={0,2}\b/g, "[REDACTED]")
.replace(/\b[0-9a-fA-F]{32,}\b/g, "[REDACTED]")
);
}
/**
* Accumulate stderr into a bounded, secret-redacted buffer.
* Returns a getter for the current (redacted) buffer contents.
*/
export function captureStderr(child: ChildProcess): () => string {
// FIX 5: redacting each chunk in isolation leaks a secret that straddles a
// chunk boundary (the token is split across two `data` events so neither half
// matches a pattern). Accumulate the RAW bytes into a bounded buffer first,
// then redact across the whole (bounded) buffer after each append so a
// boundary-spanning secret is caught. The buffer stays bounded by the existing
// ceiling; the returned getter always reports the redacted view.
let raw = "";
child.stderr?.on("data", (data: Buffer) => {
raw += data.toString();
if (raw.length > STDERR_BUFFER_CEILING) {
raw = raw.slice(raw.length - STDERR_BUFFER_CEILING);
}
});
return () => redactSecrets(raw);
}