fix(security): require auth on /mcp — closes an unauthenticated RCE (ADR-054)

[aterrylu]

Problem

Two defects composed into unauthenticated remote code execution. Both verified live against the local server and forge — not inferred from reading code.

1. /mcp had no auth at all. requireAuth was mounted on /api/* and /ws/* only. /mcp — the Streamable-HTTP transport exposing create_agent, kill_agent, set_manager — matched neither prefix and had no internal check.

2. The server bound every interface. serve() was called with no hostname, so Node bound 0.0.0.0/::. Confirmed live: node … TCP *:3100 (LISTEN).

POST /api/agents      → 401                          (auth worked)
POST /mcp initialize  → 200 + mcp-session-id         (NO auth)
POST /mcp tools/list  → create_agent, kill_agent, create_schedule, set_manager, …

Together: any host on the same network — café, office, dorm — could open an MCP session with a single curl and create_agent with permissionMode: "bypass", an arbitrary workingDirectory (runtime.ts checks existence only) and an arbitrary prompt. That is arbitrary code execution as the server's user, no credential, no skill. CVSS ≈ 9.8. The repo is public with tagged releases (latest v0.4.0), so this shipped.

The chain was proven and stopped at tools/list; create_agent was never called.

flowchart LR
    A[Any host on the network] -->|no credential| B{"bind *:3100"}
    B --> C["POST /api/agents"] --> D["401 ✅"]
    B --> E["POST /mcp"] --> F["no middleware<br/>no internal check"]
    F --> G["initialize → 200 + session"]
    G --> H["create_agent<br/>bypass + any cwd + any prompt"]
    H --> I["RCE as the server's user 💥"]

Loading

Solution

Auth is the boundary; the bind is defense-in-depth. That ordering is Terry's "always require auth" ruling, and it's what let this delete the localhost-trust category instead of gating it — no peer-address check, no exemption to get subtly wrong. It costs nothing: auth.ts has always generated and persisted a random 256-bit token on first start, so there is no tokenless state to design around.

• app.use("/mcp", requireAuth) — checked at the transport boundary, so an unauthenticated caller cannot complete initialize and never obtains the session id later calls depend on. Asserting only that tools/call 401s would have missed a regression leaving session creation open.
• Bind 127.0.0.1 by default, with --host / AUTONOMOS_HOST as explicit opt-in, threaded through install-service → install-prod-service.sh → Makefile (BIND_HOST) so a deliberately-exposed box keeps its bind across make deploy.
• An exposed bind warns at startup — and names the routes that are still open rather than claiming blanket coverage.

Known residual — deliberately NOT closed here

POST /api/hooks/* and GET /api/host remain unauthenticated on every bind. They cannot spawn or control agents, but an unauthenticated caller can forge agent status and inject dashboard notifications (including the proactive: true push path).

Deferred on purpose: authenticating the relay changes how the token travels (TokenSecurityAudit@autonomOS's scope) and risks a fleet-wide status blackout if the token fails to reach the hooks. That needs its own PR with a real-spawn /qa, not an emergency patch. Until then every operator-facing string names the gap. See ADR-054.

⚠️ Breaking change

A server reached over the network goes loopback-only on upgrade. Set in that box's .env (not rsynced, so it persists per box):

AUTONOMOS_HOST=0.0.0.0

AUTONOMOS_HOST is recommended over BIND_HOST: the service wrapper runs tsx --env-file=<repo>/.env, so it survives install-service --force, whereas a service-file-baked --host is dropped by a reinstall that omits it. One-off alternative: make deploy BIND_HOST=0.0.0.0.

Testing

Live, against a real server on an isolated port + config dir (never touching the running deployment):

check	before	after
bind	*:3100	127.0.0.1:39131
unauth /mcp initialize	200 + session id	401
unauth /mcp GET / DELETE	reachable	401
legit client w/ token	200	200 + session (not broken)
--host=0.0.0.0	n/a	binds *, warns, still 401s unauth

• make check green — 632 server/CLI + 234 dashboard tests
• AUTONOMOS_INTEGRATION=1 mcp-auth suite green locally (4/4)
• Real-spawn suites left to CI — the harness warns they're unsafe on a box with a live deployment
• New tests: mcp-auth.test.ts (real HTTP, real server), bind-host.test.ts, cli-args.test.ts, install-service-host.test.ts (asserts the actual written service file, including the absence case)

Risks

• Breaking for network-reached servers (above). Mitigated by README + changeset + ADR, but it does need one manual step before upgrading such a box.
• --host=<specific IP> (not 0.0.0.0) makes the post-install health check and the ADR-029 running-server guard — both of which probe localhost — report a false failure. Documented in --help and the README; not fixed here to keep the patch small.
• The /mcp change would break any external HTTP MCP client (Claude Desktop, CI). No in-repo client is affected — agents use the stdio channel-server, not HTTP /mcp — and the "legit client still works" test covers the path.

What /polish caught (worth reading)

Three real bugs, all found before this PR existed:

1. The PR didn't achieve its own goal. forge would still have rebound to loopback on the next deploy: its .env has no BIND_HOST (new + rsync-excluded), so $(if $(BIND_HOST),…) forwarded nothing and install-service --force rewrote the unit without --host. Fixed by recommending AUTONOMOS_HOST in .env (survives reinstalls) and documenting the pre-upgrade step.
2. BIND_HOST=0.0.0.0 # comment in .env made make prod a silent no-op — the # opened a shell comment that swallowed the installer, and the recipe is @-prefixed so there was no output. A green deploy that changed nothing. Fixed by stripping the comment + quoting; the strip itself then needed \# because make comments the line otherwise. Verified across four .env shapes.
3. My own warning text lied — it said "Every route requires the auth token" while /api/hooks/* and /api/host were exempt. That string is the last thing an operator reads before exposing a box. Now it names them.

Correction to the record

The audit first reported isLoopbackBind (zero callers) as "designed and never wired up." Wrong, and git log -S says so: #221 added it with its caller and a locking test; #264 deleted the caller and the test together, orphaning the helper.

The corrected story is worse. ADR-041 already recorded that unconditional auth-exemption is a HIGH-severity credential-injection vector on non-loopback binds — found by /polish, fixed pre-ship for one endpoint. /api/hooks/* is the "existing localhost-trust model" ADR-041 says it was mirroring but chose to be stricter than, and it was left exempt on every bind. The lesson was written down, applied once, and never applied to the neighbour it was copied from — then the only test encoding it was deleted as collateral.

A deleted test is how a security invariant dies quietly. Removing the endpoint made removing its test look like hygiene. Nothing failed. The helper just sat there looking like a guard. The new tests attach to the bind logic itself, not to any endpoint's lifetime.

Two candidate findings were also retracted as false positives after empirical checks: a slash-normalisation auth bypass (//api/agents → 200 is the SPA static fallback; handler-vs-router 404 shape disproved it) and CSWSH from a third-party origin (SameSite=Lax withholds the cookie on a subresource WS handshake).

Alternatives considered

• Revive isLoopbackBind to gate the exemptions — rejected. Preserves the "trusted context" idea that failed here in the first place.
• Peer-address gate on /api/hooks/* (hook traffic is always loopback — shared.ts hardcodes AUTONOMOS_SERVER=localhost) — designed, then dropped for Terry's always-auth rule. Kept as fallback. It carried three landmines, all empirically probed: ::ffff:127.0.0.1 must PASS (a ::1-only check 401s every hook → fleet blackout); startsWith("::ffff:") is a total bypass (it prefixes any IPv4); and loopback is all of 127/8 — 127.0.0.53 is loopback on Linux but not macOS, so === "127.0.0.1" passes QA on a Mac and is wrong on forge.
• Bind forge to its tailnet IP — smaller surface, but EADDRNOTAVAIL if tailscaled isn't up yet (forge is systemd), and with auth universal the bind isn't the boundary.
• Hotpatch + firewall — unnecessary once the real fix was small enough to ship directly.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SeVpLvCDMBZNVnrbkHzAgN

[nox-0x]

Approving — clean security fix that closes an unauthenticated RCE via /mcp + all-interfaces bind, verified live per the PR body.

Reviewed the diff carefully. The important things check out:

• The auth middleware is mounted before the /mcp route handlers (app.use("/mcp", requireAuth) at run.ts:286, handlers at 310/317/323), and the integration test explicitly verifies GET/POST/DELETE all 401 without a token, plus that no mcp-session-id is issued on the reject path. That is the correct place to gate — an unauthenticated caller cannot obtain the session id later tools/call depends on.
• resolveBindHost defaults to 127.0.0.1 and serve() now passes hostname: bindHost. Test in bind-host.test.ts locks the default. The empty-string / whitespace cases in both parseCliArgs and resolveBindHost correctly fall back to loopback rather than expanding to all interfaces (which is what listen("") would silently do).
• --host threads through install-service into the written service file; install-service-host.test.ts asserts both presence and absence, and the Makefile forwards BIND_HOST only when set locally so a deploy with no override doesn't blank the remote's own value.
• Empty --host is rejected at three layers (server parseHost, CLI parseFlags, and asserted in tests), so a service file carrying --host= can't silently expose the port on every boot.
• The startup warning names the two still-exempt routes (POST /api/hooks/*, GET /api/host) rather than claiming blanket coverage — matches the deferred-residual documented in ADR-054 and the changeset, and won't lie to an operator at the moment they decide to expose the port.

Known residual (/api/hooks/* + /api/host unauthenticated on every bind) is deferred with clear rationale and follow-up scope. Not blocking here.

Minor observations, none blocking:

• isLoopbackBind only recognises localhost/127.0.0.1/::1, so a bind to another 127/8 address (127.0.0.2) would trigger the network-exposure warning — false-positive direction, safe.
• The Makefile .env parser doesn't handle quoted values (BIND_HOST="0.0.0.0"); a quoted value would reach Node's listen() with quotes and fail loudly — not a silent-widen. Fine to defer.
• install-prod-service.sh's "[prod] Binding to HOST=$HOST (reachable from the network)" line would be inaccurate if HOST=127.0.0.1, but that combination has no legitimate use case.