mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-14 02:42:24 +03:00
fix(authz): hard-gate every credential export and CLI-config write (GHSA-5926-2w35-7h4q) (#12600)
* 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
* 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).
This commit is contained in:
committed by
GitHub
parent
3f3d27e264
commit
49c4a620ca
@@ -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
|
||||
|
||||
@@ -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))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
|
||||
132
tests/unit/authz/credential-export-always-protected.test.ts
Normal file
132
tests/unit/authz/credential-export-always-protected.test.ts
Normal 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);
|
||||
}
|
||||
});
|
||||
@@ -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(", ")}`
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user