fix(agent): strip null bytes from spawn args before launching task process (#813)

## Main-trunk migration

Replaces #804 after the trunk cutover. The three isolated commits now
start from the new `main`; original authorship and the contributor’s
implementation are preserved.

## Original PR body
## Problem

A task prompt containing a null byte crashes the whole cluster.
`spawnTaskProcess` passes the built args array, including the full
prompt, straight to `child_process.spawn()`. Node's
`normalizeSpawnArguments` throws `TypeError [ERR_INVALID_ARG_VALUE]` on
any argv string with an embedded null byte, because argv strings are
null terminated at the OS level and simply cannot carry one.

```
TypeError [ERR_INVALID_ARG_VALUE]: The argument 'args[10]' must be a string without null bytes.
    at normalizeSpawnArguments (node:child_process:584:3)
    at spawn (node:child_process:780:13)
    at spawnTaskProcess (src/agent/agent-task-executor.js:833:18)
```

Fixes #621

## Fix

Strip null bytes from string args right before the `spawn()` call in
`spawnTaskProcess` (`src/agent/agent-task-executor.js`). This is the one
place all task-run args funnel through before reaching the OS, so it
covers the prompt and any other string arg without touching
prompt-building code upstream.

Also exported `spawnTaskProcess` so it can be unit tested directly, same
as other internals already exported from this file.

## Test plan

- Added `tests/unit/spawn-null-byte-sanitization.test.js`:
- stubs `child_process.spawn` and asserts a prompt with an embedded null
byte reaches `spawn()` with the null byte stripped
- runs the real (unstubbed) `spawn()` with a null-byte-containing arg
and asserts it does not throw `ERR_INVALID_ARG_VALUE`
- Verified the test fails with `ERR_INVALID_ARG_VALUE` when the fix is
reverted, confirming it actually catches the original bug
- Full unit suite: 1212 passing, 0 new failures
- `npm run lint` / `npm run typecheck` clean on changed files (repo
currently has pre-existing, unrelated lint debt on `dev` itself, not
introduced by this change)

---------

Co-authored-by: Atharv Singh <singhatharv1919@gmail.com>

Author: tomdps

src/agent/agent-task-executor.js

@@ -875,8 +875,11 @@ function spawnTaskProcess({ agent, ctPath, args, cwd, spawnEnv }) {
   // Timeout for spawn phase - if CLI hangs during init (e.g., opencode 429 bug), kill it
   const SPAWN_TIMEOUT_MS = 30000; // 30 seconds to spawn task
 
+  // spawn() throws on null bytes in argv; strip them before they get there.
+  const safeArgs = args.map((arg) => (typeof arg === 'string' ? arg.replace(/\0/g, '') : arg));
+
   return new Promise((resolve, reject) => {
-    const proc = spawn(ctPath, args, {
+    const proc = spawn(ctPath, safeArgs, {
       cwd,
       stdio: ['ignore', 'pipe', 'pipe'],
       env: spawnEnv,
@@ -2366,6 +2369,7 @@ async function killIsolatedTask(agent, currentTask, taskId, reason, code) {
 module.exports = {
   ensureAskUserQuestionHook,
   spawnClaudeTask,
+  spawnTaskProcess,
   followClaudeTaskLogs,
   followClaudeTaskLogsIsolated,
   waitForTaskReady,

src/isolation-manager.js

@@ -727,7 +727,10 @@ class IsolationManager {
 
     args.push(containerId, ...command);
 
-    return spawn('docker', args, {
+    // spawn() throws on null bytes in argv; strip them before they get there.
+    const safeArgs = args.map((arg) => (typeof arg === 'string' ? arg.replace(/\0/g, '') : arg));
+
+    return spawn('docker', safeArgs, {
       stdio: ['pipe', 'pipe', 'pipe'],
       ...options.spawnOptions,
     });

tests/unit/spawn-null-byte-sanitization.test.js

@@ -0,0 +1,107 @@
+const assert = require('assert');
+const sinon = require('sinon');
+const { EventEmitter } = require('events');
+const childProcess = require('child_process');
+
+// Regression guard for #621: null byte in prompt crashes spawn() with ERR_INVALID_ARG_VALUE.
+describe('spawnTaskProcess null byte sanitization', function () {
+  it('strips null bytes from string args before calling spawn()', function () {
+    const fakeChild = new EventEmitter();
+    fakeChild.stdout = new EventEmitter();
+    fakeChild.stderr = new EventEmitter();
+
+    const spawnStub = sinon.stub(childProcess, 'spawn').returns(fakeChild);
+    try {
+      const executorPath = require.resolve('../../src/agent/agent-task-executor');
+      delete require.cache[executorPath];
+      const { spawnTaskProcess } = require(executorPath);
+
+      const dirtyPrompt = 'You are agent "worker".\nDo the task.\0trailing after null byte';
+      const pending = spawnTaskProcess({
+        agent: { _log: () => {} },
+        ctPath: 'zeroshot',
+        args: ['task', 'run', dirtyPrompt],
+        cwd: '/tmp',
+        spawnEnv: {},
+      });
+
+      assert.strictEqual(spawnStub.calledOnce, true);
+      const spawnedArgs = spawnStub.firstCall.args[1];
+      for (const arg of spawnedArgs) {
+        assert.strictEqual(arg.includes('\0'), false, `arg still contains a null byte: ${arg}`);
+      }
+      assert.strictEqual(
+        spawnedArgs[2],
+        'You are agent "worker".\nDo the task.trailing after null byte'
+      );
+
+      // Resolve the pending promise so it doesn't leak an unhandled rejection.
+      fakeChild.emit('close', 1);
+      return assert.rejects(pending);
+    } finally {
+      spawnStub.restore();
+      const executorPath = require.resolve('../../src/agent/agent-task-executor');
+      delete require.cache[executorPath];
+    }
+  });
+
+  it('does not throw ERR_INVALID_ARG_VALUE when spawn() runs for real on a null-byte prompt', async function () {
+    // No stub: exercises the real spawn() to prove the sanitized args are safe.
+    const executorPath = require.resolve('../../src/agent/agent-task-executor');
+    delete require.cache[executorPath];
+    const { spawnTaskProcess } = require(executorPath);
+
+    const dirtyPrompt = 'prompt with a null byte \0 in the middle';
+    const pending = spawnTaskProcess({
+      agent: { _log: () => {} },
+      ctPath: process.execPath, // node itself; unrecognized output still exercises the real spawn() call
+      args: ['--version', dirtyPrompt],
+      cwd: '/tmp',
+      spawnEnv: process.env,
+    });
+
+    try {
+      await pending;
+    } catch (err) {
+      assert.notStrictEqual(err.code, 'ERR_INVALID_ARG_VALUE');
+      assert.doesNotMatch(err.message, /null bytes/);
+    }
+  });
+});
+
+describe('IsolationManager.spawnInContainer null byte sanitization', function () {
+  it('strips null bytes from string args before calling spawn()', function () {
+    // isolation-manager.js's class is prototype-patched by other test files
+    // (e.g. orchestrator.test.js), which relies on require.cache staying stable
+    // for the rest of the mocha worker. Snapshot and restore it instead of just
+    // deleting it, so this test's own reload doesn't leak into other test files.
+    const managerPath = require.resolve('../../src/isolation-manager');
+    const originalCacheEntry = require.cache[managerPath];
+    delete require.cache[managerPath];
+
+    const spawnStub = sinon.stub(childProcess, 'spawn').returns(new EventEmitter());
+    try {
+      const IsolationManager = require(managerPath);
+      const manager = new IsolationManager();
+      manager.containers.set('cluster-1', 'container-abc');
+
+      const dirtyPrompt = 'You are agent "worker".\0trailing after null byte';
+      manager.spawnInContainer('cluster-1', ['zeroshot', 'task', 'run', dirtyPrompt]);
+
+      assert.strictEqual(spawnStub.calledOnce, true);
+      const [bin, spawnedArgs] = spawnStub.firstCall.args;
+      assert.strictEqual(bin, 'docker');
+      for (const arg of spawnedArgs) {
+        assert.strictEqual(arg.includes('\0'), false, `arg still contains a null byte: ${arg}`);
+      }
+      assert.strictEqual(spawnedArgs.at(-1), 'You are agent "worker".trailing after null byte');
+    } finally {
+      spawnStub.restore();
+      if (originalCacheEntry) {
+        require.cache[managerPath] = originalCacheEntry;
+      } else {
+        delete require.cache[managerPath];
+      }
+    }
+  });
+});