fix(execution): swarm-review follow-ups — security hardening, Windows shims, correctness nits, test gaps (#88)

docs/adr/0018-generalized-host-worker-execution.md

@@ -2,6 +2,8 @@
 
 - **Status:** Accepted
 - **Date:** 2026-07-29
+- **Updated:** 2026-07-30
+- **Update note:** Hardened trusted-policy validation, adapter invariants, plan guards, and bounded worker teardown.
 - **Deciders:** agentic-kit maintainers
 
 ## Context
@@ -68,6 +70,27 @@ permission-response contract required for a routable worker. [OpenCode CLI docum
    conformance evidence. Its routes are accepted by `ak run`, but are never auto-seeded,
    AQE-projected, primary-host eligible, or accepted by deprecated `ak dual`.
 
+### The trust boundary (stated, not weakened)
+
+`ak run` executes workers with the **user's own CLI trust posture in the target
+repository**. That means the repository itself is *inside* the trust boundary:
+
+- An owned OpenCode server reads the project `opencode.json` from the target cwd. A
+  repository that ships `{"permission":{"bash":"allow"}}` (or hostile `mcp` entries)
+  pre-approves those permissions — **no `permission.updated` event ever fires, so the
+  adapter's abort boundary does not trip by design**. The abort covers permission
+  *requests*; it is not a sandbox.
+- Claude/Codex workers likewise inherit the repo's `.claude/settings.json` hooks and
+  permissions under the user's own workspace trust for that path.
+- Repository content (`AGENTS.md`, README, source) flows into worker prompts — an
+  indirect prompt-injection channel for any agent runner, ak included.
+
+**Contract: run `ak` (and any agent runner) only in repositories you would trust with
+your full user privileges.** ak will not silently weaken, bypass, or "secure" a hostile
+repo for you; what it guarantees instead is the honest version: loopback-only owned
+servers, ephemeral per-run credentials, no `--auto`, no permission approval on your
+behalf, and terminal evidence that records what actually ran.
+
 ## Consequences
 
 - `ak run` executes the host-neutral plan and result schema while preserving the legacy

src/commands/run.mjs

@@ -19,6 +19,10 @@ export const help = `ak run — execute a host-neutral activity pipeline
 Materializes the managed per-activity routing policy and runs each worker through
 its host adapter. OpenCode is accepted only after its routing capability is enabled.
 
+Trust boundary: workers run with YOUR CLI trust posture in the target repo —
+its opencode.json / .claude settings / AGENTS.md apply. Run this only in
+repositories you would trust with your full user privileges (ADR-0018).
+
 Usage:
   ak run <template> "<task>"
 
@@ -40,19 +44,49 @@ Examples:
   ak run security "src/auth/" --route 'security-scan:opencode'
   ak run feature "fix the flaky parser" --escalate`;
 
-function positiveInt(value, name) {
+/** @param {string|undefined} value @param {string} name @param {{ ceiling?: number }} [opts] */
+function positiveInt(value, name, { ceiling } = {}) {
   if (value === undefined) return undefined;
   if (!/^\d+$/.test(value) || Number(value) < 1) throw new TypeError(`${name} must be a positive integer`);
-  return Number(value);
+  const n = Number(value);
+  // Node clamps setTimeout delays above 2^31-1 ms to ~1 ms — an uncapped
+  // --timeout would turn a huge value into an instant timeout for every
+  // worker (#88). Reject above the ceiling rather than silently mis-timing.
+  if (ceiling && n > ceiling) throw new TypeError(`${name} must not exceed ${ceiling} (a larger value is clamped to ~1ms by Node's timer)`);
+  return n;
+}
+
+/** A persisted dualRouting entry (kit.json is hand-editable — CLI routes are
+ *  validated, file entries were not, and a bad one crashed plan *printing*
+ *  far from the cause, #88). Returns an error string, or null when valid. */
+function routeEntryError(activity, entry) {
+  if (!entry || typeof entry !== 'object') return `route "${activity}" must be an object`;
+  if (typeof entry.host !== 'string' || !entry.host) return `route "${activity}" requires a non-empty host string`;
+  if (entry.model != null && typeof entry.model !== 'string') return `route "${activity}".model must be a string when present`;
+  if (entry.escalate != null) {
+    if (!Array.isArray(entry.escalate)) return `route "${activity}".escalate must be an array when present`;
+    for (const [i, rung] of entry.escalate.entries()) {
+      if (!rung || typeof rung.host !== 'string' || !rung.host) return `route "${activity}".escalate[${i}] requires a non-empty host string`;
+      if (rung.model != null && typeof rung.model !== 'string') return `route "${activity}".escalate[${i}].model must be a string when present`;
+    }
+  }
+  return null;
+}
+
+function validatePolicy(policy) {
+  const errors = Object.entries(policy).map(([a, e]) => routeEntryError(a, e)).filter(Boolean);
+  if (errors.length) throw new Error(`invalid routing policy: ${errors.join('; ')}`);
 }
 
 export function buildRunPlan(cfg, template, task, routeFlags = []) {
   let policy = { ...(cfg.providers?.dualRouting ?? {}) };
   if (routeFlags.length) {
     const { policy: overrides, warnings } = parseRouteSpecs(routeFlags);
     policy = { ...policy, ...overrides };
+    validatePolicy(policy);
     return { plan: materializeRunPlan(policy, { template, task }), warnings };
   }
+  validatePolicy(policy);
   return { plan: materializeRunPlan(policy, { template, task }), warnings: [] };
 }
 
@@ -101,7 +135,7 @@ export async function run({ flags, positionals, executePlan = executeRunPlan, cf
   let timeoutMs;
   try {
     maxConcurrent = positiveInt(flags['max-concurrent'], 'max-concurrent');
-    timeoutMs = positiveInt(flags.timeout, 'timeout');
+    timeoutMs = positiveInt(flags.timeout, 'timeout', { ceiling: 2_147_483_647 });
   } catch (error) { fail(error.message); return 2; }
   if (!flags.json) printPlan(plan);
   const results = await executePlan(plan, { maxConcurrent, timeoutMs, escalate: !!flags.escalate });

src/lib/exec.mjs

@@ -6,11 +6,10 @@
 // forced `shell:true`, which hands Node's own cmd+args JOIN of the whole
 // command line to cmd.exe as ONE string (CVE-class: any arg with `&`/`|`/`^`
 // breaks out into a second command). The actual fix is resolving the shim to
-// its real file on PATH (with extension) and calling execFile on THAT path
-// directly with shell:false: Node >=18.20.2/20.12.2/21.7.2 (this package
-// requires >=22 — see package.json engines) internally detects a .cmd/.bat
-// target and safely re-invokes cmd.exe itself, with each argv element
-// properly escaped — the CVE-2024-27980 fix. Callers never need shell:true.
+// its real file on PATH. Native .com/.exe files run directly. A .cmd shim is
+// never passed to execFile: Node does not execute batch files without a shell.
+// Instead, its sibling .ps1 shim runs through Windows PowerShell's `-File`
+// interface, preserving every caller argument as a separate argv element.
 import { execFile } from 'node:child_process';
 import { promisify } from 'node:util';
 import fs from 'node:fs';
@@ -19,34 +18,78 @@ import { isWindows } from './paths.mjs';
 
 const pexecFile = promisify(execFile);
 
-const CMD_SHIMS = new Set(['npm', 'npx', 'claude', 'opencode', 'ruflo', 'aqe', 'claude-flow']);
+const CMD_SHIMS = new Set(['npm', 'npx', 'claude', 'codex', 'opencode', 'ruflo', 'aqe', 'claude-flow']);
 
-/** Resolve `cmd` to its real file on PATH, trying Windows' shim extensions in
- *  PATHEXT order. Falls back to the bare name (execFile will ENOENT honestly)
- *  if nothing on PATH matches — never silently launches the wrong binary. */
-function resolveShim(cmd) {
-  if (!isWindows || path.isAbsolute(cmd)) return cmd;
-  const exts = (process.env.PATHEXT || '.COM;.EXE;.BAT;.CMD').split(';');
-  for (const dir of (process.env.PATH || process.env.Path || '').split(path.delimiter)) {
+/** Build a shell-free invocation for `cmd`, trying Windows' shim extensions in
+ *  PATHEXT order. A native executable is launched directly; a .cmd shim is
+ *  accepted only when its sibling .ps1 and system PowerShell both exist.
+ *  Falls back to the bare name with resolved:false when no safe target exists.
+ *  Exported: the execution adapters spawn these CLIs directly (subprocess.mjs
+ *  for claude/codex, opencode.mjs for the serve child) and must share the same
+ *  resolution `run()`/`have()` use, or readiness passes but launch ENOENTs on
+ *  Windows (swarm review, #88). */
+export function resolveShim(cmd, args = [], { windows = isWindows, env = process.env } = {}) {
+  const direct = { command: cmd, args: [...args], resolved: !windows };
+  if (!windows) return direct;
+
+  const systemRoot = env.SystemRoot || env.WINDIR;
+  const powershell = systemRoot
+    ? path.join(systemRoot, 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe')
+    : null;
+  const invocationFor = (candidate) => {
+    let stat;
+    try { stat = fs.statSync(candidate); } catch { return null; }
+    if (!stat.isFile()) return null;
+    const ext = path.extname(candidate).toLowerCase();
+    if (ext === '.com' || ext === '.exe' || !ext) {
+      return { command: candidate, args: [...args], resolved: true };
+    }
+    if (ext !== '.cmd' || !powershell) return null;
+    const script = `${candidate.slice(0, -ext.length)}.ps1`;
+    try {
+      if (!fs.statSync(script).isFile() || !fs.statSync(powershell).isFile()) return null;
+    } catch { return null; }
+    return {
+      command: powershell,
+      args: [
+        '-NoLogo', '-NoProfile', '-NonInteractive',
+        '-ExecutionPolicy', 'Bypass', '-File', script, ...args,
+      ],
+      resolved: true,
+    };
+  };
+
+  if (path.isAbsolute(cmd)) return invocationFor(cmd) ?? direct;
+  const exts = (env.PATHEXT || '.COM;.EXE;.BAT;.CMD')
+    .split(';')
+    .map((ext) => ext.trim())
+    .filter(Boolean);
+  for (const dir of (env.PATH || env.Path || '').split(path.delimiter)) {
     if (!dir) continue;
     for (const ext of exts) {
-      const candidate = path.join(dir, cmd + ext.toLowerCase());
-      try { if (fs.statSync(candidate).isFile()) return candidate; } catch { /* try the next */ }
+      const invocation = invocationFor(path.join(dir, cmd + ext.toLowerCase()));
+      if (invocation) return invocation;
     }
   }
-  return cmd;
+  return direct;
 }
 
 /** Run a command; never throws. Returns {code, stdout, stderr}. */
 export async function run(cmd, args = [], opts = {}) {
   try {
-    const resolved = CMD_SHIMS.has(cmd) ? resolveShim(cmd) : cmd;
-    const { stdout, stderr } = await pexecFile(resolved, args, {
+    const env = opts.env ? { ...process.env, ...opts.env } : process.env;
+    const invocation = CMD_SHIMS.has(cmd)
+      ? resolveShim(cmd, args, { env })
+      : { command: cmd, args };
+    if (invocation.resolved === false) {
+      return { code: 1, stdout: '', stderr: `No safe Windows invocation found for ${cmd}` };
+    }
+    const { stdout, stderr } = await pexecFile(invocation.command, invocation.args, {
       encoding: 'utf8',
       timeout: opts.timeout ?? 120_000,
       maxBuffer: 16 * 1024 * 1024,
       cwd: opts.cwd,
-      env: opts.env ? { ...process.env, ...opts.env } : process.env,
+      env,
       shell: false,
     });
     return { code: 0, stdout, stderr };
@@ -62,10 +105,7 @@ export async function run(cmd, args = [], opts = {}) {
 /** Is `cmd` invokable? (cross-platform `command -v`) */
 export async function have(cmd) {
   if (isWindows) {
-    if (path.isAbsolute(cmd)) {
-      try { return fs.statSync(cmd).isFile(); } catch { return false; }
-    }
-    return resolveShim(cmd) !== cmd;
+    return resolveShim(cmd).resolved;
   }
   return (await run('which', [cmd])).code === 0;
 }

src/lib/execution/adapters.mjs

@@ -3,9 +3,28 @@
 import { OPENCODE_EXECUTION_ADAPTER } from './opencode.mjs';
 import { CLAUDE_EXECUTION_ADAPTER } from './claude.mjs';
 import { CODEX_EXECUTION_ADAPTER } from './codex.mjs';
+import { routableHostIds } from '../adapters/index.mjs';
 
 export const EXECUTION_ADAPTERS = Object.freeze(new Map([
   ['claude', CLAUDE_EXECUTION_ADAPTER],
   ['codex', CODEX_EXECUTION_ADAPTER],
   ['opencode', OPENCODE_EXECUTION_ADAPTER],
 ]));
+
+// Construction invariant (#88, architecture leg): every routable host needs an
+// execution adapter, and every adapter needs a routable host — enforced at
+// import, exactly like the capability registries' own construction check. A
+// host flipped to canRouteActivities without an adapter (or an adapter added
+// for an unroutable host) would otherwise load cleanly and fail only at
+// runtime with cli_unavailable on every worker — issue #71's trap.
+{
+  const routable = new Set(routableHostIds());
+  const adapted = new Set(EXECUTION_ADAPTERS.keys());
+  const missing = [...routable].filter((id) => !adapted.has(id));
+  const stray = [...adapted].filter((id) => !routable.has(id));
+  if (missing.length || stray.length) {
+    throw new Error(`execution adapters out of sync with routable hosts: `
+      + `${missing.length ? `no adapter for routable host(s): ${missing.join(', ')}. ` : ''}`
+      + `${stray.length ? `adapter(s) for non-routable host(s): ${stray.join(', ')}` : ''}`.trim());
+  }
+}

src/lib/execution/claude.mjs

@@ -8,6 +8,9 @@ export function createClaudeExecutionAdapter(options = {}) {
     argumentsFor: (worker) => [
       '--print', '--output-format', 'json',
       ...(worker.configuredModel ? ['--model', worker.configuredModel] : []),
+      // Templates carry per-node turn caps (#88) — honored where the CLI has a
+      // surface; codex exec and opencode serve have none (documented there).
+      ...(Number.isInteger(worker.maxTurns) && worker.maxTurns > 0 ? ['--max-turns', String(worker.maxTurns)] : []),
       worker.prompt,
     ],
     ...options,

src/lib/execution/codex.mjs

@@ -1,6 +1,8 @@
 import { createSubprocessExecutionAdapter } from './subprocess.mjs';
 
-/** Codex's documented exec/json mode. Its configured sandbox policy is retained. */
+/** Codex's documented exec/json mode. Its configured sandbox policy is retained.
+ *  worker.maxTurns is deliberately NOT forwarded: codex exec has no turn-cap
+ *  flag (verified against its help) — the bound rides on the runner timeout. */
 /** @param {Omit<Parameters<typeof createSubprocessExecutionAdapter>[0], 'id'|'host'|'command'|'argumentsFor'>} [options] */
 export function createCodexExecutionAdapter(options = {}) {
   return createSubprocessExecutionAdapter({