From ccb3afc9ee7a3259e125330983c779f43539979b Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Mon, 20 Jul 2026 12:33:12 +0200 Subject: [PATCH] fix(multiuser): close cross-user web-layer scoping holes found in review The opt-in multi-user feature's only enforcement is web-layer scoping (all sessions share one OS account). An adversarial review found 8 critical + 7 high cross-user holes that defeated it, plus mediums; all fixed here. Single-user (flag-off) behavior stays byte-identical apart from documented consistency deltas. Ownership / confinement: - DELETE /api/sessions (bulk) + /:id now owner-scope / findSessionOrFail - quick-start, cron (create+fire), scheduled runs confine workingDir to the owner's space; case link/docker-link/docker-import confine the host path - resolveCasePath no longer resolves linked cases for non-admins; foreign remote/docker cases are skipped (fall through to the caller's own local case) - history, subagents/workflows, mux-sessions, orchestrator, cron run-history, away-digest, and remote/docker host reads are owner- or admin-scoped Permission policy (section 6.3): - non-granted users are downgraded at every spawn site incl. legacy /api/scheduled, PlanOrchestrator one-shots, remote launch, and the cron-fire gemini/codex bypass switches; resolveClaudeModeForUsername now fails closed Auth / store: - verify-first login throttle (a correct password is never locked out), /ws terminal subject to the change-password lockbox, cookie fast-path re-validates identity live, role/grant changes revoke sessions, admin delete runs the last-admin guard before any teardown - users.json: distinguish missing (ENOENT) from corrupt/unreadable so a bad read can't overwrite all accounts; unique per-process temp write path Event streams: - debounced session:updated + batched task:updated, clipboard, and push notifications route by owner (fail closed); getLightState hides machine-wide globalStats from non-admins Tests: two suites updated to assert the fixed (secure) behavior. tsc, eslint, and test:ci all green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/multiuser-mode.md | 8 +- CLAUDE.md | 4 +- docs/multi-user-plan.md | 110 ++++++++++++------------ src/cron/cron-service.ts | 29 +++++-- src/plan-orchestrator.ts | 24 +++++- src/push-store.ts | 35 +++++--- src/tmux-manager.ts | 21 ++++- src/user-store.ts | 49 ++++++++--- src/web/middleware/auth.ts | 61 ++++++++++++-- src/web/ports/auth-port.ts | 9 +- src/web/ports/infra-port.ts | 5 +- src/web/route-helpers.ts | 18 +++- src/web/routes/admin-routes.ts | 18 ++-- src/web/routes/case-routes.ts | 66 ++++++++++++--- src/web/routes/clipboard-routes.ts | 14 ++- src/web/routes/cron-routes.ts | 42 +++++++-- src/web/routes/mux-routes.ts | 23 +++-- src/web/routes/orchestrator-routes.ts | 44 +++++++--- src/web/routes/plan-routes.ts | 13 ++- src/web/routes/push-routes.ts | 4 + src/web/routes/respawn-routes.ts | 29 ++++--- src/web/routes/scheduled-routes.ts | 37 ++++++-- src/web/routes/session-routes.ts | 117 +++++++++++++++++++++----- src/web/routes/system-routes.ts | 99 ++++++++++++++++------ src/web/server.ts | 61 +++++++++++++- src/web/sse-stream-manager.ts | 12 ++- test/edge-cases.test.ts | 5 +- test/multiuser-auth.test.ts | 26 ++++-- test/routes/scheduled-routes.test.ts | 4 +- 29 files changed, 757 insertions(+), 230 deletions(-) diff --git a/.changeset/multiuser-mode.md b/.changeset/multiuser-mode.md index 6be4f5ab..2b48b54c 100644 --- a/.changeset/multiuser-mode.md +++ b/.changeset/multiuser-mode.md @@ -1,11 +1,11 @@ --- -"aicodeman": minor +'aicodeman': minor --- Opt-in multi-user mode (`--multiuser` / `CODEMAN_MULTIUSER=1`, off by default). -Named users with individually scrypt-hashed passwords in `~/.codeman/users.json`, per-user case spaces under `~/codeman-users//cases`, and full ownership scoping of sessions, cases, cron jobs, search, file previews, and real-time SSE/WS streams. Non-admin users default to Claude's classifier-guarded `--permission-mode auto`; raw shell mode, cron `launchCommand`, and skip-permissions require an explicit per-user `canBypassPermissions` grant. Machine-level resources (remote/Docker hosts, tunnel, self-update, settings) are admin-only. Admin API (`/api/admin/users*`) with one-time passwords, last-admin invariants, and an append-only audit log; self-service `/api/me` + password change; a frontend admin Users tab + change-password modal; and `codeman users add|passwd|list|rm` CLI. Also adds a global `auto` Claude startup permission mode. When off, behavior is byte-identical to single-user. +Named users with individually scrypt-hashed passwords in `~/.codeman/users.json`, per-user case spaces under `~/codeman-users//cases`, and ownership scoping of sessions (create/list/delete/mutate, incl. bulk delete), cases, cron jobs + run history, scheduled runs, search, file previews, session history, away digest, subagent/workflow monitors, and real-time SSE/WS streams (including the debounced session/task update path, clipboard, and push notifications). A non-admin's `workingDir` is realpath-confined to their own space at every spawn/link path (session create, quick-start, cron create/fire, scheduled runs, case link/docker-link, docker import). Non-admin users default to Claude's classifier-guarded `--permission-mode auto`; raw shell mode, cron `launchCommand`, skip-permissions, and the Codex/Gemini bypass switches require an explicit per-user `canBypassPermissions` grant (enforced at every spawn site incl. one-shots, plan generation, scheduled runs, and remote launches). Machine-level resources (remote/Docker hosts + host reads, mux sessions, orchestrator, tunnel, self-update, settings) are admin-only. Admin API (`/api/admin/users*`) with one-time passwords, last-admin invariants (validated before any teardown), and an append-only audit log; self-service `/api/me` + password change; a frontend admin Users tab + change-password modal; and `codeman users add|passwd|list|rm` CLI. Also adds a global `auto` Claude startup permission mode. When off, behavior is byte-identical to single-user. + +Auth hardening: the login throttle verifies the password before consulting the per-account failure bucket (a correct password can never be locked out); the `mustChangePassword` lockbox covers the WebSocket terminal; the cookie fast-path re-validates identity against the store each request (so a CLI/admin delete/disable/demote takes effect promptly); a role/grant change revokes the target's sessions. (Known limitation: a bare CLI `codeman users passwd` reset — no delete — does not by itself revoke an already-active cookie until it expires; use `codeman users rm`, the admin API, or a restart to force-revoke.) Data-integrity hardening: the store distinguishes a missing users file from a corrupt/unreadable one (so a transient read error can't overwrite all accounts) and writes via a unique per-process temp file; the earlier fire-and-forget `touchLastLogin` corruption race is serialized. Note: multi-user mode separates workspaces for a trusted team; it is not a security boundary between users (all sessions share the host OS account). Pair with Docker cases for real isolation. - -Fixes a `users.json` corruption race by serializing the store's read-modify-write. diff --git a/CLAUDE.md b/CLAUDE.md index 86bba6d5..f3272a11 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -192,11 +192,11 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Attachments** (live external document references; COD-37/#119 core, COD-38/#120 previews, COD-39/#121 history): all wiring in `file-routes.ts`. **Registry** (`attachment-registry.ts`): an **in-memory** map of a stable `attachmentId` → an absolute, `realpath`-resolved, extension-allowlisted file path, so browser requests (`GET /api/sessions/:id/attachments/:attachmentId/raw`) never carry arbitrary absolute paths; `POST /api/sessions/:id/attachments` registers one. **Magic links** (`attachment-magic.ts`): parses `codeman://attach?...` out of terminal output — ⚠️ this scanner is prompt-injectable, so the scan path is **force-confined to the session workspace** (a hostile prompt could otherwise make it read arbitrary host files over SSE); emits the `attachment:detected` SSE event. Security gate is an extension **allowlist** (`isSupportedAttachmentExtension`, in the registry/magic modules), not a blocklist; a separate path layer (`config/attachment-guard.ts`) confines reads to the workspace (`attachmentConfineToWorkspace`) and blocks sensitive trees (`/root`, `/etc`). **Previews + thumbnails** (COD-38): `:attachmentId/preview` + `:attachmentId/thumbnail` (and the workspace-file equivalents `file-preview`/`file-thumbnail`) render Office docs/PDFs via external converters (`pdftoppm` / LibreOffice `soffice` / Word-COM `powershell`); `document-preview-cache.ts` is a shared disk cache (de-dups _identical_ in-flight inputs), `document-thumbnailer.ts` does best-effort first-page images, and `document-conversion-limiter.ts` is a **global converter-spawn concurrency cap** (`runWithConversionLimit`) — without it, N distinct large docs detected at once fork N multi-minute converter processes = a localhost fork-bomb-shaped resource-exhaustion vector. **History drawer** (COD-39): `session-attachment-history.ts` tracks the last `ATTACHMENT_HISTORY_LIMIT` (100) attachments per session (`Session._attachmentHistory`, persisted via `SessionState.attachmentHistory`, replayed so externals re-register on reconnect); `GET /api/sessions/:id/attachments` is the list endpoint. ⚠️ The history drawer's launcher button is desktop-only — hidden on phones (regression-guarded; see `mobile-header-buttons-policy` test). Session-local files keep using the existing workspace-scoped `file-routes` paths; the registry is only for explicit live externals. **Codex generated artifacts** (COD-166/#150, `generated-artifact-attachments.ts`): codex-mode sessions ALSO scan (ANSI-stripped) output for `Saved to: file:///…` lines and surface those files as attachment cards with a relaxed trust policy — the allow decision runs on the **realpath-resolved** path against `os.homedir()`-anchored `~/.codex` marker dirs (symlink escapes fall back to force-confinement); gated to `mode === 'codex'` only (`source` is a REQUIRED param through the listener-deps chain — a dropped arg here silently kills the feature). Image thumbnails pass through jpg/jpeg/gif/webp. -**Ultracode / Workflow-run visualization** (opt-in `showUltracodeAgents`, default OFF; released 1.1.2): the Workflow tool ("ultracode") writes a COMPLETION artifact per run at `~/.claude/projects///workflows/wf_*.json` (written only at run end); LIVE in-flight runs exist only as transcript dirs at `…/subagents/workflows/wf_/` (journal.jsonl + agent-_.jsonl). `workflow-run-watcher.ts` (STANDALONE — deliberately never imports/touches `subagent-watcher.ts`; separate singleton, though it independently reads the same `subagents/workflows/` tree) scans BOTH sources via periodic poll + per-directory chokidar watchers with per-source mtime skip (LRU agentStatCache + journalCache), synthesizing ACTIVE runs (live per-agent tokens/tools/state from transcripts, title/phases from the workflow script) until the completion `wf\__.json`appears and supersedes, and broadcasts SSE`workflow:run_discovered`/`run_updated`/`run_removed`. The watcher is started when **either** `showUltracodeAgents`**or**`ultracodeFloatingWindows` is on (`server.ts` `isWorkflowAgentTrackingEnabled()`returns`(showUltracodeAgents ?? false) || (ultracodeFloatingWindows ?? false)`). Served via `GET /api/workflows`(optional`?minutes=`filter) and`GET /api/workflows/:runId`. Frontend `ultracode-panel.js`renders a docked master-detail view (LEFT: runs + phases; RIGHT: per-agent tokens + tool-calls; click an agent card → its live transcript via client-side`agentId`join). **Additionally**,`ultracode-windows.js`auto-pops a draggable **floating window per active run** (gated on a **DEDICATED**`ultracodeFloatingWindows`toggle, default OFF — independent of the dock panel's`showUltracodeAgents`; see `\_ultracodeFloatingEnabled()`), connected by a glowing line to the originating session tab (resolved by `session.claudeSessionId === run.sessionUuid`) — same line idiom as subagent windows, drawn into the shared `#connectionLines`SVG from the tail of`\_updateConnectionLinesImmediate`. The window auto-closes ~8s after its run finishes; explicit dismissals are remembered. Clicking an agent card opens an **in-page** connected transcript window (not a browser popup); both run and transcript windows minimize **into** the originating session tab as a merged `ULTRA`badge (🧬 runs / 📄 transcripts) with a restore/dismiss dropdown — minimized runs are skipped by auto-pop. Gesture beta: floating subagent/ultracode windows are pinch-draggable (a`window`grab kind in`entry.ts`). Types: `src/types/workflow-run.ts`. Config: `src/config/workflow-config.ts`. +**Ultracode / Workflow-run visualization** (opt-in `showUltracodeAgents`, default OFF; released 1.1.2): the Workflow tool ("ultracode") writes a COMPLETION artifact per run at `~/.claude/projects///workflows/wf_*.json` (written only at run end); LIVE in-flight runs exist only as transcript dirs at `…/subagents/workflows/wf_/` (journal.jsonl + agent-\_.jsonl). `workflow-run-watcher.ts` (STANDALONE — deliberately never imports/touches `subagent-watcher.ts`; separate singleton, though it independently reads the same `subagents/workflows/` tree) scans BOTH sources via periodic poll + per-directory chokidar watchers with per-source mtime skip (LRU agentStatCache + journalCache), synthesizing ACTIVE runs (live per-agent tokens/tools/state from transcripts, title/phases from the workflow script) until the completion `wf\__.json`appears and supersedes, and broadcasts SSE`workflow:run_discovered`/`run_updated`/`run_removed`. The watcher is started when **either** `showUltracodeAgents`**or**`ultracodeFloatingWindows` is on (`server.ts` `isWorkflowAgentTrackingEnabled()`returns`(showUltracodeAgents ?? false) || (ultracodeFloatingWindows ?? false)`). Served via `GET /api/workflows`(optional`?minutes=`filter) and`GET /api/workflows/:runId`. Frontend `ultracode-panel.js`renders a docked master-detail view (LEFT: runs + phases; RIGHT: per-agent tokens + tool-calls; click an agent card → its live transcript via client-side`agentId`join). **Additionally**,`ultracode-windows.js`auto-pops a draggable **floating window per active run** (gated on a **DEDICATED**`ultracodeFloatingWindows`toggle, default OFF — independent of the dock panel's`showUltracodeAgents`; see `\_ultracodeFloatingEnabled()`), connected by a glowing line to the originating session tab (resolved by `session.claudeSessionId === run.sessionUuid`) — same line idiom as subagent windows, drawn into the shared `#connectionLines`SVG from the tail of`\_updateConnectionLinesImmediate`. The window auto-closes ~8s after its run finishes; explicit dismissals are remembered. Clicking an agent card opens an **in-page** connected transcript window (not a browser popup); both run and transcript windows minimize **into** the originating session tab as a merged `ULTRA`badge (🧬 runs / 📄 transcripts) with a restore/dismiss dropdown — minimized runs are skipped by auto-pop. Gesture beta: floating subagent/ultracode windows are pinch-draggable (a`window`grab kind in`entry.ts`). Types: `src/types/workflow-run.ts`. Config: `src/config/workflow-config.ts`. **Cross-session search** (COD-113/#133): `GET /api/search?q=&types=&limit=` federates an **in-memory** search across all live sessions — session metadata (name/workingDir/id), run-summary events, and per-session attachment-history file entries (workspace-relative path only; the server-private `externalPath` is never read). Pure core `searchSources()` in `search-service.ts` (substring-matches with hard per-type caps — no regex, so no ReDoS; no filesystem reads, so no traversal); `harvestSources()` in `search-routes.ts` gathers the in-memory sources. `SearchQuerySchema` bounds `q` (1–200), allowlists `types` (`session,event,file`), clamps `limit` (1–60). Returns the `{success,data}` envelope. Frontend: history-panel search box in `terminal-ui.js`. Types: `src/types/search.ts`. -**Multi-user mode** (opt-in `--multiuser` / `CODEMAN_MULTIUSER=1`, OFF by default; branch `feat/multiuser-mode`, design `docs/multi-user-plan.md`): named users with individually scrypt-hashed passwords in `~/.codeman/users.json` (via `src/user-store.ts`: atomic 0600 write, short-TTL cache, SERIALIZED read-modify-write so a fire-and-forget `touchLastLogin` can't clobber a concurrent route write, last-admin invariants). Gated everywhere by `isMultiUserMode()` (`src/config/multiuser.ts`); when OFF, behavior is byte-identical to single-user (all scoping helpers short-circuit). ⚠️ **Not a security boundary at the agent layer** — every session still runs as the SAME OS account; this separates WORKSPACES, it does not sandbox users (Docker cases are the isolation story). Auth: a PARALLEL async branch in `middleware/auth.ts` (single-user branch untouched) verifies `username:password` against the store, mints identity-carrying cookies (`AuthSessionRecord` gains `username`/`role`/`mustChangePassword`), decorates `req.authUser` (Fastify augmentation; single-user leaves it undefined and the ownership helpers default to a synthetic admin), enforces a per-username failure bucket + the `mustChangePassword` lockbox. Ownership threads through `Session.owner` (stamped from `req.authUser`/`job.owner` at every `new Session()`, round-tripped via `MuxSession.owner` on recovery); `findSessionOrFail(ctx,id,req)` does a NOT_FOUND owner check; list endpoints + `getLightState` + SSE (`deriveSseHint` routes session-scoped events by owner, fail-closed; machine-level + host-plan telemetry admin-only) + WS + search + file-preview all filter by owner. §6.3 permission policy: non-granted users are forced to `--permission-mode auto` (via `resolveClaudeModeForUser` at all spawn sites, incl. one-shots because `buildPromptArgs` now respects the session mode), and shell mode / cron `launchCommand` require the `canBypassPermissions` grant. Cases live in per-user `~/codeman-users//cases` (`resolveCasesDir`); a non-admin's `workingDir` is realpath-confined there; host CRUD is admin-only. Admin API `src/web/routes/admin-routes.ts` (`/api/admin/users*`, one-time passwords, audit log `admin-audit.jsonl`) + self-service `/api/me` + `/api/me/password` (`me-routes.ts`); frontend `public/admin-ui.js` (identity boot, change-password modal + interceptor, admin Users tab). CLI `codeman users add|passwd|list|rm`. Per-user session cap via `assertSessionCapacity`/`sessionCapacityMessage`. Tests: `test/user-store.test.ts`, `test/multiuser-auth.test.ts`, `test/ownership-scoping.test.ts`, `test/admin-routes.test.ts`, `test/admin-ui.test.ts`. +**Multi-user mode** (opt-in `--multiuser` / `CODEMAN_MULTIUSER=1`, OFF by default; branch `feat/multiuser-mode`, design `docs/multi-user-plan.md`): named users with individually scrypt-hashed passwords in `~/.codeman/users.json` (via `src/user-store.ts`: atomic 0600 write, short-TTL cache, SERIALIZED read-modify-write so a fire-and-forget `touchLastLogin` can't clobber a concurrent route write, last-admin invariants). Gated everywhere by `isMultiUserMode()` (`src/config/multiuser.ts`); when OFF, behavior is byte-identical to single-user (all scoping helpers short-circuit). ⚠️ **Not a security boundary at the agent layer** — every session still runs as the SAME OS account; this separates WORKSPACES, it does not sandbox users (Docker cases are the isolation story). Auth: a PARALLEL async branch in `middleware/auth.ts` (single-user branch untouched) verifies `username:password` against the store, mints identity-carrying cookies (`AuthSessionRecord` gains `username`/`role`/`mustChangePassword`), decorates `req.authUser` (Fastify augmentation; single-user leaves it undefined and the ownership helpers default to a synthetic admin), enforces a per-username failure bucket + the `mustChangePassword` lockbox. Ownership threads through `Session.owner` (stamped from `req.authUser`/`job.owner` at every `new Session()`, round-tripped via `MuxSession.owner` on recovery); `findSessionOrFail(ctx,id,req)` does a NOT_FOUND owner check; list endpoints + `getLightState` + SSE (`deriveSseHint` routes session-scoped events by owner, fail-closed; machine-level + host-plan telemetry admin-only) + WS + search + file-preview all filter by owner. §6.3 permission policy: non-granted users are forced to `--permission-mode auto` (via `resolveClaudeModeForUser` at all spawn sites, incl. one-shots because `buildPromptArgs` now respects the session mode), and shell mode / cron `launchCommand` require the `canBypassPermissions` grant. Cases live in per-user `~/codeman-users//cases` (`resolveCasesDir`); a non-admin's `workingDir` is realpath-confined there; host CRUD is admin-only. Admin API `src/web/routes/admin-routes.ts` (`/api/admin/users*`, one-time passwords, audit log `admin-audit.jsonl`) + self-service `/api/me` + `/api/me/password` (`me-routes.ts`); frontend `public/admin-ui.js` (identity boot, change-password modal + interceptor, admin Users tab). CLI `codeman users add|passwd|list|rm`. Per-user session cap via `sessionCapacityState`/`sessionCapacityMessage`. Tests: `test/user-store.test.ts`, `test/multiuser-auth.test.ts`, `test/ownership-scoping.test.ts`, `test/admin-routes.test.ts`, `test/admin-ui.test.ts`. **Away digest** (COD-41/#136): `GET /api/away-digest?range=&since=&until=&lastViewed=` aggregates "what happened while you were away" from the lifecycle log + run-summary events + live sessions + daily token stats + recently-completed subagents into needs-attention/completed/still-running/idle/informational sections. Pure aggregator in `web/away-digest.ts` (`resolveAwayDigestRange()` validates the window — `since-last-visit`/`1h`/`today`/`24h`/`custom`, server-local TZ; `buildAwayDigest()` classifies). Header-button modal in `panels-ui.js` (button hidden on phones — regression-guarded). ⚠️ Returns `{success:true,digest}` (a legacy raw-ish shape, consistent with the other raw GET handlers in `system-routes.ts` — `{entries}`/`{config}`/`{files}`/`getSystemStats()`); frontend + tests read `.digest`. Subagent lookback is a fixed 60-min window regardless of range. diff --git a/docs/multi-user-plan.md b/docs/multi-user-plan.md index 8f2c2170..a3438971 100644 --- a/docs/multi-user-plan.md +++ b/docs/multi-user-plan.md @@ -6,7 +6,7 @@ Shipped by phase: - **Phase 1** (user store + mode plumbing + CLI): `src/user-store.ts` (scrypt, atomic 0600 writes, last-admin invariants, serialized read-modify-write), `src/config/multiuser.ts`, `codeman users add|passwd|list|rm`, `--multiuser` flag, bootstrap-on-first-boot. Tests: `test/user-store.test.ts`. - **Phase 2** (multi-user auth): parallel async auth branch (`src/web/middleware/auth.ts`), `req.authUser`, per-username rate bucket, `mustChangePassword` lockbox, `GET /api/me` + `POST /api/me/password`, QR identity-bound minting, network-bind + tunnel exemptions, new error codes. Tests: `test/multiuser-auth.test.ts`. -- **Phase 3** (ownership threading): `Session.owner` at every create path + recovery mirror; `findSessionOrFail` owner check + list filtering; §6.3 permission policy (`resolveClaudeModeForUser` at all spawn sites incl. one-shots via `buildPromptArgs`; shell/launchCommand grant); per-user case spaces (`resolveCasesDir`) + owner-scoped case list + admin-only host CRUD; `workingDir` confinement; `assertSessionCapacity` per-user cap. Tests: `test/ownership-scoping.test.ts`. +- **Phase 3** (ownership threading): `Session.owner` at every create path + recovery mirror; `findSessionOrFail` owner check + list filtering; §6.3 permission policy (`resolveClaudeModeForUser` at all spawn sites incl. one-shots via `buildPromptArgs`; shell/launchCommand grant); per-user case spaces (`resolveCasesDir`) + owner-scoped case list + admin-only host CRUD; `workingDir` confinement; `sessionCapacityState` per-user cap. Tests: `test/ownership-scoping.test.ts`. - **Phase 4** (event fan-out): WS owner gate; SSE per-client identity + `broadcast`/terminal-batch routing (`deriveSseHint`, fail-closed); `getLightState` per-identity filtering; file-route preview/thumbnail/history + `GET /api/search` scoping. - **Phase 5** (admin API + frontend): `src/web/routes/admin-routes.ts` (user CRUD, one-time passwords, last-admin guards, session revoke/kill) + `src/web/admin-audit.ts`; `public/admin-ui.js` (identity boot, change-password modal + interceptor, admin Users tab). Tests: `test/admin-routes.test.ts`, `test/admin-ui.test.ts`. @@ -33,17 +33,17 @@ Multi-user mode is **workspace separation for a trusted team, NOT security isola This must be stated loudly in `docs/security-architecture.md`, the README section, and the admin panel UI ("Users share the host account; this separates workspaces, it does not sandbox users from each other"). -Also note the flip side: multi-user mode strictly *improves* today's network posture, because it removes the single shared password and gives every person their own revocable credential. +Also note the flip side: multi-user mode strictly _improves_ today's network posture, because it removes the single shared password and gives every person their own revocable credential. ## 3. Activation and Mode Rules -| Condition | Behavior | -| --- | --- | -| No flag (default) | Exactly today's behavior. `users.json` is never read. Single-user auth via `CODEMAN_PASSWORD` if set. | -| `--multiuser` / `CODEMAN_MULTIUSER=1`, `users.json` has users | Multi-user auth active. `CODEMAN_PASSWORD` is ignored for login (warn if set). | -| `--multiuser`, no `users.json` (first boot) | Bootstrap: if `CODEMAN_USERNAME`/`CODEMAN_PASSWORD` are set, create that user as the initial admin and continue. Otherwise refuse to start with instructions to run `codeman users add --admin`. Never start multi-user with zero users (there would be no way in). | -| `--multiuser` on a non-loopback bind | Allowed without `CODEMAN_PASSWORD`: `server.ts start()` treats "multi-user with >= 1 enabled user" as satisfying the auth requirement in the loud-warning check (wire into the existing `isLoopbackBindHost()` branch). | -| Flag later removed | Single-user mode again. Sessions/state that carry `owner` fields keep working (owner is simply ignored); user spaces remain on disk untouched. | +| Condition | Behavior | +| ------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| No flag (default) | Exactly today's behavior. `users.json` is never read. Single-user auth via `CODEMAN_PASSWORD` if set. | +| `--multiuser` / `CODEMAN_MULTIUSER=1`, `users.json` has users | Multi-user auth active. `CODEMAN_PASSWORD` is ignored for login (warn if set). | +| `--multiuser`, no `users.json` (first boot) | Bootstrap: if `CODEMAN_USERNAME`/`CODEMAN_PASSWORD` are set, create that user as the initial admin and continue. Otherwise refuse to start with instructions to run `codeman users add --admin`. Never start multi-user with zero users (there would be no way in). | +| `--multiuser` on a non-loopback bind | Allowed without `CODEMAN_PASSWORD`: `server.ts start()` treats "multi-user with >= 1 enabled user" as satisfying the auth requirement in the loud-warning check (wire into the existing `isLoopbackBindHost()` branch). | +| Flag later removed | Single-user mode again. Sessions/state that carry `owner` fields keep working (owner is simply ignored); user spaces remain on disk untouched. | Plumbing: flag in `src/cli.ts` (web command), env in a new `src/config/multiuser.ts` exporting `isMultiUserMode()`. Per-instance like everything else: a beta instance (`CODEMAN_INSTANCE=beta`) has its own `users.json` via `dataPath()`. @@ -56,21 +56,23 @@ Plumbing: flag in `src/cli.ts` (web command), env in a new `src/config/multiuser "version": 1, "users": [ { - "username": "alice", // canonical lowercase slug - "role": "admin", // "admin" | "user" + "username": "alice", // canonical lowercase slug + "role": "admin", // "admin" | "user" "password": { - "algo": "scrypt", // node:crypto scrypt, no new deps - "N": 16384, "r": 8, "p": 1, + "algo": "scrypt", // node:crypto scrypt, no new deps + "N": 16384, + "r": 8, + "p": 1, "salt": "", - "hash": "" + "hash": "", }, "disabled": false, - "mustChangePassword": false, // set by admin reset; gates all API access until changed - "canBypassPermissions": false, // permission-mode grant, see section 6.3; false for new users + "mustChangePassword": false, // set by admin reset; gates all API access until changed + "canBypassPermissions": false, // permission-mode grant, see section 6.3; false for new users "createdAt": 1752900000000, - "lastLoginAt": 1752900000000 - } - ] + "lastLoginAt": 1752900000000, + }, + ], } ``` @@ -101,12 +103,12 @@ Plumbing: flag in `src/cli.ts` (web command), env in a new `src/config/multiuser Keep the existing single-user branch untouched. Add a parallel multi-user branch selected once at registration time: 1. **Credential check**: Basic header parsed into `username:password`, verified against the user store (scrypt + `timingSafeEqual`). Disabled users fail closed. -2. **Cookie sessions**: same `codeman_session` cookie and `StaleExpirationMap`, but `AuthSessionRecord` gains `username` and `role`. All existing TTL/sliding/eviction logic reused. Eviction cap becomes per-user aware (evict oldest *of that user* first) so one user cannot flush everyone's sessions by logging in 100 times. +2. **Cookie sessions**: same `codeman_session` cookie and `StaleExpirationMap`, but `AuthSessionRecord` gains `username` and `role`. All existing TTL/sliding/eviction logic reused. Eviction cap becomes per-user aware (evict oldest _of that user_ first) so one user cannot flush everyone's sessions by logging in 100 times. 3. **Request identity**: decorate `req.authUser = { username, role }` (Fastify decorateRequest). In single-user mode `req.authUser` is `{ username: 'admin', role: 'admin' }` when auth is on, and a synthetic admin when auth is off, so downstream code has ONE code path. 4. **Rate limiting**: keep the per-IP bucket; add a per-username failure bucket (same `StaleExpirationMap` pattern) so a botnet cannot brute-force one account across IPs, and one flaky user behind a NAT cannot lock out the rest. 5. **`mustChangePassword` gate**: when set, every API request except `GET /api/me`, `POST /api/me/password`, and static assets returns 403 with `errorCode: 'PASSWORD_CHANGE_REQUIRED'`; the frontend intercepts that code and shows the change-password modal. 6. **Password change vs Basic-auth caching**: browsers cache Basic credentials. After a password change we revoke all of that user's cookie sessions; the next request falls to Basic with stale creds, gets 401, and the browser re-prompts. Acceptable for v1; a proper login form is Phase 6 (see 15). -7. **Unchanged**: hook-secret loopback bypass (hooks authenticate the *instance*, not a user; the event maps to a session which has an owner), host guard, Origin/CSRF guard, security headers. +7. **Unchanged**: hook-secret loopback bypass (hooks authenticate the _instance_, not a user; the event maps to a session which has an owner), host guard, Origin/CSRF guard, security headers. 8. **WS upgrade identity** (`ws-routes.ts`): the global auth `onRequest` hook does run on the upgrade request (`@fastify/websocket` v11 runs hooks before the handshake; browsers send the session cookie), but the route handler itself only checks Host/Origin and never learns WHO authenticated. Multi-user: the handler reads the decorated `req.authUser` and closes 4003 unless owner or admin (section 6.4; identity plumbing lands in Phase 2, the owner check in Phase 4 once sessions have owners). Add a regression test that an upgrade with no credentials is rejected while auth is active: the handler-level Host/Origin gate alone must never be mistaken for auth. 9. **QR auth** (`/q/:code` redemption in `system-routes.ts`, minting in `tunnel-manager.ts`): today there is ONE global token, auto-rotated every 60s with a 90s grace window. A globally-rotating token cannot carry an identity (every logged-in user sees the same code), so multi-user mode replaces rotation with **on-demand minting**: an authenticated `POST /api/tunnel/qr` mints a single-use, short-TTL token bound to `req.authUser.username` (field on `QrTokenRecord`); redemption creates a cookie session for that user. Existing rate-limit buckets (`qrAuthFailures`, global `QR_RATE_LIMIT_MAX`) apply unchanged. Single-user mode keeps the rotating token. @@ -128,7 +130,7 @@ Role guard helper in `route-helpers.ts`: `requireAdmin(req, reply): boolean` use - All `CASES_DIR` call sites switch to `resolveCasesDir(req.authUser)`: `case-routes.ts` (list/create/delete/CLAUDE.md scaffolding, name-collision checks, docker quickcreate), `session-routes.ts` (quick-start case resolution, the workingDir-inside-cases env-strip check), `ralph-routes.ts` (case path resolution), and `plan-routes.ts:231` (easy to miss). Case-name-to-path resolution is currently DUPLICATED (`resolveCasePath` in case-routes.ts:82 and an inline copy in quick-start, session-routes.ts:1846-1863); consolidate into one owner-aware resolver as part of this refactor instead of patching both copies. - Registries that map case names to metadata become owner-scoped. `remote-cases.json`/`docker-cases.json` are arrays of objects, so entries simply gain `owner?: string` (absent = legacy: admin-only). `linked-cases.json` is a flat `Record` with no room for a field: it needs a v2 shape (`{ "version": 2, "cases": { "": { "path": "...", "owner": "..." } } }`) with read-time migration of the v1 form; it is read in two places (case-routes AND inline in quick-start), both must move to the new reader. Case names only need to be unique per user. -- **Remote hosts and Docker hosts are machine-level resources**: CRUD on `/api/docker-hosts` and remote-host endpoints becomes admin-only in multi-user mode; regular users can *use* hosts on their own cases but not define them. (Docker containers exec as the host account; letting any user define arbitrary `docker run` args is admin-equivalent.) +- **Remote hosts and Docker hosts are machine-level resources**: CRUD on `/api/docker-hosts` and remote-host endpoints becomes admin-only in multi-user mode; regular users can _use_ hosts on their own cases but not define them. (Docker containers exec as the host account; letting any user define arbitrary `docker run` args is admin-equivalent.) - Case deletion, exports (`docker-exports/`), and imports check ownership; export filenames get an owner prefix to avoid collisions (fits the existing `^[a-zA-Z0-9._-]+\.tgz$` download guard). - **Workspace confinement for non-admins (the linchpin, do not skip)**: today `POST /api/sessions` accepts ANY host directory as `workingDir` (the only check is `statSync().isDirectory()`, session-routes.ts:305-318), and file-routes/attachments confine reads to `session.workingDir`. Without a new rule the whole scoping story is circular: a user points a session at `~/codeman-users/bob` (or `/home`) and the web layer itself serves that subtree, no agent needed. Rule: in multi-user mode a non-admin's `workingDir` must realpath-resolve inside their own space, enforced at `POST /api/sessions`, `POST /api/run`, cron job create AND fire time (the dir can change owners between the two), and Ralph auto-configure. Admins are unrestricted. This one rule is what makes the section 6.4 file-route line ("own space or own sessions' workingDirs") meaningful. @@ -148,20 +150,20 @@ Codeman now ships a global **Startup Mode** picker (App Settings, Claude CLI tab ### 6.4 Everything else that lists or streams -| Surface | Scoping rule | -| --- | --- | -| SSE `/api/events` | Per-connection filter (see 7) | -| WS terminal (`ws-routes.ts`) | Handler reads `req.authUser` (section 5.8) and closes 4003 unless owner or admin; today it checks Host/Origin only and has no identity | -| `GET /api/search` | `harvestSources()` only over owned sessions | -| `GET /api/away-digest` | Aggregate only owned sessions/events | -| `GET /api/subagents`, workflow runs | Filter by owning session (`claudeSessionId -> session -> owner`); agents not attributable to any session: admin-only | -| Push (`push-routes.ts`) | Subscription records currently carry NO identity (keyed by endpoint only): `subscribe` stamps `username`. All 8 `PUSH_EVENT_MAP` events are session-scoped, so routing = resolve owner from `data.sessionId`, deliver to that owner's (plus admins') subscriptions. Legacy identity-less subscriptions: admin-only delivery | -| Screenshots `/api/screenshots` | Per-user subdir `~/.codeman/screenshots//` in multi-user mode. Note: `GET /:name` deliberately rejects `/` in names as traversal, so derive the subdir server-side from `req.authUser` and keep client-visible names flat | -| Attachments | Already session-scoped; inherits the session owner check. `attachmentConfineToWorkspace` is a global, default-OFF setting today: in multi-user mode it is FORCED ON for non-admins regardless of the setting (their attachments must resolve inside their own space); the setting keeps meaning what it means for admins | -| File routes (browse/preview) | Path allowlist adds: non-admin paths must resolve (realpath) inside their own space or their own sessions' workingDirs | -| Settings (`settings.json`) | Global, admin-only writes in multi-user mode; reads allowed (per-device display keys stay in localStorage as today). Per-user server settings: out of scope v1 | -| System ops (self-update, tunnel toggle, span-displays, docker image build) | Admin-only | -| `getLightState` init snapshot | Filtered per connection. Actual contents to filter (verified): `sessions`, `scheduledRuns`, `respawnStatus`, `subagents`, `workflowRuns`, `planUsage` (host-plan telemetry: admin-only); `globalStats` stays coarse-global. Cron jobs are NOT in the snapshot (they have their own REST route; filter there). The snapshot is cached process-wide (`LIGHT_STATE_CACHE_TTL_MS`): either key the cache per role/user or filter AFTER the cache on each send | +| Surface | Scoping rule | +| -------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| SSE `/api/events` | Per-connection filter (see 7) | +| WS terminal (`ws-routes.ts`) | Handler reads `req.authUser` (section 5.8) and closes 4003 unless owner or admin; today it checks Host/Origin only and has no identity | +| `GET /api/search` | `harvestSources()` only over owned sessions | +| `GET /api/away-digest` | Aggregate only owned sessions/events | +| `GET /api/subagents`, workflow runs | Filter by owning session (`claudeSessionId -> session -> owner`); agents not attributable to any session: admin-only | +| Push (`push-routes.ts`) | Subscription records currently carry NO identity (keyed by endpoint only): `subscribe` stamps `username`. All 8 `PUSH_EVENT_MAP` events are session-scoped, so routing = resolve owner from `data.sessionId`, deliver to that owner's (plus admins') subscriptions. Legacy identity-less subscriptions: admin-only delivery | +| Screenshots `/api/screenshots` | Per-user subdir `~/.codeman/screenshots//` in multi-user mode. Note: `GET /:name` deliberately rejects `/` in names as traversal, so derive the subdir server-side from `req.authUser` and keep client-visible names flat | +| Attachments | Already session-scoped; inherits the session owner check. `attachmentConfineToWorkspace` is a global, default-OFF setting today: in multi-user mode it is FORCED ON for non-admins regardless of the setting (their attachments must resolve inside their own space); the setting keeps meaning what it means for admins | +| File routes (browse/preview) | Path allowlist adds: non-admin paths must resolve (realpath) inside their own space or their own sessions' workingDirs | +| Settings (`settings.json`) | Global, admin-only writes in multi-user mode; reads allowed (per-device display keys stay in localStorage as today). Per-user server settings: out of scope v1 | +| System ops (self-update, tunnel toggle, span-displays, docker image build) | Admin-only | +| `getLightState` init snapshot | Filtered per connection. Actual contents to filter (verified): `sessions`, `scheduledRuns`, `respawnStatus`, `subagents`, `workflowRuns`, `planUsage` (host-plan telemetry: admin-only); `globalStats` stays coarse-global. Cron jobs are NOT in the snapshot (they have their own REST route; filter there). The snapshot is cached process-wide (`LIGHT_STATE_CACHE_TTL_MS`): either key the cache per role/user or filter AFTER the cache on each send | ## 7. SSE Event Filtering @@ -177,17 +179,17 @@ Codeman now ships a global **Startup Mode** picker (App Settings, Claude CLI tab All handlers: multi-user mode only (404 otherwise), `requireAdmin`, Zod schemas in `schemas.ts`, `ApiResponse` envelope, audit-logged. -| Endpoint | Behavior | -| --- | --- | -| `GET /api/admin/users` | List users + stats: role, disabled, createdAt, lastLoginAt, live session count, case count, space disk usage (best-effort async walk, cached 60s), active cookie-session count | -| `POST /api/admin/users` | Create: `{ username, role, password? }`. No password given: generate a one-time password, return it ONCE in the response, set `mustChangePassword` | -| `PATCH /api/admin/users/:username` | `{ role?, disabled?, canBypassPermissions? }`. Demoting/disabling the last enabled admin: 409 `LAST_ADMIN`. Disable also revokes cookie sessions. `canBypassPermissions` is the section 6.3 grant (default false) | -| `POST /api/admin/users/:username/reset-password` | Generates one-time password (returned once), sets `mustChangePassword`, revokes cookie sessions | -| `POST /api/admin/users/:username/logout` | Revoke all cookie sessions for that user. Honest limit under Basic auth: the browser silently re-sends cached credentials and gets a fresh cookie on the next request, so logout only truly ends QR-issued sessions; to actually lock someone out, disable the account or reset the password. Say so in the panel tooltip until Phase 6 | -| `DELETE /api/admin/users/:username` | `{ deleteSpace?: boolean }` (default false). Refuses last admin. Kills the user's live sessions first (normal kill flow, incl. docker/remote teardown per case), revokes cookies, removes from store. With `deleteSpace`: guarded recursive delete of `~/codeman-users/` (realpath must be inside `USER_SPACES_DIR`, top-level dir must not be a symlink), plus their registry entries and push subscriptions | -| `POST /api/admin/cases/assign` | Move a legacy `~/codeman-cases/` into a user's space (`fs.rename`) | -| Self-service `GET /api/me` | `{ username, role, mustChangePassword }` (works in single-user mode too: synthetic admin; the frontend uses it to decide whether to render admin UI) | -| Self-service `POST /api/me/password` | `{ currentPassword, newPassword }`, verifies current, min length 8, revokes other sessions, clears `mustChangePassword` | +| Endpoint | Behavior | +| ------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET /api/admin/users` | List users + stats: role, disabled, createdAt, lastLoginAt, live session count, case count, space disk usage (best-effort async walk, cached 60s), active cookie-session count | +| `POST /api/admin/users` | Create: `{ username, role, password? }`. No password given: generate a one-time password, return it ONCE in the response, set `mustChangePassword` | +| `PATCH /api/admin/users/:username` | `{ role?, disabled?, canBypassPermissions? }`. Demoting/disabling the last enabled admin: 409 `LAST_ADMIN`. Disable also revokes cookie sessions. `canBypassPermissions` is the section 6.3 grant (default false) | +| `POST /api/admin/users/:username/reset-password` | Generates one-time password (returned once), sets `mustChangePassword`, revokes cookie sessions | +| `POST /api/admin/users/:username/logout` | Revoke all cookie sessions for that user. Honest limit under Basic auth: the browser silently re-sends cached credentials and gets a fresh cookie on the next request, so logout only truly ends QR-issued sessions; to actually lock someone out, disable the account or reset the password. Say so in the panel tooltip until Phase 6 | +| `DELETE /api/admin/users/:username` | `{ deleteSpace?: boolean }` (default false). Refuses last admin. Kills the user's live sessions first (normal kill flow, incl. docker/remote teardown per case), revokes cookies, removes from store. With `deleteSpace`: guarded recursive delete of `~/codeman-users/` (realpath must be inside `USER_SPACES_DIR`, top-level dir must not be a symlink), plus their registry entries and push subscriptions | +| `POST /api/admin/cases/assign` | Move a legacy `~/codeman-cases/` into a user's space (`fs.rename`) | +| Self-service `GET /api/me` | `{ username, role, mustChangePassword }` (works in single-user mode too: synthetic admin; the frontend uses it to decide whether to render admin UI) | +| Self-service `POST /api/me/password` | `{ currentPassword, newPassword }`, verifies current, min length 8, revokes other sessions, clears `mustChangePassword` | **Audit log**: append-only `~/.codeman/admin-audit.jsonl` (same idiom as `session-lifecycle.jsonl`): timestamp, acting admin, action, target, request IP. User management without an audit trail is not acceptable even for a homelab tool. @@ -222,13 +224,13 @@ These operate directly on `users.json` via `user-store.ts` (no server needed), h ## 12. Compatibility Matrix -| Concern | Guarantee | -| --- | --- | -| Default (no flag) | No behavior change. No new file reads on the hot path. All new fields optional in state | -| State round-trip | `SessionState.owner`, `MuxSession.owner`, `CronJob.owner`, registry `owner` fields are optional; old state loads clean; new state loaded by an old build ignores unknown fields (existing tolerant parsing) | -| Instance isolation | `users.json`, audit log, screenshots subdirs all via `dataPath()`; user spaces dir is shared across instances like `~/codeman-cases` is today (documented) | -| API versioning | HTTP API is internal per `docs/versioning-policy.md`; still, all changes are additive. Ship as a **minor** version | -| Hooks | Unchanged (instance-level hook secret; owner resolved from the session) | +| Concern | Guarantee | +| ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Default (no flag) | No behavior change. No new file reads on the hot path. All new fields optional in state | +| State round-trip | `SessionState.owner`, `MuxSession.owner`, `CronJob.owner`, registry `owner` fields are optional; old state loads clean; new state loaded by an old build ignores unknown fields (existing tolerant parsing) | +| Instance isolation | `users.json`, audit log, screenshots subdirs all via `dataPath()`; user spaces dir is shared across instances like `~/codeman-cases` is today (documented) | +| API versioning | HTTP API is internal per `docs/versioning-policy.md`; still, all changes are additive. Ship as a **minor** version | +| Hooks | Unchanged (instance-level hook secret; owner resolved from the session) | ## 13. Implementation Phases diff --git a/src/cron/cron-service.ts b/src/cron/cron-service.ts index 17080b1e..63c83064 100644 --- a/src/cron/cron-service.ts +++ b/src/cron/cron-service.ts @@ -16,7 +16,7 @@ import { CronJobSchema } from '../web/schemas.js'; import { getErrorMessage, createErrorResponse, ApiErrorCode } from '../types/api.js'; import { MAX_CONCURRENT_SESSIONS, MAX_CRON_JOBS, MAX_CRON_RUN_HISTORY } from '../config/map-limits.js'; import { canUsernameRunPrivilegedCommands, resolveClaudeModeForUsername } from '../user-store.js'; -import { sessionCapacityState } from '../web/route-helpers.js'; +import { sessionCapacityState, isWorkingDirAllowedForUsername } from '../web/route-helpers.js'; import { CRON_READY_MAX_ATTEMPTS, CRON_READY_SETTLE_MS } from '../config/server-timing.js'; import { DEFAULT_BLOCKED_TREES, @@ -27,6 +27,7 @@ import { validateSessionFilePath } from '../web/route-helpers.js'; import { computeNextRunAt, dueKeyFor } from './cron-time.js'; import type { SessionPort, EventPort, ConfigPort, InfraPort } from '../web/ports/index.js'; import type { CronJob, CronJobRun, CronJobRunStatus, TriggerType } from '../types/cron.js'; +import type { GeminiConfig } from '../types/session.js'; import type { CronJobInput } from './cron-input.js'; /** The subset of the route context the cron depends on. */ @@ -331,6 +332,12 @@ export class CronService { return this.failRun(job, run, 'workingDir does not exist'); } + // Section 6.3: defense-in-depth workingDir confinement re-check at FIRE time against the + // owner's CURRENT space (complements the create/update gate). No-op in single-user / unset owner. + if (!(await isWorkingDirAllowedForUsername(job.owner, job.workingDir))) { + return this.failRun(job, run, 'workingDir is outside the owner workspace'); + } + // Recurring jobs: close the still-open session created by this job's // previous run before launching the next (default ON, opt-out via // autoClosePreviousSession:false) — otherwise an unattended interval/daily @@ -348,13 +355,11 @@ export class CronService { return this.failRun(job, run, `Owner's per-user session limit reached`); } - // Section 6.3: shell mode / launchCommand are arbitrary host-account execution. - // Re-check the owner's grant at FIRE time (it may have been revoked since create). - if (job.agentType === 'shell' || job.launchCommand) { - const allowed = await canUsernameRunPrivilegedCommands(job.owner); - if (!allowed) { - return this.failRun(job, run, 'Owner lacks the can-bypass-permissions grant for shell/launchCommand jobs'); - } + // Section 6.3: re-resolve the owner's grant at FIRE time (it may have been revoked + // since create). Gates shell/launchCommand AND clamps the external-CLI bypass below. + const ownerGranted = await canUsernameRunPrivilegedCommands(job.owner); + if ((job.agentType === 'shell' || job.launchCommand) && !ownerGranted) { + return this.failRun(job, run, 'Owner lacks the can-bypass-permissions grant for shell/launchCommand jobs'); } // Create + start the session (mirrors the quick-start route flow). @@ -366,6 +371,13 @@ export class CronService { const claudeModeConfig = await this.deps.getClaudeModeConfig(); const effectiveClaudeMode = await resolveClaudeModeForUsername(claudeModeConfig.claudeMode, job.owner); const model = mode !== 'shell' ? modelConfig?.defaultModel || undefined : undefined; + // Section 6.3: cron carries no per-CLI config, so buildGeminiCommand(undefined) + // would default a non-granted owner to `--approval-mode yolo` (classifier-free) — + // materialize auto_edit for a non-granted gemini owner, mirroring the route clamp + // (#15). Granted/admin/single-user leave it undefined → yolo parity. Codex's absent + // config already defaults to the safe sandbox, so no clamp is needed there. + const geminiConfig: GeminiConfig | undefined = + mode === 'gemini' && !ownerGranted ? { approvalMode: 'auto_edit' } : undefined; session = new Session({ workingDir: job.workingDir, mode, @@ -376,6 +388,7 @@ export class CronService { model, claudeMode: effectiveClaudeMode, allowedTools: claudeModeConfig.allowedTools, + geminiConfig, owner: job.owner, }); this.deps.addSession(session); diff --git a/src/plan-orchestrator.ts b/src/plan-orchestrator.ts index 5edddb1e..4a5f7db5 100644 --- a/src/plan-orchestrator.ts +++ b/src/plan-orchestrator.ts @@ -20,7 +20,7 @@ import type { TerminalMultiplexer } from './mux-interface.js'; import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { RESEARCH_AGENT_PROMPT, PLANNER_PROMPT } from './prompts/index.js'; -import { getErrorMessage, type PlanItem } from './types.js'; +import { getErrorMessage, type PlanItem, type ClaudeMode } from './types.js'; // Re-export for backward compatibility export type { PlanItem }; @@ -130,18 +130,28 @@ export class PlanOrchestrator { private taskDescription = ''; private researchModel: string; private plannerModel: string; + // Multi-user permission threading: the resolved claudeMode/owner/allowedTools for the + // internal research/planner one-shots. Left undefined = today's single-user behavior + // (the caller threads the resolved global mode, byte-identical when !isMultiUserMode()). + private claudeMode?: ClaudeMode; + private owner?: string; + private allowedTools?: string; constructor( mux: TerminalMultiplexer, workingDir: string = process.cwd(), outputDir?: string, - modelConfig?: { defaultModel?: string; agentTypeOverrides?: Record } + modelConfig?: { defaultModel?: string; agentTypeOverrides?: Record }, + security?: { claudeMode?: ClaudeMode; owner?: string; allowedTools?: string } ) { this.mux = mux; this.workingDir = workingDir; this.outputDir = outputDir; this.researchModel = modelConfig?.agentTypeOverrides?.explore || modelConfig?.defaultModel || DEFAULT_MODEL; this.plannerModel = modelConfig?.agentTypeOverrides?.review || modelConfig?.defaultModel || DEFAULT_MODEL; + this.claudeMode = security?.claudeMode; + this.owner = security?.owner; + this.allowedTools = security?.allowedTools; } private saveAgentOutput(agentType: string, prompt: string, result: unknown, durationMs: number): void { @@ -424,6 +434,12 @@ export class PlanOrchestrator { mux: this.mux, useMux: false, mode: 'claude', + // Section 6.3: run this one-shot under the caller-resolved permission mode/owner so a + // non-granted multi-user user cannot regain --dangerously-skip-permissions. Undefined + // (single-user, not threaded) is byte-identical to today (Session keeps its default). + claudeMode: this.claudeMode, + allowedTools: this.allowedTools, + owner: this.owner, }); this.runningSessions.add(session); @@ -580,6 +596,10 @@ export class PlanOrchestrator { mux: this.mux, useMux: false, mode: 'claude', + // Section 6.3: same permission-mode/owner threading as the research one-shot above. + claudeMode: this.claudeMode, + allowedTools: this.allowedTools, + owner: this.owner, }); this.runningSessions.add(session); diff --git a/src/push-store.ts b/src/push-store.ts index 5cee424f..059cf612 100644 --- a/src/push-store.ts +++ b/src/push-store.ts @@ -9,10 +9,23 @@ import { existsSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs'; import { join } from 'node:path'; import webpush from 'web-push'; -import type { VapidKeys, PushSubscriptionRecord } from './types.js'; +import type { VapidKeys, PushSubscriptionRecord, UserRole } from './types.js'; import { Debouncer } from './utils/index.js'; import { getDataDir } from './config/instance.js'; +/** + * A push subscription plus the multi-user owner identity stamped at subscribe time. + * `username`/`role` are undefined in single-user mode (and for legacy records saved + * before this field existed). sendPushNotifications uses them to scope a + * session-notification to its owner's devices (+ admins) instead of fanning out to + * every user. Kept as a store-local widening of PushSubscriptionRecord so the shared + * type stays untouched; the extra keys serialize/persist transparently. + */ +export type OwnedPushSubscriptionRecord = PushSubscriptionRecord & { + username?: string; + role?: UserRole; +}; + const DATA_DIR = getDataDir(); const KEYS_FILE = join(DATA_DIR, 'push-keys.json'); const SUBS_FILE = join(DATA_DIR, 'push-subscriptions.json'); @@ -20,7 +33,7 @@ const SAVE_DEBOUNCE_MS = 500; export class PushSubscriptionStore { private vapidKeys: VapidKeys | null = null; - private subscriptions: Map = new Map(); + private subscriptions: Map = new Map(); private saveDeb = new Debouncer(SAVE_DEBOUNCE_MS); private _disposed = false; @@ -67,17 +80,19 @@ export class PushSubscriptionStore { } /** Register or update a push subscription (deduplicates by endpoint) */ - addSubscription(sub: Omit): PushSubscriptionRecord { + addSubscription(sub: Omit): OwnedPushSubscriptionRecord { // Check for existing subscription with same endpoint for (const [existingId, existing] of this.subscriptions) { if (existing.endpoint === sub.endpoint) { - // Update existing - const updated: PushSubscriptionRecord = { + // Update existing (re-stamp owner identity so it tracks the current caller) + const updated: OwnedPushSubscriptionRecord = { ...existing, keys: sub.keys, userAgent: sub.userAgent, lastUsedAt: Date.now(), pushPreferences: sub.pushPreferences, + username: sub.username, + role: sub.role, }; this.subscriptions.set(existingId, updated); this.scheduleSave(); @@ -86,7 +101,7 @@ export class PushSubscriptionStore { } // New subscription - const record: PushSubscriptionRecord = { + const record: OwnedPushSubscriptionRecord = { ...sub, lastUsedAt: Date.now(), }; @@ -96,7 +111,7 @@ export class PushSubscriptionStore { } /** Update push preferences for a subscription */ - updatePreferences(id: string, preferences: Record): PushSubscriptionRecord | null { + updatePreferences(id: string, preferences: Record): OwnedPushSubscriptionRecord | null { const sub = this.subscriptions.get(id); if (!sub) return null; sub.pushPreferences = preferences; @@ -124,12 +139,12 @@ export class PushSubscriptionStore { } /** Get all subscriptions */ - getAll(): PushSubscriptionRecord[] { + getAll(): OwnedPushSubscriptionRecord[] { return Array.from(this.subscriptions.values()); } /** Get a single subscription by ID */ - get(id: string): PushSubscriptionRecord | null { + get(id: string): OwnedPushSubscriptionRecord | null { return this.subscriptions.get(id) ?? null; } @@ -138,7 +153,7 @@ export class PushSubscriptionStore { if (!existsSync(SUBS_FILE)) return; try { const raw = readFileSync(SUBS_FILE, 'utf-8'); - const arr = JSON.parse(raw) as PushSubscriptionRecord[]; + const arr = JSON.parse(raw) as OwnedPushSubscriptionRecord[]; for (const sub of arr) { this.subscriptions.set(sub.id, sub); } diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 904ace6c..76ec2f05 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -779,9 +779,22 @@ export function buildRemoteLaunchCommand(options: { mode: SessionMode; remote: SessionRemote; sessionId: string; + claudeMode?: ClaudeMode; + allowedTools?: string; }): string { - const { mode, remote, sessionId } = options; - const modeCommand = remote.commands?.[mode] || defaultRemoteCommandForMode(mode); + const { mode, remote, sessionId, claudeMode, allowedTools } = options; + // §6.3: honor the session's EFFECTIVE claude permission mode on remote instead of + // hardcoding --dangerously-skip-permissions, so a non-granted multi-user user's + // downgraded 'auto' actually reaches the remote agent (the default command otherwise + // ignored claudeMode). A per-host `commands.claude` override stays authoritative + // (admin's explicit choice). For the DEFAULT single-user config (skip), the emitted + // command is byte-identical to before. Non-claude modes are unchanged. + const override = remote.commands?.[mode]; + const modeCommand = override + ? override + : mode === 'claude' + ? `exec claude${buildClaudePermissionFlags(claudeMode, allowedTools)}` + : defaultRemoteCommandForMode(mode); const remoteName = remoteTmuxSessionName(sessionId); // Innermost: the command tmux runs in the new pane. Run via `/bin/sh -c` by @@ -1559,7 +1572,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { const fullCmd = docker ? buildDockerLaunchCommand(resolveDockerLaunchOptions(mode, docker, sessionId, resumeSessionId)) : remote - ? buildRemoteLaunchCommand({ mode, remote, sessionId }) + ? buildRemoteLaunchCommand({ mode, remote, sessionId, claudeMode, allowedTools }) : localFullCmd; // Create tmux session in three steps to handle cold-start (no server running) @@ -1814,7 +1827,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { const fullCmd = docker ? buildDockerLaunchCommand(resolveDockerLaunchOptions(mode, docker, sessionId, resumeSessionId)) : remote - ? buildRemoteLaunchCommand({ mode, remote, sessionId }) + ? buildRemoteLaunchCommand({ mode, remote, sessionId, claudeMode, allowedTools }) : localFullCmd; try { diff --git a/src/user-store.ts b/src/user-store.ts index a3ab9404..869a08c4 100644 --- a/src/user-store.ts +++ b/src/user-store.ts @@ -174,27 +174,46 @@ export function invalidateUsersCache(): void { export async function readUsers(force = false): Promise { const now = Date.now(); if (!force && cache && now - cache.ts < CACHE_TTL_MS) return cache.users; + let raw: string; try { - const raw = await fs.readFile(dataPath(USERS_FILE), 'utf-8'); - const parsed = JSON.parse(raw) as Partial; - const users = Array.isArray(parsed.users) ? parsed.users : []; - cache = { users, ts: now }; - return users; - } catch { - cache = { users: [], ts: now }; - return []; + raw = await fs.readFile(dataPath(USERS_FILE), 'utf-8'); + } catch (err) { + // ENOENT is the ONLY legitimately-empty store (first boot). Any other read + // error (EIO/EACCES/EMFILE/EBUSY) is a transient/permission failure, NOT an + // empty store — do NOT cache [] and do NOT let it look empty, or a following + // createUser/bootstrap would overwrite users.json and destroy every account. + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + cache = { users: [], ts: now }; + return []; + } + throw err; } + // A present-but-corrupt file (invalid JSON) must also fail loud rather than + // read as empty, so mutators/bootstrap abort instead of clobbering it. + const parsed = JSON.parse(raw) as Partial; + const users = Array.isArray(parsed.users) ? parsed.users : []; + cache = { users, ts: now }; + return users; } async function writeUsers(users: UserRecord[]): Promise { const dir = getDataDir(); if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); const finalPath = dataPath(USERS_FILE); - const tmpPath = `${finalPath}.tmp`; + // Unique per-writer tmp name (pid + random) so the CLI (`codeman users …`) and + // the live server — designed to write this file concurrently across processes — + // never share a single `users.json.tmp` inode and tear each other's payload. + // Matches the state-store.ts / self-update.ts convention. + const tmpPath = `${finalPath}.${process.pid}.${randomBytes(6).toString('hex')}.tmp`; const payload: UsersFile = { version: 1, users }; - await fs.writeFile(tmpPath, JSON.stringify(payload, null, 2), { mode: 0o600 }); - await fs.chmod(tmpPath, 0o600).catch(() => {}); - await fs.rename(tmpPath, finalPath); + try { + await fs.writeFile(tmpPath, JSON.stringify(payload, null, 2), { mode: 0o600 }); + await fs.chmod(tmpPath, 0o600).catch(() => {}); + await fs.rename(tmpPath, finalPath); + } catch (err) { + await fs.unlink(tmpPath).catch(() => {}); + throw err; + } cache = { users, ts: Date.now() }; } @@ -461,7 +480,9 @@ export async function resolveClaudeModeForUsername( ): Promise { const fallback: ClaudeMode = globalMode ?? 'dangerously-skip-permissions'; if (!isMultiUserMode() || !username) return fallback; + // Fail closed: an unknown/deleted owner in multi-user mode is treated as a + // non-granted regular user so a stale-owned spawn (e.g. an orphaned cron job) + // is downgraded to `auto` rather than inheriting the global bypass. const user = await findUser(username); - if (!user) return fallback; - return resolveClaudeModeForUser(globalMode, user); + return resolveClaudeModeForUser(globalMode, user ?? { role: 'user' }); } diff --git a/src/web/middleware/auth.ts b/src/web/middleware/auth.ts index 061f0745..41b9b160 100644 --- a/src/web/middleware/auth.ts +++ b/src/web/middleware/auth.ts @@ -21,7 +21,7 @@ import { } from '../../config/auth-config.js'; import { getHookSecret, HOOK_SECRET_HEADER } from '../../config/hook-secret.js'; import { isMultiUserMode } from '../../config/multiuser.js'; -import { setPassword, touchLastLogin, verifyPassword } from '../../user-store.js'; +import { findUser, setPassword, touchLastLogin, verifyPassword } from '../../user-store.js'; import { ApiErrorCode, createErrorResponse, type AuthUser } from '../../types.js'; // Request-scoped identity (multi-user). Single-user leaves it undefined and the @@ -114,6 +114,9 @@ function checkHookSecretBypass( function isPasswordChangeExempt(req: FastifyRequest): boolean { const url = (req.url ?? '').split('?')[0]; if (url === '/api/me' || url === '/api/me/password') return true; + // Security: the WebSocket terminal (/ws/...) is a functional channel, not a static + // asset, so it must NOT be exempt, or a locked user keeps a working terminal. + if (url.startsWith('/ws/')) return false; return !url.startsWith('/api/'); } @@ -344,13 +347,40 @@ function registerMultiUserAuthHook( const sessionToken = req.cookies[AUTH_COOKIE_NAME]; const record = sessionToken ? authSessions.get(sessionToken) : undefined; if (record && record.username) { - req.authUser = { username: record.username, role: record.role ?? 'user' }; + // Security: re-validate the cookie identity against the store on every request so + // an out-of-band mutation the in-memory map can't see (the `codeman users` CLI, + // a separate process, deleting/disabling/demoting a user) takes effect promptly + // instead of riding the 24h cookie. findUser is cached ~1s, so this is cheap. + let live: Awaited>; + try { + live = await findUser(record.username); + } catch { + // The store is transiently unreadable/corrupt (readUsers throws on a non-ENOENT + // read, #23). Fall back to the cookie's snapshot for THIS request rather than + // 500-ing an already-authenticated client (pre-#24 behaviour); a persistently + // corrupt store still fails all WRITES loudly at the mutator/bootstrap layer. + req.authUser = { username: record.username, role: record.role ?? 'user' }; + setSessionCookie(reply, sessionToken!); + enforcePasswordChange(req, reply, !!record.mustChangePassword); + return; + } + if (!live || live.disabled) { + authSessions.delete(sessionToken!); + reply.clearCookie(AUTH_COOKIE_NAME, { path: '/' }); + reply.code(401).send('Unauthorized'); + return; + } + // Trust the LIVE role/mustChangePassword, not the (possibly stale) cookie snapshot + // (also defends #9/#13: a CLI demotion is reflected without a revoke). + req.authUser = { username: live.username, role: live.role }; setSessionCookie(reply, sessionToken!); // sliding re-issue - enforcePasswordChange(req, reply, !!record.mustChangePassword); + enforcePasswordChange(req, reply, !!live.mustChangePassword); return; } // 2. Basic Auth against the user store (scrypt verify). + // Per-IP pre-gate bounds scrypt CPU cost from one source (does NOT gate on the + // per-username bucket here; see below). const ipFail = authFailures.get(clientIp) ?? 0; if (ipFail >= AUTH_FAILURE_MAX) { sendAuthRateLimit(reply, authFailures, clientIp); @@ -359,11 +389,10 @@ function registerMultiUserAuthHook( const creds = parseBasicAuth(req.headers.authorization); if (creds) { const normUser = creds.username.trim().toLowerCase(); - const uFail = userFailures.get(normUser) ?? 0; - if (uFail >= AUTH_FAILURE_MAX) { - sendAuthRateLimit(reply, userFailures, normUser); - return; - } + // Security: VERIFY FIRST, then throttle only FAILED attempts. Consulting the + // per-username bucket before verifying let throwaway IPs lock out a known account + // (incl. admin) even with the correct password. A correct password must always + // win and self-heal both buckets, regardless of the username-failure count. const result = await verifyPassword(creds.username, creds.password); if (result) { const { user, needsRehash: rehash } = result; @@ -388,9 +417,23 @@ function registerMultiUserAuthHook( enforcePasswordChange(req, reply, !!user.mustChangePassword); return; } - userFailures.set(normUser, uFail + 1); + // Failed guess: count it against BOTH buckets. Once the per-username bucket + // reaches the cap, further FAILED attempts get 429 (throttles distributed + // brute-force), but this path is only reached on a wrong password, so it can + // never deny a correct one. + const uFail = (userFailures.get(normUser) ?? 0) + 1; + userFailures.set(normUser, uFail); + authFailures.set(clientIp, ipFail + 1); + if (uFail >= AUTH_FAILURE_MAX) { + sendAuthRateLimit(reply, userFailures, normUser); + return; + } + reply.header('WWW-Authenticate', 'Basic realm="Codeman"'); + reply.code(401).send('Unauthorized'); + return; } + // No credentials presented: count against the per-IP bucket and challenge. authFailures.set(clientIp, ipFail + 1); reply.header('WWW-Authenticate', 'Basic realm="Codeman"'); reply.code(401).send('Unauthorized'); diff --git a/src/web/ports/auth-port.ts b/src/web/ports/auth-port.ts index f1f06198..80059111 100644 --- a/src/web/ports/auth-port.ts +++ b/src/web/ports/auth-port.ts @@ -13,9 +13,12 @@ export interface AuthSessionRecord { method: 'qr' | 'basic'; /** * Multi-user identity carried by the cookie (single-user leaves these unset). - * Snapshotted at mint time; state transitions that would change them (password - * reset, disable, delete) revoke the user's sessions so a stale snapshot can't - * outlive the change. See docs/multi-user-plan.md section 5. + * Snapshotted at mint time. Authorization-relevant admin changes (password reset, + * disable, delete, role change, bypass-grant change) revoke the user's sessions so + * a stale snapshot can't outlive the change; additionally the cookie fast-path + * re-reads role/disabled/mustChangePassword live from the store each request, so an + * out-of-band CLI mutation also takes effect promptly. See docs/multi-user-plan.md + * section 5. */ username?: string; role?: 'admin' | 'user'; diff --git a/src/web/ports/infra-port.ts b/src/web/ports/infra-port.ts index 322cdf37..21e5beca 100644 --- a/src/web/ports/infra-port.ts +++ b/src/web/ports/infra-port.ts @@ -23,6 +23,9 @@ export interface ScheduledRun { completedTasks: number; totalCost: number; logs: string[]; + /** Multi-user owner (username) — undefined in single-user mode. Used to scope + * list/delete and to downgrade the spawned Session's permission mode. */ + owner?: string; } export interface InfraPort { @@ -33,6 +36,6 @@ export interface InfraPort { readonly teamWatcher: TeamWatcher; readonly tunnelManager: TunnelManager; readonly pushStore: PushSubscriptionStore; - startScheduledRun(prompt: string, workingDir: string, durationMinutes: number): Promise; + startScheduledRun(prompt: string, workingDir: string, durationMinutes: number, owner?: string): Promise; stopScheduledRun(id: string): Promise; } diff --git a/src/web/route-helpers.ts b/src/web/route-helpers.ts index 0459473f..e2a2c2fc 100644 --- a/src/web/route-helpers.ts +++ b/src/web/route-helpers.ts @@ -22,7 +22,7 @@ import type { AuthSessionRecord } from './ports/auth-port.js'; import type { StaleExpirationMap } from '../utils/index.js'; import { dataPath } from '../config/instance.js'; import { isMultiUserMode, maxSessionsPerUser, userCasesDir } from '../config/multiuser.js'; -import { SYNTHETIC_ADMIN } from '../user-store.js'; +import { SYNTHETIC_ADMIN, findUser } from '../user-store.js'; // Shared path constants used across route modules. CASES_DIR (project folders) // stays shared across instances; SETTINGS_PATH is per-instance runtime state. @@ -166,6 +166,22 @@ export function isWorkingDirAllowed(user: AuthUser, workingDir: string): boolean return rel !== '' && !rel.startsWith('..') && !isAbsolute(rel); } +/** + * Username-keyed variant of `isWorkingDirAllowed` for spawn sites that only carry + * an owner username (cron fire-time, scheduled-run loop) rather than a live request. + * Resolves the owner's role from the store; a missing/deleted user is treated as a + * non-privileged regular user (fails closed to their deterministic case space). + * No-op (true) in single-user mode or for an unset owner. + */ +export async function isWorkingDirAllowedForUsername( + username: string | undefined, + workingDir: string +): Promise { + if (!isMultiUserMode() || !username) return true; + const user = await findUser(username); + return isWorkingDirAllowed({ username, role: user?.role ?? 'user' }, workingDir); +} + /** Whether the caller is an admin (or single-user mode, where the sole user is admin). */ export function isAdmin(req: FastifyRequest): boolean { return !isMultiUserMode() || getAuthUser(req).role === 'admin'; diff --git a/src/web/routes/admin-routes.ts b/src/web/routes/admin-routes.ts index 635c49ad..2f94e6b7 100644 --- a/src/web/routes/admin-routes.ts +++ b/src/web/routes/admin-routes.ts @@ -134,8 +134,12 @@ export function registerAdminRoutes(app: FastifyInstance, ctx: SessionPort & Aut } try { const user = await updateUser(username, parsed.data); - // Disabling revokes the user's cookie sessions. - if (parsed.data.disabled) revokeUserSessions(ctx.authSessions, username); + // Security: revoke the target's cookie sessions on ANY successful update. role, + // disabled, and canBypassPermissions are all authorization-relevant, and the + // cookie snapshots role, so a stale cookie could otherwise retain old privileges + // (a demoted admin staying admin). Idempotent, affects only the target, and + // forces a re-auth that re-snapshots the new record. + revokeUserSessions(ctx.authSessions, user.username); audit(req, 'user.update', user.username, parsed.data); ctx.broadcast(SseEvent.AdminUsersChanged, {}); return { success: true, data: { user: toPublicUser(user) } }; @@ -177,14 +181,18 @@ export function registerAdminRoutes(app: FastifyInstance, ctx: SessionPort & Aut const parsed = DeleteUserSchema.safeParse(req.body ?? {}); const deleteSpace = parsed.success ? parsed.data.deleteSpace : false; try { - // Kill the user's live sessions first (normal kill flow, incl. docker/remote - // teardown), before removing the record. + // Security: validate BEFORE any teardown. deleteUser runs the authoritative + // existence + last-admin guard under lock with no side effects, so a refusal + // (409 LAST_ADMIN / 404 USER_NOT_FOUND) leaves the user's live sessions and + // cookies untouched. Only after it succeeds do we irreversibly kill sessions and + // revoke cookies. (owned is captured from the in-memory map, independent of the + // record, so it is safe to read before the delete.) const owned = [...ctx.sessions.values()].filter((s) => s.owner === username).map((s) => s.id); + await deleteUser(username); // throws LAST_ADMIN / USER_NOT_FOUND (no side effects) for (const id of owned) { await ctx.cleanupSession(id, true, 'admin_delete_user').catch(() => {}); } revokeUserSessions(ctx.authSessions, username); - await deleteUser(username); // throws LAST_ADMIN / USER_NOT_FOUND if (deleteSpace) await deleteUserSpace(username); audit(req, 'user.delete', username, { deleteSpace, killedSessions: owned.length }); ctx.broadcast(SseEvent.AdminUsersChanged, {}); diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 8a51d77c..1cc82165 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -32,6 +32,7 @@ import { canAccessOwned, getAuthUser, isAdmin, + isWorkingDirAllowed, ownerFor, resolveCasesDir, SETTINGS_PATH, @@ -39,6 +40,7 @@ import { parseBody, readJsonConfig, } from '../route-helpers.js'; +import { isMultiUserMode } from '../../config/multiuser.js'; import type { AuthUser } from '../../types.js'; import { SseEvent } from '../sse-events.js'; import type { EventPort, ConfigPort } from '../ports/index.js'; @@ -96,7 +98,9 @@ async function readLinkedCases(): Promise> { */ async function resolveCasePath(name: string, user?: AuthUser): Promise { const linkedCases = await readLinkedCases(); - if (linkedCases[name]) return linkedCases[name]; + // Linked cases carry no owner (legacy/admin-only registry): a non-admin must not + // resolve arbitrary linked paths by name in multi-user mode (path-escape guard). + if (linkedCases[name] && (!isMultiUserMode() || user?.role === 'admin')) return linkedCases[name]; return join(resolveCasesDir(user), name); } @@ -295,7 +299,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config } }); - app.get('/api/remote-hosts', async () => readRemoteHosts(CODEMAN_CONFIG_DIR)); + // Hosts are machine-level infra config (ssh users/identity paths): non-admins get an + // empty list in multi-user mode, matching the admin-only write side. No-op otherwise. + app.get('/api/remote-hosts', async (req) => + isMultiUserMode() && !isAdmin(req) ? [] : readRemoteHosts(CODEMAN_CONFIG_DIR) + ); // Hosts are machine-level resources: only admins may define them in multi-user mode. const adminOnly = (req: FastifyRequest, reply: { code: (n: number) => unknown }): ApiResponse | null => @@ -376,7 +384,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config // ========== Docker hosts + docker cases (COD-Docker) ========== - app.get('/api/docker-hosts', async () => readDockerHosts(CODEMAN_CONFIG_DIR)); + // Hosts are machine-level infra config (images/mounts/env): non-admins get an empty + // list in multi-user mode, matching the admin-only write side. No-op otherwise. + app.get('/api/docker-hosts', async (req) => + isMultiUserMode() && !isAdmin(req) ? [] : readDockerHosts(CODEMAN_CONFIG_DIR) + ); app.post('/api/docker-hosts', async (req, reply): Promise> => { const denied = adminOnly(req, reply); @@ -446,6 +458,12 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config return createErrorResponse(ApiErrorCode.ALREADY_EXISTS, 'Case already exists'); } + // Confine the bind-mounted workspace to the caller's own space BEFORE creating it + // (also removes the arbitrary-dir-creation primitive). No-op for admins/single-user. + if (!isWorkingDirAllowed(getAuthUser(req), dockerCase.hostWorkspacePath)) { + return createErrorResponse(ApiErrorCode.FORBIDDEN, 'hostWorkspacePath is outside your workspace'); + } + // The workspace is a REAL host directory (bind-mounted into the container), so // create it now if missing. Scaffolding (.claude/settings.local.json + CLAUDE.md) // is written by quick-start on first launch, matching local-case behaviour. @@ -700,6 +718,12 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config return createErrorResponse(ApiErrorCode.ALREADY_EXISTS, 'Case already exists'); } + // Import extracts a tar into destWorkspacePath (later becomes Session.workingDir): + // confine it to the caller's own space. No-op for admins/single-user. + if (!isWorkingDirAllowed(getAuthUser(req), destWorkspacePath)) { + return createErrorResponse(ApiErrorCode.FORBIDDEN, 'destWorkspacePath is outside your workspace'); + } + const timestamp = Date.now(); let result; try { @@ -740,7 +764,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config }); // Link an existing folder as a case - app.post('/api/cases/link', async (req): Promise> => { + app.post('/api/cases/link', async (req, reply): Promise> => { + // Linking writes an arbitrary absolute path into the shared ownerless registry: + // admin-only in multi-user mode (mirrors host CRUD + the admin-only GET listing). + const denied = adminOnly(req, reply); + if (denied) return denied; const { name, path: folderPath } = parseBody(LinkCaseSchema, req.body, 'Invalid request body'); // Expand ~ to home directory @@ -787,27 +815,31 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config app.delete('/api/cases/:name', async (req): Promise> => { const { name } = req.params as { name: string }; + const user = getAuthUser(req); - if (!validatePathWithinBase(name, resolveCasesDir(getAuthUser(req)))) { + if (!validatePathWithinBase(name, resolveCasesDir(user))) { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case name'); } + // Fold ownership INTO the match (don't early-return): a non-owned same-named remote/ + // docker case is skipped so control falls through to the caller's own local delete. + // canAccessOwned is all-true for admins/single-user, so flag-OFF stays byte-identical. const remoteCases = await readRemoteCases(CODEMAN_CONFIG_DIR); - if (remoteCases.some((item) => item.name === name)) { + if (remoteCases.some((item) => item.name === name && canAccessOwned(user, item.owner))) { await writeRemoteCases( CODEMAN_CONFIG_DIR, - remoteCases.filter((item) => item.name !== name) + remoteCases.filter((item) => !(item.name === name && canAccessOwned(user, item.owner))) ); ctx.broadcast(SseEvent.CaseDeleted, { name, type: 'remote-unlinked' }); return { success: true, data: { name } }; } const dockerCases = await readDockerCases(CODEMAN_CONFIG_DIR); - const dockerCase = dockerCases.find((item) => item.name === name); + const dockerCase = dockerCases.find((item) => item.name === name && canAccessOwned(user, item.owner)); if (dockerCase) { await writeDockerCases( CODEMAN_CONFIG_DIR, - dockerCases.filter((item) => item.name !== name) + dockerCases.filter((item) => item !== dockerCase) ); // Best-effort `docker rm -f` the per-case container (case-delete is the // explicit teardown that removes it; the bind-mounted workspace survives). @@ -828,9 +860,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config return { success: true, data: { name } }; } - // Check linked cases first — unlink only, don't delete the actual directory + // Check linked cases first — unlink only, don't delete the actual directory. + // Linked cases carry no owner (admin-only WRITE in multi-user mode), so a non-admin + // must not unlink one either; skip so control falls through to their local delete. const linkedCases = await readLinkedCases(); - if (linkedCases[name]) { + if (linkedCases[name] && (!isMultiUserMode() || isAdmin(req))) { delete linkedCases[name]; try { await fs.writeFile(LINKED_CASES_FILE, JSON.stringify(linkedCases, null, 2)); @@ -888,8 +922,12 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case name'); } + // Fold ownership INTO the match (don't early-return): a non-owned same-named remote/ + // docker case is skipped so control falls through to the caller's own LOCAL case + // (remote/docker names are globally unique, local names per-user). No metadata is + // disclosed for a foreign case. canAccessOwned is allow-all for admins/single-user. const remoteCases = await readRemoteCases(CODEMAN_CONFIG_DIR); - const remoteCase = remoteCases.find((item) => item.name === name); + const remoteCase = remoteCases.find((item) => item.name === name && canAccessOwned(getAuthUser(req), item.owner)); if (remoteCase) { const host = (await readRemoteHosts(CODEMAN_CONFIG_DIR)).find((item) => item.id === remoteCase.hostId); if (!host) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Remote host not found'); @@ -907,7 +945,9 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config }; } - const dockerCase = (await readDockerCases(CODEMAN_CONFIG_DIR)).find((item) => item.name === name); + const dockerCase = (await readDockerCases(CODEMAN_CONFIG_DIR)).find( + (item) => item.name === name && canAccessOwned(getAuthUser(req), item.owner) + ); if (dockerCase) { const host = (await readDockerHosts(CODEMAN_CONFIG_DIR)).find((item) => item.id === dockerCase.hostId); if (!host) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Docker host not found'); diff --git a/src/web/routes/clipboard-routes.ts b/src/web/routes/clipboard-routes.ts index 32f22c4b..ecede0a2 100644 --- a/src/web/routes/clipboard-routes.ts +++ b/src/web/routes/clipboard-routes.ts @@ -5,19 +5,29 @@ import { FastifyInstance } from 'fastify'; import { SseEvent } from '../sse-events.js'; -import type { EventPort } from '../ports/index.js'; +import type { EventPort, SessionPort } from '../ports/index.js'; +import { getAuthUser, canAccessOwned } from '../route-helpers.js'; import { createErrorResponse, ApiErrorCode } from '../../types.js'; -export function registerClipboardRoutes(app: FastifyInstance, ctx: EventPort): void { +export function registerClipboardRoutes(app: FastifyInstance, ctx: EventPort & SessionPort): void { app.post('/api/clipboard', async (req) => { const body = req.body as { text?: string; sessionId?: string }; const text = body?.text; if (typeof text !== 'string' || text.length === 0) { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Missing or empty "text" field'); } + // Multi-user: a supplied sessionId must belong to the caller — never let a + // client target another user's session (no-op in single-user). + if (body.sessionId && !canAccessOwned(getAuthUser(req), ctx.sessions.get(body.sessionId)?.owner)) { + return createErrorResponse(ApiErrorCode.FORBIDDEN, 'Cannot target another user session'); + } ctx.broadcast(SseEvent.ClipboardWrite, { text, sessionId: body.sessionId ?? null, + // Stamp the trusted caller identity so deriveSseHint routes this write to the + // caller's own tabs only (multi-user). Undefined in single-user → JSON drops + // the field and delivery stays global to that one user's browsers. + callerUsername: req.authUser?.username, timestamp: Date.now(), }); return {}; diff --git a/src/web/routes/cron-routes.ts b/src/web/routes/cron-routes.ts index 273a1deb..d98488f7 100644 --- a/src/web/routes/cron-routes.ts +++ b/src/web/routes/cron-routes.ts @@ -9,8 +9,8 @@ import { FastifyInstance } from 'fastify'; import { ApiErrorCode, createErrorResponse } from '../../types.js'; import { CronJobSchema, CronJobUpdateSchema, CronJobEnabledSchema } from '../schemas.js'; -import { canAccessOwned, getAuthUser, ownerFor, parseBody } from '../route-helpers.js'; -import { canRunPrivilegedCommands } from '../../user-store.js'; +import { canAccessOwned, getAuthUser, isWorkingDirAllowed, ownerFor, parseBody } from '../route-helpers.js'; +import { canUsernameRunPrivilegedCommands } from '../../user-store.js'; import { isMultiUserMode } from '../../config/multiuser.js'; import type { CronJob } from '../../types/cron.js'; import type { CronPort } from '../ports/index.js'; @@ -35,8 +35,19 @@ export function registerCronRoutes(app: FastifyInstance, ctx: CronPort): void { // No custom errorMessage: surface the schema's field-specific messages // (e.g. "runAt is required for a one-time schedule"). const body = parseBody(CronJobSchema, req.body); + // Section 6.2: confine the job's workingDir to the owner's case space (mirrors + // POST /api/sessions). No-op allow-all for admins/single-user. workingDir is + // required by CronJobSchema so it is always present here. + if (!isWorkingDirAllowed(getAuthUser(req), body.workingDir)) { + return createErrorResponse(ApiErrorCode.FORBIDDEN, 'workingDir is outside your workspace'); + } // Section 6.3: shell mode / a launchCommand is arbitrary host-account execution. - if ((body.agentType === 'shell' || body.launchCommand) && !canRunPrivilegedCommands(getAuthUser(req))) { + // Resolve the owner's grant from the store (AuthUser.role alone can't tell a GRANTED + // regular user from a plain one); mirrors session-routes + the cron fire-time re-check. + if ( + (body.agentType === 'shell' || body.launchCommand) && + !(await canUsernameRunPrivilegedCommands(ownerFor(req))) + ) { return createErrorResponse( ApiErrorCode.FORBIDDEN, 'Shell/launchCommand cron jobs require the can-bypass-permissions grant' @@ -56,7 +67,14 @@ export function registerCronRoutes(app: FastifyInstance, ctx: CronPort): void { const { id } = req.params as { id: string }; if (!canTouch(req, ctx.cron.getJob(id))) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Cron job not found'); const body = parseBody(CronJobUpdateSchema, req.body); - if ((body.agentType === 'shell' || body.launchCommand) && !canRunPrivilegedCommands(getAuthUser(req))) { + // Section 6.2: the update body is partial, so only confine when workingDir is set. + if (body.workingDir !== undefined && !isWorkingDirAllowed(getAuthUser(req), body.workingDir)) { + return createErrorResponse(ApiErrorCode.FORBIDDEN, 'workingDir is outside your workspace'); + } + if ( + (body.agentType === 'shell' || body.launchCommand) && + !(await canUsernameRunPrivilegedCommands(ownerFor(req))) + ) { return createErrorResponse( ApiErrorCode.FORBIDDEN, 'Shell/launchCommand cron jobs require the can-bypass-permissions grant' @@ -98,10 +116,22 @@ export function registerCronRoutes(app: FastifyInstance, ctx: CronPort): void { app.get('/api/cron/jobs/:id/runs', async (req) => { const { id } = req.params as { id: string }; + // Owner-gate like every other :id handler so a foreign job's run history (session + // ids, names, deep links) isn't leaked; NOT_FOUND avoids disclosing existence. + if (!canTouch(req, ctx.cron.getJob(id))) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Cron job not found'); return ctx.cron.listRuns(id); }); - app.get('/api/cron/runs', async () => { - return ctx.cron.listRuns(); + app.get('/api/cron/runs', async (req) => { + const runs = ctx.cron.listRuns(); + if (!isMultiUserMode()) return runs; + const user = getAuthUser(req); + if (user.role === 'admin') return runs; + // Non-admin: keep only runs whose owning job the caller can access (drops runs + // whose job is absent from the map — defensive; deleteJob already cascades). + const ownerByJobId = new Map( + ctx.cron.listJobs().map((j): [string, string | undefined] => [j.id, j.owner]) + ); + return runs.filter((run) => canAccessOwned(user, ownerByJobId.get(run.cronJobId))); }); } diff --git a/src/web/routes/mux-routes.ts b/src/web/routes/mux-routes.ts index 14b93760..12b22229 100644 --- a/src/web/routes/mux-routes.ts +++ b/src/web/routes/mux-routes.ts @@ -6,9 +6,14 @@ import { FastifyInstance } from 'fastify'; import type { InfraPort } from '../ports/index.js'; import { STATS_COLLECTION_INTERVAL_MS } from '../../config/server-timing.js'; +import { requireAdmin } from '../route-helpers.js'; +import { isMultiUserMode } from '../../config/multiuser.js'; export function registerMuxRoutes(app: FastifyInstance, ctx: InfraPort): void { - app.get('/api/mux-sessions', async () => { + app.get('/api/mux-sessions', async (req, reply) => { + // Multi-user: this recovery/debug surface exposes every user's tmux + workdirs → admin-only + // (requireAdmin is a no-op allow-all in single-user mode, so flag-off is unchanged). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const sessions = await ctx.mux.getSessionsWithStats(); return { sessions, @@ -16,23 +21,31 @@ export function registerMuxRoutes(app: FastifyInstance, ctx: InfraPort): void { }; }); - app.delete('/api/mux-sessions/:sessionId', async (req) => { + app.delete('/api/mux-sessions/:sessionId', async (req, reply) => { + // Multi-user: killing any tmux session by name is a cross-user destructive action → admin-only. + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const { sessionId } = req.params as { sessionId: string }; const success = await ctx.mux.killSession(sessionId); return { killed: success }; }); - app.post('/api/mux-sessions/reconcile', async () => { + app.post('/api/mux-sessions/reconcile', async (req, reply) => { + // Multi-user: process-wide reconcile → admin-only. + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const result = await ctx.mux.reconcileSessions(); return result; }); - app.post('/api/mux-sessions/stats/start', async () => { + app.post('/api/mux-sessions/stats/start', async (req, reply) => { + // Multi-user: process-wide stats collection toggle → admin-only. + if (isMultiUserMode() && !requireAdmin(req, reply)) return; ctx.mux.startStatsCollection(STATS_COLLECTION_INTERVAL_MS); return {}; }); - app.post('/api/mux-sessions/stats/stop', async () => { + app.post('/api/mux-sessions/stats/stop', async (req, reply) => { + // Multi-user: process-wide stats collection toggle → admin-only. + if (isMultiUserMode() && !requireAdmin(req, reply)) return; ctx.mux.stopStatsCollection(); return {}; }); diff --git a/src/web/routes/orchestrator-routes.ts b/src/web/routes/orchestrator-routes.ts index fa0d4f6f..1fce72d0 100644 --- a/src/web/routes/orchestrator-routes.ts +++ b/src/web/routes/orchestrator-routes.ts @@ -19,7 +19,8 @@ import { FastifyInstance } from 'fastify'; import { ApiErrorCode, createErrorResponse, getErrorMessage } from '../../types.js'; import { OrchestratorStartSchema, OrchestratorRejectSchema } from '../schemas.js'; -import { parseBody } from '../route-helpers.js'; +import { parseBody, requireAdmin } from '../route-helpers.js'; +import { isMultiUserMode } from '../../config/multiuser.js'; import { SseEvent } from '../sse-events.js'; import type { EventPort, OrchestratorPort } from '../ports/index.js'; @@ -79,7 +80,10 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat // Start // ═══════════════════════════════════════════════════════════════ - app.post('/api/orchestrator/start', async (req) => { + app.post('/api/orchestrator/start', async (req, reply) => { + // Multi-user: the orchestrator is a process-wide singleton with no per-user + // isolation → admin-only (requireAdmin is a no-op allow-all in single-user mode). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const { goal, config } = parseBody(OrchestratorStartSchema, req.body, 'Invalid request body'); // Initialize loop if needed @@ -115,7 +119,9 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat // Approve / Reject Plan // ═══════════════════════════════════════════════════════════════ - app.post('/api/orchestrator/approve', async () => { + app.post('/api/orchestrator/approve', async (req, reply) => { + // Multi-user: shared-singleton orchestrator → admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const loop = getLoop(); try { @@ -128,7 +134,9 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat } }); - app.post('/api/orchestrator/reject', async (req) => { + app.post('/api/orchestrator/reject', async (req, reply) => { + // Multi-user: shared-singleton orchestrator → admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const loop = getLoop(); const { feedback } = parseBody(OrchestratorRejectSchema, req.body, 'Feedback is required'); @@ -147,7 +155,9 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat // Pause / Resume / Stop // ═══════════════════════════════════════════════════════════════ - app.post('/api/orchestrator/pause', async () => { + app.post('/api/orchestrator/pause', async (req, reply) => { + // Multi-user: shared-singleton orchestrator → admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const loop = getLoop(); try { @@ -158,7 +168,9 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat } }); - app.post('/api/orchestrator/resume', async () => { + app.post('/api/orchestrator/resume', async (req, reply) => { + // Multi-user: shared-singleton orchestrator → admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const loop = getLoop(); try { @@ -171,7 +183,9 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat } }); - app.post('/api/orchestrator/stop', async () => { + app.post('/api/orchestrator/stop', async (req, reply) => { + // Multi-user: shared-singleton orchestrator → admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const loop = getLoop(); try { @@ -186,7 +200,9 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat // Status / Plan // ═══════════════════════════════════════════════════════════════ - app.get('/api/orchestrator/status', async () => { + app.get('/api/orchestrator/status', async (req, reply) => { + // Multi-user: shared-singleton orchestrator → admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const loop = ctx.orchestratorLoop; if (!loop) { return { ok: true, state: 'idle', plan: null, stats: null }; @@ -198,7 +214,9 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat }; }); - app.get('/api/orchestrator/plan', async () => { + app.get('/api/orchestrator/plan', async (req, reply) => { + // Multi-user: shared-singleton orchestrator → admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const loop = ctx.orchestratorLoop; if (!loop) { return { ok: true, plan: null }; @@ -215,7 +233,9 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat // Phase Operations // ═══════════════════════════════════════════════════════════════ - app.post('/api/orchestrator/phase/:id/skip', async (req) => { + app.post('/api/orchestrator/phase/:id/skip', async (req, reply) => { + // Multi-user: shared-singleton orchestrator → admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const loop = getLoop(); const { id } = req.params as { id: string }; @@ -227,7 +247,9 @@ export function registerOrchestratorRoutes(app: FastifyInstance, ctx: Orchestrat } }); - app.post('/api/orchestrator/phase/:id/retry', async (req) => { + app.post('/api/orchestrator/phase/:id/retry', async (req, reply) => { + // Multi-user: shared-singleton orchestrator → admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const loop = getLoop(); const { id } = req.params as { id: string }; diff --git a/src/web/routes/plan-routes.ts b/src/web/routes/plan-routes.ts index 2bc347e4..09d257d8 100644 --- a/src/web/routes/plan-routes.ts +++ b/src/web/routes/plan-routes.ts @@ -261,7 +261,18 @@ NOW: Generate the implementation plan for the task above. Think step by step.`; } const detailedModelConfig = await ctx.getModelConfig(); - const orchestrator = new PlanOrchestrator(ctx.mux, process.cwd(), outputDir, detailedModelConfig ?? undefined); + // Section 6.3: resolve the owner's permission mode (mirrors /api/generate-plan above) and + // thread it + owner + allowedTools into the orchestrator's internal research/planner one-shots + // so a non-granted multi-user user cannot run them under --dangerously-skip-permissions. + // In single-user, resolveClaudeModeForUsername returns the global mode = byte-identical. + const detailedOwner = ownerFor(req); + const detailedClaudeModeConfig = await ctx.getClaudeModeConfig(); + const detailedClaudeMode = await resolveClaudeModeForUsername(detailedClaudeModeConfig.claudeMode, detailedOwner); + const orchestrator = new PlanOrchestrator(ctx.mux, process.cwd(), outputDir, detailedModelConfig ?? undefined, { + claudeMode: detailedClaudeMode, + owner: detailedOwner, + allowedTools: detailedClaudeModeConfig.allowedTools, + }); // Store orchestrator for potential cancellation via API (not on disconnect) // Plan generation continues even if browser disconnects - only explicit cancel stops it diff --git a/src/web/routes/push-routes.ts b/src/web/routes/push-routes.ts index 0fb27b93..a45f88db 100644 --- a/src/web/routes/push-routes.ts +++ b/src/web/routes/push-routes.ts @@ -24,6 +24,10 @@ export function registerPushRoutes(app: FastifyInstance, ctx: InfraPort): void { userAgent: userAgent ?? req.headers['user-agent'] ?? '', createdAt: Date.now(), pushPreferences: pushPreferences ?? {}, + // Multi-user: stamp the trusted caller identity so sendPushNotifications can + // scope session notifications to the owner (+ admins). Undefined in single-user. + username: req.authUser?.username, + role: req.authUser?.role, }); return { success: true, data: { id: record.id } }; }); diff --git a/src/web/routes/respawn-routes.ts b/src/web/routes/respawn-routes.ts index c5533668..56543997 100644 --- a/src/web/routes/respawn-routes.ts +++ b/src/web/routes/respawn-routes.ts @@ -8,7 +8,7 @@ import { ApiErrorCode, createErrorResponse, getErrorMessage, type PersistedRespa import { RespawnController, type RespawnConfig } from '../../respawn-controller.js'; import { RespawnConfigSchema, InteractiveRespawnSchema, RespawnEnableSchema } from '../schemas.js'; import { SseEvent } from '../sse-events.js'; -import { findSessionOrFail, autoConfigureRalph, parseBody } from '../route-helpers.js'; +import { findSessionOrFail, autoConfigureRalph, parseBody, canAccessOwned, getAuthUser } from '../route-helpers.js'; import type { SessionPort, EventPort, RespawnPort, ConfigPort, InfraPort } from '../ports/index.js'; import { getLifecycleLog } from '../../session-lifecycle-log.js'; import { isExternalCliMode } from '../../session.js'; @@ -46,7 +46,11 @@ export function registerRespawnRoutes( const { id } = req.params as { id: string }; const controller = ctx.respawnControllers.get(id); - if (!controller) { + // Multi-user: gate on the owner from the same source the data comes from, and + // return the existing neutral shape (not 404) when foreign so existence isn't + // leaked. canAccessOwned is allow-all in single-user mode → byte-identical. + const owner = ctx.sessions.get(id)?.owner ?? ctx.mux.getSession(id)?.owner; + if (!controller || !canAccessOwned(getAuthUser(req), owner)) { return { enabled: false, status: null }; } @@ -60,16 +64,21 @@ export function registerRespawnRoutes( app.get('/api/sessions/:id/respawn/config', async (req) => { const { id } = req.params as { id: string }; + // Multi-user: owner-gate each branch against the source of the data, preserving + // the neutral {config:null,active:false} shape when foreign (no existence leak). + // canAccessOwned is allow-all in single-user mode → byte-identical, and this keeps + // the mux-only pre-config path working (findSessionOrFail would break it). + const user = getAuthUser(req); const controller = ctx.respawnControllers.get(id); - if (controller) { + if (controller && canAccessOwned(user, ctx.sessions.get(id)?.owner)) { return { config: controller.getConfig(), active: true }; } // Return pre-saved config from mux-sessions.json - const preConfig = ctx.mux.getSession(id)?.respawnConfig; - if (preConfig) { - return { config: preConfig, active: false }; + const mux = ctx.mux.getSession(id); + if (mux?.respawnConfig && canAccessOwned(user, mux.owner)) { + return { config: mux.respawnConfig, active: false }; } return { config: null, active: false }; @@ -122,6 +131,9 @@ export function registerRespawnRoutes( app.post('/api/sessions/:id/respawn/stop', async (req) => { const { id } = req.params as { id: string }; + // Owner-gate before any side effects (matches start/config/enable): a non-owner + // gets NOT_FOUND and never reaches stop/delete/clearRespawnConfig/persist. + const session = findSessionOrFail(ctx, id, req); const controller = ctx.respawnControllers.get(id); if (!controller) { @@ -144,10 +156,7 @@ export function registerRespawnRoutes( ctx.mux.clearRespawnConfig(id); // Update state.json (respawnConfig removed) - const session = ctx.sessions.get(id); - if (session) { - ctx.persistSessionState(session); - } + ctx.persistSessionState(session); ctx.broadcast(SseEvent.RespawnStopped, { sessionId: id }); diff --git a/src/web/routes/scheduled-routes.ts b/src/web/routes/scheduled-routes.ts index 614f3824..0d56fe08 100644 --- a/src/web/routes/scheduled-routes.ts +++ b/src/web/routes/scheduled-routes.ts @@ -7,17 +7,35 @@ import { FastifyInstance } from 'fastify'; import { statSync } from 'node:fs'; import { ApiErrorCode, createErrorResponse, type ApiResponse } from '../../types.js'; import { ScheduledRunSchema } from '../schemas.js'; -import { parseBody } from '../route-helpers.js'; +import { + parseBody, + getAuthUser, + ownerFor, + isWorkingDirAllowed, + canAccessOwned, + resolveCasesDir, +} from '../route-helpers.js'; +import { isMultiUserMode } from '../../config/multiuser.js'; import type { SessionPort, EventPort, InfraPort, ScheduledRun } from '../ports/index.js'; export function registerScheduledRoutes(app: FastifyInstance, ctx: SessionPort & EventPort & InfraPort): void { - app.get('/api/scheduled', async () => { - return Array.from(ctx.scheduledRuns.values()); + app.get('/api/scheduled', async (req) => { + // Multi-user: non-admins see only their own runs (no-op in single-user). + const user = getAuthUser(req); + return Array.from(ctx.scheduledRuns.values()).filter((r) => canAccessOwned(user, r.owner)); }); app.post('/api/scheduled', async (req): Promise<{ run: ScheduledRun } | ApiResponse> => { const { prompt, workingDir, durationMinutes } = parseBody(ScheduledRunSchema, req.body, 'Invalid request body'); + // Multi-user: confine the run's workingDir to the caller's own case space. + // The spawned Session (--dangerously-skip-permissions by default) trusts this + // dir; without confinement a non-admin could point it at another user's files. + // No-op for admins / single-user (isWorkingDirAllowed returns true). + if (workingDir && !isWorkingDirAllowed(getAuthUser(req), workingDir)) { + return createErrorResponse(ApiErrorCode.FORBIDDEN, 'workingDir is not within your allowed workspace'); + } + // Validate workingDir exists and is a directory if (workingDir) { try { @@ -30,7 +48,11 @@ export function registerScheduledRoutes(app: FastifyInstance, ctx: SessionPort & } } - const run = await ctx.startScheduledRun(prompt, workingDir || process.cwd(), durationMinutes ?? 60); + // Multi-user: default a missing workingDir to the user's own cases dir rather + // than the server's cwd. Single-user keeps process.cwd() (byte-identical). + const effectiveWorkingDir = workingDir || (isMultiUserMode() ? resolveCasesDir(getAuthUser(req)) : process.cwd()); + + const run = await ctx.startScheduledRun(prompt, effectiveWorkingDir, durationMinutes ?? 60, ownerFor(req)); return { run }; }); @@ -38,7 +60,9 @@ export function registerScheduledRoutes(app: FastifyInstance, ctx: SessionPort & const { id } = req.params as { id: string }; const run = ctx.scheduledRuns.get(id); - if (!run) { + // NOT_FOUND (not FORBIDDEN) for a foreign run so existence isn't leaked; no-op + // for admins / single-user (canAccessOwned returns true). + if (!run || !canAccessOwned(getAuthUser(req), run.owner)) { return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Scheduled run not found'); } @@ -50,7 +74,8 @@ export function registerScheduledRoutes(app: FastifyInstance, ctx: SessionPort & const { id } = req.params as { id: string }; const run = ctx.scheduledRuns.get(id); - if (!run) { + // Owner-scoped read: a foreign run reads as NOT_FOUND (no-op in single-user). + if (!run || !canAccessOwned(getAuthUser(req), run.owner)) { return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Scheduled run not found'); } diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index a9e38f53..2029dc66 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -17,6 +17,8 @@ import { getErrorMessage, type ApiResponse, type SessionColor, + type CodexConfig, + type GeminiConfig, } from '../../types.js'; import { Session, isAltScreenStripMode } from '../../session.js'; import { SseEvent } from '../sse-events.js'; @@ -42,6 +44,7 @@ import { CASES_DIR, findSessionOrFail, getAuthUser, + isAdmin, isWorkingDirAllowed, ownerFor, parseBody, @@ -51,7 +54,7 @@ import { SETTINGS_PATH, validatePathWithinBase, } from '../route-helpers.js'; -import { canRunPrivilegedCommands, resolveClaudeModeForUsername } from '../../user-store.js'; +import { canUsernameRunPrivilegedCommands, resolveClaudeModeForUsername } from '../../user-store.js'; import { isMultiUserMode } from '../../config/multiuser.js'; import { AUTH_COOKIE_NAME } from '../middleware/auth.js'; import { @@ -266,6 +269,29 @@ export function _resetPasteRateBuckets(): void { pasteRateBuckets.clear(); } +/** + * Security (multi-user §6.3): the Claude-only permission-mode downgrade does not + * cover the other CLIs' bypass switches. Codex `--dangerously-bypass-approvals-and-sandbox` + * and Gemini `--approval-mode yolo` disable the safety classifier the non-granted-user + * downgrade is meant to keep on, so clamp them for a non-granted owner. buildGeminiCommand + * defaults an ABSENT approvalMode to yolo, so the gemini config must be MATERIALIZED + * (auto_edit) even when the request sent none. No-op in single-user mode / for a granted + * owner (canUsernameRunPrivilegedCommands returns true when !isMultiUserMode()). + */ +async function clampExternalCliBypassForOwner( + owner: string | undefined, + codexConfig: CodexConfig | undefined, + geminiConfig: GeminiConfig | undefined +): Promise<{ codexConfig: CodexConfig | undefined; geminiConfig: GeminiConfig | undefined }> { + const granted = await canUsernameRunPrivilegedCommands(owner); + if (granted) return { codexConfig, geminiConfig }; + // Non-granted: force codex bypass off (only meaningful when a config was sent) and + // materialize gemini to auto_edit (clamps an explicit 'yolo' and the yolo default). + const clampedCodex = codexConfig ? { ...codexConfig, dangerouslyBypassApprovals: false } : codexConfig; + const clampedGemini: GeminiConfig = { ...(geminiConfig ?? {}), approvalMode: 'auto_edit' }; + return { codexConfig: clampedCodex, geminiConfig: clampedGemini }; +} + export function registerSessionRoutes( app: FastifyInstance, ctx: SessionPort & EventPort & ConfigPort & InfraPort & AuthPort @@ -312,8 +338,9 @@ export function registerSessionRoutes( const workingDir = body.workingDir || process.cwd(); // Multi-user: shell mode is arbitrary command execution as the host account, - // gated behind the same grant as bypass (section 6.3). - if (body.mode === 'shell' && !canRunPrivilegedCommands(getAuthUser(req))) { + // gated behind the same grant as bypass (section 6.3). Resolve the owner's grant + // from the store so a GRANTED regular user is not wrongly denied (AuthUser role alone can't tell). + if (body.mode === 'shell' && !(await canUsernameRunPrivilegedCommands(owner))) { return createErrorResponse(ApiErrorCode.FORBIDDEN, 'Shell sessions require the can-bypass-permissions grant'); } @@ -463,6 +490,12 @@ export function registerSessionRoutes( const claudeModeConfig = await ctx.getClaudeModeConfig(); // Section 6.3: force non-granted users to a classifier-guarded mode. const effectiveClaudeMode = await resolveClaudeModeForUsername(claudeModeConfig.claudeMode, owner); + // Section 6.3: clamp Codex/Gemini bypass switches for a non-granted owner (no-op single-user/granted). + const { codexConfig: gatedCodexConfig, geminiConfig: gatedGeminiConfig } = await clampExternalCliBypassForOwner( + owner, + body.codexConfig, + body.geminiConfig + ); const terminalHistoryConfig = await ctx.getTerminalHistoryConfig(); const session = new Session({ workingDir, @@ -475,8 +508,8 @@ export function registerSessionRoutes( claudeMode: effectiveClaudeMode, allowedTools: claudeModeConfig.allowedTools, openCodeConfig: mode === 'opencode' ? body.openCodeConfig : undefined, - codexConfig: mode === 'codex' ? body.codexConfig : undefined, - geminiConfig: mode === 'gemini' ? body.geminiConfig : undefined, + codexConfig: mode === 'codex' ? gatedCodexConfig : undefined, + geminiConfig: mode === 'gemini' ? gatedGeminiConfig : undefined, resumeSessionId: validatedResumeId, envOverrides: body.envOverrides, effort: body.effort, @@ -536,18 +569,22 @@ export function registerSessionRoutes( const query = req.query as { killMux?: string }; const killMux = query.killMux !== 'false'; // Default to true - if (!ctx.sessions.has(id)) { - return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Session not found'); - } + // Security: owner-scoped lookup 404s foreign/missing sessions uniformly (no existence leak, no cross-user kill). + const session = findSessionOrFail(ctx, id, req); - await ctx.cleanupSession(id, killMux, 'user_delete'); + await ctx.cleanupSession(session.id, killMux, 'user_delete'); return {}; }); // ========== Delete All Sessions ========== - app.delete('/api/sessions', async (): Promise> => { - const sessionIds = Array.from(ctx.sessions.keys()); + app.delete('/api/sessions', async (req): Promise> => { + // Security: scope the bulk sweep to sessions the caller can access — a non-admin + // must not wipe other users' sessions (canAccessOwned is allow-all for admin/single-user). + const user = getAuthUser(req); + const sessionIds = Array.from(ctx.sessions.values()) + .filter((s) => canAccessOwned(user, s.owner)) + .map((s) => s.id); let killed = 0; for (const id of sessionIds) { @@ -1724,7 +1761,8 @@ export function registerSessionRoutes( } = parseBody(QuickStartSchema, req.body); // Multi-user: shell mode is arbitrary host-account execution, gated by the grant. - if (mode === 'shell' && !canRunPrivilegedCommands(getAuthUser(req))) { + // Resolve the owner's grant from the store so a GRANTED regular user is not wrongly denied. + if (mode === 'shell' && !(await canUsernameRunPrivilegedCommands(owner))) { return createErrorResponse(ApiErrorCode.FORBIDDEN, 'Shell sessions require the can-bypass-permissions grant'); } @@ -1735,11 +1773,20 @@ export function registerSessionRoutes( let docker = undefined; let dockerResumeId: string | undefined; let casePath: string | null = null; + // Security: fold ownership INTO the match (don't early-return) so a NON-OWNED + // same-named remote/docker case is skipped and control falls through to the caller's + // own LOCAL case — remote/docker names are globally unique but local names are + // per-user, so a name collision must not shadow the caller's own case. canAccessOwned + // is allow-all for admins/single-user, so flag-OFF stays byte-identical. const remoteCases = await readRemoteCases(CODEMAN_CONFIG_DIR); - const remoteCase = remoteCases.find((item) => item.name === caseName); + const remoteCase = remoteCases.find( + (item) => item.name === caseName && canAccessOwned(getAuthUser(req), item.owner) + ); const dockerCase = remoteCase ? undefined - : (await readDockerCases(CODEMAN_CONFIG_DIR)).find((item) => item.name === caseName); + : (await readDockerCases(CODEMAN_CONFIG_DIR)).find( + (item) => item.name === caseName && canAccessOwned(getAuthUser(req), item.owner) + ); if (remoteCase) { const host = (await readRemoteHosts(CODEMAN_CONFIG_DIR)).find((item) => item.id === remoteCase.hostId); if (!host) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Remote host not found'); @@ -1775,7 +1822,7 @@ export function registerSessionRoutes( // Docker case: the CLI executes INSIDE a container via local tmux + `docker // exec`, so the LOCAL availability gates below don't apply. Mirror the remote // branch's rejection of per-session config that would not cross into the - // container (it would silently no-op). + // container (it would silently no-op). (Ownership is enforced in the .find above.) const host = (await readDockerHosts(CODEMAN_CONFIG_DIR)).find((item) => item.id === dockerCase.hostId); if (!host) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Docker host not found'); if ( @@ -1872,8 +1919,11 @@ export function registerSessionRoutes( } catch { // File missing or unparseable — treat as empty registry } - // Multi-user: resolve local cases inside the requesting user's case space. - casePath = linkedCases[caseName] || validatePathWithinBase(caseName, resolveCasesDir(getAuthUser(req))); + // Multi-user: the linked-cases registry is ownerless/global, so only admins may + // resolve a name to an arbitrary linked path. A non-admin resolves inside their + // OWN case space only (single-user: isAdmin true, so linked cases still honoured). + const linked = isAdmin(req) ? linkedCases[caseName] : undefined; + casePath = linked || validatePathWithinBase(caseName, resolveCasesDir(getAuthUser(req))); if (!casePath) { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case path'); } @@ -1883,6 +1933,15 @@ export function registerSessionRoutes( // for local cases the !casePath guard above returned early. TypeScript can't narrow across the if/else. const resolvedCasePath = casePath as string; + // Multi-user linchpin (section 6.2): confine the resolved workingDir to the caller's + // own case space BEFORE any mkdir/scaffold below creates or mutates it. Applies to + // LOCAL and DOCKER cases (docker.hostWorkspacePath is a real host dir the file routes + // trust); skipped for REMOTE, whose path is an ssh path that would spuriously fail + // realpath confinement. No-op for admins / single-user mode. + if (!remote && !isWorkingDirAllowed(getAuthUser(req), resolvedCasePath)) { + return createErrorResponse(ApiErrorCode.FORBIDDEN, 'case path is outside your workspace'); + } + // Create case folder and CLAUDE.md if it doesn't exist (only for non-linked, non-remote, // non-docker cases — docker workspaces are scaffolded in their own block below) if (!remote && !docker && !existsSync(resolvedCasePath)) { @@ -1962,6 +2021,12 @@ export function registerSessionRoutes( : undefined; const qsClaudeModeConfig = await ctx.getClaudeModeConfig(); const qsEffectiveClaudeMode = await resolveClaudeModeForUsername(qsClaudeModeConfig.claudeMode, owner); + // Section 6.3: clamp Codex/Gemini bypass switches for a non-granted owner (no-op single-user/granted). + const { codexConfig: qsGatedCodexConfig, geminiConfig: qsGatedGeminiConfig } = await clampExternalCliBypassForOwner( + owner, + codexConfig, + geminiConfig + ); const qsTerminalHistoryConfig = await ctx.getTerminalHistoryConfig(); const session = new Session({ workingDir: resolvedCasePath, @@ -1975,8 +2040,8 @@ export function registerSessionRoutes( allowedTools: qsClaudeModeConfig.allowedTools, owner, openCodeConfig: mode === 'opencode' ? openCodeConfig : undefined, - codexConfig: mode === 'codex' ? codexConfig : undefined, - geminiConfig: mode === 'gemini' ? geminiConfig : undefined, + codexConfig: mode === 'codex' ? qsGatedCodexConfig : undefined, + geminiConfig: mode === 'gemini' ? qsGatedGeminiConfig : undefined, envOverrides, effort, remote, @@ -2319,6 +2384,12 @@ export function registerSessionRoutes( const query = req.query as { projectKey?: string; offset?: string; limit?: string }; const projectsDir = join(process.env.HOME || '/tmp', '.claude', 'projects'); const headBuf = Buffer.alloc(16384); + // Multi-user: this scans the host-wide ~/.claude/projects tree, so a non-admin + // must only see history whose decoded workingDir is inside their own case space. + // Do NOT trust the caller-supplied projectKey — confine on the decoded path. + // No-op for admins / single-user mode. + const user = getAuthUser(req); + const scopeHistory = isMultiUserMode() && user.role !== 'admin'; // Single-folder drill-down: when projectKey is provided, scan only that // directory, bypass the 50-cap, and honor offset/limit pagination. @@ -2330,13 +2401,15 @@ export function registerSessionRoutes( const offset = Math.max(0, parseInt(query.offset || '0', 10) || 0); const limit = Math.min(100, Math.max(1, parseInt(query.limit || '20', 10) || 20)); const projPath = join(projectsDir, query.projectKey); - const all = await scanProjectDir(projPath, query.projectKey, headBuf); + let all = await scanProjectDir(projPath, query.projectKey, headBuf); + // Confine to the caller's workspace (a projectKey maps to a single foreign cwd). + if (scopeHistory) all = all.filter((r) => isWorkingDirAllowed(user, r.workingDir)); all.sort((a, b) => new Date(b.lastModified).getTime() - new Date(a.lastModified).getTime()); return { sessions: all.slice(offset, offset + limit), total: all.length }; } // Global overview: scan all projects, return up to 50 most-recent sessions. - const results: HistorySession[] = []; + let results: HistorySession[] = []; try { const projectDirs = await fs.readdir(projectsDir); for (const projDir of projectDirs) { @@ -2348,6 +2421,8 @@ export function registerSessionRoutes( // Projects dir may not exist } + // Multi-user: drop rows outside the non-admin caller's own case space. + if (scopeHistory) results = results.filter((r) => isWorkingDirAllowed(user, r.workingDir)); results.sort((a, b) => new Date(b.lastModified).getTime() - new Date(a.lastModified).getTime()); return { sessions: results.slice(0, 50) }; }); diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index 1730fa3f..021791fe 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -17,7 +17,7 @@ import { ApiErrorCode, createErrorResponse, getErrorMessage, type NiceConfig } f import { isUnauthenticatedNetworkAcknowledged } from '../network-auth-policy.js'; import { isMultiUserMode } from '../../config/multiuser.js'; import { findUser } from '../../user-store.js'; -import { getAuthUser } from '../route-helpers.js'; +import { getAuthUser, requireAdmin, canAccessOwned } from '../route-helpers.js'; import { ConfigUpdateSchema, SettingsUpdateSchema, @@ -224,14 +224,16 @@ export function registerSystemRoutes( } // Resolve the role for the bound user (disabled/deleted users fail closed). - let identity: { username: string; role: 'admin' | 'user' } | undefined; + // Carry the bound user's real mustChangePassword flag out of this block so the + // minted cookie enforces the lockbox instead of hardcoding false. + let identity: { username: string; role: 'admin' | 'user'; mustChangePassword: boolean } | undefined; if (multiUser && consumed.username) { const user = await findUser(consumed.username); if (!user || user.disabled) { ctx.qrAuthFailures?.set(clientIp, qrFailures + 1); return reply.code(401).send('Invalid or expired QR code'); } - identity = { username: user.username, role: user.role }; + identity = { username: user.username, role: user.role, mustChangePassword: !!user.mustChangePassword }; } // Issue session cookie (same pattern as Basic Auth success path) @@ -244,7 +246,7 @@ export function registerSystemRoutes( method: 'qr', username: identity?.username, role: identity?.role, - mustChangePassword: false, + mustChangePassword: !!identity?.mustChangePassword, }); ctx.qrAuthFailures?.delete(clientIp); @@ -493,23 +495,52 @@ export function registerSystemRoutes( limit: 1000, }); - const sessions: AwayDigestSession[] = Array.from(ctx.sessions.values()).map((session) => ({ - id: session.id, - name: session.name, - status: session.status, - inputTokens: session.inputTokens, - outputTokens: session.outputTokens, - totalCost: session.totalCost, - })); + // Multi-user: scope the digest's aggregated activity to sessions the caller + // owns (canAccessOwned is a no-op allow-all for admins/single-user). + const user = getAuthUser(req); + const sessions: AwayDigestSession[] = Array.from(ctx.sessions.values()) + .filter((session) => canAccessOwned(user, session.owner)) + .map((session) => ({ + id: session.id, + name: session.name, + status: session.status, + inputTokens: session.inputTokens, + outputTokens: session.outputTokens, + totalCost: session.totalCost, + })); - const runSummaries = Array.from(ctx.runSummaryTrackers.values()).map((tracker) => tracker.getSummary()); + // Run-summary trackers are keyed by Codeman session id → filter by that session's owner. + const runSummaries = Array.from(ctx.runSummaryTrackers.entries()) + .filter(([id]) => canAccessOwned(user, ctx.sessions.get(id)?.owner)) + .map(([, tracker]) => tracker.getSummary()); + + // Map each subagent's Claude conversation id back to its owning session so the + // recent-subagent lookback is owner-scoped too (fails closed when unattributable). + const ownerByClaudeSessionId = new Map(); + for (const s of ctx.sessions.values()) { + if (s.claudeSessionId) ownerByClaudeSessionId.set(s.claudeSessionId, s.owner); + } + const subagents = subagentWatcher + .getRecentSubagents(60) + .filter((sa) => canAccessOwned(user, ownerByClaudeSessionId.get(sa.sessionId))) as AwayDigestSubagent[]; + + // Multi-user: the lifecycle log and daily token stats carry no owner, so scope them + // for a non-admin: keep only lifecycle entries attributable to an owned LIVE session + // (fail closed — an ended session's owner can't be resolved, so it is dropped rather + // than leaked), and withhold the machine-wide daily token totals entirely (they can't + // be per-user attributed, same as globalStats in #29). Admins/single-user keep all + // (canAccessOwned allow-all, role check false → byte-identical). + const scopedLifecycle = lifecycleEntries.filter((e) => + canAccessOwned(user, ctx.sessions.get(e.sessionId ?? '')?.owner) + ); + const nonAdminScoped = isMultiUserMode() && user.role !== 'admin'; const digest = buildAwayDigest({ range, - lifecycleEntries, + lifecycleEntries: scopedLifecycle, runSummaries, sessions, - dailyTokenStats: ctx.store.getDailyStats(30), - subagents: subagentWatcher.getRecentSubagents(60) as AwayDigestSubagent[], + dailyTokenStats: nonAdminScoped ? [] : ctx.store.getDailyStats(30), + subagents, now: range.until, }); @@ -827,7 +858,10 @@ export function registerSystemRoutes( // ========== Workflow Run Monitoring (ultracode) ========== // LEFT-pane list: lightweight run summaries (no agents[]). - app.get('/api/workflows', async (req) => { + app.get('/api/workflows', async (req, reply) => { + // Multi-user stopgap: these aggregates are process-wide (no owner concept), so + // restrict cross-user reads to admins (no-op allow-all in single-user mode). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const { minutes } = req.query as { minutes?: string }; const runs = minutes ? workflowRunWatcher.getRecentRunSummaries(parseInt(minutes, 10)) @@ -836,7 +870,9 @@ export function registerSystemRoutes( }); // RIGHT-pane detail: full run incl. agents[] (tokens/toolCalls/state per agent). - app.get('/api/workflows/:runId', async (req) => { + app.get('/api/workflows/:runId', async (req, reply) => { + // Multi-user stopgap: cross-user run detail is admin-only (no-op in single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const { runId } = req.params as { runId: string }; const run = workflowRunWatcher.getRun(runId); if (!run) { @@ -847,7 +883,10 @@ export function registerSystemRoutes( // ========== Subagent Monitoring ========== - app.get('/api/subagents', async (req) => { + app.get('/api/subagents', async (req, reply) => { + // Multi-user stopgap: the global subagent list spans all users → admin-only + // (no-op allow-all in single-user mode). Per-session variant below stays scoped. + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const { minutes } = req.query as { minutes?: string }; const subagents = minutes ? subagentWatcher.getRecentSubagents(parseInt(minutes, 10)) @@ -862,7 +901,9 @@ export function registerSystemRoutes( return { success: true, data: subagents }; }); - app.get('/api/subagents/:agentId', async (req) => { + app.get('/api/subagents/:agentId', async (req, reply) => { + // Multi-user stopgap: cross-user subagent metadata is admin-only (no-op single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const { agentId } = req.params as { agentId: string }; const info = subagentWatcher.getSubagent(agentId); if (!info) { @@ -871,7 +912,10 @@ export function registerSystemRoutes( return { success: true, data: info }; }); - app.get('/api/subagents/:agentId/transcript', async (req) => { + app.get('/api/subagents/:agentId/transcript', async (req, reply) => { + // Multi-user stopgap: transcript CONTENT of any user's subagent is admin-only + // (no-op allow-all in single-user mode). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const { agentId } = req.params as { agentId: string }; const { limit, format } = req.query as { limit?: string; format?: 'raw' | 'formatted' }; const limitNum = limit ? parseInt(limit, 10) : undefined; @@ -885,7 +929,10 @@ export function registerSystemRoutes( return { success: true, data: transcript }; }); - app.delete('/api/subagents/:agentId', async (req) => { + app.delete('/api/subagents/:agentId', async (req, reply) => { + // Multi-user stopgap: killing any user's subagent is a cross-user write → admin-only + // (no-op allow-all in single-user mode). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const { agentId } = req.params as { agentId: string }; const info = subagentWatcher.getSubagent(agentId); if (!info) { @@ -899,12 +946,16 @@ export function registerSystemRoutes( return createErrorResponse(ApiErrorCode.OPERATION_FAILED, 'Subagent not found or already completed'); }); - app.post('/api/subagents/cleanup', async () => { + app.post('/api/subagents/cleanup', async (req, reply) => { + // Multi-user stopgap: process-wide cleanup affects every user → admin-only (no-op single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const removed = subagentWatcher.cleanupNow(); return { success: true, data: { removed, remaining: subagentWatcher.getSubagents().length } }; }); - app.delete('/api/subagents', async () => { + app.delete('/api/subagents', async (req, reply) => { + // Multi-user stopgap: clearing ALL users' subagents is a cross-user write → admin-only (no-op single-user). + if (isMultiUserMode() && !requireAdmin(req, reply)) return; const cleared = subagentWatcher.clearAll(); return { success: true, data: { cleared } }; }); diff --git a/src/web/server.ts b/src/web/server.ts index a82e818e..d07abd55 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1554,7 +1554,12 @@ export class WebServer extends EventEmitter { ); } - private async startScheduledRun(prompt: string, workingDir: string, durationMinutes: number): Promise { + private async startScheduledRun( + prompt: string, + workingDir: string, + durationMinutes: number, + owner?: string + ): Promise { const id = uuidv4(); const now = Date.now(); @@ -1570,6 +1575,9 @@ export class WebServer extends EventEmitter { completedTasks: 0, totalCost: 0, logs: [`[${new Date().toISOString()}] Scheduled run started`], + // Multi-user: stamp the requesting user so the spawned Session is owned + + // permission-downgraded, and list/delete stay owner-scoped. + owner, }; this.scheduledRuns.set(id, run); @@ -1608,8 +1616,23 @@ export class WebServer extends EventEmitter { let session: Session | null = null; try { - // Create a session for this iteration - session = new Session({ workingDir: run.workingDir }); + // Create a session for this iteration. + if (isMultiUserMode()) { + // §6.3: resolve the permission mode with the RUN OWNER (a non-granted user + // must not regain --dangerously-skip-permissions here) and stamp the owner so + // list/delete stay scoped. owner + mode + allowedTools mirror quick-start. + const scheduledClaudeCfg = await this.getClaudeModeConfig(); + session = new Session({ + workingDir: run.workingDir, + owner: run.owner, + claudeMode: await resolveClaudeModeForUsername(scheduledClaudeCfg.claudeMode, run.owner), + allowedTools: scheduledClaudeCfg.allowedTools, + }); + } else { + // Single-user: build EXACTLY as master (bare workingDir → Session's default + // mode) so the flag-off path stays byte-identical. + session = new Session({ workingDir: run.workingDir }); + } this.sessions.set(session.id, session); this.store.incrementSessionsCreated(); this.persistSessionState(session); @@ -1785,7 +1808,7 @@ export class WebServer extends EventEmitter { Array.isArray(arr) ? (arr as Array>).filter((x) => ownedClaudeIds.has(String(x[key]))) : arr; - return { + const filtered: Record = { ...base, sessions, respawnStatus, @@ -1794,6 +1817,11 @@ export class WebServer extends EventEmitter { workflowRuns: bySession(base.workflowRuns, 'sessionUuid'), planUsage: null, // host-plan telemetry is admin-only }; + // #29: globalStats is a machine-wide aggregate (all users' tokens/cost + active + // count) with no per-user attribution — never expose it to a non-admin. The + // header falls back to per-active-session totals when it is absent. + delete filtered.globalStats; + return filtered; } private computeLightState() { @@ -1890,6 +1918,14 @@ export class WebServer extends EventEmitter { const owner = sessionId ? this.sessions.get(sessionId)?.owner : undefined; return { owner, sessionScoped: true }; } + // #20/#38: clipboard:write writes into the receiver's OS clipboard — route it to + // the POSTING user's own tabs only (never other users). The route stamps the + // trusted caller identity as `callerUsername`. sessionScoped:true fails closed + // (withhold from non-admins) if the caller identity is somehow unresolved, rather + // than falling through to global delivery. + if (event.startsWith('clipboard:')) { + return { username: (data as { callerUsername?: string }).callerUsername, sessionScoped: true }; + } // Unrecognized / genuinely global events (connection status, needsRefresh): all. return undefined; } @@ -1948,6 +1984,13 @@ export class WebServer extends EventEmitter { const sessionName = (data.sessionName as string) || ''; const sessionId = (data.sessionId as string) || ''; + // Multi-user: a session-scoped push (all PUSH_EVENT_MAP events carry a sessionId) + // must reach only the owner's devices (+ admins) — the body embeds the session + // name + activity, so cross-user delivery would leak it. Resolved once here; the + // per-subscription gate below is a no-op in single-user (send to all). + const multiUserPush = isMultiUserMode(); + const pushSessionOwner = sessionId ? this.sessions.get(sessionId)?.owner : undefined; + // Build body text from event data let body = sessionName ? `[${sessionName}]` : ''; if (event === SseEvent.SessionError && data.error) { @@ -1984,6 +2027,16 @@ export class WebServer extends EventEmitter { // Check per-subscription preferences if (sub.pushPreferences[event] === false) continue; + // Multi-user recipient scoping: admins receive all; a session-scoped event + // reaches only subscriptions owned by the session owner (fail closed if the + // owner is unresolved — legacy subs with no stamped username are excluded); + // a genuinely session-less event reaches everyone. + if (multiUserPush && sub.role !== 'admin') { + if (sessionId) { + if (sub.username === undefined || sub.username !== pushSessionOwner) continue; + } + } + // Re-validate the stored endpoint before fetching it server-side (SSRF, M7). // Defense-in-depth: subscribe-time validation already rejects unsafe URLs. if (!isSafePushEndpoint(sub.endpoint)) { diff --git a/src/web/sse-stream-manager.ts b/src/web/sse-stream-manager.ts index cb3592f4..32c65f71 100644 --- a/src/web/sse-stream-manager.ts +++ b/src/web/sse-stream-manager.ts @@ -413,7 +413,11 @@ export class SseStreamManager { return; } for (const [, { sessionId, task }] of this.taskUpdateBatches) { - this.broadcast(SseEvent.TaskUpdated, { sessionId, task }); + // Multi-user: batched task updates carry session state — route to the owner + // only (fail closed if unknown), matching flushSessionTerminalBatch. No-op for + // identity-less single-user clients (canDeliver short-circuits on no identity). + const owner = this.deps.resolveSessionOwner?.(sessionId); + this.broadcast(SseEvent.TaskUpdated, { sessionId, task }, { owner, sessionScoped: true }); } this.taskUpdateBatches.clear(); } @@ -453,7 +457,11 @@ export class SseStreamManager { // Single expensive serialization per batch interval const state = this.deps.getSessionStateWithRespawn(sessionId); if (state) { - this.broadcast(SseEvent.SessionUpdated, state); + // Multi-user: the debounced session:updated blob carries name/workingDir/ + // tokens/cost — route to the session owner only (fail closed if unknown), + // matching flushSessionTerminalBatch. No-op for single-user clients. + const owner = this.deps.resolveSessionOwner?.(sessionId); + this.broadcast(SseEvent.SessionUpdated, state, { owner, sessionScoped: true }); } } this.stateUpdatePending.clear(); diff --git a/test/edge-cases.test.ts b/test/edge-cases.test.ts index d3c1a4bd..8ceec2ae 100644 --- a/test/edge-cases.test.ts +++ b/test/edge-cases.test.ts @@ -221,7 +221,10 @@ describe('Edge Cases and Error Handling', () => { }); const data = await response.json(); - expect(data.error).toBe('Respawn controller not found'); + // respawn/stop now owner-gates via findSessionOrFail first (multi-user #18), so a + // non-existent session id 404s as "Session ... not found" (same not-found semantics, + // matching the sibling start/config/enable handlers). + expect(data.error).toContain('not found'); }); it('should handle updating config on non-existent session', async () => { diff --git a/test/multiuser-auth.test.ts b/test/multiuser-auth.test.ts index 0271f912..25128914 100644 --- a/test/multiuser-auth.test.ts +++ b/test/multiuser-auth.test.ts @@ -17,6 +17,7 @@ import { WebServer } from '../src/web/server.js'; import { TmuxManager } from '../src/tmux-manager.js'; import { TunnelManager } from '../src/tunnel-manager.js'; import { createUser, invalidateUsersCache } from '../src/user-store.js'; +import { AUTH_FAILURE_MAX } from '../src/config/auth-config.js'; vi.spyOn(TmuxManager, 'isTmuxAvailable').mockReturnValue(true); @@ -159,17 +160,32 @@ describe('multi-user auth', () => { expect(after.status).toBe(200); }); - it('rate-limits repeated failures for an account', async () => { + it('verify-first: a correct password is never rate-limited and self-heals failures (#17)', async () => { rateServer = new WebServer(RATE_PORT, false, true); await rateServer.start(); const rurl = (p: string) => `http://localhost:${RATE_PORT}${p}`; - for (let i = 0; i < 10; i++) { + + // Nine wrong passwords (one below the cap) are each rejected 401 — not throttled yet. + for (let i = 0; i < AUTH_FAILURE_MAX - 1; i++) { const res = await fetch(rurl('/api/status'), { headers: { Authorization: basic('bob', `bad-${i}`) } }); expect(res.status).toBe(401); } - // 11th attempt (even with correct creds) is rate-limited. - const limited = await fetch(rurl('/api/status'), { headers: { Authorization: basic('bob', 'bobpass123') } }); - expect(limited.status).toBe(429); + // Finding #17: the CORRECT password must ALWAYS win (verified BEFORE the per-username + // throttle) — the accumulated failures can never lock the account out — and success + // clears the failure buckets. Previously this returned 429 (the DoS being fixed). + const good = await fetch(rurl('/api/status'), { headers: { Authorization: basic('bob', 'bobpass123') } }); + expect(good.status).toBe(200); + // Self-heal: a fresh wrong attempt is 401 again (the counter was reset by the success). + const afterReset = await fetch(rurl('/api/status'), { headers: { Authorization: basic('bob', 'nope') } }); + expect(afterReset.status).toBe(401); + + // Sustained wrong passwords ARE still throttled: 429 once the cap is reached. + let limited = false; + for (let i = 0; i < AUTH_FAILURE_MAX + 1 && !limited; i++) { + const res = await fetch(rurl('/api/status'), { headers: { Authorization: basic('bob', `x-${i}`) } }); + limited = res.status === 429; + } + expect(limited).toBe(true); }); }); diff --git a/test/routes/scheduled-routes.test.ts b/test/routes/scheduled-routes.test.ts index b9e532f4..dda0b7bf 100644 --- a/test/routes/scheduled-routes.test.ts +++ b/test/routes/scheduled-routes.test.ts @@ -174,8 +174,8 @@ describe('scheduled-routes', () => { // Bare { run } return (envelope-wrapped to { success:true, data:{ run } } // in production; harness sees the bare return). expect(body.run).toBeDefined(); - // Should default to 60 minutes - expect(harness.ctx.startScheduledRun).toHaveBeenCalledWith('test', expect.any(String), 60); + // Should default to 60 minutes; 4th arg is the multi-user owner (undefined in single-user). + expect(harness.ctx.startScheduledRun).toHaveBeenCalledWith('test', expect.any(String), 60, undefined); }); });