mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-01 21:02:12 +03:00
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.
This commit is contained in:
committed by
GitHub
parent
9ebb53e432
commit
7e2b839935
@@ -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
|
||||
|
||||
@@ -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];
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -21,8 +21,25 @@ export function hasManageScope(scopes: string[] = []): boolean {
|
||||
return hasManageScopeShared(scopes);
|
||||
}
|
||||
|
||||
export async function requireManagementAuth(request: Request): Promise<Response | null> {
|
||||
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<Response | null> {
|
||||
if (!options.alwaysRequireAuth && !(await isAuthRequired(request))) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -73,11 +90,7 @@ export async function requireManagementAuth(request: Request): Promise<Response
|
||||
let meta: Awaited<ReturnType<typeof getApiKeyMetadata>>;
|
||||
try {
|
||||
if (!(await isValidApiKey(apiKey))) {
|
||||
return createErrorResponse({
|
||||
status: 403,
|
||||
message: "Invalid management token",
|
||||
type: "invalid_request",
|
||||
});
|
||||
return invalidManagementTokenResponse(options);
|
||||
}
|
||||
meta = await getApiKeyMetadata(apiKey);
|
||||
} catch {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
|
||||
221
tests/unit/cloud-write-auth.test.ts
Normal file
221
tests/unit/cloud-write-auth.test.ts
Normal file
@@ -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<ApiKeyRecord> {
|
||||
return localDb.createApiKey(`cloud-write-${scopes.join("-") || "none"}`, "machine-test", scopes);
|
||||
}
|
||||
|
||||
async function createActiveConnection(): Promise<ProviderConnectionRecord> {
|
||||
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<ProviderConnectionRecord> {
|
||||
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<T>(fn: () => Promise<T>): 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);
|
||||
});
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user