From 7e2b8399359d70b00fee81ea142a2e1f26441c0f Mon Sep 17 00:00:00 2001 From: Vittor Guilherme Borges de Oliveira Date: Sun, 5 Jul 2026 05:33:19 -0300 Subject: [PATCH] fix(security): require management auth for mutable cloud routes (#6233) (#6233) fix(security): require management auth for mutable cloud routes (#6233). Verified: 3 PR tests + full authz/route-guard suite 241/241 green. Thanks @vittoroliveira-dev. Integrated into release/v3.8.45. --- CHANGELOG.md | 2 + src/app/api/cloud/credentials/update/route.ts | 21 +- src/app/api/cloud/models/alias/route.ts | 19 +- src/lib/api/requireManagementAuth.ts | 27 ++- src/server/authz/classify.ts | 10 +- src/shared/constants/publicApiRoutes.ts | 22 +- tests/unit/authz/classify.test.ts | 34 ++- tests/unit/cloud-write-auth.test.ts | 221 ++++++++++++++++++ tests/unit/public-api-routes.test.ts | 10 + 9 files changed, 323 insertions(+), 43 deletions(-) create mode 100644 tests/unit/cloud-write-auth.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f467c5c73..6cc1e627c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ - **fix(antigravity):** strip a trailing assistant prefill turn for Vertex Claude models to avoid upstream 400s ([#6114](https://github.com/diegosouzapw/OmniRoute/pull/6114)). Regression guard: `tests/unit/antigravity-claude-prefill-strip.test.ts`. (thanks @anki1kr) +- **fix(security):** the mutable cloud-agent routes (`/api/cloud/credentials/update`, `/api/cloud/models/alias`) now require management auth instead of being treated as public. They were classified as public API routes, so a request without management credentials could update stored cloud-agent credentials and model aliases. They are removed from the public-route set, classified as management routes in the authz pipeline, and gated by `requireManagementAuth`; cloud **read**/auth routes stay public. Regression guards: `tests/unit/cloud-write-auth.test.ts`, `tests/unit/authz/classify.test.ts`, `tests/unit/public-api-routes.test.ts`. ([#6233](https://github.com/diegosouzapw/OmniRoute/pull/6233) — thanks @vittoroliveira-dev) + --- ## [3.8.43] — 2026-07-02 diff --git a/src/app/api/cloud/credentials/update/route.ts b/src/app/api/cloud/credentials/update/route.ts index 25b492e082..ad58f5fbd4 100644 --- a/src/app/api/cloud/credentials/update/route.ts +++ b/src/app/api/cloud/credentials/update/route.ts @@ -1,10 +1,17 @@ import { NextResponse } from "next/server"; -import { validateApiKey, getProviderConnections, updateProviderConnection } from "@/models"; +import { getProviderConnections, updateProviderConnection } from "@/models"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { cloudCredentialUpdateSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; // Update provider credentials (for cloud token refresh) export async function PUT(request: Request) { + const authError = await requireManagementAuth(request, { + alwaysRequireAuth: true, + invalidApiKeyStatus: 401, + }); + if (authError) return authError; + let rawBody; try { rawBody = await request.json(); @@ -16,24 +23,12 @@ export async function PUT(request: Request) { } try { - const authHeader = request.headers.get("Authorization"); - if (!authHeader?.startsWith("Bearer ")) { - return NextResponse.json({ error: "Missing API key" }, { status: 401 }); - } - - const apiKey = authHeader.slice(7); const validation = validateBody(cloudCredentialUpdateSchema, rawBody); if (isValidationFailure(validation)) { return NextResponse.json({ error: validation.error }, { status: 400 }); } const { provider, credentials } = validation.data; - // Validate API key - const isValid = await validateApiKey(apiKey); - if (!isValid) { - return NextResponse.json({ error: "Invalid API key" }, { status: 401 }); - } - // Find active connection for provider const connections = await getProviderConnections({ provider, isActive: true }); const connection = connections[0]; diff --git a/src/app/api/cloud/models/alias/route.ts b/src/app/api/cloud/models/alias/route.ts index cbc7a2923f..1f2ae36ad5 100644 --- a/src/app/api/cloud/models/alias/route.ts +++ b/src/app/api/cloud/models/alias/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { validateApiKey, getModelAliases, setModelAlias, isCloudEnabled } from "@/models"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { getConsistentMachineId } from "@/shared/utils/machineId"; import { syncToCloud } from "@/lib/cloudSync"; import { cloudModelAliasUpdateSchema } from "@/shared/validation/schemas"; @@ -7,6 +8,12 @@ import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; // PUT /api/cloud/models/alias - Set model alias (for cloud/CLI) export async function PUT(request: Request) { + const authError = await requireManagementAuth(request, { + alwaysRequireAuth: true, + invalidApiKeyStatus: 401, + }); + if (authError) return authError; + let rawBody; try { rawBody = await request.json(); @@ -18,18 +25,6 @@ export async function PUT(request: Request) { } try { - const authHeader = request.headers.get("authorization"); - const apiKey = authHeader?.replace("Bearer ", ""); - - if (!apiKey) { - return NextResponse.json({ error: "Missing API key" }, { status: 401 }); - } - - const isValid = await validateApiKey(apiKey); - if (!isValid) { - return NextResponse.json({ error: "Invalid API key" }, { status: 401 }); - } - const validation = validateBody(cloudModelAliasUpdateSchema, rawBody); if (isValidationFailure(validation)) { return NextResponse.json({ error: validation.error }, { status: 400 }); diff --git a/src/lib/api/requireManagementAuth.ts b/src/lib/api/requireManagementAuth.ts index 90b4fa7dd5..8540d17c20 100644 --- a/src/lib/api/requireManagementAuth.ts +++ b/src/lib/api/requireManagementAuth.ts @@ -21,8 +21,25 @@ export function hasManageScope(scopes: string[] = []): boolean { return hasManageScopeShared(scopes); } -export async function requireManagementAuth(request: Request): Promise { - if (!(await isAuthRequired(request))) { +interface RequireManagementAuthOptions { + alwaysRequireAuth?: boolean; + invalidApiKeyStatus?: 401 | 403; +} + +function invalidManagementTokenResponse(options: RequireManagementAuthOptions): Response { + const status = options.invalidApiKeyStatus ?? 403; + return createErrorResponse({ + status, + message: status === 401 ? "Invalid API key" : "Invalid management token", + type: "invalid_request", + }); +} + +export async function requireManagementAuth( + request: Request, + options: RequireManagementAuthOptions = {} +): Promise { + if (!options.alwaysRequireAuth && !(await isAuthRequired(request))) { return null; } @@ -73,11 +90,7 @@ export async function requireManagementAuth(request: Request): Promise>; try { if (!(await isValidApiKey(apiKey))) { - return createErrorResponse({ - status: 403, - message: "Invalid management token", - type: "invalid_request", - }); + return invalidManagementTokenResponse(options); } meta = await getApiKeyMetadata(apiKey); } catch { diff --git a/src/server/authz/classify.ts b/src/server/authz/classify.ts index f8fc361565..a5270860d6 100644 --- a/src/server/authz/classify.ts +++ b/src/server/authz/classify.ts @@ -1,7 +1,7 @@ import { - PUBLIC_API_ROUTE_PREFIXES, PUBLIC_READONLY_API_ROUTE_PREFIXES, PUBLIC_READONLY_METHODS, + isPublicApiRoute, } from "../../shared/constants/publicApiRoutes"; import type { ClassificationReason, RouteClassification } from "./types"; @@ -131,11 +131,5 @@ function matchesReadonlyPublic(path: string, method: string): boolean { } function isClassifiedAsPublic(path: string, method: string): boolean { - const isV1ApiPrefix = (p: string) => - p === "/api/v1" || p === "/api/v1/" || p.startsWith("/api/v1/"); - const filtered = PUBLIC_API_ROUTE_PREFIXES.filter((p) => p !== "/api/v1/"); - if (filtered.some((prefix) => path.startsWith(prefix)) && !isV1ApiPrefix(path)) { - return true; - } - return matchesReadonlyPublic(path, method); + return isPublicApiRoute(path, method); } diff --git a/src/shared/constants/publicApiRoutes.ts b/src/shared/constants/publicApiRoutes.ts index 26df9df12b..70853e142a 100644 --- a/src/shared/constants/publicApiRoutes.ts +++ b/src/shared/constants/publicApiRoutes.ts @@ -4,7 +4,6 @@ const PUBLIC_API_ROUTE_PREFIXES = [ "/api/auth/status", "/api/init", "/api/v1/", - "/api/cloud/", "/api/sync/bundle", "/api/oauth/", // Public, ticket-gated Codex device-flow completion (validate + persist). @@ -27,7 +26,28 @@ const PUBLIC_READONLY_API_ROUTE_PREFIXES = [ const PUBLIC_READONLY_METHODS = new Set(["GET", "HEAD", "OPTIONS"]); +const PUBLIC_CLOUD_API_ROUTES = [ + { path: "/api/cloud/auth", methods: new Set(["POST", "OPTIONS"]) }, + { path: "/api/cloud/model/resolve", methods: new Set(["POST", "OPTIONS"]) }, + { path: "/api/cloud/models/alias", methods: new Set(["GET", "HEAD", "OPTIONS"]) }, +]; + +function pathMatchesExactRoute(pathname: string, routePath: string): boolean { + return pathname === routePath || pathname === `${routePath}/`; +} + +function isPublicCloudApiRoute(pathname: string, method: string): boolean { + const normalizedMethod = String(method).toUpperCase(); + return PUBLIC_CLOUD_API_ROUTES.some( + ({ path, methods }) => pathMatchesExactRoute(pathname, path) && methods.has(normalizedMethod) + ); +} + export function isPublicApiRoute(pathname: string, method = "GET"): boolean { + if (isPublicCloudApiRoute(pathname, method)) { + return true; + } + if (PUBLIC_API_ROUTE_PREFIXES.some((route) => pathname.startsWith(route))) { return true; } diff --git a/tests/unit/authz/classify.test.ts b/tests/unit/authz/classify.test.ts index c9a76bb950..f091bb2bf9 100644 --- a/tests/unit/authz/classify.test.ts +++ b/tests/unit/authz/classify.test.ts @@ -136,11 +136,41 @@ const cases: Case[] = [ expectedClass: "PUBLIC", }, { - name: "/api/cloud/* is PUBLIC", - path: "/api/cloud/something", + name: "/api/cloud/auth POST is PUBLIC", + path: "/api/cloud/auth", + method: "POST", + expectedClass: "PUBLIC", + }, + { + name: "/api/cloud/model/resolve POST is PUBLIC", + path: "/api/cloud/model/resolve", + method: "POST", + expectedClass: "PUBLIC", + }, + { + name: "/api/cloud/models/alias GET is PUBLIC", + path: "/api/cloud/models/alias", method: "GET", expectedClass: "PUBLIC", }, + { + name: "/api/cloud/credentials/update PUT is MANAGEMENT", + path: "/api/cloud/credentials/update", + method: "PUT", + expectedClass: "MANAGEMENT", + }, + { + name: "/api/cloud/models/alias PUT is MANAGEMENT", + path: "/api/cloud/models/alias", + method: "PUT", + expectedClass: "MANAGEMENT", + }, + { + name: "/api/cloud/unknown GET is MANAGEMENT", + path: "/api/cloud/unknown", + method: "GET", + expectedClass: "MANAGEMENT", + }, { name: "/api/oauth/* is PUBLIC", path: "/api/oauth/callback", diff --git a/tests/unit/cloud-write-auth.test.ts b/tests/unit/cloud-write-auth.test.ts new file mode 100644 index 0000000000..7a04b1e40b --- /dev/null +++ b/tests/unit/cloud-write-auth.test.ts @@ -0,0 +1,221 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cloud-write-auth-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.DISABLE_SQLITE_AUTO_BACKUP = "true"; +process.env.JWT_SECRET = "cloud-write-auth-jwt"; +process.env.INITIAL_PASSWORD = "bootstrap-password"; +process.env.API_KEY_SECRET = "cloud-write-auth-api-key-secret"; + +type ApiKeyRecord = { key: string }; +type ProviderConnectionRecord = { + id: string; + accessToken?: string | null; + refreshToken?: string | null; + expiresAt?: string | null; +}; + +const core = await import("../../src/lib/db/core.ts"); +const localDb = await import("../../src/lib/localDb.ts"); +const credentialsRoute = await import("../../src/app/api/cloud/credentials/update/route.ts"); +const aliasRoute = await import("../../src/app/api/cloud/models/alias/route.ts"); + +async function resetStorage() { + delete process.env.OMNIROUTE_API_KEY; + delete process.env.ROUTER_API_KEY; + process.env.INITIAL_PASSWORD = "bootstrap-password"; + process.env.JWT_SECRET = "cloud-write-auth-jwt"; + process.env.API_KEY_SECRET = "cloud-write-auth-api-key-secret"; + core.resetDbInstance(); + localDb.resetApiKeyState(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); + await localDb.updateSettings({ requireLogin: true, password: "" }); +} + +async function createKey(scopes: string[] = []): Promise { + return localDb.createApiKey(`cloud-write-${scopes.join("-") || "none"}`, "machine-test", scopes); +} + +async function createActiveConnection(): Promise { + const connection = await localDb.createProviderConnection({ + provider: "openai", + authType: "oauth", + name: "OpenAI OAuth", + email: "owner@example.test", + isActive: true, + accessToken: "old-access-token", + refreshToken: "old-refresh-token", + expiresAt: "2026-01-01T00:00:00.000Z", + }); + assert.ok(connection?.id); + return connection as ProviderConnectionRecord; +} + +async function readActiveConnection(): Promise { + const [connection] = (await localDb.getProviderConnections({ + provider: "openai", + isActive: true, + })) as ProviderConnectionRecord[]; + assert.ok(connection); + return connection; +} + +function credentialUpdateBody() { + return { + provider: "openai", + credentials: { + accessToken: "new-access-secret", + refreshToken: "new-refresh-secret", + expiresIn: 3600, + }, + }; +} + +function aliasUpdateBody() { + return { + model: "openai/gpt-4o-mini", + alias: "fast-default", + }; +} + +function cloudCredentialsRequest(token: string | null, body = credentialUpdateBody()) { + const headers = new Headers({ "content-type": "application/json" }); + if (token) headers.set("authorization", `Bearer ${token}`); + return new Request("http://localhost/api/cloud/credentials/update", { + method: "PUT", + headers, + body: JSON.stringify(body), + }); +} + +function cloudAliasRequest(token: string | null, body = aliasUpdateBody()) { + const headers = new Headers({ "content-type": "application/json" }); + if (token) headers.set("authorization", `Bearer ${token}`); + return new Request("http://localhost/api/cloud/models/alias", { + method: "PUT", + headers, + body: JSON.stringify(body), + }); +} + +async function captureConsoleLog(fn: () => Promise): Promise<{ value: T; logs: string }> { + const originalLog = console.log; + const entries: string[] = []; + console.log = (...args: unknown[]) => { + entries.push(args.map((arg) => String(arg)).join(" ")); + }; + try { + return { value: await fn(), logs: entries.join("\n") }; + } finally { + console.log = originalLog; + } +} + +function assertTextDoesNotLeakSecrets(text: string, label: string, secrets: string[]) { + for (const secret of secrets) { + assert.equal(text.includes(secret), false, `${label} leaked secret: ${secret}`); + } +} + +async function assertResponseDoesNotLeakSecrets(response: Response, secrets: string[]) { + const text = await response.text(); + assertTextDoesNotLeakSecrets(text, "response", secrets); + return text.length > 0 ? JSON.parse(text) : null; +} + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(async () => { + core.resetDbInstance(); + localDb.resetApiKeyState(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("PUT /api/cloud/credentials/update rejects valid API key without manage/admin scope and leaves credentials unchanged", async () => { + await createActiveConnection(); + const key = await createKey(); + + const { value: response, logs } = await captureConsoleLog(() => + credentialsRoute.PUT(cloudCredentialsRequest(key.key)) + ); + const body = await assertResponseDoesNotLeakSecrets(response, [ + "new-access-secret", + "new-refresh-secret", + ]); + assertTextDoesNotLeakSecrets(logs, "logs", ["new-access-secret", "new-refresh-secret"]); + const connection = await readActiveConnection(); + + assert.equal(response.status, 403); + assert.match(body.error?.message || "", /manage/); + assert.equal(connection.accessToken, "old-access-token"); + assert.equal(connection.refreshToken, "old-refresh-token"); + assert.equal(connection.expiresAt, "2026-01-01T00:00:00.000Z"); +}); + +test("PUT /api/cloud/models/alias rejects valid API key without manage/admin scope and leaves aliases unchanged", async () => { + await localDb.setModelAlias("fast-default", "openai/original-model"); + const key = await createKey(); + + const { value: response, logs } = await captureConsoleLog(() => + aliasRoute.PUT(cloudAliasRequest(key.key)) + ); + const body = await assertResponseDoesNotLeakSecrets(response, ["openai/gpt-4o-mini"]); + assertTextDoesNotLeakSecrets(logs, "logs", ["openai/gpt-4o-mini"]); + const aliases = await localDb.getModelAliases(); + + assert.equal(response.status, 403); + assert.match(body.error?.message || "", /manage/); + assert.equal(aliases["fast-default"], "openai/original-model"); +}); + +test("PUT /api/cloud/credentials/update accepts API key with manage scope", async () => { + await createActiveConnection(); + const key = await createKey(["manage"]); + + const response = await credentialsRoute.PUT(cloudCredentialsRequest(key.key)); + const body = await response.json(); + const connection = await readActiveConnection(); + + assert.equal(response.status, 200); + assert.equal(body.success, true); + assert.equal(connection.accessToken, "new-access-secret"); + assert.equal(connection.refreshToken, "new-refresh-secret"); + assert.notEqual(connection.expiresAt, "2026-01-01T00:00:00.000Z"); +}); + +test("PUT /api/cloud/models/alias accepts API key with manage scope", async () => { + const key = await createKey(["manage"]); + + const response = await aliasRoute.PUT(cloudAliasRequest(key.key)); + const body = await response.json(); + const aliases = await localDb.getModelAliases(); + + assert.equal(response.status, 200); + assert.equal(body.success, true); + assert.equal(aliases["fast-default"], "openai/gpt-4o-mini"); +}); + +test("cloud write routes keep 401 for missing or invalid Bearer credentials", async () => { + await createActiveConnection(); + + const { value: missing, logs: missingLogs } = await captureConsoleLog(() => + credentialsRoute.PUT(cloudCredentialsRequest(null)) + ); + const { value: invalid, logs: invalidLogs } = await captureConsoleLog(() => + aliasRoute.PUT(cloudAliasRequest("sk-invalid")) + ); + await assertResponseDoesNotLeakSecrets(missing, ["new-access-secret", "new-refresh-secret"]); + await assertResponseDoesNotLeakSecrets(invalid, ["openai/gpt-4o-mini"]); + assertTextDoesNotLeakSecrets(missingLogs, "logs", ["new-access-secret", "new-refresh-secret"]); + assertTextDoesNotLeakSecrets(invalidLogs, "logs", ["openai/gpt-4o-mini"]); + + assert.equal(missing.status, 401); + assert.equal(invalid.status, 401); +}); diff --git a/tests/unit/public-api-routes.test.ts b/tests/unit/public-api-routes.test.ts index 00b2b02ecf..e480c528aa 100644 --- a/tests/unit/public-api-routes.test.ts +++ b/tests/unit/public-api-routes.test.ts @@ -9,6 +9,16 @@ test("isPublicApiRoute allows public management prefixes", () => { assert.equal(isPublicApiRoute("/api/oauth/cursor/callback"), true); }); +test("isPublicApiRoute keeps cloud read/auth routes public but not cloud write routes", () => { + assert.equal(isPublicApiRoute("/api/cloud/auth", "POST"), true); + assert.equal(isPublicApiRoute("/api/cloud/model/resolve", "POST"), true); + assert.equal(isPublicApiRoute("/api/cloud/models/alias", "GET"), true); + + assert.equal(isPublicApiRoute("/api/cloud/credentials/update", "PUT"), false); + assert.equal(isPublicApiRoute("/api/cloud/models/alias", "PUT"), false); + assert.equal(isPublicApiRoute("/api/cloud/unknown", "GET"), false); +}); + test("isPublicApiRoute allows readonly health and require-login bootstrap routes", () => { assert.equal(isPublicApiRoute("/api/monitoring/health", "GET"), true); assert.equal(isPublicApiRoute("/api/monitoring/health", "HEAD"), true);