Compare commits

...

2 Commits

Author SHA1 Message Date
diegosouzapw
2711b2d8af chore(quality): register the credential-export authz test in stryker tap.testFiles
The new tests/unit/authz/credential-export-always-protected.test.ts covers
src/server/authz/routeGuard.ts, so check:mutation-test-coverage --strict fails
until it is listed — its mutant kills would not count otherwise.

Inserted in place (no re-serialization: a JSON round-trip on this file reorders
~10 curated entries that are already out of alphabetical order, cf. #11438).
2026-09-03 09:05:50 -03:00
diegosouzapw
0c647ff1a4 fix(authz): hard-gate every credential export and CLI-config write
GHSA-5926-2w35-7h4q: `POST /api/providers/{id}/claude-auth/export` and
`.../codex-auth/export` gate on `requireManagementAuth(request)` with no
`alwaysRequireAuth`, and neither path was in ALWAYS_PROTECTED_API_PATHS. Under
`requireLogin=false` — the local-first default — both fail open, so anyone who
knows a connection id downloads the operator's raw Claude/Codex OAuth
access_token / refresh_token (plus the Codex 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
mghq had missed; these two are the siblings both missed. So the fix is written
against the class, not the two reported routes.

Sweeping every route that hands out stored credentials, dumps captured traffic,
or writes the operator's CLI config turned up four more on the fail-open tier:

- GET /api/logs/export — dumps call_logs (prompts and responses) and proxy_logs
  for up to 168h.
- /api/cli-tools/codex-profiles — GET leaks the operator's account label; PUT
  writes attacker-supplied auth.json and config.toml straight into the host's
  Codex CLI config. Its only guard is ensureCliConfigWriteAllowed() with no
  targetPath, which checks CLI_ALLOW_CONFIG_WRITES — default true. Paired with
  the POST that stores an arbitrary profile, that is: save a profile holding the
  attacker's auth.json, apply it, and the operator's CLI now runs on attacker
  credentials (or, via config.toml, an attacker base URL).
- {claude,codex}-auth/apply-local and providers/agy-auth/apply-local — write a
  stored credential into ~/.codex/auth.json and
  ~/.gemini/antigravity-cli/antigravity-oauth-token.

The traffic-inspector HAR exports were already covered by LOCAL_ONLY.

Routes with a dynamic segment cannot be expressed in the exact/prefix list — a
`/api/providers/` prefix would hard-gate the whole provider surface and break
every keyless install — so this adds ALWAYS_PROTECTED_API_PATTERNS, mirroring
the existing LOCAL_ONLY_API_PATTERNS, and `isAlwaysProtectedPath` consults both.

The apply-local routes get ALWAYS_PROTECTED rather than LOCAL_ONLY on purpose:
it closes the anonymous hole without breaking an operator driving the dashboard
through a tunnel.

Deliberately NOT adding `{ alwaysRequireAuth: true }` at the handlers. Tier 2 is
the architecture's designated mechanism and the guard runs before the handler; a
second copy of the same decision inside each route is exactly the kind of
duplicate that drifts out of sync (cf. the dashboardCsrf prefix scan that had to
be unified in #11417).

tests/unit/authz/credential-export-always-protected.test.ts — 5 tests, red
before the fix. Written as an inventory of the whole class rather than two more
assertions, plus negative cases: the neighbouring provider routes must stay on
MANAGEMENT, and a connection id containing a slash must not slip past `[^/]+`.

openapi.yaml marks the seven newly-gated operations `x-always-protected`, and
openapi-security-tiers.test.ts now resolves `{param}` placeholders so it can
validate the pattern entries too.

Reported by @skeletonsec.

Closes GHSA-5926-2w35-7h4q
2026-09-03 08:49:42 -03:00
5 changed files with 203 additions and 7 deletions

View File

@@ -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 <id> claude auth apply local"
x-always-protected: true
responses:
"200":
description: OK
@@ -10238,6 +10245,7 @@ paths:
tags:
- Providers
summary: "POST providers <id> claude auth export"
x-always-protected: true
responses:
"200":
description: OK
@@ -10246,6 +10254,7 @@ paths:
tags:
- Providers
summary: "POST providers <id> codex auth apply local"
x-always-protected: true
responses:
"200":
description: OK
@@ -10254,6 +10263,7 @@ paths:
tags:
- Providers
summary: "POST providers <id> codex auth export"
x-always-protected: true
responses:
"200":
description: OK

View File

@@ -140,6 +140,42 @@ export const ALWAYS_PROTECTED_API_PATHS: ReadonlyArray<string> = [
// 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<RegExp> = [
// `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))
);
}

View File

@@ -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",

View File

@@ -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);
}
});

View File

@@ -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<string, any> = 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<string, any>)) {
if (!["get", "post", "put", "patch", "delete"].includes(method)) continue;
if (spec?.["x-always-protected"] !== true) continue;
const matchesPath = (ALWAYS_PROTECTED_API_PATHS as ReadonlyArray<string>).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<string>).some(
(p: string) => pathStr === p || pathStr.startsWith(`${p}/`)
) ||
(ALWAYS_PROTECTED_API_PATTERNS as ReadonlyArray<RegExp>).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<string>).join(", ")}`
);
}