diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 2f83d88080..16a39eef8d 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2297,6 +2297,7 @@ paths: post: tags: [Providers] summary: Auto-detect and import the local Antigravity CLI (agy) login from disk + x-always-protected: true responses: "200": description: Created or updated provider connection @@ -4050,12 +4051,14 @@ paths: get: tags: [CLI Tools] summary: Get Codex profiles + x-always-protected: true responses: "200": description: Codex profile list post: tags: [CLI Tools] summary: Create Codex profile + x-always-protected: true requestBody: required: true content: @@ -4068,6 +4071,7 @@ paths: put: tags: [CLI Tools] summary: Update Codex profile + x-always-protected: true requestBody: required: true content: @@ -4080,6 +4084,7 @@ paths: delete: tags: [CLI Tools] summary: Delete Codex profile + x-always-protected: true responses: "200": description: Profile deleted @@ -9751,6 +9756,7 @@ paths: tags: - Logs summary: "GET logs › export" + x-always-protected: true responses: "200": description: OK @@ -10230,6 +10236,7 @@ paths: tags: - Providers summary: "POST providers › › claude auth › apply local" + x-always-protected: true responses: "200": description: OK @@ -10238,6 +10245,7 @@ paths: tags: - Providers summary: "POST providers › › claude auth › export" + x-always-protected: true responses: "200": description: OK @@ -10246,6 +10254,7 @@ paths: tags: - Providers summary: "POST providers › › codex auth › apply local" + x-always-protected: true responses: "200": description: OK @@ -10254,6 +10263,7 @@ paths: tags: - Providers summary: "POST providers › › codex auth › export" + x-always-protected: true responses: "200": description: OK diff --git a/src/server/authz/routeGuard.ts b/src/server/authz/routeGuard.ts index 60a8d8e15a..b987f4fe49 100644 --- a/src/server/authz/routeGuard.ts +++ b/src/server/authz/routeGuard.ts @@ -140,6 +140,42 @@ export const ALWAYS_PROTECTED_API_PATHS: ReadonlyArray = [ // which is false under requireLogin=false. (GHSA-v7g9-7f55-5g46) "/api/settings/export-json", "/api/settings/import-json", + // Bulk log export: call_logs carries prompts and responses, proxy_logs carries + // client/public IPs, and the handler only calls requireManagementAuth() with no + // alwaysRequireAuth. Found sweeping the GHSA-5926-2w35-7h4q class. + "/api/logs/export", + // Codex CLI profile store. GET leaks the operator's account label; PUT writes + // attacker-supplied auth.json + config.toml straight into the operator's Codex + // CLI config (ensureCliConfigWriteAllowed() only checks CLI_ALLOW_CONFIG_WRITES, + // which defaults to true), so a POST+PUT pair repoints the CLI at attacker + // credentials or an attacker base URL. Found sweeping the same class. + "/api/cli-tools/codex-profiles", + // Writes into ~/.gemini/antigravity-cli/antigravity-oauth-token. Same family + // as the {claude,codex}-auth/apply-local pattern below; a plain path because + // it carries no dynamic segment. + "/api/providers/agy-auth/apply-local", +]; + +/** + * ALWAYS_PROTECTED routes whose path carries a dynamic segment, so the plain + * exact/prefix list above cannot express them: a `/api/providers/` prefix would + * hard-gate the entire provider surface and break every keyless local-first + * install. Mirrors LOCAL_ONLY_API_PATTERNS. + * + * The Claude/Codex OAuth export routes return the connection's raw + * access_token / refresh_token (and the Codex id_token) and gate only on + * `requireManagementAuth(request)` with no `alwaysRequireAuth`, which fails open + * under requireLogin=false (GHSA-5926-2w35-7h4q). They are the siblings that + * both GHSA-mghq-58h3-qcqj and GHSA-v7g9-7f55-5g46 missed. + */ +export const ALWAYS_PROTECTED_API_PATTERNS: ReadonlyArray = [ + // `export` hands the caller the raw token; `apply-local` writes it into the + // host's CLI config (~/.codex/auth.json and the Claude equivalent). The second + // does not disclose the credential, but "anonymous" is still the wrong + // audience for it. ALWAYS_PROTECTED rather than LOCAL_ONLY on purpose: it + // closes the anonymous hole without breaking an operator driving the dashboard + // through a tunnel. + /^\/api\/providers\/[^/]+\/(claude|codex)-auth\/(export|apply-local)\/?$/, ]; export function isLoopbackHost(hostHeader: string | null): boolean { @@ -295,5 +331,8 @@ export function isLocalOnlyBypassableByManageScope(path: string): boolean { } export function isAlwaysProtectedPath(path: string): boolean { - return ALWAYS_PROTECTED_API_PATHS.some((p) => path === p || path.startsWith(p)); + return ( + ALWAYS_PROTECTED_API_PATHS.some((p) => path === p || path.startsWith(p)) || + ALWAYS_PROTECTED_API_PATTERNS.some((re) => re.test(path)) + ); } diff --git a/stryker.conf.json b/stryker.conf.json index d13bf2c1d4..23dd4fb2d5 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -90,6 +90,7 @@ "tests/unit/auth-opencode-zen-noauth-fallback.test.ts", "tests/unit/auth-passthrough-per-model-402-12242.test.ts", "tests/unit/auth-terminal-status.test.ts", + "tests/unit/authz/credential-export-always-protected.test.ts", "tests/unit/authz/discovery-routes-local-only.test.ts", "tests/unit/authz/oauth-autoimport-local-only.test.ts", "tests/unit/authz/route-guard-local-prefix.test.ts", diff --git a/tests/unit/authz/credential-export-always-protected.test.ts b/tests/unit/authz/credential-export-always-protected.test.ts new file mode 100644 index 0000000000..c297bad003 --- /dev/null +++ b/tests/unit/authz/credential-export-always-protected.test.ts @@ -0,0 +1,132 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + isAlwaysProtectedPath, + isLocalOnlyPath, + ALWAYS_PROTECTED_API_PATHS, +} from "../../../src/server/authz/routeGuard.ts"; + +// GHSA-5926-2w35-7h4q — the Claude/Codex OAuth export routes gate on +// `requireManagementAuth(request)` with no `alwaysRequireAuth`, which fails open +// under requireLogin=false, and neither path was in ALWAYS_PROTECTED_API_PATHS. +// An unauthenticated caller who knows a connection id could download the +// operator's raw access_token / refresh_token / id_token. +// +// This is the THIRD recurrence of one class: GHSA-mghq-58h3-qcqj added +// /api/db-backups, GHSA-v7g9-7f55-5g46 added the /api/settings/*-json siblings +// it had missed, and this one is the siblings BOTH missed. So the test is +// written as an inventory of the whole class rather than two more assertions: +// a route that hands out stored credentials, dumps captured traffic, or writes +// the operator's CLI config must be hard-gated (ALWAYS_PROTECTED or +// LOCAL_ONLY), never left on the fail-open MANAGEMENT tier. + +const HARD_GATED_INVENTORY: ReadonlyArray<{ path: string; why: string }> = [ + // ── Reported in GHSA-5926-2w35-7h4q ────────────────────────────────────── + { + path: "/api/providers/6f3c1b7e-0000-4000-8000-000000000000/claude-auth/export", + why: "returns the connection's raw Claude OAuth access_token/refresh_token", + }, + { + path: "/api/providers/6f3c1b7e-0000-4000-8000-000000000000/codex-auth/export", + why: "returns the connection's raw Codex access_token/refresh_token/id_token", + }, + // ── Found sweeping the class while fixing the above ────────────────────── + { + path: "/api/logs/export", + why: "dumps call_logs (prompts and responses) and proxy_logs for up to 168h", + }, + { + path: "/api/cli-tools/codex-profiles", + why: "PUT writes attacker-supplied auth.json and config.toml into the operator's Codex CLI config", + }, + // ── Same family: WRITE the operator's credentials into host CLI files ─── + // These do not hand the credential to the caller, so they are a step below + // the export routes — but anonymous is still the wrong audience for "write + // this connection's token into ~/.codex/auth.json". ALWAYS_PROTECTED rather + // than LOCAL_ONLY on purpose: it closes the anonymous hole without breaking + // an operator driving the dashboard through a tunnel. + { + path: "/api/providers/6f3c1b7e-0000-4000-8000-000000000000/codex-auth/apply-local", + why: "writes the connection's credential into the host's ~/.codex/auth.json", + }, + { + path: "/api/providers/6f3c1b7e-0000-4000-8000-000000000000/claude-auth/apply-local", + why: "writes the connection's credential into the host's Claude CLI config", + }, + { + path: "/api/providers/agy-auth/apply-local", + why: "writes into ~/.gemini/antigravity-cli/antigravity-oauth-token", + }, + // ── Already fixed; pinned so a refactor cannot silently drop them ──────── + { path: "/api/db-backups/export", why: "GHSA-mghq-58h3-qcqj" }, + { path: "/api/db-backups/exportAll", why: "GHSA-mghq-58h3-qcqj" }, + { path: "/api/settings/export-json", why: "GHSA-v7g9-7f55-5g46" }, + { path: "/api/settings/import-json", why: "GHSA-v7g9-7f55-5g46" }, + { path: "/api/settings/database", why: "irreversible database replace" }, + { path: "/api/shutdown", why: "stops the server" }, + // ── Hard-gated by the LOCAL_ONLY tier instead ──────────────────────────── + { + path: "/api/tools/traffic-inspector/export.har", + why: "captured traffic can contain Authorization headers (LOCAL_ONLY)", + }, + { + path: "/api/tools/traffic-inspector/sessions/abc/export.har", + why: "same, per session (LOCAL_ONLY)", + }, +]; + +test("every credential/traffic export and CLI-config write is hard-gated", () => { + for (const { path, why } of HARD_GATED_INVENTORY) { + const gated = isAlwaysProtectedPath(path) || isLocalOnlyPath(path); + assert.ok( + gated, + `${path} is on the fail-open MANAGEMENT tier — anonymous under requireLogin=false. ${why}` + ); + } +}); + +test("the trailing-slash spelling is gated too", () => { + for (const path of [ + "/api/providers/abc/claude-auth/export/", + "/api/providers/abc/codex-auth/export/", + "/api/logs/export/", + "/api/cli-tools/codex-profiles/", + ]) { + assert.ok(isAlwaysProtectedPath(path) || isLocalOnlyPath(path), path); + } +}); + +test("the new patterns do not over-protect their neighbours", () => { + // The dynamic-segment entries must not swallow the rest of /api/providers/, + // which is ordinary MANAGEMENT and has to keep working under requireLogin=false. + for (const path of [ + "/api/providers", + "/api/providers/abc", + "/api/providers/abc/models", + "/api/providers/abc/claude-auth", + "/api/providers/abc/codex-auth", + "/api/providers/abc/claude-auth/apply", + "/api/providers/agy-auth", + "/api/logs", + "/api/cli-tools", + ]) { + assert.equal( + isAlwaysProtectedPath(path), + false, + `${path} must stay on the MANAGEMENT tier — hard-gating it breaks keyless local-first installs` + ); + } +}); + +test("a connection id cannot escape the pattern with a slash", () => { + // `[^/]+` is deliberate: a traversal-ish id must not match and silently drop + // back to the fail-open tier by looking like a different route. + assert.equal(isAlwaysProtectedPath("/api/providers/a/b/claude-auth/export"), false); +}); + +test("the plain-path allowlist keeps its existing entries", () => { + for (const p of ["/api/shutdown", "/api/settings/database", "/api/db-backups"]) { + assert.ok(ALWAYS_PROTECTED_API_PATHS.includes(p), p); + } +}); diff --git a/tests/unit/openapi-security-tiers.test.ts b/tests/unit/openapi-security-tiers.test.ts index 8203ab6f2d..d1380a5c08 100644 --- a/tests/unit/openapi-security-tiers.test.ts +++ b/tests/unit/openapi-security-tiers.test.ts @@ -7,8 +7,12 @@ import * as yaml from "js-yaml"; const ROOT = process.cwd(); const OPENAPI_PATH = path.join(ROOT, "docs", "openapi.yaml"); -const { LOCAL_ONLY_API_PREFIXES, LOCAL_ONLY_API_PATTERNS, ALWAYS_PROTECTED_API_PATHS } = - await import("../../src/server/authz/routeGuard.ts"); +const { + LOCAL_ONLY_API_PREFIXES, + LOCAL_ONLY_API_PATTERNS, + ALWAYS_PROTECTED_API_PATHS, + ALWAYS_PROTECTED_API_PATTERNS, +} = await import("../../src/server/authz/routeGuard.ts"); const raw: any = yaml.load(fs.readFileSync(OPENAPI_PATH, "utf-8")); const paths: Record = raw.paths || {}; @@ -131,12 +135,22 @@ test("every x-always-protected path matches ALWAYS_PROTECTED_API_PATHS in routeG for (const [method, spec] of Object.entries(methods as Record)) { if (!["get", "post", "put", "patch", "delete"].includes(method)) continue; if (spec?.["x-always-protected"] !== true) continue; - const matchesPath = (ALWAYS_PROTECTED_API_PATHS as ReadonlyArray).some( - (p: string) => pathStr === p || pathStr.startsWith(`${p}/`) - ); + // Routes with a dynamic segment cannot be expressed in the plain + // exact/prefix list, so routeGuard also carries ALWAYS_PROTECTED_API_PATTERNS + // (GHSA-5926-2w35-7h4q). Substitute a concrete value for the OpenAPI + // `{param}` placeholders before testing those. + const concretePath = pathStr.replace(/\{[^}]+\}/g, "sample-id"); + const matchesPath = + (ALWAYS_PROTECTED_API_PATHS as ReadonlyArray).some( + (p: string) => pathStr === p || pathStr.startsWith(`${p}/`) + ) || + (ALWAYS_PROTECTED_API_PATTERNS as ReadonlyArray).some((re) => + re.test(concretePath) + ); assert.ok( matchesPath, - `YAML path "${pathStr}" ${method.toUpperCase()} has x-always-protected but is NOT in ALWAYS_PROTECTED_API_PATHS. ` + + `YAML path "${pathStr}" ${method.toUpperCase()} has x-always-protected but is NOT in ALWAYS_PROTECTED_API_PATHS ` + + `nor matched by ALWAYS_PROTECTED_API_PATTERNS. ` + `Entries: ${(ALWAYS_PROTECTED_API_PATHS as ReadonlyArray).join(", ")}` ); }