From fc0ecc6051373e0f8fd98588035ff41e6b30543c Mon Sep 17 00:00:00 2001 From: JD Date: Mon, 26 Oct 2026 02:11:16 -0400 Subject: [PATCH] fix(tunnel): give the unauthenticated tunnel its own acknowledgement The network acknowledgement that install.sh writes for every passwordless LAN install also satisfied the tunnel guard, so one click on the welcome screen's tunnel link published full terminal control to a public URL with no login and no confirm. The tunnel now needs CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL or --allow-unauthenticated-tunnel (carried by web -d and service install), and with no login the browser asks before every enable, reading a new loginConfigured field from GET /api/tunnel/status. The guard is now decided once per request and required again where the tunnel starts, so a save that found the tunnel running can no longer restart it unguarded after a concurrent stop. Closes #706 --- .changeset/tunnel-own-acknowledgement.md | 5 + CLAUDE.md | 2 +- docs/api-reference.md | 3 +- docs/architecture-invariants.md | 2 +- docs/security-architecture.md | 18 +- docs/tailscale-installer-plan.md | 2 +- docs/wiki/Remote-Access.md | 6 +- docs/wiki/Security.md | 3 +- docs/wiki/Settings-Reference.md | 3 +- src/cli.ts | 18 +- src/daemon-control.ts | 2 + src/web/network-auth-policy.ts | 19 +- src/web/ports/infra-port.ts | 2 + src/web/public/settings-ui.js | 103 +++++- src/web/routes/system-routes.ts | 111 ++++-- src/web/schemas.ts | 2 +- src/web/server.ts | 21 +- test/cli-commands.test.ts | 26 ++ test/cli-tunnel-flag.test.ts | 102 ++++++ test/daemon-control.test.ts | 30 ++ test/install-sh-invariants.test.ts | 4 + test/mocks/mock-route-context.ts | 1 + .../routes/system-routes-tunnel-guard.test.ts | 336 +++++++++++++----- test/service-installer.test.ts | 24 ++ test/tunnel-enable-confirm-ui.test.ts | 230 ++++++++++++ test/tunnel-flag-plumbing.test.ts | 101 ++++++ 26 files changed, 1020 insertions(+), 156 deletions(-) create mode 100644 .changeset/tunnel-own-acknowledgement.md create mode 100644 test/cli-tunnel-flag.test.ts create mode 100644 test/tunnel-enable-confirm-ui.test.ts create mode 100644 test/tunnel-flag-plumbing.test.ts diff --git a/.changeset/tunnel-own-acknowledgement.md b/.changeset/tunnel-own-acknowledgement.md new file mode 100644 index 000000000..1b8858ad0 --- /dev/null +++ b/.changeset/tunnel-own-acknowledgement.md @@ -0,0 +1,5 @@ +--- +'aicodeman': patch +--- + +Security fix: `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK` (and `--allow-unauthenticated-network`) no longer lets the Cloudflare tunnel start without a password. It now acknowledges only an unauthenticated network bind, which is what the installer writes it for on a LAN install without a password, so on those installs one click on the welcome screen's tunnel link published full terminal control to a public URL with no login and no confirm. An unauthenticated tunnel has its own opt-in now, `CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1` or `codeman web --allow-unauthenticated-tunnel` (carried by `codeman web -d` and `codeman service install`), which the installer never writes. If you relied on the network variable to start the tunnel without a password, set one of those instead. Without it the server still accepts an enable that carries `acknowledgeUnauthTunnel: true`, as before. With no password and no multi-user mode, the browser now asks before every tunnel enable from the welcome link, the tunnel panel and App Settings, whatever the server would accept, using the new `loginConfigured` field of `GET /api/tunnel/status`. diff --git a/CLAUDE.md b/CLAUDE.md index bd2594acd..8b10dc8eb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -281,7 +281,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph | **Rate limit** | 10 failed auth/IP → 429 (15min decay). Multi-user mode instead checks 50/IP (`AUTH_IP_FAILURE_MAX`) and 10 per (username, IP) BEFORE the password, so past either even a correct password from that IP waits (the account still logs in from another IP), plus 10 per username AFTER it (429 to wrong guesses from any IP, never to a correct password). QR and hook-secret have separate buckets, so neither can lock out login | | **Hook bypass** | `/api/hook-event` + `/api/status-telemetry` skip Basic auth (localhost-only, schema-validated), but when auth is active the loopback bypass requires `X-Codeman-Hook-Secret` **unconditionally** (Codeman cannot detect a user's own loopback reverse proxy) | | **Lost-frame page** | The THIRD unauthenticated 200, beside the two hook routes, and the only one decided by request headers alone: a `GET`/`HEAD` carrying `Sec-Fetch-Dest: iframe\|frame`, `Accept: text/html` and mode `navigate` (or none), for a path that is NOT a registered route (never `/api/`, `/ws/`, `/q/`), is answered BEFORE the credential checks with the static web-tab recovery page (`lostWebviewFramePage`: no reflected input, `default-src 'none'` plus its own script hash, `no-store`). `/` is the one registered route also admitted, only when the request carries neither `codeman_session` nor `Authorization` (nothing in Codeman frames its own root; a sandboxed frame has neither), since the landing page masks to exactly `/` and its reload otherwise rendered Codeman inside the web tab. ⚠️ A non-browser client can set those headers, so an unauthenticated caller can tell a registered route (401) from a non-route (200) and enumerate the route table; accepted, the routes are public in `docs/api-reference.md`. Pinned by `test/webview-auth-exemption.test.ts` + `test/webview-lost-root-frame.test.ts` | -| **Tunnel** | Enabling a tunnel **refuses** without `CODEMAN_PASSWORD` unless exposure is acknowledged via `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` or the per-request `acknowledgeUnauthTunnel:true` action field (never persisted) | +| **Tunnel** | Enabling a tunnel **refuses** without a login (`CODEMAN_PASSWORD` or multi-user mode) unless exposure is acknowledged via `CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1` / `--allow-unauthenticated-tunnel` or the per-request `acknowledgeUnauthTunnel:true` action field (never persisted). `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK` acknowledges the BIND only and never satisfies this guard (#706: `install.sh` writes it for every passwordless LAN install). The authorization is decided ONCE per request, running or not, and required again at the `start()` site: the settings-file awaits between them let a concurrent stop land, and an unauthorized enable never writes `tunnelEnabled:true`. With no login the browser asks before every enable (welcome link, tunnel panel, App Settings save that turns it on) through ONE helper, `_confirmTunnelEnable()` (settings-ui.js), which reads `loginConfigured` from `GET /api/tunnel/status` (computed in the route by the guard's own test, never in `TunnelManager`); the 403 handler is the fallback when that read fails | | **Validation** | Zod schemas, Unicode-aware path allowlist regex, env prefix allowlist (`CLAUDE_CODE_*`/`OPENCODE_*`/`CODEX_*`/`GEMINI_*`/`GOOGLE_*`/`ANTIGRAVITY_*`/`PI_*`/`GROK_*`/`XAI_*`/`DSH_*`/`DEEPSEEK_*`/`OMP_*`/`COPILOT_*`) | | **Headers** | CORS localhost-only, CSP, X-Frame-Options, HSTS if HTTPS | diff --git a/docs/api-reference.md b/docs/api-reference.md index 95a28b178..7d892c480 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1218,7 +1218,8 @@ A local case whose folder is the top of a git repository can keep **worktrees** | Method | Path | Body | Notes | | ------ | ----------------------------- | ------------------------------------ | ----- | | `GET` | `/api/settings` | none | The stored settings object. Never writes. For a non-admin in multi-user mode, without `voiceSettings.apiKey`, `caseOrder` and `lastUsedCase`. | -| `PUT` | `/api/settings` | partial settings (strict schema) | Merged into the stored file; services (watchers, tunnel, tmux history limit) reconcile to the merged result. `403 FORBIDDEN` for a non-admin in multi-user mode. A tunnel enable without `CODEMAN_PASSWORD` is refused separately (`403 OPERATION_FAILED`, resend with `acknowledgeUnauthTunnel: true`). | +| `PUT` | `/api/settings` | partial settings (strict schema) | Merged into the stored file; services (watchers, tunnel, tmux history limit) reconcile to the merged result. `403 FORBIDDEN` for a non-admin in multi-user mode. A tunnel enable with no login (no `CODEMAN_PASSWORD`, not multi-user) is refused separately (`403 OPERATION_FAILED`, resend with `acknowledgeUnauthTunnel: true`) unless the server runs with `CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1` or `--allow-unauthenticated-tunnel`. `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK` does not count; when it is set, the refusal says so. An enable without that authorization is accepted only while the tunnel is already running and never stores `tunnelEnabled: true`; if the tunnel stops before the request would start it, it is not restarted and the answer is the same `403 OPERATION_FAILED` (the request's other settings are saved). | +| `GET` | `/api/tunnel/status` | none | `{ running, url, ..., loginConfigured }`. `loginConfigured` is `true` when a login protects the server (`CODEMAN_PASSWORD` set or multi-user mode on), the same test the tunnel guard above applies; with `false` the browser asks before every tunnel enable. Neither opt-in variable makes it `true`. | | `GET` | `/api/execution/model-config` | none | `modelConfig` (cron's and the plan orchestrator's default model). | | `PUT` | `/api/execution/model-config` | `{ defaultModel?, agentTypeOverrides?, ... }` | Replaces `modelConfig`. `403 FORBIDDEN` for a non-admin in multi-user mode. | | `PUT` | `/api/cases/order` | `{ order: string[] }` | The one case order every user's `GET /api/cases` is sorted by. `403 FORBIDDEN` for a non-admin in multi-user mode. | diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 7a743bb62..f36714124 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -1182,7 +1182,7 @@ Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not u | **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 logins/IP → 429 (15min decay). Multi-user mode instead has two gates checked before scrypt, so past either even a correct password from that IP gets 429: 50/IP (`AUTH_IP_FAILURE_MAX`) and 10 per (username, IP) (`AUTH_FAILURE_MAX`; the same account still logs in from another IP). A 10-per-username bucket (multi-user only) is checked after the verify: 429 to failed guesses from the 10th on, from any IP, never to a correct password. QR has separate limiter | -| **Hook bypass** | `/api/hook-event` (and `/api/status-telemetry`, the statusLine exporter) skip Basic auth (localhost-only, schema-validated). When auth is active (`CODEMAN_PASSWORD` set), the loopback bypass requires the per-instance `X-Codeman-Hook-Secret` header **unconditionally** — COD-54 introduced it tunnel-gated; COD-91 (PR #127) made it always-on because Codeman can't detect a user's own loopback reverse proxy (own cloudflared/`tailscale serve`/nginx → 127.0.0.1), closing that residual plain-bypass gap. Hook curls cat the secret file at exec time via `$CODEMAN_HOOK_SECRET_FILE` (session env, `config/hook-secret.ts`); a missing/wrong secret gets 401 and rate-limits in a dedicated bucket (never locks out login). Tunnel enable **refuses** without `CODEMAN_PASSWORD` unless exposure is acknowledged — via `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` (env, COD-55) **or** the per-request `acknowledgeUnauthTunnel:true` action field (1.1.9): the welcome/settings tunnel toggle pops a security confirm dialog and, on confirm, resends with that flag (server logs a loud warning on every passwordless tunnel start; curl/API stay refused without password/env/flag). The flag is an action field, never persisted | +| **Hook bypass** | `/api/hook-event` (and `/api/status-telemetry`, the statusLine exporter) skip Basic auth (localhost-only, schema-validated). When auth is active (`CODEMAN_PASSWORD` set), the loopback bypass requires the per-instance `X-Codeman-Hook-Secret` header **unconditionally** — COD-54 introduced it tunnel-gated; COD-91 (PR #127) made it always-on because Codeman can't detect a user's own loopback reverse proxy (own cloudflared/`tailscale serve`/nginx → 127.0.0.1), closing that residual plain-bypass gap. Hook curls cat the secret file at exec time via `$CODEMAN_HOOK_SECRET_FILE` (session env, `config/hook-secret.ts`); a missing/wrong secret gets 401 and rate-limits in a dedicated bucket (never locks out login). Tunnel enable **refuses** without a login (`CODEMAN_PASSWORD` or multi-user mode) unless exposure is acknowledged — via `CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1` / `--allow-unauthenticated-tunnel` (#706; until then the guard took `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1`, COD-55, which `install.sh` writes for every passwordless LAN install, so the bind acknowledgement silently published the tunnel too) **or** the per-request `acknowledgeUnauthTunnel:true` action field (1.1.9). With no login the browser asks BEFORE every enable (welcome link, tunnel panel, an App Settings save that turns it on) through `_confirmTunnelEnable()`, reading `loginConfigured` from `GET /api/tunnel/status`, and on confirm sends that flag; the 403 handler stays the fallback when the status read fails (server logs a loud warning on every passwordless tunnel start; curl/API stay refused without password/env/flag). The flag is an action field, never persisted | | **Env vars** | `CODEMAN_MUX` (managed session), `CODEMAN_API_URL` (auto-set for hooks), `CODEMAN_ALLOWED_HOSTS` (extra Host/Origin allowlist entries for reverse proxies, comma-separated; bare `.suffix` matches subdomains), `CODEMAN_DOCKER_BRIDGE_HOOKS`=1 (opt-in hooks-only listener on the docker bridge gateway so in-container hooks reach a loopback-bound server; bind IP from `CODEMAN_DOCKER_BRIDGE_HOST` or auto-detect) | | **Validation** | Zod schemas, Unicode-aware path allowlist regex, env prefix allowlist (`CLAUDE_CODE_*`/`OPENCODE_*`/`CODEX_*`/`GEMINI_*`/`GOOGLE_*`/`ANTIGRAVITY_*`/`PI_*`) | | **Headers** | CORS localhost-only, CSP, X-Frame-Options, HSTS if HTTPS | diff --git a/docs/security-architecture.md b/docs/security-architecture.md index 21f0b9e56..0a6eb6eaa 100644 --- a/docs/security-architecture.md +++ b/docs/security-architecture.md @@ -105,7 +105,9 @@ The warning points at three ways to secure the instance: 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. + flag is **only** an acknowledgement; it does not change reachability. It covers + the bind and nothing else: it does **not** let the managed Cloudflare tunnel + start without a password, which has its own opt‑in (§2, the tunnel caveat). `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 @@ -209,8 +211,15 @@ requests as local: **Mitigation:** set `CODEMAN_PASSWORD` whenever a loopback‑connecting tunnel is up — it gates everything except the (secret‑gated) hook exemption and is the documented practice; since COD‑55 enabling the managed tunnel **refuses** to start -without it unless `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` explicitly -acknowledges the exposure. Prefer `tailscale serve` (below), which authenticates +without a login (`CODEMAN_PASSWORD` or multi‑user mode) unless +`CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1` / `--allow-unauthenticated-tunnel` +acknowledges the exposure, or the request carries `acknowledgeUnauthTunnel: true` +(what the browser sends after its confirm). The network acknowledgement +(`CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK`) does **not** count: the installer writes +it for every LAN install without a password, so it once turned a click on the +welcome screen's tunnel link into a public, login‑free URL with no confirm. With +no login the browser asks before every enable, whatever the server would accept, +reading `loginConfigured` from `GET /api/tunnel/status`. Prefer `tailscale serve` (below), which authenticates at the tailnet layer so untrusted clients never reach the loopback port at all. ### Host‑header & Origin allowlist (DNS‑rebinding & CSRF defense) @@ -585,7 +594,8 @@ Opt-in and off by default: the server POSTs the Web Push events (permission prom | `--host` / `CODEMAN_HOST` | Bind host (default `127.0.0.1`) | | `CODEMAN_ALLOWED_HOSTS` | Extra `Host`/`Origin` allowlist entries for reverse proxies (comma‑separated; exact host, or leading‑dot `.suffix` for subdomains) — see §3 | | `--base-url` / `CODEMAN_BASE_URL` | Sub‑path prefix Codeman is mounted under behind a reverse proxy, e.g. `/codeman` (default `/`); the proxy must forward the prefix unchanged. Independent of `CODEMAN_ALLOWED_HOSTS` | -| `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK` | Acknowledge an unauthenticated non‑loopback bind (downgrades the warning) | +| `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK` | Acknowledge an unauthenticated non‑loopback bind (downgrades the warning). Does not cover the tunnel | +| `--allow-unauthenticated-tunnel` / `CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL` | Let the managed Cloudflare tunnel start without a login (the browser still asks first). Never written by the installer | | `--https` | Enable TLS (adds HSTS) | | `CODEMAN_INSTANCE` | Scope tmux socket + data dir for isolation | | `CODEMAN_GESTURE=1` | Make the gesture overlay available (widens CSP) | diff --git a/docs/tailscale-installer-plan.md b/docs/tailscale-installer-plan.md index aac7d4df2..2322ed096 100644 --- a/docs/tailscale-installer-plan.md +++ b/docs/tailscale-installer-plan.md @@ -192,7 +192,7 @@ start/stop wrapper to manage; tailscaled owns the lifecycle). | First cert issuance latency | Verify step retries ~30s and says why the first load may be slow. | | `tailscale up` needs auth | Print the auth URL prominently, poll with timeout, skip gracefully. Works headless. | | Custom `CODEMAN_PORT` | Serve target uses the actual port; `install.sh tailscale` re-reads it from the service file. | -| Funnel (public internet) | OUT OF SCOPE for v1. If ever added it must mirror the tunnel guard: refuse without `CODEMAN_PASSWORD` (`isUnauthenticatedNetworkAcknowledged`). Funnel exposes to the whole internet and is a different risk class than tailnet-only serve. Mention `tailscale funnel` in docs only, with the password warning. | +| Funnel (public internet) | OUT OF SCOPE for v1. If ever added it must mirror the tunnel guard: refuse without `CODEMAN_PASSWORD` (`isUnauthenticatedTunnelAcknowledged` and the login check in the `PUT /api/settings` tunnel guard). Funnel exposes to the whole internet and is a different risk class than tailnet-only serve. Mention `tailscale funnel` in docs only, with the password warning. | | Uninstall | Best effort: if `serve status --json` shows 443 proxying to our port, run the targeted `tailscale serve --https=443 off` (still accepted by current CLIs); if the CLI rejects it, print manual instructions. Never touch other mappings, never uninstall tailscale itself. | | User already fronting Codeman some other way (reverse proxy etc.) | The serve check only looks at tailscale state; other proxies are invisible and unaffected (same stance as the loopback-exemption note in security-architecture). | diff --git a/docs/wiki/Remote-Access.md b/docs/wiki/Remote-Access.md index 4cba2a6cd..49e577cba 100644 --- a/docs/wiki/Remote-Access.md +++ b/docs/wiki/Remote-Access.md @@ -115,8 +115,10 @@ There is also a toggle in **App Settings → System → Remote access**. **The tunnel refuses to start without a password.** That is on purpose: a public URL with no authentication is a terminal on your machine handed to the internet. Acknowledging the risk -explicitly is possible from the UI toggle, and only from there; the API will not do it for -you. +explicitly is possible from the UI toggle, which asks you every time, or for the whole server +with `CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1` (or `codeman web --allow-unauthenticated-tunnel`). +`CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK`, which the installer sets for a LAN install without a +password, does not count. ## LAN plus password diff --git a/docs/wiki/Security.md b/docs/wiki/Security.md index c3686143e..678e1ff6c 100644 --- a/docs/wiki/Security.md +++ b/docs/wiki/Security.md @@ -20,7 +20,8 @@ In order of how much they matter: 1. **Do not expose it without `CODEMAN_PASSWORD`.** Binding a non-loopback host without one starts, but warns loudly. A tunnel refuses outright unless you acknowledge the exposure - in the UI. + in the UI, which asks every time, or set `CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL`. The + installer's `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK` covers the LAN bind only. 2. **Prefer Tailscale over a public tunnel.** Keeping the loopback bind and putting a private network in front of it removes the public attack surface entirely, and gives you real HTTPS. See [Remote Access](Remote-Access). diff --git a/docs/wiki/Settings-Reference.md b/docs/wiki/Settings-Reference.md index 19d1676be..e09056b37 100644 --- a/docs/wiki/Settings-Reference.md +++ b/docs/wiki/Settings-Reference.md @@ -244,7 +244,8 @@ Some things are configured before the server starts, not in the UI: | `CODEMAN_GESTURE` | Makes gesture control available to be enabled. | | `CODEMAN_DOCKER_BRIDGE_HOOKS` | Lets in-container hooks reach the host on a loopback bind. | | `CODEMAN_FILE_PICKER_ROOTS` | Extra roots for the path picker. | -| `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK` | Acknowledges exposing the server with no password. | +| `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK` | Acknowledges binding a non-loopback address with no password. Does not cover the tunnel. | +| `CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL` | Lets the Cloudflare tunnel start with no password (`--allow-unauthenticated-tunnel` on `codeman web`). The browser still asks first. | | `CODEMAN_BASE_URL` | Mounts Codeman under a sub-path behind a reverse proxy that forwards the prefix unchanged. See [Remote Access](Remote-Access). | | `CODEMAN_MAX_SESSIONS` | Cap on concurrent sessions. 50 by default, 1 to 462 (a larger value is clamped to 462, anything that is not a positive integer falls back to 50). Read at startup. The 462 assumes the default `CODEMAN_MAX_WEBVIEWS` and no pinned closed sessions, which share the same 512-entry tab layout. In multi-user mode the per-user cap defaults to half of it unless `CODEMAN_MAX_SESSIONS_PER_USER` is set. | | `CODEMAN_MAX_DOWNLOAD_BYTES` | Cap on raw file bodies and downloads. 2 GB by default, `0` for none. | diff --git a/src/cli.ts b/src/cli.ts index ba07793b3..a6bfda1ca 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -950,6 +950,10 @@ function addWebLaunchOptions(cmd: Command): Command { '--allow-unauthenticated-network', 'Allow non-loopback web access without CODEMAN_PASSWORD (dangerous; terminal control is exposed)' ) + .option( + '--allow-unauthenticated-tunnel', + 'Allow starting the Cloudflare tunnel without CODEMAN_PASSWORD (dangerous; publishes terminal control to a public URL; env: CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL)' + ) .option( '--multiuser', 'Enable opt-in multi-user mode (named users in ~/.codeman/users.json; env: CODEMAN_MULTIUSER)' @@ -964,6 +968,7 @@ function toWebLaunchOptions(options: { baseUrl?: string; titleHostname?: string; allowUnauthenticatedNetwork?: boolean; + allowUnauthenticatedTunnel?: boolean; multiuser?: boolean; }): WebLaunchOptions { const port = parseInt(options.port, 10); @@ -985,6 +990,7 @@ function toWebLaunchOptions(options: { basePath, titleHostname: options.titleHostname, allowUnauthenticatedNetwork: !!options.allowUnauthenticatedNetwork, + allowUnauthenticatedTunnel: !!options.allowUnauthenticatedTunnel, multiuser: !!options.multiuser, }; } @@ -1072,6 +1078,7 @@ webCmd.action(async (options) => { const https = launch.https; const titleHostname = options.titleHostname; const allowUnauthenticatedNetwork = launch.allowUnauthenticatedNetwork ?? false; + const allowUnauthenticatedTunnel = launch.allowUnauthenticatedTunnel ?? false; const basePath = launch.basePath ?? ''; // Single source of truth for subsystems that read it directly (e.g. renderers). if (basePath) process.env.CODEMAN_BASE_URL = basePath; @@ -1086,7 +1093,16 @@ webCmd.action(async (options) => { try { // The server prints its own "running at" line (it also covers the daemon and // service launch paths), so this one used to be a duplicate of it. - const server = await startWebServer(port, https, false, host, titleHostname, allowUnauthenticatedNetwork, basePath); + const server = await startWebServer( + port, + https, + false, + host, + titleHostname, + allowUnauthenticatedNetwork, + basePath, + allowUnauthenticatedTunnel + ); if (https) { console.log(palette.warn(' Note: Accept the self-signed certificate in your browser on first visit')); } diff --git a/src/daemon-control.ts b/src/daemon-control.ts index cb09f3d7d..c52efd5ee 100644 --- a/src/daemon-control.ts +++ b/src/daemon-control.ts @@ -49,6 +49,7 @@ export interface WebLaunchOptions { basePath?: string; titleHostname?: string; allowUnauthenticatedNetwork?: boolean; + allowUnauthenticatedTunnel?: boolean; multiuser?: boolean; } @@ -92,6 +93,7 @@ export function buildWebArgs(options: WebLaunchOptions): string[] { if (options.basePath) args.push('--base-url', options.basePath); if (options.titleHostname) args.push('--title-hostname', options.titleHostname); if (options.allowUnauthenticatedNetwork) args.push('--allow-unauthenticated-network'); + if (options.allowUnauthenticatedTunnel) args.push('--allow-unauthenticated-tunnel'); if (options.multiuser) args.push('--multiuser'); return args; } diff --git a/src/web/network-auth-policy.ts b/src/web/network-auth-policy.ts index 3fb049747..4e775460b 100644 --- a/src/web/network-auth-policy.ts +++ b/src/web/network-auth-policy.ts @@ -7,15 +7,28 @@ export function isExplicitlyEnabled(value: string | undefined): boolean { } /** - * True when unauthenticated network exposure is acceptable: either a password is - * set (auth active) or the operator explicitly acknowledged it. Used by the - * tunnel-enable guard (COD-55) to refuse publishing an unauthenticated public URL. + * True when an unauthenticated non-loopback BIND is acceptable: either a password is + * set (auth active) or the operator explicitly acknowledged it. Quiets the startup + * warning only; it does not cover the Cloudflare tunnel (#706), which has its own + * acknowledgement below. */ export function isUnauthenticatedNetworkAcknowledged(allowFlag = false): boolean { if (process.env.CODEMAN_PASSWORD) return true; return allowFlag || isExplicitlyEnabled(process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK); } +/** + * True when the operator opted in to an unauthenticated public Cloudflare tunnel: + * `--allow-unauthenticated-tunnel` (passed in as `allowFlag`) or + * CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL. Used by the tunnel-enable guard (COD-55) in + * place of a login. Kept apart from the network acknowledgement on purpose (#706): + * install.sh writes that one for every passwordless LAN install, and it must never + * also publish the machine to the internet. install.sh never writes this one. + */ +export function isUnauthenticatedTunnelAcknowledged(allowFlag = false): boolean { + return allowFlag || isExplicitlyEnabled(process.env.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL); +} + export function isLoopbackBindHost(host: string): boolean { const normalized = host .trim() diff --git a/src/web/ports/infra-port.ts b/src/web/ports/infra-port.ts index 21e5beca0..38d819327 100644 --- a/src/web/ports/infra-port.ts +++ b/src/web/ports/infra-port.ts @@ -35,6 +35,8 @@ export interface InfraPort { readonly scheduledRuns: Map; readonly teamWatcher: TeamWatcher; readonly tunnelManager: TunnelManager; + /** `--allow-unauthenticated-tunnel` was passed to `codeman web` (the env var is read per request, #706). */ + readonly allowUnauthenticatedTunnel: boolean; readonly pushStore: PushSubscriptionStore; startScheduledRun(prompt: string, workingDir: string, durationMinutes: number, owner?: string): Promise; stopScheduledRun(id: string): Promise; diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index db745f450..f906e8dba 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -2271,10 +2271,16 @@ Object.assign(CodemanApp.prototype, { btn.disabled = true; try { const newEnabled = !isActive; + const gate = newEnabled ? await this._confirmTunnelEnable() : { proceed: true, acknowledge: false }; + if (!gate.proceed) { + this._updateWelcomeTunnelBtn(false); + btn.disabled = false; + return; + } const res = await fetch('/api/settings', { method: 'PUT', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ tunnelEnabled: newEnabled }), + body: JSON.stringify({ tunnelEnabled: newEnabled, ...(gate.acknowledge ? { acknowledgeUnauthTunnel: true } : {}) }), }); // COD-55: server refuses an unauthenticated public tunnel (403). Surface it. if (newEnabled && (await this._handleTunnelEnableRefusal(res))) { @@ -2582,11 +2588,67 @@ Object.assign(CodemanApp.prototype, { return `${Math.floor(hrs / 24)}d ago`; }, + /** + * End an App Settings save whose tunnel enable did not go through (confirm cancelled, or + * refused by the server): the WHOLE settings PUT is off, so turn the tunnel toggle back + * off in the UI and localStorage. Other settings persisted to localStorage still apply locally. + */ + _abandonTunnelEnable(settings, keepOpen) { + settings.tunnelEnabled = false; + this.saveAppSettingsToStorage(settings); + const cb = document.getElementById('appSettingsTunnelEnabled'); + if (cb) cb.checked = false; + if (!keepOpen) this.closeAppSettings(); + }, + + /** The one SECURITY WARNING prompt for an unauthenticated public tunnel. */ + _askUnauthTunnel() { + return confirm( + '⚠️ SECURITY WARNING — no password set\n\n' + + 'Enabling the Cloudflare tunnel will publish THIS machine to a public URL with ' + + 'NO login. Anyone who gets the URL has full terminal control — effectively remote ' + + 'code execution on your computer.\n\n' + + 'Strongly recommended: set CODEMAN_PASSWORD instead.\n\n' + + 'Enable the unauthenticated public tunnel anyway?' + ); + }, + + /** + * #706: ask BEFORE a tunnel enable whenever the server has no login (no CODEMAN_PASSWORD, + * not multi-user), every time, whatever the server would accept. With + * CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL set the PUT is never refused, so the refusal + * handler below would never ask. The ONE gate for the welcome link, the tunnel panel and + * the App Settings save. A tunnel that is already running is not being turned on, and + * the server does not guard it either. When the status cannot be read, the PUT goes out + * without the acknowledgement and `_handleTunnelEnableRefusal` stays the fallback. + * @returns {Promise<{ proceed: boolean, acknowledge: boolean }>} proceed=false: cancelled, + * send nothing; acknowledge: send `acknowledgeUnauthTunnel: true` with the enable. + */ + async _confirmTunnelEnable() { + let status = null; + try { + const res = await fetch('/api/tunnel/status'); + if (res.ok) { + const env = await res.json(); + status = env?.success === true ? env.data : env; + } + } catch { + /* unknown: the 403 handler asks instead */ + } + if (!status || status.running || status.loginConfigured !== false) return { proceed: true, acknowledge: false }; + if (!this._askUnauthTunnel()) { + this.showToast('Tunnel not enabled', 'info'); + return { proceed: false, acknowledge: false }; + } + return { proceed: true, acknowledge: true }; + }, + /** * COD-55: detect the server's refusal to start an unauthenticated public tunnel. * The PUT /api/settings route returns a 4xx with { success:false, error } when no - * CODEMAN_PASSWORD is set and the unauthenticated-network opt-in is not acknowledged. - * Shows the server's (actionable) message as an error toast. + * CODEMAN_PASSWORD is set and the unauthenticated-tunnel opt-in is not acknowledged. + * Shows the server's (actionable) message as an error toast. Since #706 this is the + * fallback for when `_confirmTunnelEnable` could not read the tunnel status. * @param {Response|null} res - the fetch Response from the settings PUT * @returns {Promise} true if the tunnel-enable was refused (caller should abort) */ @@ -2612,14 +2674,7 @@ Object.assign(CodemanApp.prototype, { // 403 = the no-password safety refusal (COD-55). Warn loudly and let the // operator acknowledge the risk; on confirm, retry with explicit acknowledgment. if (res.status === 403) { - const confirmed = confirm( - '⚠️ SECURITY WARNING — no password set\n\n' + - 'Enabling the Cloudflare tunnel will publish THIS machine to a public URL with ' + - 'NO login. Anyone who gets the URL has full terminal control — effectively remote ' + - 'code execution on your computer.\n\n' + - 'Strongly recommended: set CODEMAN_PASSWORD instead.\n\n' + - 'Enable the unauthenticated public tunnel anyway?' - ); + const confirmed = this._askUnauthTunnel(); if (!confirmed) { this._dismissTunnelConnecting?.(); this.showToast('Tunnel not enabled', 'info'); @@ -2664,10 +2719,15 @@ Object.assign(CodemanApp.prototype, { return; } try { + const gate = enable ? await this._confirmTunnelEnable() : { proceed: true, acknowledge: false }; + if (!gate.proceed) { + this.closeTunnelPanel(); + return; + } const res = await fetch('/api/settings', { method: 'PUT', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ tunnelEnabled: enable }), + body: JSON.stringify({ tunnelEnabled: enable, ...(gate.acknowledge ? { acknowledgeUnauthTunnel: true } : {}) }), }); // COD-55: server refuses an unauthenticated public tunnel (403). Surface it. if (enable && (await this._handleTunnelEnableRefusal(res))) { @@ -3270,11 +3330,21 @@ Object.assign(CodemanApp.prototype, { let webhookError = ''; let serverSaved = false; try { + // #706: a save that turns the tunnel on asks first when the server has no login + // (one already running is not asked about, see _confirmTunnelEnable). + const tunnelGate = settings.tunnelEnabled && !settingsReadOnly + ? await this._confirmTunnelEnable() + : { proceed: true, acknowledge: false }; + if (!tunnelGate.proceed) { + this._abandonTunnelEnable(settings, keepOpen); + return; + } // A non-admin's save is not sent at all (see settingsReadOnly above): the server // would refuse it with a 403, the plan-usage collection flip included. const res = settingsReadOnly ? null : await this._apiPut('/api/settings', { ...serverSettings, ...(_chipFlip !== undefined ? { showPlanUsageLimits: _chipFlip } : {}), + ...(tunnelGate.acknowledge ? { acknowledgeUnauthTunnel: true } : {}), notificationPreferences: notifPrefsToSave, voiceSettings, }); @@ -3292,14 +3362,9 @@ Object.assign(CodemanApp.prototype, { // COD-55: the server refuses an unauthenticated public tunnel with a 403 — which // rejects the WHOLE settings PUT. Surface the message and revert the tunnel toggle - // (in the UI + localStorage) so it doesn't look enabled. Other settings persisted - // to localStorage above still apply locally. + // so it doesn't look enabled. if (!refusedAsNonAdmin && settings.tunnelEnabled && (await this._handleTunnelEnableRefusal(res))) { - settings.tunnelEnabled = false; - this.saveAppSettingsToStorage(settings); - const cb = document.getElementById('appSettingsTunnelEnabled'); - if (cb) cb.checked = false; - if (!keepOpen) this.closeAppSettings(); + this._abandonTunnelEnable(settings, keepOpen); return; } diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index 1ebbef006..c72453635 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -15,7 +15,7 @@ import { randomBytes } from 'node:crypto'; import { dataPath } from '../../config/instance.js'; import { readHostSensors } from '../../host-sensors.js'; import { ApiErrorCode, createErrorResponse, getErrorMessage, type NiceConfig } from '../../types.js'; -import { isUnauthenticatedNetworkAcknowledged } from '../network-auth-policy.js'; +import { isExplicitlyEnabled, isUnauthenticatedTunnelAcknowledged } from '../network-auth-policy.js'; import { isMultiUserMode } from '../../config/multiuser.js'; import { findUser, canUsernameRunPrivilegedCommands } from '../../user-store.js'; import { getAuthUser, isAdmin, requireAdmin, canAccessOwned, revokeUserSessions } from '../route-helpers.js'; @@ -177,6 +177,43 @@ export function settingsForNonAdmin(settings: unknown): unknown { return out; } +/** + * True when a login protects the server: CODEMAN_PASSWORD is set, or multi-user mode is + * on (every person has their own credential). The tunnel guard and the `loginConfigured` + * field of GET /api/tunnel/status both read this, so the page's answer mirrors the guard. + */ +function isLoginConfigured(): boolean { + return !!process.env.CODEMAN_PASSWORD || isMultiUserMode(); +} + +/** + * The refusal of a tunnel enable with no login and no tunnel opt-in or acknowledgement: logs + * the diagnostic line and returns the message for the 403. `late` is the case where the tunnel + * was running when the request arrived and had stopped by the time it would be started; the + * request's other settings were saved by then. + */ +function unauthenticatedTunnelRefusal(late: boolean): string { + // Someone who set the network variable to get here before #706 is told why it stopped working. + const networkAcknowledged = isExplicitlyEnabled(process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK); + console.warn( + `[tunnel] Refused an unauthenticated tunnel ${late ? 'restart after a concurrent stop' : 'enable'} (no login, no tunnel opt-in, no acknowledgement${networkAcknowledged ? '; CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK does not cover the tunnel' : ''})` + ); + return ( + (late + ? 'The Cloudflare tunnel stopped while these settings were being saved and was not restarted; ' + + 'the other settings were saved. ' + : '') + + 'Refusing to start the Cloudflare tunnel without authentication: it would publish ' + + 'full terminal control to a public URL with no password. Set CODEMAN_PASSWORD to ' + + 'require login, set CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1, or resend with ' + + 'acknowledgeUnauthTunnel:true to acknowledge an unauthenticated public tunnel.' + + (networkAcknowledged + ? ' CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK is set, but it only acknowledges the network ' + + 'bind and no longer covers the tunnel.' + : '') + ); +} + export function registerSystemRoutes( app: FastifyInstance, ctx: SessionPort & EventPort & ConfigPort & InfraPort & AuthPort & TabLayoutPort & KeepWarmPort @@ -194,7 +231,14 @@ export function registerSystemRoutes( // ========== Tunnel ========== - app.get('/api/tunnel/status', async () => ctx.tunnelManager.getStatus()); + // `loginConfigured` (#706) tells the page whether a tunnel would have a login in front + // of it, so it can ask before an enable even when the server would accept one. It is + // the guard's own login test (isLoginConfigured above), never TunnelManager's, which + // knows nothing about auth. + app.get('/api/tunnel/status', async () => ({ + ...ctx.tunnelManager.getStatus(), + loginConfigured: isLoginConfigured(), + })); app.get('/api/tunnel/info', async (req) => { const status = ctx.tunnelManager.getStatus(); @@ -1063,36 +1107,29 @@ export function registerSystemRoutes( // tunnel binds to loopback, server.ts's non-loopback bind guard never trips, and // with no CODEMAN_PASSWORD the auth middleware is inactive — so the tunnel URL is // unauthenticated. Refuse to start a tunnel unless auth is configured OR exposure - // is acknowledged: either the CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK env var, or an + // is acknowledged: either the CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL opt-in, or an // explicit per-request `acknowledgeUnauthTunnel:true` (the UI sends this after a // confirm dialog). This keeps curl/API/CLI callers protected by default while // letting an operator opt in from the browser without setting the env var. // Guard runs BEFORE persisting so a refused tunnelEnabled:true is not saved. - if (settings.tunnelEnabled === true && !ctx.tunnelManager.isRunning()) { - // Multi-user mode makes the tunnel authenticated (every person has their own - // credential), so it satisfies the same requirement as CODEMAN_PASSWORD. - const acknowledged = - isMultiUserMode() || isUnauthenticatedNetworkAcknowledged() || settings.acknowledgeUnauthTunnel === true; - if (!acknowledged) { - const msg = - 'Refusing to start the Cloudflare tunnel without authentication: it would publish ' + - 'full terminal control to a public URL with no password. Set CODEMAN_PASSWORD to ' + - 'require login, set CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1, or resend with ' + - 'acknowledgeUnauthTunnel:true to acknowledge an unauthenticated public tunnel.'; - throw Object.assign(new Error(msg), { - statusCode: 403, - body: createErrorResponse(ApiErrorCode.OPERATION_FAILED, msg), - }); - } - // Loud warning whenever a public tunnel is started with no password — whether - // acknowledged via env var or the per-request UI confirmation. - if (!process.env.CODEMAN_PASSWORD) { - console.warn( - '⚠️ [tunnel] Starting an UNAUTHENTICATED public Cloudflare tunnel — no CODEMAN_PASSWORD set. ' + - 'Anyone with the tunnel URL gets full terminal control (effectively RCE). ' + - 'Set CODEMAN_PASSWORD to require login.' - ); - } + // The tunnel opt-in is CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL / --allow-unauthenticated-tunnel, + // never the network one (#706): install.sh writes CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK + // for every passwordless LAN install, and that acknowledges the LAN bind only. + // Decided ONCE per request, whether or not the tunnel is running now, and required again + // where the tunnel is really started below: the settings file awaits in between leave room + // for a concurrent stop, and a save that found the tunnel running (so nothing was refused + // here) must not then restart it unguarded. + const tunnelEnableAuthorized = + settings.tunnelEnabled === true && + (isLoginConfigured() || + isUnauthenticatedTunnelAcknowledged(ctx.allowUnauthenticatedTunnel) || + settings.acknowledgeUnauthTunnel === true); + if (settings.tunnelEnabled === true && !tunnelEnableAuthorized && !ctx.tunnelManager.isRunning()) { + const msg = unauthenticatedTunnelRefusal(false); + throw Object.assign(new Error(msg), { + statusCode: 403, + body: createErrorResponse(ApiErrorCode.OPERATION_FAILED, msg), + }); } try { @@ -1109,6 +1146,10 @@ export function registerSystemRoutes( // acknowledgeUnauthTunnel is an ACTION field (not a stored setting) — strip // it before persisting so settings.json stays clean. const { acknowledgeUnauthTunnel, ...settingsToStore } = settings; + // An enable that is not authorized only got this far because the tunnel was running. + // It must never write tunnelEnabled:true: a concurrent stop has written false by now, + // and true on disk would show a stopped tunnel as on. The stored value stays as it is. + if (settings.tunnelEnabled === true && !tunnelEnableAuthorized) delete settingsToStore.tunnelEnabled; const merged = { ...existing, ...settingsToStore }; await fs.writeFile(SETTINGS_PATH, JSON.stringify(merged, null, 2)); @@ -1182,6 +1223,20 @@ export function registerSystemRoutes( if ('tunnelEnabled' in settings) { const tunnelEnabled = settings.tunnelEnabled as boolean; if (tunnelEnabled && !ctx.tunnelManager.isRunning()) { + // The tunnel stopped since the guard above saw it running: the same authorization applies. + if (!tunnelEnableAuthorized) { + reply.code(403); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, unauthenticatedTunnelRefusal(true)); + } + // Loud warning whenever a public tunnel is started with no password, whether + // acknowledged via the tunnel opt-in or the per-request UI confirmation. + if (!process.env.CODEMAN_PASSWORD) { + console.warn( + '⚠️ [tunnel] Starting an UNAUTHENTICATED public Cloudflare tunnel — no CODEMAN_PASSWORD set. ' + + 'Anyone with the tunnel URL gets full terminal control (effectively RCE). ' + + 'Set CODEMAN_PASSWORD to require login.' + ); + } ctx.tunnelManager.start(ctx.port, ctx.https); console.log('Tunnel started via settings change'); } else if (tunnelEnabled && ctx.tunnelManager.isRunning() && ctx.tunnelManager.getUrl()) { diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 8336a2e3a..7276d8975 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -1554,7 +1554,7 @@ export const SettingsUpdateSchema = z // Action field (NOT persisted): explicit per-request acknowledgment that the // operator accepts exposing an UNAUTHENTICATED public tunnel (no CODEMAN_PASSWORD). // Lets the UI enable a tunnel after a confirm dialog without the - // CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK env var. Stripped before persisting. + // CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL opt-in. Stripped before persisting. acknowledgeUnauthTunnel: z.boolean().optional(), tabTwoRows: z.boolean().optional(), /** diff --git a/src/web/server.ts b/src/web/server.ts index 2b3e582c4..14e02dc35 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -472,6 +472,8 @@ export class WebServer extends EventEmitter { private windowTitle: string; private readonly indexHtmlTemplate: string; private readonly allowUnauthenticatedNetwork: boolean; + /** `--allow-unauthenticated-tunnel`; the tunnel guard reads it through the route context (#706). */ + private readonly allowUnauthenticatedTunnel: boolean; private _pasteImageGcStop: (() => void) | null = null; private _eventLoopMonitor: EventLoopMonitorHandle | null = null; /** Opt-in hooks-only listener on the docker bridge gateway (CODEMAN_DOCKER_BRIDGE_HOOKS). */ @@ -489,7 +491,8 @@ export class WebServer extends EventEmitter { host: string = '127.0.0.1', titleHostname?: string, allowUnauthenticatedNetwork: boolean = false, - basePath: string = '' + basePath: string = '', + allowUnauthenticatedTunnel: boolean = false ) { super(); this.setMaxListeners(0); @@ -501,6 +504,7 @@ export class WebServer extends EventEmitter { this.testMode = testMode; this.allowUnauthenticatedNetwork = allowUnauthenticatedNetwork || isExplicitlyEnabled(process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK); + this.allowUnauthenticatedTunnel = allowUnauthenticatedTunnel; this.titleHostname = titleHostname || getHostname(); this.windowTitle = `codeman:${this.titleHostname}`; this.indexHtmlTemplate = readFileSync(join(__dirname, 'public', 'index.html'), 'utf-8'); @@ -880,6 +884,7 @@ export class WebServer extends EventEmitter { scheduledRuns: this.scheduledRuns, teamWatcher: this.teamWatcher, tunnelManager: this.tunnelManager, + allowUnauthenticatedTunnel: this.allowUnauthenticatedTunnel, pushStore: this.pushStore, startScheduledRun: this.startScheduledRun.bind(this), stopScheduledRun: this.stopScheduledRun.bind(this), @@ -4399,9 +4404,19 @@ export async function startWebServer( host: string = '127.0.0.1', titleHostname?: string, allowUnauthenticatedNetwork: boolean = false, - basePath: string = '' + basePath: string = '', + allowUnauthenticatedTunnel: boolean = false ): Promise { - const server = new WebServer(port, https, testMode, host, titleHostname, allowUnauthenticatedNetwork, basePath); + const server = new WebServer( + port, + https, + testMode, + host, + titleHostname, + allowUnauthenticatedNetwork, + basePath, + allowUnauthenticatedTunnel + ); await server.start(); return server; } diff --git a/test/cli-commands.test.ts b/test/cli-commands.test.ts index 9714313e7..689907851 100644 --- a/test/cli-commands.test.ts +++ b/test/cli-commands.test.ts @@ -208,3 +208,29 @@ describe('help text', () => { } }); }); + +describe('unauthenticated tunnel launch option', () => { + it.each(['web', 'service install'])('registers --allow-unauthenticated-tunnel on %s', (path) => { + const command = path === 'web' ? find(program, 'web')! : find(find(program, 'service')!, 'install')!; + const options = command.options.filter((option) => option.long === '--allow-unauthenticated-tunnel'); + expect( + options.map((option) => ({ + flags: option.flags, + attribute: option.attributeName(), + required: option.required, + optional: option.optional, + })) + ).toEqual([ + { + flags: '--allow-unauthenticated-tunnel', + attribute: 'allowUnauthenticatedTunnel', + required: false, + optional: false, + }, + ]); + }); + + it('documents --allow-unauthenticated-tunnel in web help', () => { + expect(find(program, 'web')!.helpInformation()).toContain('--allow-unauthenticated-tunnel'); + }); +}); diff --git a/test/cli-tunnel-flag.test.ts b/test/cli-tunnel-flag.test.ts new file mode 100644 index 000000000..575be2ca5 --- /dev/null +++ b/test/cli-tunnel-flag.test.ts @@ -0,0 +1,102 @@ +/** + * @fileoverview Issue #706 tunnel flag plumbing through the real command parser into daemon, foreground and service launches. Launch boundaries are mocked, so the CLI's valid nonzero port never binds a socket or creates a service unit. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const launches = vi.hoisted(() => ({ + startDaemon: vi.fn(), + startWebServer: vi.fn(), + installService: vi.fn(), + stop: vi.fn(), +})); + +vi.mock('../src/daemon-control.js', async (importOriginal) => ({ + ...(await importOriginal()), + startDaemon: launches.startDaemon, +})); +vi.mock('../src/service-installer.js', async (importOriginal) => ({ + ...(await importOriginal()), + installService: launches.installService, +})); +vi.mock('../src/web/server.js', () => ({ startWebServer: launches.startWebServer })); + +const SIGNALS = ['SIGTERM', 'SIGINT', 'SIGHUP'] as const; +let originalListeners: Map>; +let exitSpy: ReturnType>; + +beforeEach(() => { + // Commander retains parsed option values; use a fresh real program for each invocation. + vi.resetModules(); + vi.clearAllMocks(); + originalListeners = new Map(SIGNALS.map((signal) => [signal, process.listeners(signal)])); + exitSpy = vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`Unexpected process.exit(${code})`); + }); + launches.startDaemon.mockResolvedValue({ ok: true, url: 'http://127.0.0.1:8080', pid: 123, logPath: 'web.log' }); + launches.stop.mockResolvedValue(undefined); + launches.startWebServer.mockResolvedValue({ stop: launches.stop }); + launches.installService.mockResolvedValue({ ok: true, message: 'Installed', unitPath: 'codeman-web.service' }); +}); + +afterEach(() => { + for (const signal of SIGNALS) { + for (const listener of process.listeners(signal)) { + if (!originalListeners.get(signal)!.includes(listener)) process.removeListener(signal, listener); + } + } + vi.restoreAllMocks(); +}); + +const expectedOptions = (allowUnauthenticatedTunnel: boolean) => ({ + host: '127.0.0.1', + port: 8080, + https: false, + basePath: '', + titleHostname: undefined, + allowUnauthenticatedNetwork: false, + allowUnauthenticatedTunnel, + multiuser: false, +}); + +async function parse(args: string[], enabled: boolean) { + const { program } = await import('../src/cli.js'); + await program.parseAsync([ + 'node', + 'codeman', + ...args, + '--port', + '8080', + '--host', + '127.0.0.1', + '--base-url', + '/', + ...(enabled ? ['--allow-unauthenticated-tunnel'] : []), + ]); + expect(exitSpy).not.toHaveBeenCalled(); +} + +describe('tunnel flag CLI plumbing', () => { + it.each([true, false])('web -d passes allowUnauthenticatedTunnel=%s to startDaemon', async (enabled) => { + await parse(['web', '-d'], enabled); + expect(launches.startDaemon.mock.calls).toEqual([[expectedOptions(enabled)]]); + expect(launches.startWebServer.mock.calls).toEqual([]); + expect(launches.installService.mock.calls).toEqual([]); + }); + + it.each([true, false])('foreground web passes allowUnauthenticatedTunnel=%s as argument eight', async (enabled) => { + await parse(['web'], enabled); + expect(launches.startWebServer.mock.calls).toEqual([ + [8080, false, false, '127.0.0.1', undefined, false, '', enabled], + ]); + expect(launches.startDaemon.mock.calls).toEqual([]); + expect(launches.installService.mock.calls).toEqual([]); + expect(launches.stop.mock.calls).toEqual([]); + }); + + it.each([true, false])('service install passes allowUnauthenticatedTunnel=%s to installService', async (enabled) => { + await parse(['service', 'install'], enabled); + expect(launches.installService.mock.calls).toEqual([[expectedOptions(enabled)]]); + expect(launches.startDaemon.mock.calls).toEqual([]); + expect(launches.startWebServer.mock.calls).toEqual([]); + }); +}); diff --git a/test/daemon-control.test.ts b/test/daemon-control.test.ts index 54c19ff9a..3aefd40c4 100644 --- a/test/daemon-control.test.ts +++ b/test/daemon-control.test.ts @@ -30,6 +30,36 @@ async function closedPort(): Promise { } describe('buildWebArgs', () => { + it('forwards the tunnel opt-in between the network and multi-user flags', () => { + const options = { + host: '0.0.0.0', + port: 8080, + https: true, + titleHostname: 'tower', + allowUnauthenticatedNetwork: true, + allowUnauthenticatedTunnel: true, + multiuser: true, + }; + expect(buildWebArgs(options)).toEqual([ + 'web', + '--host', + '0.0.0.0', + '--port', + '8080', + '--https', + '--title-hostname', + 'tower', + '--allow-unauthenticated-network', + '--allow-unauthenticated-tunnel', + '--multiuser', + ]); + }); + + it.each([false, undefined])('omits the tunnel opt-in when it is %s', (allowUnauthenticatedTunnel) => { + const options = { host: '127.0.0.1', port: 8080, https: false, allowUnauthenticatedTunnel }; + expect(buildWebArgs(options)).toEqual(['web', '--host', '127.0.0.1', '--port', '8080']); + }); + it('always passes host and port through explicitly', () => { expect(buildWebArgs({ host: '127.0.0.1', port: 3000, https: false })).toEqual([ 'web', diff --git a/test/install-sh-invariants.test.ts b/test/install-sh-invariants.test.ts index 2d1faebfc..31eee9270 100644 --- a/test/install-sh-invariants.test.ts +++ b/test/install-sh-invariants.test.ts @@ -531,3 +531,7 @@ describe('install.sh review fixes for #460', () => { expect(drive('--tailscale')).toContain('host=127.0.0.1 pw=s3cret ack=0'); }); }); + +it('install.sh never contains the unauthenticated tunnel opt-in', () => { + expect(SOURCE).not.toContain('CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL'); +}); diff --git a/test/mocks/mock-route-context.ts b/test/mocks/mock-route-context.ts index f74dddf28..9f75130ae 100644 --- a/test/mocks/mock-route-context.ts +++ b/test/mocks/mock-route-context.ts @@ -169,6 +169,7 @@ export function createMockRouteContext(options?: { scheduledRuns: new Map(), teamWatcher: { getTeams: vi.fn(() => []), getTeamTasks: vi.fn(() => []), hasActiveTeammates: vi.fn(() => false) }, tunnelManager: null, + allowUnauthenticatedTunnel: false, pushStore: null, startScheduledRun: vi.fn(), stopScheduledRun: vi.fn(), diff --git a/test/routes/system-routes-tunnel-guard.test.ts b/test/routes/system-routes-tunnel-guard.test.ts index 8da2b6740..42604388b 100644 --- a/test/routes/system-routes-tunnel-guard.test.ts +++ b/test/routes/system-routes-tunnel-guard.test.ts @@ -1,20 +1,11 @@ /** - * @fileoverview COD-55 — tunnel password guard. - * - * Enabling the Cloudflare tunnel publishes the whole app (full terminal control = - * effectively RCE) to a public *.trycloudflare.com URL. When no CODEMAN_PASSWORD - * is set, requests through that URL are unauthenticated. These tests assert the - * PUT /api/settings tunnel-enable path REFUSES to start the tunnel unless a - * password is set OR the unauthenticated-network opt-in is acknowledged. - * - * Uses app.inject() — no real HTTP ports needed. Port: N/A. + * @fileoverview Issue #706 keeps LAN acknowledgement separate from public tunnel consent. A login, tunnel-specific env opt-in, launch flag or per-request acknowledgement permits enabling. Early refusals persist nothing; late race refusals save unrelated settings without enabling the tunnel. Status reports login configuration independently of exposure opt-ins. */ - import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js'; import { registerSystemRoutes } from '../../src/web/routes/system-routes.js'; -// Settings are written to disk via fs/promises — stub so the guard test never +// Settings are written to disk via fs/promises; stub so the guard test never // touches the real settings.json, and so we can assert "not persisted on refusal". vi.mock('node:fs/promises', () => ({ default: { @@ -34,129 +25,296 @@ vi.mock('node:fs', async (importOriginal) => { }); import fs from 'node:fs/promises'; +import { SETTINGS_PATH } from '../../src/web/route-helpers.js'; const mockedWriteFile = vi.mocked(fs.writeFile); -/** Build a tunnelManager stub the route's ctx can use. */ -function makeTunnelManager(running = false) { +const STATUS = { running: false, url: null }; + +function makeTunnelManager() { return { start: vi.fn(), stop: vi.fn(), - isRunning: vi.fn(() => running), + isRunning: vi.fn(() => false), getUrl: vi.fn(() => null), - getStatus: vi.fn(() => ({ running })), + getStatus: vi.fn(() => ({ ...STATUS })), }; } -describe('COD-55 tunnel password guard (PUT /api/settings tunnelEnabled)', () => { +describe('tunnel authentication policy', () => { let harness: RouteTestHarness; let tunnel: ReturnType; - const savedPassword = process.env.CODEMAN_PASSWORD; - const savedOptIn = process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK; + const envKeys = [ + 'CODEMAN_PASSWORD', + 'CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK', + 'CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL', + 'CODEMAN_MULTIUSER', + ] as const; + let savedEnv: (string | undefined)[]; beforeEach(async () => { - harness = await createRouteTestHarness(registerSystemRoutes); + savedEnv = envKeys.map((key) => process.env[key]); + for (const key of envKeys) delete process.env[key]; + harness = await createRouteTestHarness(registerSystemRoutes, { + authUser: { username: 'root', role: 'admin' }, + }); vi.clearAllMocks(); + vi.mocked(fs.readFile).mockResolvedValue('{}'); mockedWriteFile.mockResolvedValue(undefined); - tunnel = makeTunnelManager(false); - // tunnelManager is null in the default mock ctx — inject our spy. + tunnel = makeTunnelManager(); (harness.ctx as unknown as { tunnelManager: unknown }).tunnelManager = tunnel; - delete process.env.CODEMAN_PASSWORD; - delete process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK; + (harness.ctx as unknown as { allowUnauthenticatedTunnel: boolean }).allowUnauthenticatedTunnel = false; }); afterEach(async () => { - await harness.app.close(); - if (savedPassword === undefined) delete process.env.CODEMAN_PASSWORD; - else process.env.CODEMAN_PASSWORD = savedPassword; - if (savedOptIn === undefined) delete process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK; - else process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK = savedOptIn; + try { + await harness?.app.close(); + } finally { + envKeys.forEach((key, i) => { + if (savedEnv[i] === undefined) delete process.env[key]; + else process.env[key] = savedEnv[i]; + }); + } }); - it('REFUSES tunnel-enable with no password and no opt-in (4xx, start not called)', async () => { - const res = await harness.app.inject({ - method: 'PUT', - url: '/api/settings', - payload: { tunnelEnabled: true }, + const put = (payload: Record = { tunnelEnabled: true }) => + harness.app.inject({ method: 'PUT', url: '/api/settings', payload }); + + function expectRefused(res: Awaited>) { + expect.soft(res.statusCode).toBe(403); + expect.soft(res.json()).toEqual({ + success: false, + errorCode: 'OPERATION_FAILED', + error: expect.any(String), }); + expect.soft(tunnel.start.mock.calls).toEqual([]); + expect.soft(mockedWriteFile.mock.calls).toEqual([]); + } + + function expectEnabled(res: Awaited>) { + expect(res.statusCode).toBe(200); + expect(tunnel.start.mock.calls).toEqual([[harness.ctx.port, harness.ctx.https]]); + expect(tunnel.stop.mock.calls).toEqual([]); + expect(mockedWriteFile.mock.calls).toEqual([[SETTINGS_PATH, JSON.stringify({ tunnelEnabled: true }, null, 2)]]); + } - expect(res.statusCode).toBeGreaterThanOrEqual(400); - expect(res.statusCode).toBeLessThan(500); - const body = JSON.parse(res.body); - expect(body.success).toBe(false); - // Message should tell the user how to fix it. - expect(body.error).toMatch(/CODEMAN_PASSWORD|CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK/); - // The tunnel must NOT have been started. - expect(tunnel.start).not.toHaveBeenCalled(); - // And tunnelEnabled:true must NOT have been persisted. - expect(mockedWriteFile).not.toHaveBeenCalled(); + it('refuses the incident: network acknowledgement does not authorize a public tunnel', async () => { + process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK = '1'; + const res = await put(); + expectRefused(res); + expect.soft(res.json().error).toContain('CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL'); + expect.soft(res.json().error).toContain('CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK'); + expect.soft(res.json().error).toMatch(/no longer/i); }); - it('ALLOWS tunnel-enable when CODEMAN_PASSWORD is set (start called, 200)', async () => { - process.env.CODEMAN_PASSWORD = 'hunter2'; + it('refuses without credentials or acknowledgement and names the tunnel opt-in', async () => { + const res = await put(); + expectRefused(res); + expect(res.json().error).toContain('CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL'); + }); - const res = await harness.app.inject({ - method: 'PUT', - url: '/api/settings', - payload: { tunnelEnabled: true }, - }); + it.each(['1', 'true', 'yes', 'on', ' TRUE '])( + 'allows tunnel env value %j without an acknowledgement', + async (value) => { + process.env.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL = value; + expectEnabled(await put()); + } + ); - expect(res.statusCode).toBe(200); - expect(tunnel.start).toHaveBeenCalledTimes(1); + it('allows the route context flag without an env opt-in or acknowledgement', async () => { + (harness.ctx as unknown as { allowUnauthenticatedTunnel: boolean }).allowUnauthenticatedTunnel = true; + expectEnabled(await put()); }); - it('ALLOWS tunnel-enable with the unauthenticated-network opt-in acknowledged (start called, 200)', async () => { - process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK = '1'; + it('allows acknowledgement alone and persists only tunnelEnabled', async () => { + expectEnabled(await put({ tunnelEnabled: true, acknowledgeUnauthTunnel: true })); + }); - const res = await harness.app.inject({ - method: 'PUT', - url: '/api/settings', - payload: { tunnelEnabled: true }, - }); + it('refuses a false acknowledgement', async () => { + expectRefused(await put({ tunnelEnabled: true, acknowledgeUnauthTunnel: false })); + }); - expect(res.statusCode).toBe(200); - expect(tunnel.start).toHaveBeenCalledTimes(1); + it('allows a configured password without acknowledgement', async () => { + process.env.CODEMAN_PASSWORD = 'test-password'; + expectEnabled(await put()); }); - it('ALLOWS tunnel-enable with per-request acknowledgeUnauthTunnel:true (start called, 200, flag not persisted)', async () => { - const res = await harness.app.inject({ - method: 'PUT', - url: '/api/settings', - payload: { tunnelEnabled: true, acknowledgeUnauthTunnel: true }, - }); + it('allows a multi-user admin without acknowledgement', async () => { + process.env.CODEMAN_MULTIUSER = '1'; + expectEnabled(await put()); + }); - expect(res.statusCode).toBe(200); - expect(tunnel.start).toHaveBeenCalledTimes(1); - // The action flag must NOT be persisted to settings.json. - expect(mockedWriteFile).toHaveBeenCalled(); - const persisted = JSON.parse(mockedWriteFile.mock.calls[0][1] as string); - expect(persisted.acknowledgeUnauthTunnel).toBeUndefined(); - expect(persisted.tunnelEnabled).toBe(true); + it.each(['0', 'false', '', 'enabled'])('refuses non-true tunnel env value %j', async (value) => { + process.env.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL = value; + expectRefused(await put()); }); - it('still REFUSES when acknowledgeUnauthTunnel is false (4xx, start not called)', async () => { - const res = await harness.app.inject({ - method: 'PUT', - url: '/api/settings', - payload: { tunnelEnabled: true, acknowledgeUnauthTunnel: false }, - }); + it('does not guard disabling a running tunnel', async () => { + tunnel.isRunning.mockReturnValue(true); + const res = await put({ tunnelEnabled: false }); + expect(res.statusCode).toBe(200); + expect(tunnel.start.mock.calls).toEqual([]); + expect(tunnel.stop.mock.calls).toEqual([[]]); + expect(mockedWriteFile.mock.calls).toEqual([[SETTINGS_PATH, JSON.stringify({ tunnelEnabled: false }, null, 2)]]); + }); - expect(res.statusCode).toBeGreaterThanOrEqual(400); - expect(res.statusCode).toBeLessThan(500); - expect(tunnel.start).not.toHaveBeenCalled(); - expect(mockedWriteFile).not.toHaveBeenCalled(); + it.each([ + ['password', 'CODEMAN_PASSWORD', 'test-password', false, true], + ['multi-user', 'CODEMAN_MULTIUSER', '1', false, true], + ['neither', undefined, undefined, false, false], + ['empty password', 'CODEMAN_PASSWORD', '', false, false], + ['network opt-in only', 'CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK', '1', false, false], + ['tunnel opt-in only', 'CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL', '1', false, false], + ['tunnel flag only', undefined, undefined, true, false], + ] as const)('status reports loginConfigured for %s', async (_label, key, value, flag, loginConfigured) => { + if (key !== undefined) process.env[key] = value; + (harness.ctx as unknown as { allowUnauthenticatedTunnel: boolean }).allowUnauthenticatedTunnel = flag; + const res = await harness.app.inject({ method: 'GET', url: '/api/tunnel/status' }); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ ...STATUS, loginConfigured }); + expect(tunnel.getStatus.mock.calls).toEqual([[]]); + expect(mockedWriteFile.mock.calls).toEqual([]); }); +}); + +describe('tunnel enable authorization across a concurrent stop', () => { + let harness: RouteTestHarness; + let tunnel: ReturnType; + let running: boolean; + let stopDuringWrite: boolean; + let afterWrite: (() => void) | undefined; + let storedSettings: Record; + const envKeys = [ + 'CODEMAN_PASSWORD', + 'CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK', + 'CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL', + 'CODEMAN_MULTIUSER', + ] as const; + let savedEnv: (string | undefined)[]; - it('does not guard tunnel-disable (tunnelEnabled:false always allowed)', async () => { - tunnel = makeTunnelManager(true); + beforeEach(async () => { + savedEnv = envKeys.map((key) => process.env[key]); + for (const key of envKeys) delete process.env[key]; + harness = await createRouteTestHarness(registerSystemRoutes, { + authUser: { username: 'root', role: 'admin' }, + }); + vi.clearAllMocks(); + running = true; + stopDuringWrite = true; + afterWrite = undefined; + storedSettings = { tunnelEnabled: false, subagentTrackingEnabled: false }; + tunnel = makeTunnelManager(); + tunnel.isRunning.mockImplementation(() => running); (harness.ctx as unknown as { tunnelManager: unknown }).tunnelManager = tunnel; + (harness.ctx as unknown as { allowUnauthenticatedTunnel: boolean }).allowUnauthenticatedTunnel = false; + vi.mocked(fs.readFile).mockImplementation(async (path) => { + expect(path).toBe(SETTINGS_PATH); + return JSON.stringify(storedSettings); + }); + mockedWriteFile.mockImplementation(async (path, contents) => { + expect(path).toBe(SETTINGS_PATH); + storedSettings = JSON.parse(contents as string); + // The stop becomes visible during the awaited write, regardless of isRunning() call count. + if (stopDuringWrite) running = false; + afterWrite?.(); + }); + }); - const res = await harness.app.inject({ + afterEach(async () => { + try { + await harness?.app.close(); + } finally { + envKeys.forEach((key, i) => { + if (savedEnv[i] === undefined) delete process.env[key]; + else process.env[key] = savedEnv[i]; + }); + vi.mocked(fs.readFile).mockResolvedValue('{}'); + mockedWriteFile.mockResolvedValue(undefined); + } + }); + + const put = (acknowledgement: Record = {}) => + harness.app.inject({ method: 'PUT', url: '/api/settings', - payload: { tunnelEnabled: false }, + payload: { tunnelEnabled: true, autoNameSessions: false, ...acknowledgement }, }); + function expectWritten(tunnelEnabled: boolean) { + const expected = { tunnelEnabled, subagentTrackingEnabled: false, autoNameSessions: false }; + // Check every write so a transient unauthorized enable followed by a rollback also fails. + expect + .soft(mockedWriteFile.mock.calls.map(([path, contents]) => ({ path, settings: JSON.parse(contents as string) }))) + .toEqual([{ path: SETTINGS_PATH, settings: expected }]); + expect.soft(storedSettings).toEqual(expected); + } + + function expectLateRefusal(res: Awaited>) { + expect.soft(res.statusCode).toBe(403); + expect.soft(res.json()).toEqual({ + success: false, + errorCode: 'OPERATION_FAILED', + error: expect.any(String), + }); + expect.soft(res.json().error).toContain('CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL'); + expect.soft(res.json().error).toMatch(/not (re)?start/i); + expect.soft(tunnel.start.mock.calls).toEqual([]); + expect.soft(tunnel.stop.mock.calls).toEqual([]); + expectWritten(false); + } + + function expectAuthorizedRestart(res: Awaited>) { expect(res.statusCode).toBe(200); - expect(tunnel.stop).toHaveBeenCalledTimes(1); + expect(res.json()).toEqual({}); + expect(tunnel.start.mock.calls).toEqual([[harness.ctx.port, harness.ctx.https]]); + expect(tunnel.stop.mock.calls).toEqual([]); + expectWritten(true); + } + + it('refuses an unauthorized restart after a concurrent stop while saving other settings', async () => { + expectLateRefusal(await put()); + }); + + it.each(['password', 'tunnel env', 'route flag', 'per-request acknowledgement', 'multi-user admin'] as const)( + 'allows a restart with %s authorization', + async (authorization) => { + if (authorization === 'password') process.env.CODEMAN_PASSWORD = 'test-password'; + if (authorization === 'tunnel env') process.env.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL = '1'; + if (authorization === 'route flag') { + (harness.ctx as unknown as { allowUnauthenticatedTunnel: boolean }).allowUnauthenticatedTunnel = true; + } + if (authorization === 'multi-user admin') process.env.CODEMAN_MULTIUSER = '1'; + expectAuthorizedRestart( + await put(authorization === 'per-request acknowledgement' ? { acknowledgeUnauthTunnel: true } : {}) + ); + } + ); + + it.each([false, true])( + 'preserves stored tunnelEnabled=%s when an unauthorized tunnel stays running', + async (tunnelEnabled) => { + stopDuringWrite = false; + storedSettings.tunnelEnabled = tunnelEnabled; + const res = await put(); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({}); + expect(tunnel.start.mock.calls).toEqual([]); + expect(tunnel.stop.mock.calls).toEqual([]); + expectWritten(tunnelEnabled); + } + ); + + it('does not acquire restart authorization from an env opt-in added during the write', async () => { + afterWrite = () => { + process.env.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL = '1'; + }; + expectLateRefusal(await put()); + }); + + it('retains the request authorization when the env opt-in is removed during the write', async () => { + process.env.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL = '1'; + afterWrite = () => { + delete process.env.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL; + }; + expectAuthorizedRestart(await put()); }); }); diff --git a/test/service-installer.test.ts b/test/service-installer.test.ts index 0115a59ff..01cf1ad5a 100644 --- a/test/service-installer.test.ts +++ b/test/service-installer.test.ts @@ -6,6 +6,7 @@ import { describe, it, expect } from 'vitest'; import { + resolveServicePlan, buildLaunchAgentPlist, buildServiceEnv, buildServicePath, @@ -177,3 +178,26 @@ describe('detectServiceKind', () => { expect(detectServiceKind()).toBe(expected); }); }); + +describe('resolveServicePlan tunnel opt-in', () => { + it.each(['systemd', 'launchd'] as const)('carries the tunnel flag into %s args', (kind) => { + const options = { + host: '0.0.0.0', + port: 8080, + https: false, + allowUnauthenticatedNetwork: true, + allowUnauthenticatedTunnel: true, + multiuser: true, + }; + expect(resolveServicePlan(kind, options).args).toEqual([ + 'web', + '--host', + '0.0.0.0', + '--port', + '8080', + '--allow-unauthenticated-network', + '--allow-unauthenticated-tunnel', + '--multiuser', + ]); + }); +}); diff --git a/test/tunnel-enable-confirm-ui.test.ts b/test/tunnel-enable-confirm-ui.test.ts new file mode 100644 index 000000000..c69ae9c55 --- /dev/null +++ b/test/tunnel-enable-confirm-ui.test.ts @@ -0,0 +1,230 @@ +/** + * @fileoverview Issue #706 browser confirmation policy through the real welcome, tunnel panel and App Settings entry points. HTTP and browser prompts are stubbed; settings and acknowledgement logic run unchanged. + */ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { JSDOM } from 'jsdom'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const PUBLIC = join(process.cwd(), 'src/web/public'); +const read = (name: string) => readFileSync(join(PUBLIC, name), 'utf8'); +const INDEX = read('index.html'); +const SCRIPTS = ['constants.js', 'app.js', 'settings-ui.js', 'api-client.js'].map(read); +const STORAGE_KEY = 'codeman-app-settings'; +const NO_LOGIN = { running: false, url: null, loginConfigured: false }; +type Status = { running: boolean; url: string | null; loginConfigured?: boolean }; +type Call = { method: string; url: string; body: Record | undefined }; +type Entry = 'welcome' | 'panel' | 'save'; +const windows: JSDOM[] = []; + +afterEach(() => { + for (const dom of windows.splice(0)) dom.window.close(); +}); + +async function boot( + status: Status = NO_LOGIN, + accept = true, + statusFailure?: 'reject' | 'non-ok', + refuseFirstPut = false +) { + const dom = new JSDOM(INDEX, { url: 'http://localhost/', runScripts: 'outside-only' }); + windows.push(dom); + const win = dom.window; + if (win.document.readyState !== 'complete') await new Promise((resolve) => win.addEventListener('load', resolve)); + // Keep status polling and unrelated rendering timers outside this user action. + win.setTimeout = (() => 0) as unknown as typeof win.setTimeout; + win.setInterval = (() => 0) as unknown as typeof win.setInterval; + win.localStorage.setItem(STORAGE_KEY, JSON.stringify({ tunnelEnabled: false })); + const calls: Call[] = []; + const order: string[] = []; + const confirm = vi.fn(() => { + order.push('confirm'); + return accept; + }); + win.confirm = confirm; + let putCount = 0; + const response = (body: unknown, statusCode = 200) => ({ + ok: statusCode >= 200 && statusCode < 300, + status: statusCode, + json: async () => body, + clone: () => response(body, statusCode), + }); + const fetch = vi.fn(async (url: string, opts: { method?: string; body?: string } = {}) => { + const method = opts.method || 'GET'; + calls.push({ method, url, body: opts.body === undefined ? undefined : JSON.parse(opts.body) }); + order.push(`${method} ${url}`); + if (url === '/api/tunnel/status' && method === 'GET') { + if (statusFailure === 'reject') throw new Error('Status unavailable'); + // A failed HTTP response must not be trusted, even if its JSON resembles a status. + return response(status, statusFailure === 'non-ok' ? 503 : 200); + } + if (url === '/api/settings' && method === 'PUT') { + putCount++; + if (refuseFirstPut && putCount === 1) { + return response( + { success: false, error: 'Tunnel acknowledgement required', errorCode: 'OPERATION_FAILED' }, + 403 + ); + } + return response({ success: true, data: {} }); + } + if (url === '/api/execution/model-config' && method === 'PUT') return response({ success: true, data: {} }); + throw new Error(`Unexpected request: ${method} ${url}`); + }); + Object.assign(win, { fetch }); + win.eval( + [ + `var MobileDetection = { + getDeviceType: () => 'desktop', isHandheldDevice: () => false, + isMobile: () => false, isTouchDevice: () => false, + }; + var KeyboardHandler = {}, SwipeHandler = {}, DeepgramProvider = {}, NotificationManager = function () {}; + var KeyboardAccessoryBar = { setMode() {} }; + var VoiceInput = { _getDeepgramConfig: () => ({}), _saveDeepgramConfig() {}, refreshClaudeStatus: () => Promise.resolve() }; + var FocusTrap = function () { this.activate = () => {}; this.deactivate = () => {}; }; + var DEFAULT_VOICE_KEYTERMS = '';`, + ...SCRIPTS, + 'window.__App = CodemanApp;', + ].join('\n') + ); + // Only peripheral rendering is stubbed. All tunnel logic, settings reads/writes and HTTP helpers run for real. + const app = Object.create((win as unknown as { __App: { prototype: object } }).__App.prototype); + app.sessions = new Map(); + app.sessionOrder = []; + for (const name of [ + 'showToast', + 'closeAppSettings', + '_updateLocalEchoState', + 'applyTerminalFontFamily', + 'applyTerminalFontWeights', + '_syncPushPreferences', + 'applyHeaderVisibilitySettings', + 'applySkin', + 'applyLocalization', + 'applySessionListLayout', + 'applyTabOrientation', + 'applyLineageLineSettings', + '_updateTokensImmediate', + 'applyMonitorVisibility', + 'renderApprovals', + 'renderProjectInsightsPanel', + 'updateSubagentWindowVisibility', + '_updateCjkInputState', + 'showWelcome', + ]) + app[name] = vi.fn(); + const box = win.document.getElementById('appSettingsTunnelEnabled') as HTMLInputElement; + const button = win.document.getElementById('welcomeTunnelBtn') as HTMLButtonElement; + // The welcome screen has finished its status-dependent reveal before the user clicks. + button.style.display = ''; + const puts = () => calls.filter((call) => call.method === 'PUT' && call.url === '/api/settings'); + const events = () => + order.filter( + (event) => event === 'confirm' || event.endsWith('/api/settings') || event.endsWith('/api/tunnel/status') + ); + const stored = () => JSON.parse(win.localStorage.getItem(STORAGE_KEY) || '{}'); + const act = async (entry: Entry, enable = true) => { + if (entry === 'save') box.checked = enable; + if (entry === 'welcome') button.classList.toggle('active', !enable); + vi.clearAllMocks(); + calls.length = 0; + order.length = 0; + if (entry === 'welcome') await app.toggleTunnelFromWelcome(); + if (entry === 'panel') await app._tunnelPanelToggle(enable); + if (entry === 'save') await app.saveAppSettings(); + }; + return { act, puts, events, confirm, box, button, stored }; +} + +function expectEnablePut(calls: Call[], entry: Entry, acknowledge: boolean) { + expect(calls).toHaveLength(1); + const expected = { tunnelEnabled: true, ...(acknowledge ? { acknowledgeUnauthTunnel: true } : {}) }; + if (entry === 'save') { + // The spec leaves the other App Settings fields unchanged. + expect(calls[0].body).toMatchObject(expected); + if (!acknowledge) expect(calls[0].body).not.toHaveProperty('acknowledgeUnauthTunnel'); + } else { + expect(calls).toEqual([{ method: 'PUT', url: '/api/settings', body: expected }]); + } +} + +describe.each(['welcome', 'panel', 'save'] as const)('%s tunnel enable', (entry) => { + it('confirms before a successful PUT and sends the acknowledgement', async () => { + const h = await boot(); + await h.act(entry); + expect.soft(h.confirm).toHaveBeenCalledTimes(1); + expect.soft(h.events()).toEqual(['GET /api/tunnel/status', 'confirm', 'PUT /api/settings']); + expectEnablePut(h.puts(), entry, true); + }); + + it('asks again on every enable attempt', async () => { + const h = await boot(); + for (let attempt = 0; attempt < 2; attempt++) { + await h.act(entry); + expect.soft(h.confirm).toHaveBeenCalledTimes(1); + expect.soft(h.events()).toEqual(['GET /api/tunnel/status', 'confirm', 'PUT /api/settings']); + } + }); + + it('cancels without a PUT and leaves the tunnel off', async () => { + const h = await boot(NO_LOGIN, false); + await h.act(entry); + expect.soft(h.confirm).toHaveBeenCalledTimes(1); + expect.soft(h.puts()).toEqual([]); + expect.soft(h.events()).toEqual(['GET /api/tunnel/status', 'confirm']); + expect.soft(h.button.classList.contains('active')).toBe(false); + expect.soft(h.button.disabled).toBe(false); + expect.soft(h.button.style.display).not.toBe('none'); + expect.soft(h.box.disabled).toBe(false); + expect.soft(h.box.checked).toBe(false); + if (entry === 'save') expect.soft(h.stored().tunnelEnabled).toBe(false); + }); + + it('does not confirm or acknowledge when login is configured', async () => { + const h = await boot({ ...NO_LOGIN, loginConfigured: true }); + await h.act(entry); + expect(h.confirm).not.toHaveBeenCalled(); + expectEnablePut(h.puts(), entry, false); + }); + + it('does not confirm or acknowledge when loginConfigured is absent', async () => { + const h = await boot({ running: false, url: null }); + await h.act(entry); + expect(h.confirm).not.toHaveBeenCalled(); + expect(h.events()).toEqual(['GET /api/tunnel/status', 'PUT /api/settings']); + expectEnablePut(h.puts(), entry, false); + }); + + it.each(['reject', 'non-ok'] as const)('sends no acknowledgement when the status read is %s', async (failure) => { + const h = await boot(NO_LOGIN, true, failure); + await h.act(entry); + expect(h.confirm).not.toHaveBeenCalled(); + expect.soft(h.events()).toEqual(['GET /api/tunnel/status', 'PUT /api/settings']); + expectEnablePut(h.puts(), entry, false); + }); +}); + +it('save does not confirm or acknowledge an already running tunnel', async () => { + const h = await boot({ running: true, url: 'https://test.trycloudflare.com', loginConfigured: false }); + await h.act('save'); + expect(h.confirm).not.toHaveBeenCalled(); + expectEnablePut(h.puts(), 'save', false); +}); + +it.each(['reject', 'non-ok'] as const)('welcome keeps the 403 fallback when the status read is %s', async (failure) => { + const h = await boot(NO_LOGIN, true, failure, true); + await h.act('welcome'); + expect.soft(h.confirm).toHaveBeenCalledTimes(1); + expect.soft(h.events()).toEqual(['GET /api/tunnel/status', 'PUT /api/settings', 'confirm', 'PUT /api/settings']); + expect(h.puts()).toEqual([ + { method: 'PUT', url: '/api/settings', body: { tunnelEnabled: true } }, + { method: 'PUT', url: '/api/settings', body: { tunnelEnabled: true, acknowledgeUnauthTunnel: true } }, + ]); +}); + +it.each(['welcome', 'panel'] as const)('%s disables without confirmation', async (entry) => { + const h = await boot(); + await h.act(entry, false); + expect(h.confirm).not.toHaveBeenCalled(); + expect(h.puts()).toEqual([{ method: 'PUT', url: '/api/settings', body: { tunnelEnabled: false } }]); +}); diff --git a/test/tunnel-flag-plumbing.test.ts b/test/tunnel-flag-plumbing.test.ts new file mode 100644 index 000000000..9f7f92e9e --- /dev/null +++ b/test/tunnel-flag-plumbing.test.ts @@ -0,0 +1,101 @@ +/** + * @fileoverview Issue #706 carries the WebServer tunnel opt-in into its real route context. Requests use Fastify injection and settings use the isolated HOME from test/setup.ts; the tunnel process and socket listener are stubbed. + */ +import { mkdir, readFile, writeFile } from 'node:fs/promises'; +import { dirname } from 'node:path'; +import type { FastifyInstance } from 'fastify'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { startWebServer, WebServer } from '../src/web/server.js'; +import { TunnelManager } from '../src/tunnel-manager.js'; +import { TmuxManager } from '../src/tmux-manager.js'; +import { SETTINGS_PATH } from '../src/web/route-helpers.js'; + +const ENV_KEYS = [ + 'CODEMAN_PASSWORD', + 'CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK', + 'CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL', + 'CODEMAN_MULTIUSER', +] as const; +const INITIAL = { tunnelEnabled: false, subagentTrackingEnabled: false }; +let savedEnv: (string | undefined)[]; +let server: WebServer | undefined; +let startTunnel: ReturnType>; + +beforeEach(async () => { + savedEnv = ENV_KEYS.map((key) => process.env[key]); + for (const key of ENV_KEYS) delete process.env[key]; + vi.spyOn(TmuxManager, 'isTmuxAvailable').mockReturnValue(true); + startTunnel = vi.spyOn(TunnelManager.prototype, 'start').mockImplementation(() => {}); + vi.spyOn(TunnelManager.prototype, 'isRunning').mockReturnValue(false); + await mkdir(dirname(SETTINGS_PATH), { recursive: true }); + await writeFile(SETTINGS_PATH, JSON.stringify(INITIAL)); +}); + +afterEach(async () => { + try { + await server?.stop(); + } finally { + server = undefined; + vi.restoreAllMocks(); + ENV_KEYS.forEach((key, i) => { + if (savedEnv[i] === undefined) delete process.env[key]; + else process.env[key] = savedEnv[i]; + }); + } +}); + +async function enableTunnel(instance: WebServer) { + server = instance; + const app = (server as unknown as { app: FastifyInstance }).app; + // Only the transport is replaced: start() still builds the production route context and middleware. + const listen = vi.spyOn(app, 'listen').mockImplementation(async () => { + await app.ready(); + return 'http://127.0.0.1:0'; + }); + await server.start(); + expect(listen.mock.calls).toEqual([[{ port: 0, host: '127.0.0.1' }]]); + startTunnel.mockClear(); + return app.inject({ method: 'PUT', url: '/api/settings', payload: { tunnelEnabled: true } }); +} + +describe('WebServer tunnel flag plumbing', () => { + it('permits an unauthenticated tunnel through the startWebServer factory flag', async () => { + const start = WebServer.prototype.start; + // Keep the real factory and startup path, replacing only the socket transport before startup. + vi.spyOn(WebServer.prototype, 'start').mockImplementation(async function (this: WebServer) { + // eslint-disable-next-line @typescript-eslint/no-this-alias -- Teardown must also stop an instance whose factory startup fails. + server = this; + const app = (this as unknown as { app: FastifyInstance }).app; + const listen = vi.spyOn(app, 'listen').mockImplementation(async () => { + await app.ready(); + return 'http://127.0.0.1:0'; + }); + await start.call(this); + expect(listen.mock.calls).toEqual([[{ port: 0, host: '127.0.0.1' }]]); + }); + server = await startWebServer(0, false, true, '127.0.0.1', undefined, false, '', true); + const app = (server as unknown as { app: FastifyInstance }).app; + startTunnel.mockClear(); + const res = await app.inject({ method: 'PUT', url: '/api/settings', payload: { tunnelEnabled: true } }); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ success: true, data: {} }); + expect(startTunnel.mock.calls).toEqual([[server.boundPort, false]]); + expect(JSON.parse(await readFile(SETTINGS_PATH, 'utf8'))).toEqual({ ...INITIAL, tunnelEnabled: true }); + }); + + it('permits an unauthenticated tunnel through the constructor flag', async () => { + const res = await enableTunnel(new WebServer(0, false, true, '127.0.0.1', undefined, false, '', true)); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ success: true, data: {} }); + expect(startTunnel.mock.calls).toEqual([[0, false]]); + expect(JSON.parse(await readFile(SETTINGS_PATH, 'utf8'))).toEqual({ ...INITIAL, tunnelEnabled: true }); + }); + + it('refuses an unauthenticated tunnel when the constructor flag is omitted', async () => { + const res = await enableTunnel(new WebServer(0, false, true, '127.0.0.1', undefined, false, '')); + expect(res.statusCode).toBe(403); + expect(res.json()).toEqual({ success: false, errorCode: 'OPERATION_FAILED', error: expect.any(String) }); + expect(startTunnel.mock.calls).toEqual([]); + expect(await readFile(SETTINGS_PATH, 'utf8')).toBe(JSON.stringify(INITIAL)); + }); +});