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)); + }); +});