Skip to content

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

Merged
pacphi merged 2 commits into
pacphi:mainfrom
robertelee78:fix/swarm-followups
Jul 30, 2026
Merged

fix(execution): swarm-review follow-ups — security hardening, Windows shims, correctness nits, test gaps (#88)#91
pacphi merged 2 commits into
pacphi:mainfrom
robertelee78:fix/swarm-followups

Conversation

@robertelee78

Copy link
Copy Markdown
Contributor

Summary

The #88 swarm-review follow-ups, all verified against the code before acceptance. Grouped by theme; each item names its evidence.

Security hardening

  • S2 — port-reservation TOCTOU eliminated. The owned opencode server now binds :0 and reports its port on stdout — there is no freed port for a squatter to win (the rogue would then have intercepted the per-run credential and the worker prompt). reservePort stays as the fallback for stdout-less (injected) children, and a child that dies before reporting — or during the health check — fails honestly with its exit code instead of polling into the timeout.
  • S3 — SSE per-line accumulator capped at 256 KB (mirrors the subprocess cap): a never-terminating line is a bounded protocol error, not unbounded memory growth.
  • Teardown HTTP is bounded. /abort + /instance/dispose go through a 10 s (injectable teardownTimeoutMs) deadline — a wedged server cannot hang the runner's cancel/cleanup path.
  • S1 disposition (document, per review). The trust boundary is now stated in ADR-0018 and ak run --help: workers run with the user's own CLI trust posture; a hostile repo's opencode.json permissions / .claude/settings.json / AGENTS.md are inside that boundary (a repo that pre-allows permissions generates no permission.updated event, so the abort boundary does not trip by design — the abort covers requests, it is not a sandbox). Run ak only in repos you trust with full user privileges.

Windows shim path

resolveShim is exported (with a {windows, env} test seam) and applied at both spawn sites (subprocess claude/codex; the opencode serve child), and codex is added to CMD_SHIMS. Readiness and launch now resolve the same binary — fixing launch-ENOENT on Windows where .cmd shims are required. Includes a simulated-PATH PATHEXT-order test.

Correctness nits

  • worker.maxTurns reaches the claude CLI as --max-turns (codex exec + opencode serve have no equivalent — documented, not faked).
  • Worker prompt rendering uses replacer functions — $& $` $' in a prompt/model id are data, never replacement syntax.
  • --timeout above Node's 2^31-1 ms timer ceiling is rejected with a clear error (would otherwise clamp to ~1 ms for every worker).
  • Persisted dualRouting is schema-checked at load: malformed entries fail with the entry named, not a padEnd crash at print time.
  • Construction invariant: routable hosts and EXECUTION_ADAPTERS keys are enforced at import — a routable host with no adapter (or an adapter for an unroutable host) throws at import instead of cli_unavailable on every worker at runtime.

Test gaps filled

SSE foreign-session isolation; plan-validation guards (duplicate/unknown/self deps, bad maxConcurrent, dependency cycle); readiness-false short-circuit; TERM-then-KILL signal order; construction invariant via fresh-process import.

Verification

  • 20+ new/updated tests across the touched suites.
  • pnpm run check exit 0 (1083 kit + cjs), pnpm run test:surface exit 0 (25).

Closes #88. Touches run.mjs (timeout ceiling, policy validation) — merges cleanly with #90 (escalation) or rebases trivially on it.

Rebase the contributor follow-ups onto current main and preserve the original security, correctness, and test-gap repairs.

Use one shell-free invocation descriptor across Claude, Codex, and OpenCode. Native Windows executables run directly; package-manager shims require a sibling PowerShell script and pass hostile arguments literally through PowerShell -File. Bound OpenCode startup, SSE, teardown, and subprocess termination; validate routing inputs and adapter construction; forward supported turn caps; document the repository trust boundary.

Verification: pnpm run check; pnpm run test:surface; pnpm audit --audit-level=moderate; pnpm run lint:links:internal.
@pacphi
pacphi force-pushed the fix/swarm-followups branch from 8f1413f to 5acd4fb Compare July 30, 2026 21:16
@pacphi
pacphi merged commit b7e4f8f into pacphi:main Jul 30, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Swarm-review follow-ups for the #76 execution layer: security hardening, Windows shims, correctness nits, named test gaps

2 participants