From c8b24ffc30627884cd81c48a3714ee574c9266eb Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Tue, 15 Sep 2026 13:25:35 -0300 Subject: [PATCH] fix(authz): gate the cli-tools status and skills execution routes to LOCAL_ONLY (#13745) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GHSA-35fw-cv32-2373 and GHSA-jx89-f37j-pq89 — the same defect class as /api/acp/agents (GHSA-hf57): a route whose handler chain spawns a host process was classified Tier 3 MANAGEMENT only, and requireManagementAuth() waives auth when requireLogin=false. Hard Rules #15/#17 require the LOCAL_ONLY gate, which runs on the stamped real peer before any auth check. cli-tools (GHSA-35fw): 14 routes reach getCliRuntimeStatus() -> locateCommand() -> runProcess("sh", ["-c", 'command -v -- "$1"']) -> spawn(), exactly like their six gated siblings (forge/grok-build/jcode/qwen/omp/letta-settings): all-statuses, status, and the claude/cline/codewhale/codex/crush/deepseek-tui/ droid/kilo/openclaw/pi/smelt-settings routes. The advisory counted 13; it missed /api/cli-tools/detect, which is heavier — detectAllTools() runs execFile(binary, ["--version"]) and execFile("which") per tool. skills (GHSA-jx89): POST /api/skills/install stores the request's handlerCode verbatim as the skill handler with no allowlist, so a value equal to a built-in name (execute_command / eval_code) aliases the real sandboxed built-in; POST /api/skills/executions then runs it. The sandbox is a real container, but the spawn is transitive, which is why the 6A.8 source scan never flagged it. Entries are exact paths, not a /api/cli-tools/ blanket prefix: apply, backups, config, guide-settings, hermes-agent-settings, keys, logs, openclaw/auto-order and codex-profiles do not spawn and remote dashboards use them. All 16 are mirrored into SPAWN_CAPABLE_PREFIXES (no manage-scope bypass) and added to the route-guard-membership roots so the gate enforces them from now on. Functional trade-off, same one already accepted for grok/forge/jcode/qwen: a dashboard served through a tunnel no longer shows the CLI Tools status badges. Tests are red-first. Two existing negative controls pointed at routes that turn out to spawn (/api/cli-tools/all-statuses, /api/skills/install); they now point at routes that genuinely do not (/api/cli-tools/config, /api/skills/marketplace, /api/skills/skillssh/install), so the non-over-gating assertions are kept. --- .../fixes/ghsa-35fw-jx89-local-only-gates.md | 1 + docs/openapi.yaml | 39 +++++++ docs/security/ROUTE_GUARD_TIERS.md | 73 +++++++------ scripts/check/check-route-guard-membership.ts | 20 ++++ src/server/authz/routeGuard.ts | 23 ++++ src/shared/constants/spawnCapablePrefixes.ts | 19 ++++ stryker.conf.json | 2 + .../authz/route-guard-skills-collect.test.ts | 10 +- ...spawn-capable-prefixes-client-safe.test.ts | 24 ++++- .../unit/check-route-guard-membership.test.ts | 65 +++++++++-- ...uard-cli-tools-settings-local-only.test.ts | 102 ++++++++++++++++++ ...rd-forge-jcode-settings-local-only.test.ts | 7 +- ...ard-grok-build-settings-local-only.test.ts | 7 +- ...te-guard-skills-execute-local-only.test.ts | 74 +++++++++++++ 14 files changed, 412 insertions(+), 54 deletions(-) create mode 100644 changelog.d/fixes/ghsa-35fw-jx89-local-only-gates.md create mode 100644 tests/unit/route-guard-cli-tools-settings-local-only.test.ts create mode 100644 tests/unit/route-guard-skills-execute-local-only.test.ts diff --git a/changelog.d/fixes/ghsa-35fw-jx89-local-only-gates.md b/changelog.d/fixes/ghsa-35fw-jx89-local-only-gates.md new file mode 100644 index 0000000000..cb74cb6fc4 --- /dev/null +++ b/changelog.d/fixes/ghsa-35fw-jx89-local-only-gates.md @@ -0,0 +1 @@ +- **fix(authz):** classify the 14 remaining spawn-capable `/api/cli-tools/*` routes (`all-statuses`, `status`, `detect` and the `claude/cline/codewhale/codex/crush/deepseek-tui/droid/kilo/openclaw/pi/smelt-settings` writers) and the `/api/skills/install` + `/api/skills/executions` pair as LOCAL_ONLY — they reach `child_process.spawn` transitively (`getCliRuntimeStatus()` / `detectAllTools()` / the skills sandbox) but only sat behind Tier 3 MANAGEMENT auth, which `requireLogin=false` waives; loopback/LAN enforcement now runs before any auth check, matching their already-gated siblings (GHSA-35fw-cv32-2373 — thanks Parth Narula; GHSA-jx89-f37j-pq89 — thanks Aeon). Tunnel-served dashboards lose the CLI Tools status badges, the same trade-off already accepted for grok/forge/jcode/qwen. diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 16a39eef8d..ac93ffb20e 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -3999,12 +3999,14 @@ paths: get: tags: [CLI Tools] summary: Get Claude CLI settings + x-loopback-only: true responses: "200": description: Claude CLI configuration post: tags: [CLI Tools] summary: Apply Claude CLI settings + x-loopback-only: true requestBody: required: true content: @@ -4017,6 +4019,7 @@ paths: delete: tags: [CLI Tools] summary: Reset Claude CLI settings + x-loopback-only: true responses: "200": description: Claude CLI settings reset @@ -4025,12 +4028,14 @@ paths: get: tags: [CLI Tools] summary: Get Cline CLI settings + x-loopback-only: true responses: "200": description: Cline CLI configuration post: tags: [CLI Tools] summary: Apply Cline CLI settings + x-loopback-only: true requestBody: required: true content: @@ -4043,6 +4048,7 @@ paths: delete: tags: [CLI Tools] summary: Reset Cline CLI settings + x-loopback-only: true responses: "200": description: Cline CLI settings reset @@ -4093,12 +4099,14 @@ paths: get: tags: [CLI Tools] summary: Get Codex CLI settings + x-loopback-only: true responses: "200": description: Codex CLI configuration post: tags: [CLI Tools] summary: Apply Codex CLI settings + x-loopback-only: true requestBody: required: true content: @@ -4111,6 +4119,7 @@ paths: delete: tags: [CLI Tools] summary: Reset Codex CLI settings + x-loopback-only: true responses: "200": description: Codex CLI settings reset @@ -4119,12 +4128,14 @@ paths: get: tags: [CLI Tools] summary: Get Droid CLI settings + x-loopback-only: true responses: "200": description: Droid CLI configuration post: tags: [CLI Tools] summary: Apply Droid CLI settings + x-loopback-only: true requestBody: required: true content: @@ -4137,6 +4148,7 @@ paths: delete: tags: [CLI Tools] summary: Reset Droid CLI settings + x-loopback-only: true responses: "200": description: Droid CLI settings reset @@ -4145,12 +4157,14 @@ paths: get: tags: [CLI Tools] summary: Get Kilo CLI settings + x-loopback-only: true responses: "200": description: Kilo CLI configuration post: tags: [CLI Tools] summary: Apply Kilo CLI settings + x-loopback-only: true requestBody: required: true content: @@ -4163,6 +4177,7 @@ paths: delete: tags: [CLI Tools] summary: Reset Kilo CLI settings + x-loopback-only: true responses: "200": description: Kilo CLI settings reset @@ -4171,12 +4186,14 @@ paths: get: tags: [CLI Tools] summary: Get OpenClaw CLI settings + x-loopback-only: true responses: "200": description: OpenClaw CLI configuration post: tags: [CLI Tools] summary: Apply OpenClaw CLI settings + x-loopback-only: true requestBody: required: true content: @@ -4189,6 +4206,7 @@ paths: delete: tags: [CLI Tools] summary: Reset OpenClaw CLI settings + x-loopback-only: true responses: "200": description: OpenClaw CLI settings reset @@ -8256,6 +8274,7 @@ paths: tags: - CLI Tools summary: Read Crush CLI OmniRoute config + x-loopback-only: true description: Local-only. Reads the OmniRoute provider block in Crush's config. x-internal: true responses: @@ -8265,6 +8284,7 @@ paths: tags: - CLI Tools summary: Write Crush CLI OmniRoute config + x-loopback-only: true description: Local-only. Registers OmniRoute as an `openai-compat` provider in Crush's config. x-internal: true responses: @@ -8274,6 +8294,7 @@ paths: tags: - CLI Tools summary: Remove OmniRoute from Crush CLI config + x-loopback-only: true description: Local-only. Removes the OmniRoute provider block from Crush's config. x-internal: true responses: @@ -8284,6 +8305,7 @@ paths: tags: - CLI Tools summary: Read CodeWhale CLI OmniRoute config + x-loopback-only: true description: >- Local-only. Reads the OmniRoute config block from `~/.codewhale/config.toml` (with `~/.deepseek/config.toml` legacy @@ -8296,6 +8318,7 @@ paths: tags: - CLI Tools summary: Write CodeWhale CLI OmniRoute config + x-loopback-only: true description: Local-only. Writes the OmniRoute config block in CodeWhale TOML format. x-internal: true responses: @@ -8305,6 +8328,7 @@ paths: tags: - CLI Tools summary: Remove OmniRoute from CodeWhale CLI config + x-loopback-only: true description: Local-only. Removes the OmniRoute config block from CodeWhale's config. x-internal: true responses: @@ -8566,6 +8590,7 @@ paths: tags: - Cli tools summary: "GET cli tools › all statuses" + x-loopback-only: true responses: "200": description: OK @@ -8597,6 +8622,7 @@ paths: tags: - Cli tools summary: "DELETE cli tools › deepseek tui settings" + x-loopback-only: true responses: "200": description: OK @@ -8604,6 +8630,7 @@ paths: tags: - Cli tools summary: "GET cli tools › deepseek tui settings" + x-loopback-only: true responses: "200": description: OK @@ -8611,6 +8638,7 @@ paths: tags: - Cli tools summary: "POST cli tools › deepseek tui settings" + x-loopback-only: true responses: "200": description: OK @@ -8619,6 +8647,7 @@ paths: tags: - Cli tools summary: "GET cli tools › detect" + x-loopback-only: true responses: "200": description: OK @@ -8791,6 +8820,7 @@ paths: tags: - Cli tools summary: "DELETE cli tools › pi settings" + x-loopback-only: true responses: "200": description: OK @@ -8798,6 +8828,7 @@ paths: tags: - Cli tools summary: "GET cli tools › pi settings" + x-loopback-only: true responses: "200": description: OK @@ -8805,6 +8836,7 @@ paths: tags: - Cli tools summary: "POST cli tools › pi settings" + x-loopback-only: true responses: "200": description: OK @@ -8838,6 +8870,7 @@ paths: tags: - Cli tools summary: "DELETE cli tools › smelt settings" + x-loopback-only: true responses: "200": description: OK @@ -8845,6 +8878,7 @@ paths: tags: - Cli tools summary: "GET cli tools › smelt settings" + x-loopback-only: true responses: "200": description: OK @@ -8852,6 +8886,7 @@ paths: tags: - Cli tools summary: "POST cli tools › smelt settings" + x-loopback-only: true responses: "200": description: OK @@ -8860,6 +8895,7 @@ paths: tags: - Cli tools summary: "GET cli tools › status" + x-loopback-only: true responses: "200": description: OK @@ -11709,6 +11745,7 @@ paths: tags: - Skills summary: "GET skills › executions" + x-loopback-only: true responses: "200": description: OK @@ -11716,6 +11753,7 @@ paths: tags: - Skills summary: "POST skills › executions" + x-loopback-only: true responses: "200": description: OK @@ -11724,6 +11762,7 @@ paths: tags: - Skills summary: "POST skills › install" + x-loopback-only: true responses: "200": description: OK diff --git a/docs/security/ROUTE_GUARD_TIERS.md b/docs/security/ROUTE_GUARD_TIERS.md index fe1ca48ba2..d3b69ff2d7 100644 --- a/docs/security/ROUTE_GUARD_TIERS.md +++ b/docs/security/ROUTE_GUARD_TIERS.md @@ -39,41 +39,44 @@ spawn-capable route: a leaked token over a tunnel still can't reach the spawn. `check-route-guard-membership` gate enumerates every `route.ts` under the spawn-capable prefixes and fails CI if any is not classified local-only. -| Prefix / pattern | Why it's local-only | -| -------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | -| `/api/mcp/` | MCP server — spawns stdio bridges + SSE handlers | -| `/api/cli-tools/runtime/` | CLI tool runtime — executes arbitrary plugin code | -| `/api/cli-tools/{omp,letta,grok-build,forge,jcode,qwen}-settings` | Per-tool settings writers that can touch tool binaries/config on the host | -| `/api/cli-tools/antigravity-mitm` | Antigravity MITM proxy control (spawns/points system proxy) | -| `/api/modality-bridge/video/` | Strict trusted-loopback Video Bridge runtime probe and internal extraction bridge | -| `/api/services/` | Embedded services (9Router / CLIProxy / Bifrost / Mux / Dario) — `npm install` + spawn | -| `/dashboard/providers/services/` | Reverse proxy to embedded-service UIs | -| `/api/tunnels/cloudflared` | Installs/spawns the cloudflared binary | -| `/api/tunnels/tailscale/{install,enable,disable,login,start-daemon}` | Installs/controls tailscaled on the host | -| `/api/copilot/` | Unauthenticated LLM driver — CLI-only by default | -| `/api/tools/agent-bridge/` | AgentBridge — spawns MITM server + DNS edits | -| `/api/tools/traffic-inspector/` | Traffic Inspector — http-proxy listener + system proxy | -| `/api/settings/mitm` | Enables MITM interception (system-level proxy state) | -| `/api/issue-agent/` | Issue agent — spawns local tooling against the repo | -| `/api/plugins/`, `/api/plugins` | Plugins — load/execute via `worker_threads` + `child_process` | -| `/api/middleware/` | User middleware — loads/executes operator code in-process | -| `/api/system/version` | Auto-update (POST only; GET/HEAD/OPTIONS exempt) — spawns `git checkout` + `npm install` | -| `/api/db-backups/exportAll` | Spawns `tar` for the export archive | -| `/api/local/` | 1-click local launchers (Redis today) — spawns podman/docker | -| `/api/headroom/start`, `/api/headroom/stop` | Headroom proxy lifecycle — spawns python CLI / signals PID | -| `/api/jobs`, `/api/jobs/` | Job runner control — executes scheduled host-side work | -| `/api/oauth/cursor/auto-import` | `execFile("which", ["cursor"])` before importing creds | -| `/api/oauth/kiro/auto-import` | Reads Kiro CLI credential files from the host | -| `/api/skills/collect/` | Skill collection — detects/installs local tooling | -| `/api/discovery/` | Local network/provider discovery probes | -| `/api/vnc-session` (`VNC_ROUTE_PREFIX`) | Spawns a headful browser + VNC session for interactive logins | -| `/api/acp/agents` | ACP — discovers and spawns local CLI agent binaries | -| `/api/resilience/connections`, `/dashboard/resilience/connections` | Connection maintenance actions that can touch local CLI state | -| `/api/providers/cursor/agent-availability` | Dashboard install-nudge check — spawns `cursor-agent status --format json` | -| `/api/providers/{id}/login` (regex) | Launches a headful Playwright Chromium for web-cookie login | -| `/api/providers/volcengine-plan/connect` (regex) | Manual headful flow + session-based phone/SMS auto-login (spawns Playwright) | -| `/api/providers/{id}/refresh-cursor` (regex) | Manual Cursor session renewal — nudges `cursor-agent` | -| `/api/providers/{id}/chatgpt-web-codex-doctor` (regex) | Diagnoses the local Codex CLI install (spawns the binary) | +| Prefix / pattern | Why it's local-only | +| -------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | +| `/api/mcp/` | MCP server — spawns stdio bridges + SSE handlers | +| `/api/cli-tools/runtime/` | CLI tool runtime — executes arbitrary plugin code | +| `/api/cli-tools/{omp,letta,grok-build,forge,jcode,qwen}-settings` | Per-tool settings writers that can touch tool binaries/config on the host | +| `/api/cli-tools/{claude,cline,codewhale,codex,crush,deepseek-tui,droid,kilo,openclaw,pi,smelt}-settings` | Same `getCliRuntimeStatus()` spawn as the six siblings above (GHSA-35fw-cv32-2373) | +| `/api/cli-tools/{all-statuses,status,detect}` | CLI inventory probes — spawn `command -v` / `--version` per tool (GHSA-35fw-cv32-2373) | +| `/api/cli-tools/antigravity-mitm` | Antigravity MITM proxy control (spawns/points system proxy) | +| `/api/modality-bridge/video/` | Strict trusted-loopback Video Bridge runtime probe and internal extraction bridge | +| `/api/services/` | Embedded services (9Router / CLIProxy / Bifrost / Mux / Dario) — `npm install` + spawn | +| `/dashboard/providers/services/` | Reverse proxy to embedded-service UIs | +| `/api/tunnels/cloudflared` | Installs/spawns the cloudflared binary | +| `/api/tunnels/tailscale/{install,enable,disable,login,start-daemon}` | Installs/controls tailscaled on the host | +| `/api/copilot/` | Unauthenticated LLM driver — CLI-only by default | +| `/api/tools/agent-bridge/` | AgentBridge — spawns MITM server + DNS edits | +| `/api/tools/traffic-inspector/` | Traffic Inspector — http-proxy listener + system proxy | +| `/api/settings/mitm` | Enables MITM interception (system-level proxy state) | +| `/api/issue-agent/` | Issue agent — spawns local tooling against the repo | +| `/api/plugins/`, `/api/plugins` | Plugins — load/execute via `worker_threads` + `child_process` | +| `/api/middleware/` | User middleware — loads/executes operator code in-process | +| `/api/system/version` | Auto-update (POST only; GET/HEAD/OPTIONS exempt) — spawns `git checkout` + `npm install` | +| `/api/db-backups/exportAll` | Spawns `tar` for the export archive | +| `/api/local/` | 1-click local launchers (Redis today) — spawns podman/docker | +| `/api/headroom/start`, `/api/headroom/stop` | Headroom proxy lifecycle — spawns python CLI / signals PID | +| `/api/jobs`, `/api/jobs/` | Job runner control — executes scheduled host-side work | +| `/api/oauth/cursor/auto-import` | `execFile("which", ["cursor"])` before importing creds | +| `/api/oauth/kiro/auto-import` | Reads Kiro CLI credential files from the host | +| `/api/skills/collect/` | Skill collection — detects/installs local tooling | +| `/api/skills/install`, `/api/skills/executions` | Skill handler registration + execution — reach the sandbox container spawn (GHSA-jx89) | +| `/api/discovery/` | Local network/provider discovery probes | +| `/api/vnc-session` (`VNC_ROUTE_PREFIX`) | Spawns a headful browser + VNC session for interactive logins | +| `/api/acp/agents` | ACP — discovers and spawns local CLI agent binaries | +| `/api/resilience/connections`, `/dashboard/resilience/connections` | Connection maintenance actions that can touch local CLI state | +| `/api/providers/cursor/agent-availability` | Dashboard install-nudge check — spawns `cursor-agent status --format json` | +| `/api/providers/{id}/login` (regex) | Launches a headful Playwright Chromium for web-cookie login | +| `/api/providers/volcengine-plan/connect` (regex) | Manual headful flow + session-based phone/SMS auto-login (spawns Playwright) | +| `/api/providers/{id}/refresh-cursor` (regex) | Manual Cursor session renewal — nudges `cursor-agent` | +| `/api/providers/{id}/chatgpt-web-codex-doctor` (regex) | Diagnoses the local Codex CLI install (spawns the binary) | **Response on violation:** `403 LOCAL_ONLY` diff --git a/scripts/check/check-route-guard-membership.ts b/scripts/check/check-route-guard-membership.ts index 14e5968278..3122fd8ecd 100644 --- a/scripts/check/check-route-guard-membership.ts +++ b/scripts/check/check-route-guard-membership.ts @@ -53,6 +53,26 @@ export const SPAWN_CAPABLE_ROUTE_ROOTS: ReadonlyArray = [ "src/app/api/cli-tools/forge-settings", // GET calls getCliRuntimeStatus() to detect the `forge` CLI install (Hard Rules #15 + #17, #7263) "src/app/api/cli-tools/jcode-settings", // GET calls getCliRuntimeStatus() to detect the `jcode` CLI install (Hard Rules #15 + #17, #7263) "src/app/api/cli-tools/qwen-settings", // GET calls getCliRuntimeStatus("qwen") and writes local ~/.qwen config files (Hard Rules #15 + #17) + // GHSA-35fw-cv32-2373: the 14 cli-tools routes that reach the same spawn as the siblings + // above via getCliRuntimeStatus() (13) or detectAllTools() -> execFile (detect). + "src/app/api/cli-tools/all-statuses", // GET calls getCliRuntimeStatus() per CLI_TOOL_IDS entry (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/claude-settings", // GET calls getCliRuntimeStatus() to detect the `claude` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/cline-settings", // GET calls getCliRuntimeStatus() to detect the `cline` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/codewhale-settings", // GET calls getCliRuntimeStatus() to detect the `codewhale` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/codex-settings", // GET calls getCliRuntimeStatus() to detect the `codex` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/crush-settings", // GET calls getCliRuntimeStatus() to detect the `crush` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/deepseek-tui-settings", // GET calls getCliRuntimeStatus() to detect the `deepseek-tui` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/detect", // GET calls detectAllTools() -> execFile(binary, --version) + execFile("which") per tool via src/lib/cli-helper/tool-detector.ts (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/droid-settings", // GET calls getCliRuntimeStatus() to detect the `droid` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/kilo-settings", // GET calls getCliRuntimeStatus() to detect the `kilo` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/openclaw-settings", // GET calls getCliRuntimeStatus() to detect the `openclaw` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/pi-settings", // GET calls getCliRuntimeStatus() to detect the `pi` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/smelt-settings", // GET calls getCliRuntimeStatus() to detect the `smelt` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "src/app/api/cli-tools/status", // GET calls getCliRuntimeStatus() per CLI_TOOL_IDS entry (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + // GHSA-jx89-f37j-pq89: skills install + execute reach childProcess.spawn transitively + // (executor.ts -> builtins.ts -> sandbox.ts) — invisible to the source-scan subcheck. + "src/app/api/skills/install", // POST stores handlerCode verbatim; a built-in name aliases execute_command / eval_code (Hard Rules #15 + #17, GHSA-jx89-f37j-pq89) + "src/app/api/skills/executions", // POST runs skillExecutor.execute() -> sandbox container spawn (Hard Rules #15 + #17, GHSA-jx89-f37j-pq89) ]; // Frozen pre-existing exceptions: spawn-capable routes NOT yet classified diff --git a/src/server/authz/routeGuard.ts b/src/server/authz/routeGuard.ts index b987f4fe49..a2b6a3571b 100644 --- a/src/server/authz/routeGuard.ts +++ b/src/server/authz/routeGuard.ts @@ -39,6 +39,27 @@ export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray = [ "/api/cli-tools/forge-settings", // spawns via getCliRuntimeStatus() to detect the `forge` CLI install (Hard Rules #15 + #17, #7263) "/api/cli-tools/jcode-settings", // spawns via getCliRuntimeStatus() to detect the `jcode` CLI install (Hard Rules #15 + #17, #7263) "/api/cli-tools/qwen-settings", // GET probes the local `qwen` binary; writes target ~/.qwen config files (Hard Rules #15 + #17) + // GHSA-35fw-cv32-2373: the 14 cli-tools routes below reach the SAME spawn as their six + // gated siblings above — getCliRuntimeStatus() -> locateCommand() -> runProcess("sh", -c + // 'command -v -- "$1"') -> spawn() — but sat on Tier 3 MANAGEMENT only, which + // requireManagementAuth() waives under requireLogin=false (incl. the fresh-install window). + // Exact entries on purpose: a blanket "/api/cli-tools/" prefix would also lock the + // non-spawning apply/backups/config/guide-settings/hermes-agent-settings/keys/logs/ + // openclaw/auto-order routes that tunnel-served dashboards legitimately use. + "/api/cli-tools/all-statuses", // GET calls getCliRuntimeStatus() per CLI_TOOL_IDS entry (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/claude-settings", // spawns via getCliRuntimeStatus() to detect the `claude` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/cline-settings", // spawns via getCliRuntimeStatus() to detect the `cline` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/codewhale-settings", // spawns via getCliRuntimeStatus() to detect the `codewhale` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/codex-settings", // spawns via getCliRuntimeStatus() to detect the `codex` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/crush-settings", // spawns via getCliRuntimeStatus() to detect the `crush` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/deepseek-tui-settings", // spawns via getCliRuntimeStatus() to detect the `deepseek-tui` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/detect", // GET calls detectAllTools() -> execFile(binary, --version) + execFile("which") per tool (src/lib/cli-helper/tool-detector.ts) (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/droid-settings", // spawns via getCliRuntimeStatus() to detect the `droid` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/kilo-settings", // spawns via getCliRuntimeStatus() to detect the `kilo` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/openclaw-settings", // spawns via getCliRuntimeStatus() to detect the `openclaw` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373). Does NOT cover the non-spawning sibling /api/cli-tools/openclaw/auto-order (different segment). + "/api/cli-tools/pi-settings", // spawns via getCliRuntimeStatus() to detect the `pi` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/smelt-settings", // spawns via getCliRuntimeStatus() to detect the `smelt` CLI install (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) + "/api/cli-tools/status", // GET calls getCliRuntimeStatus() per CLI_TOOL_IDS entry (Hard Rules #15 + #17, GHSA-35fw-cv32-2373) "/api/services/", // T-10: embedded service lifecycle (spawn child processes) "/api/tunnels/cloudflared", // POST installs/starts/stops cloudflared; safe methods are exempted below "/api/tunnels/tailscale/disable", // stops Funnel and may stop tailscaled/Tailscale service @@ -66,6 +87,8 @@ export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray = [ "/api/oauth/cursor/auto-import", // spawns execFile("which", argv-array-of-one-arg "cursor") to verify a local Cursor install before importing creds — RCE-via-tunnel surface (Hard Rules #15 + #17, found by 6A.8 route-guard gate). Specific path only: the rest of /api/oauth/ (browser redirect/callback flows) must stay remote-reachable. Note: this comment intentionally avoids a literal closing square bracket character — check-openapi-security-tiers.mjs's naive regex parser for this array stops at the first one it finds, silently truncating its view of every entry after this one. "/api/oauth/kiro/auto-import", // reads host-local Kiro credential files (homedir kiro-cli data) — must reach the loopback-only gate, not the PUBLIC /api/oauth/ prefix (GHSA-wgwc-crjm-pmwv, GHSA-gxv4-955v-v6cm). Excluded from PUBLIC in publicApiRoutes.ts. "/api/skills/collect/", // Skill Collector CLI detection: GET .../detect probes getCliRuntimeStatus() per CLI_TOOL_IDS entry, which spawns a child process to check each tool — RCE-via-tunnel surface (Hard Rules #15 + #17, PR #6294 review). + "/api/skills/install", // POST stores the request's handlerCode verbatim as the skill handler with no allowlist; a value equal to the built-in `execute_command` / `eval_code` name aliases the real sandboxed built-in (src/lib/skills/executor.ts -> builtins.ts -> sandbox.ts childProcess.spawn). Transitive spawn the 6A.8 source-scan cannot see. Same class as /api/acp/agents (Hard Rules #15 + #17, GHSA-jx89-f37j-pq89) + "/api/skills/executions", // POST runs skillExecutor.execute() on any global/system skill with caller-chosen input — reaches the container spawn in src/lib/skills/sandbox.ts; only isAuthenticated()-gated, which requireLogin=false waives (Hard Rules #15 + #17, GHSA-jx89-f37j-pq89). Registry list/delete, marketplace and skillssh stay remote-reachable. "/api/discovery/", // Discovery tool (opt-in provider scanner): the scan route makes outbound probes to provider endpoints (SSRF-adjacent) and the whole surface is an admin research tool — strict-loopback only, no manage-scope bypass (NOT in LOCAL_ONLY_MANAGE_SCOPE_BYPASS_PREFIXES). See _tasks/features-v3.8.42/gaps/DISCOVERY_TOOL_DESIGN.md. VNC_ROUTE_PREFIX, // #7892: /api/vnc-session/* spawns Docker containers via child_process.spawn (src/lib/vncSession/service.ts) — RCE-via-tunnel surface (Hard Rules #15 + #17), same CVE class (GHSA-fhh6-4qxv-rpqj). "/api/acp/agents", // ACP custom-agent registry: POST registers a client-chosen `binary`; GET / POST {action:"refresh"} runs detectInstalledAgents() -> execFileSync(probe.command, probe.args, { shell }) transitively (src/lib/acp/registry.ts) — RCE-via-tunnel surface (Hard Rules #15 + #17, #7948) diff --git a/src/shared/constants/spawnCapablePrefixes.ts b/src/shared/constants/spawnCapablePrefixes.ts index d6392b202b..1c9261347f 100644 --- a/src/shared/constants/spawnCapablePrefixes.ts +++ b/src/shared/constants/spawnCapablePrefixes.ts @@ -26,6 +26,23 @@ export const SPAWN_CAPABLE_PREFIXES: ReadonlyArray = [ "/api/cli-tools/runtime/", "/api/cli-tools/qwen-settings", // GET probes the Qwen Code binary; the route also mutates local ~/.qwen files + // GHSA-35fw-cv32-2373: 14 cli-tools routes that reach the same getCliRuntimeStatus() / + // detectAllTools() spawn as their gated siblings — must never be whitelistable via + // manage-scope bypass (Hard Rules #15 + #17). Exact entries; NOT a "/api/cli-tools/" blanket. + "/api/cli-tools/all-statuses", // GET calls getCliRuntimeStatus() per CLI_TOOL_IDS entry + "/api/cli-tools/claude-settings", // GET probes the `claude` binary via getCliRuntimeStatus() + "/api/cli-tools/cline-settings", // GET probes the `cline` binary via getCliRuntimeStatus() + "/api/cli-tools/codewhale-settings", // GET probes the `codewhale` binary via getCliRuntimeStatus() + "/api/cli-tools/codex-settings", // GET probes the `codex` binary via getCliRuntimeStatus() + "/api/cli-tools/crush-settings", // GET probes the `crush` binary via getCliRuntimeStatus() + "/api/cli-tools/deepseek-tui-settings", // GET probes the `deepseek-tui` binary via getCliRuntimeStatus() + "/api/cli-tools/detect", // GET calls detectAllTools() -> execFile(binary, --version) + execFile("which") per tool + "/api/cli-tools/droid-settings", // GET probes the `droid` binary via getCliRuntimeStatus() + "/api/cli-tools/kilo-settings", // GET probes the `kilo` binary via getCliRuntimeStatus() + "/api/cli-tools/openclaw-settings", // GET probes the `openclaw` binary via getCliRuntimeStatus() + "/api/cli-tools/pi-settings", // GET probes the `pi` binary via getCliRuntimeStatus() + "/api/cli-tools/smelt-settings", // GET probes the `smelt` binary via getCliRuntimeStatus() + "/api/cli-tools/status", // GET calls getCliRuntimeStatus() per CLI_TOOL_IDS entry "/api/services/", // T-10: can run npm install + spawn node processes "/api/tunnels/cloudflared", // POST installs/starts/stops cloudflared; safe methods remain read-only exempt "/api/tunnels/tailscale/disable", // stops Funnel and may stop tailscaled/Tailscale service @@ -40,6 +57,8 @@ export const SPAWN_CAPABLE_PREFIXES: ReadonlyArray = [ "/api/plugins/", // plugins: load/execute via worker_threads + child_process (Hard Rules #15 + #17) "/api/local/", // T-12: 1-click local service launchers (Redis today) — must never be whitelistable via manage-scope bypass (Hard Rules #15 + #17) "/api/skills/collect/", // Skill Collector CLI detection: GET .../detect spawns a child process per CLI_TOOL_IDS entry — must never be whitelistable via manage-scope bypass (Hard Rules #15 + #17, PR #6294 review) + "/api/skills/install", // POST registers a handler string that can alias the built-in execute_command / eval_code (src/lib/skills/executor.ts -> builtins.ts -> sandbox.ts childProcess.spawn) — must never be whitelistable via manage-scope bypass (Hard Rules #15 + #17, GHSA-jx89-f37j-pq89) + "/api/skills/executions", // POST runs skillExecutor.execute() -> container spawn in src/lib/skills/sandbox.ts — must never be whitelistable via manage-scope bypass (Hard Rules #15 + #17, GHSA-jx89-f37j-pq89) "/api/headroom/start", // spawns headroom-ai python CLI — must never be bypassable (Hard Rules #15 + #17) "/api/headroom/stop", // kills tracked PID — must never be bypassable (Hard Rules #15 + #17) "/api/vnc-session", // #7892: spawns Docker containers via child_process.spawn (src/lib/vncSession/service.ts) — must never be whitelistable via manage-scope bypass (Hard Rules #15 + #17) diff --git a/stryker.conf.json b/stryker.conf.json index 6feacd68f8..1e95da7312 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -373,6 +373,7 @@ "tests/unit/rotation-config-omniroute.test.ts", "tests/unit/route-explainability.test.ts", "tests/unit/route-guard-acp-agents-local-only.test.ts", + "tests/unit/route-guard-cli-tools-settings-local-only.test.ts", "tests/unit/route-guard-cursor-agent-availability.test.ts", "tests/unit/route-guard-cursor-refresh.test.ts", "tests/unit/route-guard-forge-jcode-settings-local-only.test.ts", @@ -382,6 +383,7 @@ "tests/unit/route-guard-private-lan.test.ts", "tests/unit/route-guard-provider-login-local-only.test.ts", "tests/unit/route-guard-qwen-settings-local-only.test.ts", + "tests/unit/route-guard-skills-execute-local-only.test.ts", "tests/unit/router-strategies.test.ts", "tests/unit/routing-adaptive-e2e.test.ts", "tests/unit/rule12-error-sanitization-sweep.test.ts", diff --git a/tests/unit/authz/route-guard-skills-collect.test.ts b/tests/unit/authz/route-guard-skills-collect.test.ts index 795fc3f59e..03b8b80df8 100644 --- a/tests/unit/authz/route-guard-skills-collect.test.ts +++ b/tests/unit/authz/route-guard-skills-collect.test.ts @@ -21,12 +21,16 @@ test("isLocalOnlyPath: /api/skills/collect/ prefix is local-only (Hard Rules #15 }); test("isLocalOnlyPath: the rest of /api/skills/ stays remote-reachable (no over-broadening)", () => { - // Only the spawn-capable collect/* subtree is loopback-locked. The rest of the - // skills surface (registry install, marketplace, skillssh) already gates on + // Only the spawn-capable subtrees are loopback-locked. The rest of the skills + // surface (registry list/delete, marketplace, skillssh) gates on // requireManagementAuth() and must remain reachable remotely. + // (/api/skills/install used to be the negative control here, but it can alias + // the sandboxed execute_command built-in and became LOCAL_ONLY under + // GHSA-jx89-f37j-pq89 — see tests/unit/route-guard-skills-execute-local-only.test.ts.) assert.equal(isLocalOnlyPath("/api/skills"), false); - assert.equal(isLocalOnlyPath("/api/skills/install"), false); + assert.equal(isLocalOnlyPath("/api/skills/marketplace"), false); assert.equal(isLocalOnlyPath("/api/skills/marketplace/install"), false); + assert.equal(isLocalOnlyPath("/api/skills/skillssh/install"), false); }); test("isLocalOnlyBypassableByManageScope: /api/skills/collect/ is NOT bypassable (defence in depth)", () => { diff --git a/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts b/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts index f28e4e8c80..ea7bef6dc8 100644 --- a/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts +++ b/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts @@ -90,11 +90,33 @@ test("SPAWN_CAPABLE_PREFIXES is defined in the server-free constants leaf with t "/api/tunnels/tailscale/install", "/api/tunnels/tailscale/login", "/api/tunnels/tailscale/start-daemon", + // GHSA-35fw-cv32-2373: the 14 cli-tools routes that reach the same + // getCliRuntimeStatus() / detectAllTools() spawn as their gated siblings. + "/api/cli-tools/all-statuses", + "/api/cli-tools/claude-settings", + "/api/cli-tools/cline-settings", + "/api/cli-tools/codewhale-settings", + "/api/cli-tools/codex-settings", + "/api/cli-tools/crush-settings", + "/api/cli-tools/deepseek-tui-settings", + "/api/cli-tools/detect", + "/api/cli-tools/droid-settings", + "/api/cli-tools/kilo-settings", + "/api/cli-tools/openclaw-settings", + "/api/cli-tools/pi-settings", + "/api/cli-tools/smelt-settings", + "/api/cli-tools/status", + // GHSA-jx89-f37j-pq89: skills handler registration + execution reach the + // sandbox container spawn transitively. + "/api/skills/install", + "/api/skills/executions", ]) { assert.ok( SPAWN_CAPABLE_PREFIXES.includes(prefix), `SPAWN_CAPABLE_PREFIXES lost the spawn-capable prefix "${prefix}" during extraction` ); } - assert.equal(SPAWN_CAPABLE_PREFIXES.length, 20); + // 20 at extraction time + 14 (GHSA-35fw-cv32-2373) + 2 (GHSA-jx89-f37j-pq89). + // qwen-settings is the one pre-existing entry not enumerated above. + assert.equal(SPAWN_CAPABLE_PREFIXES.length, 36); }); diff --git a/tests/unit/check-route-guard-membership.test.ts b/tests/unit/check-route-guard-membership.test.ts index 146772528b..b73ea025fb 100644 --- a/tests/unit/check-route-guard-membership.test.ts +++ b/tests/unit/check-route-guard-membership.test.ts @@ -9,6 +9,7 @@ import { isSpawnCapableSource, findSpawnCapableRoutes, KNOWN_UNCLASSIFIED_SOURCE_SPAWN, + SPAWN_CAPABLE_ROUTE_ROOTS, } from "../../scripts/check/check-route-guard-membership.ts"; import { isLocalOnlyPath } from "../../src/server/authz/routeGuard.ts"; @@ -64,11 +65,7 @@ test("flags a spawn-capable route that is NOT classified local-only (RCE-via-tun // this gate guards against. const leaky = (path: string): boolean => path.startsWith("/api/mcp/"); assert.deepEqual( - findUnclassifiedSpawnRoutes( - ["/api/mcp/tools", "/api/services/cliproxy/install"], - leaky, - {} - ), + findUnclassifiedSpawnRoutes(["/api/mcp/tools", "/api/services/cliproxy/install"], leaky, {}), ["/api/services/cliproxy/install"] ); }); @@ -76,11 +73,9 @@ test("flags a spawn-capable route that is NOT classified local-only (RCE-via-tun test("allowlisted routes are not flagged (frozen pre-existing exceptions)", () => { const leaky = (path: string): boolean => path.startsWith("/api/mcp/"); assert.deepEqual( - findUnclassifiedSpawnRoutes( - ["/api/mcp/tools", "/api/services/legacy/route"], - leaky, - { "/api/services/legacy/route": "frozen pre-existing exception" } - ), + findUnclassifiedSpawnRoutes(["/api/mcp/tools", "/api/services/legacy/route"], leaky, { + "/api/services/legacy/route": "frozen pre-existing exception", + }), [] ); }); @@ -128,7 +123,10 @@ test("6A.8 findSpawnCapableRoutes: detects real spawn-capable route.ts files", ( ]; const found = findSpawnCapableRoutes(repoRoot); for (const r of knownSpawnRoutes) { - assert.ok(found.includes(r), `expected ${r} in spawn-capable routes, found: ${found.join(", ")}`); + assert.ok( + found.includes(r), + `expected ${r} in spawn-capable routes, found: ${found.join(", ")}` + ); } }); @@ -153,6 +151,51 @@ test("#7948: /api/acp/agents (transitive execFileSync via registry) is classifie assert.equal(isLocalOnlyPath("/api/acp/agents"), true); }); +test("GHSA-35fw-cv32-2373: every cli-tools route that reaches getCliRuntimeStatus()/detectAllTools() is classified local-only", () => { + // Same transitive-spawn class as #7948: the spawn lives in + // src/shared/services/cliRuntime.ts (runProcess -> spawn) and + // src/lib/cli-helper/tool-detector.ts (execFile), never in the route file, so + // the source-scan subcheck is blind to it. Six siblings were already gated; + // these 14 called the same helper and were not. Each is now a + // SPAWN_CAPABLE_ROUTE_ROOT so subcheck 1 enforces membership going forward. + const routes = [ + "/api/cli-tools/all-statuses", + "/api/cli-tools/claude-settings", + "/api/cli-tools/cline-settings", + "/api/cli-tools/codewhale-settings", + "/api/cli-tools/codex-settings", + "/api/cli-tools/crush-settings", + "/api/cli-tools/deepseek-tui-settings", + "/api/cli-tools/detect", + "/api/cli-tools/droid-settings", + "/api/cli-tools/kilo-settings", + "/api/cli-tools/openclaw-settings", + "/api/cli-tools/pi-settings", + "/api/cli-tools/smelt-settings", + "/api/cli-tools/status", + ]; + for (const r of routes) { + assert.equal(isLocalOnlyPath(r), true, `${r} must be local-only`); + assert.ok( + SPAWN_CAPABLE_ROUTE_ROOTS.includes(`src/app${r}`), + `src/app${r} must be a SPAWN_CAPABLE_ROUTE_ROOT` + ); + } +}); + +test("GHSA-jx89-f37j-pq89: /api/skills/install + /api/skills/executions (transitive sandbox spawn) are classified local-only", () => { + // The spawn is three modules away from the route (executor.ts -> builtins.ts -> + // sandbox.ts childProcess.spawn), so the source-scan subcheck cannot see it. + // Both are now SPAWN_CAPABLE_ROUTE_ROOTs so subcheck 1 enforces membership. + for (const r of ["/api/skills/install", "/api/skills/executions"]) { + assert.equal(isLocalOnlyPath(r), true, `${r} must be local-only`); + assert.ok( + SPAWN_CAPABLE_ROUTE_ROOTS.includes(`src/app${r}`), + `src/app${r} must be a SPAWN_CAPABLE_ROUTE_ROOT` + ); + } +}); + test("6A.8: spawn-capable routes in SPAWN_CAPABLE_ROUTE_ROOTS are still all classified local-only", async () => { // The original subcheck (SPAWN_CAPABLE_ROUTE_ROOTS) must still pass. // This test is a regression guard — the new source-scan does not break the old check. diff --git a/tests/unit/route-guard-cli-tools-settings-local-only.test.ts b/tests/unit/route-guard-cli-tools-settings-local-only.test.ts new file mode 100644 index 0000000000..50bcda640a --- /dev/null +++ b/tests/unit/route-guard-cli-tools-settings-local-only.test.ts @@ -0,0 +1,102 @@ +/** + * Security regression (GHSA-35fw-cv32-2373): every /api/cli-tools/* route whose + * handler reaches a child-process spawn must be classified LOCAL_ONLY so loopback + * enforcement runs unconditionally before any auth check. + * + * 13 routes call getCliRuntimeStatus(toolId) directly from their exported GET + * handler (src/shared/services/cliRuntime.ts): getCliRuntimeStatus -> + * locateCommandCandidate -> locateCommand -> runProcess("sh", ["-c", + * 'command -v -- "$1"', ...]) -> spawn(). /api/cli-tools/detect reaches the same + * class via detectAllTools() -> execFile(binary, ["--version"]) + execFile("which") + * (src/lib/cli-helper/tool-detector.ts). Their six siblings (omp, letta, + * grok-build, forge, jcode, qwen -settings + runtime/) call the SAME helper and + * were already LOCAL_ONLY; these 14 were only behind Tier 3 MANAGEMENT, which + * requireManagementAuth() waives whenever requireLogin=false (including the + * fresh-install window before a password is set). + * + * Classifying them LOCAL_ONLY closes the remote-spawn vector: a non-loopback / + * non-LAN caller — with or without a leaked JWT over a Cloudflared/Ngrok tunnel — + * cannot trigger process spawning or enumerate the host's CLI inventory. + * Hard Rules #15 + #17. See docs/security/ROUTE_GUARD_TIERS.md. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { + isLocalOnlyBypassableByManageScope, + isLocalOnlyPath, +} from "../../src/server/authz/routeGuard.ts"; +import { SPAWN_CAPABLE_PREFIXES } from "../../src/shared/constants/spawnCapablePrefixes.ts"; +import { SPAWN_CAPABLE_ROUTE_ROOTS } from "../../scripts/check/check-route-guard-membership.ts"; + +/** The 14 cli-tools routes that reach a spawn (GHSA-35fw-cv32-2373). */ +const SPAWNING_CLI_TOOLS_ROUTES: ReadonlyArray = [ + "/api/cli-tools/all-statuses", + "/api/cli-tools/claude-settings", + "/api/cli-tools/cline-settings", + "/api/cli-tools/codewhale-settings", + "/api/cli-tools/codex-settings", + "/api/cli-tools/crush-settings", + "/api/cli-tools/deepseek-tui-settings", + "/api/cli-tools/detect", + "/api/cli-tools/droid-settings", + "/api/cli-tools/kilo-settings", + "/api/cli-tools/openclaw-settings", + "/api/cli-tools/pi-settings", + "/api/cli-tools/smelt-settings", + "/api/cli-tools/status", +]; + +for (const route of SPAWNING_CLI_TOOLS_ROUTES) { + test(`GHSA-35fw: ${route} is LOCAL_ONLY (reaches spawn via getCliRuntimeStatus/detectAllTools)`, () => { + assert.equal(isLocalOnlyPath(route), true); + }); + + test(`GHSA-35fw: ${route}/ (trailing slash) is LOCAL_ONLY`, () => { + assert.equal(isLocalOnlyPath(`${route}/`), true); + }); + + test(`GHSA-35fw: ${route} cannot be opened through the manage-scope bypass`, () => { + // Mirror in SPAWN_CAPABLE_PREFIXES: the zod schema rejects the prefix at + // PATCH /api/settings time and the runtime predicate refuses a malformed row. + assert.ok( + SPAWN_CAPABLE_PREFIXES.includes(route), + `${route} must be listed in SPAWN_CAPABLE_PREFIXES` + ); + assert.equal(isLocalOnlyBypassableByManageScope(route), false); + }); + + test(`GHSA-35fw: the spawn-capable route audit enumerates ${route}`, () => { + const root = `src/app${route}`; + assert.ok( + SPAWN_CAPABLE_ROUTE_ROOTS.includes(root), + `${root} must be listed in SPAWN_CAPABLE_ROUTE_ROOTS so the 6A.8 gate enforces it` + ); + }); +} + +test("GHSA-35fw: the already-gated cli-tools siblings stay LOCAL_ONLY", () => { + // Guards against a refactor dropping the precedent these entries follow. + assert.equal(isLocalOnlyPath("/api/cli-tools/omp-settings"), true); + assert.equal(isLocalOnlyPath("/api/cli-tools/letta-settings"), true); + assert.equal(isLocalOnlyPath("/api/cli-tools/grok-build-settings"), true); + assert.equal(isLocalOnlyPath("/api/cli-tools/forge-settings"), true); + assert.equal(isLocalOnlyPath("/api/cli-tools/jcode-settings"), true); + assert.equal(isLocalOnlyPath("/api/cli-tools/qwen-settings"), true); + assert.equal(isLocalOnlyPath("/api/cli-tools/runtime/claude"), true); +}); + +test("GHSA-35fw: non-spawning cli-tools routes are NOT over-gated (no blanket prefix)", () => { + // These are legitimate remote-dashboard routes: file/config reads and writes + // with no child-process reach. A blanket "/api/cli-tools/" prefix would break + // every tunnel-served dashboard, so the fix is 14 exact entries, not one. + assert.equal(isLocalOnlyPath("/api/cli-tools/apply"), false); + assert.equal(isLocalOnlyPath("/api/cli-tools/backups"), false); + assert.equal(isLocalOnlyPath("/api/cli-tools/config"), false); + assert.equal(isLocalOnlyPath("/api/cli-tools/guide-settings/claude"), false); + assert.equal(isLocalOnlyPath("/api/cli-tools/hermes-agent-settings"), false); + assert.equal(isLocalOnlyPath("/api/cli-tools/keys"), false); + assert.equal(isLocalOnlyPath("/api/cli-tools/logs"), false); + // "/api/cli-tools/openclaw-settings" must not swallow the sibling + // "/api/cli-tools/openclaw/auto-order" (different segment, no spawn). + assert.equal(isLocalOnlyPath("/api/cli-tools/openclaw/auto-order"), false); +}); diff --git a/tests/unit/route-guard-forge-jcode-settings-local-only.test.ts b/tests/unit/route-guard-forge-jcode-settings-local-only.test.ts index aef5669d9b..04c9871fdb 100644 --- a/tests/unit/route-guard-forge-jcode-settings-local-only.test.ts +++ b/tests/unit/route-guard-forge-jcode-settings-local-only.test.ts @@ -42,7 +42,10 @@ test("sibling cli-tools spawn-capable settings routes stay LOCAL_ONLY", () => { test("non-spawning cli-tools routes are NOT over-gated by this entry", () => { // The new prefixes must not accidentally widen to the whole /api/cli-tools/ subtree, - // which remote dashboards legitimately use. - assert.equal(isLocalOnlyPath("/api/cli-tools/all-statuses"), false); + // which remote dashboards legitimately use. (/api/cli-tools/all-statuses used to be + // the negative control here, but it calls getCliRuntimeStatus() too and became + // LOCAL_ONLY under GHSA-35fw-cv32-2373 — see + // route-guard-cli-tools-settings-local-only.test.ts.) assert.equal(isLocalOnlyPath("/api/cli-tools/keys"), false); + assert.equal(isLocalOnlyPath("/api/cli-tools/config"), false); }); diff --git a/tests/unit/route-guard-grok-build-settings-local-only.test.ts b/tests/unit/route-guard-grok-build-settings-local-only.test.ts index b02348f151..72da038193 100644 --- a/tests/unit/route-guard-grok-build-settings-local-only.test.ts +++ b/tests/unit/route-guard-grok-build-settings-local-only.test.ts @@ -33,7 +33,10 @@ test("sibling cli-tools spawn-capable settings routes stay LOCAL_ONLY", () => { test("non-spawning cli-tools routes are NOT over-gated by this entry", () => { // The new prefix must not accidentally widen to the whole /api/cli-tools/ subtree, - // which remote dashboards legitimately use. - assert.equal(isLocalOnlyPath("/api/cli-tools/all-statuses"), false); + // which remote dashboards legitimately use. (/api/cli-tools/all-statuses used to be + // the negative control here, but it calls getCliRuntimeStatus() too and became + // LOCAL_ONLY under GHSA-35fw-cv32-2373 — see + // route-guard-cli-tools-settings-local-only.test.ts.) assert.equal(isLocalOnlyPath("/api/cli-tools/keys"), false); + assert.equal(isLocalOnlyPath("/api/cli-tools/config"), false); }); diff --git a/tests/unit/route-guard-skills-execute-local-only.test.ts b/tests/unit/route-guard-skills-execute-local-only.test.ts new file mode 100644 index 0000000000..b08a8c30fa --- /dev/null +++ b/tests/unit/route-guard-skills-execute-local-only.test.ts @@ -0,0 +1,74 @@ +/** + * Security regression (GHSA-jx89-f37j-pq89): /api/skills/install and + * /api/skills/executions must be classified LOCAL_ONLY so loopback enforcement + * runs unconditionally before any auth check. + * + * POST /api/skills/install stores the request's `handlerCode` string verbatim as + * the skill's `handler` with no allowlist (src/app/api/skills/install/route.ts). + * POST /api/skills/executions then calls skillExecutor.execute(), whose handler + * resolution (src/lib/skills/executor.ts) falls through to the built-in table — + * so a handler string that equals `execute_command` or `eval_code` runs the real + * built-in (src/lib/skills/builtins.ts), which reaches + * childProcess.spawn() in src/lib/skills/sandbox.ts. The + * sandbox is a hardened docker/podman container, but the spawn is real and + * transitive: the 6A.8 source-scan gate only greps route.ts, so it cannot see it. + * + * Both routes were only behind requireManagementAuth() / isAuthenticated(), + * which waive auth whenever requireLogin=false — the identical class already + * closed for /api/acp/agents (GHSA-hf57-cqmx-p4gr) and /api/skills/collect/. + * Hard Rules #15 + #17. See docs/security/ROUTE_GUARD_TIERS.md. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { + isLocalOnlyBypassableByManageScope, + isLocalOnlyPath, +} from "../../src/server/authz/routeGuard.ts"; +import { SPAWN_CAPABLE_PREFIXES } from "../../src/shared/constants/spawnCapablePrefixes.ts"; +import { SPAWN_CAPABLE_ROUTE_ROOTS } from "../../scripts/check/check-route-guard-membership.ts"; + +test("GHSA-jx89: /api/skills/install is LOCAL_ONLY (registers a handler that can alias execute_command)", () => { + assert.equal(isLocalOnlyPath("/api/skills/install"), true); +}); + +test("GHSA-jx89: /api/skills/install with trailing slash is LOCAL_ONLY", () => { + assert.equal(isLocalOnlyPath("/api/skills/install/"), true); +}); + +test("GHSA-jx89: /api/skills/executions is LOCAL_ONLY (skillExecutor.execute reaches sandbox spawn)", () => { + assert.equal(isLocalOnlyPath("/api/skills/executions"), true); +}); + +test("GHSA-jx89: /api/skills/executions with trailing slash is LOCAL_ONLY", () => { + assert.equal(isLocalOnlyPath("/api/skills/executions/"), true); +}); + +test("GHSA-jx89: neither skills execution route can be opened through the manage-scope bypass", () => { + for (const route of ["/api/skills/install", "/api/skills/executions"]) { + assert.ok( + SPAWN_CAPABLE_PREFIXES.includes(route), + `${route} must be listed in SPAWN_CAPABLE_PREFIXES` + ); + assert.equal(isLocalOnlyBypassableByManageScope(route), false); + } +}); + +test("GHSA-jx89: the spawn-capable route audit enumerates both skills execution routes", () => { + assert.ok(SPAWN_CAPABLE_ROUTE_ROOTS.includes("src/app/api/skills/install")); + assert.ok(SPAWN_CAPABLE_ROUTE_ROOTS.includes("src/app/api/skills/executions")); +}); + +test("GHSA-jx89: the existing /api/skills/collect/ gate is untouched", () => { + assert.equal(isLocalOnlyPath("/api/skills/collect/detect"), true); +}); + +test("GHSA-jx89: the rest of /api/skills/ stays remote-reachable (no over-broadening)", () => { + // Registry listing / delete, marketplace and skillssh do not reach the sandbox + // spawn: they stay on requireManagementAuth() and must remain tunnel-reachable. + assert.equal(isLocalOnlyPath("/api/skills"), false); + assert.equal(isLocalOnlyPath("/api/skills/"), false); + assert.equal(isLocalOnlyPath("/api/skills/some-id"), false); + assert.equal(isLocalOnlyPath("/api/skills/marketplace"), false); + assert.equal(isLocalOnlyPath("/api/skills/marketplace/install"), false); + assert.equal(isLocalOnlyPath("/api/skills/skillssh/install"), false); +});