diff --git a/CHANGELOG.md b/CHANGELOG.md index a1969583..c67c76d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,36 @@ # aicodeman +## 0.9.0 + +### Minor Changes + +- Security hardening release: network-bind policy, auth lockout recovery, download/SVG hardening, dependency & supply-chain fixes, tmux launch reliability, and a full security-architecture doc. + + **Network binding (COD-29, #107):** + - The web server now defaults to binding `127.0.0.1` (loopback) instead of `0.0.0.0`, so a fresh install is reachable only from the same machine and needs no password. New `--host` / `-H` / `CODEMAN_HOST` flag to choose the bind host. + - Binding a non-loopback host **without** `CODEMAN_PASSWORD` no longer refuses to start — it **starts and prints a loud warning** with the three ways to secure it (set `CODEMAN_PASSWORD`, bind loopback + an authenticated tunnel / `tailscale serve`, or acknowledge with `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1`). This keeps Codeman "just working" for new users while making remote exposure a guided, explicit choice. Host classification lives in the new `src/web/network-auth-policy.ts` (handles `127.0.0.0/8`, `::1`, `::ffff:127.*`, bracketed IPv6). + - A post-install security note now explains the loopback default and how to expose safely. + + **Authentication (COD-29, #107):** + - Auth lockout now recovers gracefully: the per-IP rate-limit (`429`) check runs **after** the cookie/credential checks, so a valid session cookie or correct password is never locked out by a prior attacker's failures from the same IP (important behind a shared-IP tunnel). Wrong credentials are still counted and still hit the limit, and a `Retry-After` header is returned. + + **Downloads & content-type hardening (COD-29, #107):** + - New session-scoped `POST /api/download` route: realpath-bounded to the session working dir, a sensitive-path blocklist (`/etc/shadow`, `~/.ssh/`, `.env`, `*credentials*`, …), `isFile()` + 50 MB cap, forced `attachment`. + - Workspace `.svg` files are served as `application/octet-stream` + `attachment` + `nosniff` (closes a stored-XSS-via-SVG vector); `nosniff` now applies to all `file-raw` responses. + + **Dependencies & supply chain (COD-28, #106):** + - Bumped security-sensitive deps to patched versions (`@fastify/static` 9, `fastify` 5.8, `uuid` 14, `vitest` 4.1, …) and added `overrides` for patched transitives (`picomatch`, `basic-ftp`, `fast-uri`, `flatted`); `npm audit` goes from 7 advisories to 0. + - New `npm run check:public-assets` (`scripts/check-public-assets.mjs`): scans `src/web/public/**` for literal NUL bytes and runs `node --check` on every `.js` file, plus a Prettier pass on maintained files. Removed literal NUL placeholders from `app.js`. Added `test/dependency-security.test.ts` and `test/frontend-public-tooling.test.ts`. + + **tmux launch reliability (COD-31, #110):** + - New tmux sessions and respawns launch from a stable `/tmp` and `cd` into the workspace inside the pane, avoiding `new-session` crashes when a FUSE/rclone-mounted workspace has a transient mount blip at launch. The `cd "" && ` form is fail-safe (the CLI never runs in `/tmp`) and the path is validated + double-quoted. + + **Test stability (COD-30, #108):** + - Cleared leaked auth env in the Vitest setup, corrected stale route status-code / SSE-lifecycle expectations to match shipped behavior, updated the mobile keyboard accessory expectations, and measured DOMContentLoaded via browser navigation timing. Also fixed the `WebServer` title tests for the new `host` constructor arg + async `renderIndexHtml`. + + **Docs:** + - New `docs/security-architecture.md` documenting the full model (network binding, auth pipeline, the tunnel `req.ip` caveat, file-serving hardening, supply-chain, multi-instance isolation, security headers, and recommended secure setups). CLAUDE.md updated accordingly. + ## 0.8.2 ### Patch Changes diff --git a/CLAUDE.md b/CLAUDE.md index 183352ab..d68a5268 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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.8.2 (must match `package.json`) +**Version**: 0.9.0 (must match `package.json`) ## Project Overview @@ -78,7 +78,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph |------|---------| | Dev with TLS | `npx tsx src/index.ts web --https` | | Override window title hostname | `npx tsx src/index.ts web --title-hostname ` (default: `os.hostname()` — `codeman:` is used for tab title, title-flash, and OS desktop notification prefix) | -| Bind a non-loopback host | `npx tsx src/index.ts web --host 0.0.0.0` (or `-H`; env `CODEMAN_HOST`; default `127.0.0.1`). **Requires `CODEMAN_PASSWORD`** or it refuses to start — see Common Gotchas | +| Bind a non-loopback host | `npx tsx src/index.ts web --host 0.0.0.0` (or `-H`; env `CODEMAN_HOST`; default `127.0.0.1`). Without `CODEMAN_PASSWORD` it **starts but warns loudly** — see Common Gotchas + `docs/security-architecture.md` | | Continuous typecheck | `tsc --noEmit --watch` | | Test coverage | `npm run test:coverage` | | Dead-code sweep | `npm run knip` (config in `knip.json`) | @@ -101,7 +101,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph - **Dual-CLI prefix discipline** — Codeman supports both Claude Code and OpenCode (`claude-cli-resolver.ts` / `opencode-cli-resolver.ts`); env-var prefix is CLI-specific (`CLAUDE_CODE_*` vs `OPENCODE_*`) and the allowlist in `schemas.ts` enforces this. When adding settings, decide which CLI(s) it applies to and gate the env export accordingly — don't blindly forward both prefixes. See `docs/opencode-integration.md` for the OpenCode resolver design - **Zod `.optional()` rejects `null`** — accepts `undefined` only. When the frontend builds a request body with `JSON.stringify`, an explicit `null` field is preserved on the wire and fails validation with `INVALID_INPUT`. Convert `null` → `undefined` before stringifying (e.g. `field: value ?? undefined`), or declare the schema `.nullish()`. Real bugs caused: 0.6.4 (`durationMinutes` for ∞ respawn), and the same shape pattern hit `opusContext1mEnabled` in 0.6.3 - **`xterm-zerolag-input` is duplicated** — the local-echo overlay lives in BOTH `packages/xterm-zerolag-input/src/` (published to npm as a standalone library for external consumers — see README "Published Packages") AND inline inside `src/web/public/app.js` (runtime copy the web UI actually loads, since the page ships as plain JS without a bundler). Any change to overlay behavior MUST be applied to both, or dev and prod diverge — and a public API break in the package warrants a separate version bump for `xterm-zerolag-input` in the changeset. Always test on mobile after touching it. See `docs/local-echo-overlay-plan.md`. -- **Default bind is loopback-only — non-loopback fails closed without a password** — since COD-29 (PR #107) the web server defaults to `--host 127.0.0.1` (was `0.0.0.0`). Binding any non-loopback host (`--host`/`-H`/`CODEMAN_HOST`) **throws at startup unless `CODEMAN_PASSWORD` is set** (or `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` is given). The thrower is `isLoopbackBindHost()` in `server.ts`; flags are wired in `cli.ts`. ⚠️ Operational trap: the production systemd unit runs `node dist/index.js web --https` with no `--host`, so after this change it became reachable **only on localhost** — remote access (LAN/Tailscale/tunnel) silently breaks until you add `Environment=CODEMAN_HOST=0.0.0.0` + `Environment=CODEMAN_PASSWORD=…` to `~/.config/systemd/user/codeman-web.service`. A loopback-bound server is still reachable through a same-host tunnel (cloudflared → `127.0.0.1`), but NOT by a browser hitting the box's LAN/Tailscale IP. Auth user defaults to `admin`. +- **Default bind is loopback-only; non-loopback without a password starts but warns** — since COD-29 (PR #107) the web server defaults to `--host 127.0.0.1` (was `0.0.0.0`). As of **0.9.0** binding a non-loopback host (`--host`/`-H`/`CODEMAN_HOST`) without `CODEMAN_PASSWORD` **no longer refuses to start — it starts and prints a loud warning** listing the fixes (set `CODEMAN_PASSWORD`, bind loopback + tunnel/`tailscale serve`, or `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` to acknowledge → terser note). Host classification is `isLoopbackBindHost()` in `network-auth-policy.ts`; the warn-vs-start logic is in `server.ts` `start()`; flags wired in `cli.ts`. ⚠️ Operational note: the production systemd unit runs `node dist/index.js web --https` with no `--host`, so it binds **localhost only** — reach it remotely via `tailscale serve`/tunnel to `127.0.0.1`, or add `Environment=CODEMAN_HOST=0.0.0.0` + `Environment=CODEMAN_PASSWORD=…` to `~/.config/systemd/user/codeman-web.service`. A loopback bind is reachable through a same-host tunnel (cloudflared/tailscale → `127.0.0.1`) but NOT by a browser hitting the box's LAN IP. Auth user defaults to `admin`. **Full model: `docs/security-architecture.md`.** - **Instance isolation / multi-instance attach danger** — data dir (`~/.codeman`) and tmux socket (`tmux -L codeman`) are PROCESS-WIDE and shared by every Codeman on the machine, derived from `CODEMAN_INSTANCE` via `src/config/instance.ts` (`getDataDir()`/`dataPath()`/`DEFAULT_TMUX_SOCKET`). ⚠️ A 2nd instance on the SAME socket **discovers and attaches PTYs to the first instance's live sessions** (`tmux -L codeman attach-session …`), resizing/mutating them — `$HOME` isolation is NOT enough (tmux is system-global). To run two instances, give each a distinct `CODEMAN_INSTANCE` (scopes BOTH dir+socket: `~/.codeman-` + `-L codeman-`), or set `CODEMAN_TMUX_SOCKET` + `CODEMAN_DATA_DIR` individually. **`CODEMAN_INSTANCE` defaults to empty = the production layout (`~/.codeman`, `-L codeman`, port 3000)**, so this branch is safe to ship to master without disturbing existing installs. To run THIS beta alongside prod, launch with `scripts/run-beta.sh` (`CODEMAN_INSTANCE=beta` + `CODEMAN_PORT=5000`) — it never collides with prod's data dir/socket/port. Any new `~/.codeman/...` path MUST go through `dataPath()`, never `join(homedir(), '.codeman', …)`. **Import conventions**: Utils from `./utils`, types from `./types` (barrel), config from specific `./config/*` files. @@ -175,10 +175,12 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L ### Security +**Full model: [`docs/security-architecture.md`](docs/security-architecture.md)** — network binding, auth pipeline, the tunnel caveat, file-serving hardening, supply-chain, instance isolation, and recommended secure setups. + | Layer | Details | |-------|---------| | **Auth** | Optional HTTP Basic via `CODEMAN_USERNAME` (defaults to `admin`) / `CODEMAN_PASSWORD` env vars. Active only when `CODEMAN_PASSWORD` is set (`middleware/auth.ts`) | -| **Network bind** | Defaults to `127.0.0.1` (loopback). Binding a non-loopback host (`--host`/`CODEMAN_HOST`) **fails closed** without `CODEMAN_PASSWORD` unless `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1`. Added by COD-29 (PR #107) — see Common Gotchas | +| **Network bind** | Defaults to `127.0.0.1` (loopback). A non-loopback bind (`--host`/`CODEMAN_HOST`) without `CODEMAN_PASSWORD` **starts but warns loudly** (0.9.0; was fail-closed in COD-29/#107). `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` acknowledges the warning. Classifier: `network-auth-policy.ts` | | **QR Auth** | Single-use 6-char tokens (60s TTL) for tunnel login. See `docs/qr-auth-plan.md` | | **Sessions** | 24h cookie (`codeman_session`), auto-extend, device context audit | | **Rate limit** | 10 failed auth/IP → 429 (15min decay). QR has separate limiter | diff --git a/docs/security-architecture.md b/docs/security-architecture.md new file mode 100644 index 00000000..6a19f33d --- /dev/null +++ b/docs/security-architecture.md @@ -0,0 +1,319 @@ +# Security Architecture + +This document describes Codeman's security model: how it decides who may reach +the web UI, how requests are authenticated, how the file-serving and tmux layers +are hardened, and the recommended ways to expose an instance safely. + +Codeman spawns and drives Claude/OpenCode CLIs with +`--dangerously-skip-permissions`. **Anyone who can reach an unauthenticated +instance can run arbitrary commands as your user.** The defaults below are chosen +so that a fresh install is safe on the machine it runs on, while remote access is +an explicit, guided opt‑in. + +> TL;DR — Codeman binds **loopback only (`127.0.0.1`) by default**, so out of the +> box it is reachable only from the same machine and needs no password. To reach +> it from elsewhere, either put it behind an **authenticated tunnel** +> (`tailscale serve` / `cloudflared`) **or** bind a wider host **and set +> `CODEMAN_PASSWORD`**. If you bind a non‑loopback host with no password, Codeman +> still starts but prints a **loud warning** telling you how to secure it. + +--- + +## 1. Network binding model + +| Setting | Default | Source | +|---------|---------|--------| +| Bind host | `127.0.0.1` (loopback) | `--host` / `CODEMAN_HOST` → `WebServer` ctor | +| Port | `3000` | `--port` / `CODEMAN_PORT` | +| TLS | off (`--https` to enable) | `--https` | + +### Bind host classification + +`isLoopbackBindHost()` (`src/web/network-auth-policy.ts`) decides whether a bind +host is loopback-only. It returns `true` for: + +- `localhost` +- any IPv4 in `127.0.0.0/8` (e.g. `127.0.0.1`, `127.42.0.9`) +- IPv6 loopback `::1` (bracketed `[::1]` and the long form `0:0:0:0:0:0:0:1`) +- IPv4‑mapped loopback `::ffff:127.*` + +It returns `false` for `0.0.0.0`, `::` (all interfaces), LAN IPs, and hostnames. +The classification is **fail‑safe in the dangerous direction**: any host that is +not provably loopback is treated as non‑loopback (it never mistakes `0.0.0.0` +for loopback). Shorthand forms like `127.1` or integer/octal IPs classify as +non‑loopback (you'll get a warning, not a silent wide‑open bind) — use +`127.0.0.1` for an unambiguous loopback bind. + +### Startup policy (the "warn, don't block" rule) + +At `WebServer.start()`: + +| Bind host | `CODEMAN_PASSWORD` | Behavior | +|-----------|--------------------|----------| +| loopback (default) | unset | **Start.** Safe — reachable only from this machine. | +| loopback | set | **Start.** Auth required even locally. | +| non‑loopback | set | **Start.** Auth protects the open bind. | +| non‑loopback | unset | **Start + LOUD warning** listing how to secure it. | +| non‑loopback | unset, `--allow-unauthenticated-network` | **Start + terse acknowledged note.** | + +> History: an earlier iteration (unreleased COD‑29) *refused to start* on a +> non‑loopback bind without a password. That surprised setups that "just worked" +> before, so **0.9.0 changed it to start‑and‑warn**. Loopback is still the safe +> default; the warning (with three concrete fixes) replaces the hard failure. + +The warning points at three ways to secure the instance: + +1. `CODEMAN_PASSWORD=` — turns on HTTP Basic auth (see §2). +2. `--host 127.0.0.1` + an authenticated tunnel (`cloudflared` / `tailscale serve`). +3. `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` + — explicitly accept the risk (downgrades the warning to a one‑line note). This + flag is **only** an acknowledgement; it does not change reachability. + +`CODEMAN_API_URL` (used by hooks/child processes) is always derived as a loopback +address (`0.0.0.0`/`localhost`/`::1` → `127.0.0.1`) so in‑process hooks reach the +server over loopback regardless of the public bind. + +--- + +## 2. Authentication + +Auth is **optional** and controlled by env vars captured at startup: + +- `CODEMAN_USERNAME` (default `admin` when only a password is set) +- `CODEMAN_PASSWORD` + +When `CODEMAN_PASSWORD` is unset, no auth is enforced — which is why the default +loopback bind matters. The auth pipeline (`src/web/middleware/auth.ts`, +`onRequest` hook) runs in this order: + +1. **Localhost‑only exemptions** (always first): `POST /api/hook-event` and the QR + `/q/` short‑code path are exempt when `req.ip` is loopback (see §3). +2. **Session cookie** check — a valid `codeman_session` cookie short‑circuits to + allow. +3. **HTTP Basic** check — correct credentials short‑circuit to allow and clear + that IP's failure counter. +4. **Rate‑limit gate** — if neither cookie nor credentials passed and the IP is + locked out, return `429` with a `Retry-After` header. +5. Otherwise return `401`, incrementing the IP's failure counter. + +### Session cookies + +On successful Basic auth the server issues `codeman_session`, an opaque +server‑side token (`randomBytes(32)`), valid 24h with auto‑extend and device +context for the audit log. Tokens are **not** client‑signed — they're validated +by presence in a server‑side map, so they cannot be forged offline. + +### Rate limiting / lockout recovery + +Failed auth is tracked **per IP**: 10 failures → `429`, with a 15‑minute decay. +The QR path has its own separate limiter. + +The lockout check sits **after** the cookie/credential checks (step 4, not first). +This is deliberate: a user with a **valid cookie or correct password recovers +immediately** even while an attacker is hammering the same IP — important because +all traffic through a tunnel shares one source IP (loopback). Wrong credentials +are still counted and still hit the `429` at the threshold, so brute‑force +protection is unchanged. + +--- + +## 3. Request‑origin trust & the tunnel caveat + +`req.ip` is derived from the **TCP socket only** — Fastify runs with +`trustProxy: false`, so `X-Forwarded-For` / `X-Real-IP` / `Forwarded` are +**ignored**. A remote client cannot forge `req.ip` to `127.0.0.1`. + +**However**, a reverse tunnel that connects to the server over loopback (e.g. +`cloudflared --url http://localhost:3000`) makes **every tunneled request arrive +with `req.ip = 127.0.0.1`**. The localhost‑only exemptions then treat those +requests as local: + +- `POST /api/hook-event` — auth‑exempt for loopback. Bounded impact: it is + `HookEventSchema`‑validated and requires a valid in‑memory `sessionId`; it can + drive respawn signals, SSE broadcasts, push notifications, and transcript + watching — **not** arbitrary terminal input or file reads. It is a + session‑disruption / notification‑spoofing surface, not RCE. +- QR `/q/` — still protected by its own short‑code brute‑force limiter + (10 failures / 60s against a 62⁶ space). + +**Mitigation:** set `CODEMAN_PASSWORD` whenever a loopback‑connecting tunnel is +up (it does not gate the hook‑event exemption, but it gates everything else and +is the documented practice). Prefer `tailscale serve` (below), which authenticates +at the tailnet layer so untrusted clients never reach the loopback port at all. +A future hardening could gate the hook‑event exemption on a shared secret while a +tunnel is active. + +--- + +## 4. Recommended remote‑access setups + +Ordered most‑to‑least recommended: + +### A. Tailscale serve (recommended) + +Bind loopback, let Tailscale front it on your tailnet with a real cert: + +```bash +codeman web --https # binds 127.0.0.1:3000 +tailscale serve --bg https / http://127.0.0.1:3000 +``` + +Only devices on your tailnet can reach it; Tailscale handles identity. No app +password and no `0.0.0.0` bind required. (This is the maintainer's production +setup.) + +### B. Authenticated cloudflared tunnel + password + +```bash +export CODEMAN_PASSWORD= +codeman web --https +cloudflared tunnel --url https://localhost:3000 +``` + +Always set `CODEMAN_PASSWORD` here — the tunnel connects over loopback, so the +hook‑event exemption (§3) would otherwise be reachable from the public URL. + +### C. Direct LAN bind + password + +```bash +export CODEMAN_PASSWORD= +codeman web --https --host 0.0.0.0 +``` + +Exposes the port on all interfaces; the password is the only thing protecting it. + +### Avoid + +`--host 0.0.0.0` **without** a password. Codeman will start (and warn), but +anyone on the network can control your Claude sessions. Never re‑expose `0.0.0.0` +without a password. + +--- + +## 5. File‑serving hardening + +Three routes serve workspace files; all require a valid `sessionId` and run the +shared path validator `validateSessionFilePath()` (`src/web/route-helpers.ts`): +it `realpath`s the target **before** the boundary check and rejects anything that +escapes the session working directory (`..`, absolute paths, and symlinks that +resolve outside). The realpath‑before‑check ordering closes the validation‑time +TOCTOU window. + +| Route | Cap | Notes | +|-------|-----|-------| +| `file-content` | 10 MB | text preview | +| `file-raw` | 50 MB | inline MIME map; **`X-Content-Type-Options: nosniff` on all responses** | +| `POST /api/download` | 50 MB | forced `attachment`; sensitive‑path blocklist | + +### SVG / content‑type XSS + +A workspace `.svg` served inline as `image/svg+xml` is a stored‑XSS vector (SVG +can carry `