diff --git a/changelog.d/fixes/7263-settings-local-only.md b/changelog.d/fixes/7263-settings-local-only.md new file mode 100644 index 0000000000..b32a211449 --- /dev/null +++ b/changelog.d/fixes/7263-settings-local-only.md @@ -0,0 +1 @@ +- fix(authz): classify /api/cli-tools/forge-settings and /api/cli-tools/jcode-settings as LOCAL_ONLY, closing an RCE-via-tunnel gap where getCliRuntimeStatus() spawns a child process without loopback enforcement (#7263) diff --git a/scripts/check/check-route-guard-membership.ts b/scripts/check/check-route-guard-membership.ts index c73d9e2667..cb5ca34c2d 100644 --- a/scripts/check/check-route-guard-membership.ts +++ b/scripts/check/check-route-guard-membership.ts @@ -50,6 +50,8 @@ export const SPAWN_CAPABLE_ROUTE_ROOTS: ReadonlyArray = [ "src/app/api/cli-tools/runtime", "src/app/api/local", // T-12: 1-click local service launchers (Redis today) — every child here spawns podman/docker (Hard Rules #15 + #17) "src/app/api/skills/collect", // Skill Collector CLI detection: GET .../detect spawns a child process per CLI_TOOL_IDS entry via getCliRuntimeStatus() (Hard Rules #15 + #17, PR #6294 review) + "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) ]; // 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 dbb4dadf00..66ef33afa5 100644 --- a/src/server/authz/routeGuard.ts +++ b/src/server/authz/routeGuard.ts @@ -32,6 +32,8 @@ export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray = [ "/api/cli-tools/omp-settings", // spawns `which omp` to detect the CLI install (Hard Rules #15 + #17, #6318) "/api/cli-tools/letta-settings", // spawns `which letta` to detect the CLI install (Hard Rules #15 + #17, #6318) "/api/cli-tools/grok-build-settings", // GET calls getCliRuntimeStatus("grok-build"), which spawns a child process to locate + healthcheck the `grok` binary — same transitive-spawn surface that classified /api/skills/collect/ (Hard Rules #15 + #17). Writing ~/.grok/config.toml is inherently a local-machine operation, so loopback-only costs no real capability. + "/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/services/", // T-10: embedded service lifecycle (spawn child processes) "/dashboard/providers/services/", // T-07: reverse proxy to embedded service UIs "/api/copilot/", // unauthenticated LLM driver — CLI-only by default; admins can opt-in to remote access via manage-scope bypass 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 new file mode 100644 index 0000000000..aef5669d9b --- /dev/null +++ b/tests/unit/route-guard-forge-jcode-settings-local-only.test.ts @@ -0,0 +1,48 @@ +/** + * Security regression: /api/cli-tools/forge-settings and /api/cli-tools/jcode-settings + * must be classified as LOCAL_ONLY so loopback enforcement runs unconditionally before + * any auth check. + * + * GET calls getCliRuntimeStatus(TOOL_ID) (src/app/api/cli-tools/forge-settings/route.ts, + * src/app/api/cli-tools/jcode-settings/route.ts), which spawns a child process to locate + * and healthcheck the CLI binary (src/shared/services/cliRuntime.ts:332). That is the same + * transitive-spawn surface that got /api/cli-tools/grok-build-settings and + * /api/skills/collect/ classified. + * + * Classifying it LOCAL_ONLY closes the remote-RCE vector: a leaked JWT over a + * Cloudflared/Ngrok tunnel cannot trigger process spawning. + * Hard Rules #15 + #17. See docs/security/ROUTE_GUARD_TIERS.md. Issue #7263. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { isLocalOnlyPath } from "../../src/server/authz/routeGuard.ts"; + +test("/api/cli-tools/forge-settings is LOCAL_ONLY (spawns via getCliRuntimeStatus)", () => { + assert.equal(isLocalOnlyPath("/api/cli-tools/forge-settings"), true); +}); + +test("/api/cli-tools/forge-settings with trailing slash is LOCAL_ONLY", () => { + assert.equal(isLocalOnlyPath("/api/cli-tools/forge-settings/"), true); +}); + +test("/api/cli-tools/jcode-settings is LOCAL_ONLY (spawns via getCliRuntimeStatus)", () => { + assert.equal(isLocalOnlyPath("/api/cli-tools/jcode-settings"), true); +}); + +test("/api/cli-tools/jcode-settings with trailing slash is LOCAL_ONLY", () => { + assert.equal(isLocalOnlyPath("/api/cli-tools/jcode-settings/"), true); +}); + +test("sibling cli-tools spawn-capable settings routes stay LOCAL_ONLY", () => { + // Guards against a refactor dropping the established precedent this entry follows. + 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); +}); + +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); + assert.equal(isLocalOnlyPath("/api/cli-tools/keys"), false); +});