From a03cb03110fc441e5a1cf8ba81f3908b994e36e5 Mon Sep 17 00:00:00 2001 From: Xiangzhe Date: Fri, 21 Aug 2026 14:26:41 -0300 Subject: [PATCH] fix(security): route OAuth auto-import through the LOCAL_ONLY tier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cursor / kiro / raycast auto-import routes read host-local credential files, but the broad /api/oauth/ PUBLIC prefix classified them PUBLIC — which skips the LOCAL_ONLY tier entirely, so the loopback-only guard never ran. They are now excluded from PUBLIC (classify MANAGEMENT) and added to LOCAL_ONLY_API_PREFIXES, so a non-loopback caller is rejected before the handler runs. OAuth callbacks and browser flows under /api/oauth/ stay PUBLIC. Reported by @ntdat812 via GHSA-wgwc-crjm-pmwv and @koyokr via GHSA-gxv4-955v-v6cm. --- src/server/authz/routeGuard.ts | 2 ++ src/shared/constants/publicApiRoutes.ts | 19 ++++++++++ .../authz/oauth-autoimport-local-only.test.ts | 35 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 tests/unit/authz/oauth-autoimport-local-only.test.ts diff --git a/src/server/authz/routeGuard.ts b/src/server/authz/routeGuard.ts index d59e68836f..7c61d24545 100644 --- a/src/server/authz/routeGuard.ts +++ b/src/server/authz/routeGuard.ts @@ -56,6 +56,8 @@ export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray = [ "/api/jobs", // JobRegistry control (enable/disable/run-now) + run history - runtime job administration, loopback-only (Hard Rules #15 + #17) "/api/jobs/", // sub-paths: /api/jobs/:id/{runs,enable,disable,run-now} (the bare `/api/jobs` above matches the list route; this matches children) "/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/oauth/raycast/auto-import", // reads host-local Raycast credential files — same loopback-only rationale as the kiro and cursor auto-import routes above. "/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/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). diff --git a/src/shared/constants/publicApiRoutes.ts b/src/shared/constants/publicApiRoutes.ts index 7044089a59..b34e130acd 100644 --- a/src/shared/constants/publicApiRoutes.ts +++ b/src/shared/constants/publicApiRoutes.ts @@ -71,7 +71,26 @@ function isPublicCloudApiRoute(pathname: string, method: string): boolean { ); } +// OAuth "auto-import" routes read host-local credential files (Cursor / Kiro / +// Raycast tokens). The broad `/api/oauth/` PUBLIC prefix would classify them +// PUBLIC, which skips the LOCAL_ONLY tier entirely (GHSA-wgwc-crjm-pmwv) and +// exposes the host credential to a remote caller (GHSA-gxv4-955v-v6cm). Exclude +// them so they fall through to MANAGEMENT and reach the loopback-only gate. +const LOCAL_ONLY_OAUTH_IMPORT_ROUTES = [ + "/api/oauth/cursor/auto-import", + "/api/oauth/kiro/auto-import", + "/api/oauth/raycast/auto-import", +]; + export function isPublicApiRoute(pathname: string, method = "GET"): boolean { + if ( + LOCAL_ONLY_OAUTH_IMPORT_ROUTES.some( + (route) => pathname === route || pathname.startsWith(`${route}/`) + ) + ) { + return false; + } + if (isPublicCloudApiRoute(pathname, method)) { return true; } diff --git a/tests/unit/authz/oauth-autoimport-local-only.test.ts b/tests/unit/authz/oauth-autoimport-local-only.test.ts new file mode 100644 index 0000000000..586ad365eb --- /dev/null +++ b/tests/unit/authz/oauth-autoimport-local-only.test.ts @@ -0,0 +1,35 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { isPublicApiRoute } from "../../../src/shared/constants/publicApiRoutes.ts"; +import { classifyRoute } from "../../../src/server/authz/classify.ts"; +import { isLocalOnlyPath } from "../../../src/server/authz/routeGuard.ts"; + +// GHSA-wgwc-crjm-pmwv / GHSA-gxv4-955v-v6cm — the OAuth auto-import routes read +// host-local credential files. They must NOT be PUBLIC (which skips the LOCAL_ONLY +// tier); they must classify MANAGEMENT and be loopback-gated. + +const AUTO_IMPORT = [ + "/api/oauth/cursor/auto-import", + "/api/oauth/kiro/auto-import", + "/api/oauth/raycast/auto-import", +]; + +test("OAuth auto-import routes are excluded from PUBLIC classification", () => { + for (const p of AUTO_IMPORT) { + assert.equal(isPublicApiRoute(p), false, `${p} must not be PUBLIC`); + assert.equal(classifyRoute(p, "GET").routeClass, "MANAGEMENT", `${p} must classify MANAGEMENT`); + } +}); + +test("OAuth auto-import routes are LOCAL_ONLY (loopback-gated)", () => { + for (const p of AUTO_IMPORT) { + assert.equal(isLocalOnlyPath(p), true, `${p} must be LOCAL_ONLY`); + } +}); + +test("the rest of /api/oauth/ (callbacks, browser flows) stays PUBLIC", () => { + assert.equal(isPublicApiRoute("/api/oauth/cursor/callback"), true); + assert.equal(isPublicApiRoute("/api/oauth/codex/authorize"), true); + // A sibling that merely shares the prefix must not be swept in. + assert.equal(isPublicApiRoute("/api/oauth/cursor/auto-import-status"), true); +});