fix(auth): restore permission boundary safeguards
This commit is contained in:
@@ -22,3 +22,23 @@ export function requirePermission(
|
|||||||
if (hasPermission(principal, permission)) return principal;
|
if (hasPermission(principal, permission)) return principal;
|
||||||
return reply.code(403).send({ code: "auth_forbidden", error: "This operation is not permitted" });
|
return reply.code(403).send({ code: "auth_forbidden", error: "This operation is not permitted" });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Permit non-browser clients and browsers whose declared origin matches the request host. */
|
||||||
|
export function requireSameOriginOrNonBrowser(
|
||||||
|
request: FastifyRequest,
|
||||||
|
reply: FastifyReply,
|
||||||
|
): FastifyReply | undefined {
|
||||||
|
const origin = request.headers.origin;
|
||||||
|
if (origin === undefined) return undefined;
|
||||||
|
if (typeof origin !== "string" || typeof request.headers.host !== "string") {
|
||||||
|
return reply.code(403).send({ code: "auth_forbidden", error: "This operation is not permitted" });
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
const supplied = new URL(origin);
|
||||||
|
const expected = new URL(`${request.protocol}://${request.headers.host}`);
|
||||||
|
if (supplied.origin === expected.origin) return undefined;
|
||||||
|
} catch {
|
||||||
|
// Invalid browser origins are forbidden below.
|
||||||
|
}
|
||||||
|
return reply.code(403).send({ code: "auth_forbidden", error: "This operation is not permitted" });
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,5 +1,9 @@
|
|||||||
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
||||||
import { isPrincipalContext, requirePermission } from "../auth/authorization.js";
|
import {
|
||||||
|
isPrincipalContext,
|
||||||
|
requirePermission,
|
||||||
|
requireSameOriginOrNonBrowser,
|
||||||
|
} from "../auth/authorization.js";
|
||||||
import { PiManagementError, type PiManagementService } from "../pi/management.js";
|
import { PiManagementError, type PiManagementService } from "../pi/management.js";
|
||||||
|
|
||||||
export function piManagementRoutes(
|
export function piManagementRoutes(
|
||||||
@@ -26,6 +30,10 @@ async function run<T>(
|
|||||||
): Promise<T | FastifyReply> {
|
): Promise<T | FastifyReply> {
|
||||||
const principal = requirePermission(request, reply, "pi.manage");
|
const principal = requirePermission(request, reply, "pi.manage");
|
||||||
if (!isPrincipalContext(principal)) return principal;
|
if (!isPrincipalContext(principal)) return principal;
|
||||||
|
if (principal.issuer === "local" && isWrite(request.method)) {
|
||||||
|
const csrfDenied = requireSameOriginOrNonBrowser(request, reply);
|
||||||
|
if (csrfDenied) return csrfDenied;
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
return await action();
|
return await action();
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
@@ -36,3 +44,7 @@ async function run<T>(
|
|||||||
return reply.code(503).send({ code: "pi_management_unavailable", error: "Pi management is unavailable" });
|
return reply.code(503).send({ code: "pi_management_unavailable", error: "Pi management is unavailable" });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isWrite(method: string): boolean {
|
||||||
|
return method === "POST" || method === "PUT" || method === "PATCH" || method === "DELETE";
|
||||||
|
}
|
||||||
|
|||||||
@@ -102,7 +102,8 @@ export function sessionRoutes(
|
|||||||
admissionLeases.set(req, release);
|
admissionLeases.set(req, release);
|
||||||
});
|
});
|
||||||
app.addHook("preHandler", async (req, reply) => {
|
app.addHook("preHandler", async (req, reply) => {
|
||||||
if (req.url === "/runtime/prewarm" || req.url === "/sessions" || req.url.startsWith("/sessions/")) {
|
const pathname = req.url.split("?", 1)[0];
|
||||||
|
if (pathname === "/runtime/prewarm" || pathname === "/sessions" || pathname.startsWith("/sessions/")) {
|
||||||
const principal = requirePermission(req, reply, "session.use");
|
const principal = requirePermission(req, reply, "session.use");
|
||||||
if (!isPrincipalContext(principal)) return principal;
|
if (!isPrincipalContext(principal)) return principal;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import type { ThtRunner } from "../tht/tht-runner.js";
|
|||||||
import type { PrincipalContext } from "../auth/principal.js";
|
import type { PrincipalContext } from "../auth/principal.js";
|
||||||
import type { Settings } from "../settings/settings-store.js";
|
import type { Settings } from "../settings/settings-store.js";
|
||||||
import type { WorkspaceRegistry } from "../workspaces/registry.js";
|
import type { WorkspaceRegistry } from "../workspaces/registry.js";
|
||||||
import { isPrincipalContext, requirePermission } from "../auth/authorization.js";
|
import { hasPermission, isPrincipalContext, requirePermission } from "../auth/authorization.js";
|
||||||
|
|
||||||
export function sqlRoutes(app: FastifyInstance, deps: {
|
export function sqlRoutes(app: FastifyInstance, deps: {
|
||||||
tht: ThtRunner; getSettings: (principal: PrincipalContext) => Promise<Settings>;
|
tht: ThtRunner; getSettings: (principal: PrincipalContext) => Promise<Settings>;
|
||||||
@@ -13,11 +13,16 @@ export function sqlRoutes(app: FastifyInstance, deps: {
|
|||||||
const runner = deps.tht as any;
|
const runner = deps.tht as any;
|
||||||
return typeof runner.withPrincipal === "function" ? runner.withPrincipal(principal) : runner;
|
return typeof runner.withPrincipal === "function" ? runner.withPrincipal(principal) : runner;
|
||||||
};
|
};
|
||||||
|
const readPrincipal = (principal: PrincipalContext): PrincipalContext => ({
|
||||||
|
...principal,
|
||||||
|
// The harness still consumes this compatibility bit; it must never exceed session.read_all.
|
||||||
|
isAdmin: hasPermission(principal, "session.read_all"),
|
||||||
|
});
|
||||||
const isNotFound = (error: unknown) => /not found|non trovata|inesistente|404/i.test(
|
const isNotFound = (error: unknown) => /not found|non trovata|inesistente|404/i.test(
|
||||||
error instanceof Error ? error.message : String(error),
|
error instanceof Error ? error.message : String(error),
|
||||||
);
|
);
|
||||||
const locate = async (principal: PrincipalContext, id: string, legacyWorkspace?: string) => {
|
const locate = async (principal: PrincipalContext, id: string, legacyWorkspace?: string) => {
|
||||||
const runner = runnerFor(principal);
|
const runner = runnerFor(readPrincipal(principal));
|
||||||
if (typeof runner.sessionShow !== "function") return { manifest: {}, workspace: legacyWorkspace };
|
if (typeof runner.sessionShow !== "function") return { manifest: {}, workspace: legacyWorkspace };
|
||||||
const registry = deps.workspaceRegistry as Partial<WorkspaceRegistry>;
|
const registry = deps.workspaceRegistry as Partial<WorkspaceRegistry>;
|
||||||
const revisions = typeof registry.listRetainedSnapshots === "function"
|
const revisions = typeof registry.listRetainedSnapshots === "function"
|
||||||
@@ -65,7 +70,7 @@ export function sqlRoutes(app: FastifyInstance, deps: {
|
|||||||
return reply.code(503).send({ error: "session storage is unavailable" });
|
return reply.code(503).send({ error: "session storage is unavailable" });
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
return await runnerFor(principal).sqlPreview(id, { limit, offset }, workspace);
|
return await runnerFor(readPrincipal(principal)).sqlPreview(id, { limit, offset }, workspace);
|
||||||
} catch (error: any) {
|
} catch (error: any) {
|
||||||
return reply.code(500).send({ error: error.message ?? String(error) });
|
return reply.code(500).send({ error: error.message ?? String(error) });
|
||||||
}
|
}
|
||||||
@@ -87,7 +92,7 @@ export function sqlRoutes(app: FastifyInstance, deps: {
|
|||||||
return reply.code(503).send({ error: "session storage is unavailable" });
|
return reply.code(503).send({ error: "session storage is unavailable" });
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
return await runnerFor(principal).sqlExport(id, workspace);
|
return await runnerFor(readPrincipal(principal)).sqlExport(id, workspace);
|
||||||
} catch (error: any) {
|
} catch (error: any) {
|
||||||
return reply.code(500).send({ error: error.message ?? String(error) });
|
return reply.code(500).send({ error: error.message ?? String(error) });
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { expect, test } from "vitest";
|
import { expect, test } from "vitest";
|
||||||
import Fastify from "fastify";
|
import Fastify from "fastify";
|
||||||
import { authPreHandler, getPrincipal } from "../src/auth/auth.js";
|
import { authPreHandler, getPrincipal } from "../src/auth/auth.js";
|
||||||
import { hasPermission } from "../src/auth/authorization.js";
|
import { hasPermission, requireSameOriginOrNonBrowser } from "../src/auth/authorization.js";
|
||||||
import type { PrincipalContext } from "../src/auth/principal.js";
|
import type { PrincipalContext } from "../src/auth/principal.js";
|
||||||
import type { Permission } from "../src/auth/types.js";
|
import type { Permission } from "../src/auth/types.js";
|
||||||
|
|
||||||
@@ -54,3 +54,21 @@ test("compatibility adapters derive roles and permissions before routes inspect
|
|||||||
expect(elevated.json()).toMatchObject({ roles: ["admin"], isAdmin: true });
|
expect(elevated.json()).toMatchObject({ roles: ["admin"], isAdmin: true });
|
||||||
expect(malformed.json()).toMatchObject({ roles: ["user"], isAdmin: false });
|
expect(malformed.json()).toMatchObject({ roles: ["user"], isAdmin: false });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("same-origin guard permits non-browser and same-origin calls but rejects a cross-origin browser", async () => {
|
||||||
|
const app = Fastify();
|
||||||
|
app.get("/guard", async (request, reply) => {
|
||||||
|
const denied = requireSameOriginOrNonBrowser(request, reply);
|
||||||
|
return denied ?? { ok: true };
|
||||||
|
});
|
||||||
|
|
||||||
|
expect((await app.inject({ method: "GET", url: "/guard" })).statusCode).toBe(200);
|
||||||
|
expect((await app.inject({
|
||||||
|
method: "GET", url: "/guard", headers: { host: "127.0.0.1:8080", origin: "http://127.0.0.1:8080" },
|
||||||
|
})).statusCode).toBe(200);
|
||||||
|
const denied = await app.inject({
|
||||||
|
method: "GET", url: "/guard", headers: { host: "127.0.0.1:8080", origin: "https://evil.example" },
|
||||||
|
});
|
||||||
|
expect(denied.statusCode).toBe(403);
|
||||||
|
expect(denied.json()).toEqual({ code: "auth_forbidden", error: "This operation is not permitted" });
|
||||||
|
});
|
||||||
|
|||||||
@@ -105,9 +105,9 @@ test("loopback-only AUTH_MODE=none may read the sanitized Pi status", async () =
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// Route authorization is permission-based; loopback none-mode derives its local admin principal
|
// A local implicit administrator has pi.manage, but a browser origin still cannot borrow that
|
||||||
// from trusted installation configuration rather than a browser Origin check.
|
// authority to mutate local configuration or trigger provider work.
|
||||||
test("loopback-only management accepts cross-origin writes for its local administrator", async () => {
|
test("loopback-only management rejects cross-origin writes for its local administrator", async () => {
|
||||||
const service = fakeService();
|
const service = fakeService();
|
||||||
const app = appWith(service);
|
const app = appWith(service);
|
||||||
try {
|
try {
|
||||||
@@ -121,10 +121,10 @@ test("loopback-only management accepts cross-origin writes for its local adminis
|
|||||||
headers: { host: "127.0.0.1:8080", origin: "https://evil.example" },
|
headers: { host: "127.0.0.1:8080", origin: "https://evil.example" },
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(configured.statusCode).toBe(200);
|
expect(configured.statusCode).toBe(403);
|
||||||
expect(smoke.statusCode).toBe(200);
|
expect(smoke.statusCode).toBe(403);
|
||||||
expect(service.configure).toHaveBeenCalledOnce();
|
expect(service.configure).not.toHaveBeenCalled();
|
||||||
expect(service.test).toHaveBeenCalledOnce();
|
expect(service.test).not.toHaveBeenCalled();
|
||||||
} finally {
|
} finally {
|
||||||
await app.close();
|
await app.close();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -10,6 +10,8 @@ import { SseHub } from "../src/sse/sse-hub.js";
|
|||||||
import { MaintenanceBarrier } from "../src/runtime/maintenance-gate.js";
|
import { MaintenanceBarrier } from "../src/runtime/maintenance-gate.js";
|
||||||
import { PiProcessManager } from "../src/pi/pi-process-manager.js";
|
import { PiProcessManager } from "../src/pi/pi-process-manager.js";
|
||||||
import { validateDeclarativePiConfig } from "../src/pi/managed-config.js";
|
import { validateDeclarativePiConfig } from "../src/pi/managed-config.js";
|
||||||
|
import Fastify from "fastify";
|
||||||
|
import { sessionRoutes } from "../src/routes/sessions.js";
|
||||||
|
|
||||||
const FAKE = path.resolve("../harness/tests/fake_pi/fake_pi_rpc.mjs");
|
const FAKE = path.resolve("../harness/tests/fake_pi/fake_pi_rpc.mjs");
|
||||||
const SCRIPT = path.resolve("../harness/tests/fake_pi/scripts/f1_disambiguation.json");
|
const SCRIPT = path.resolve("../harness/tests/fake_pi/scripts/f1_disambiguation.json");
|
||||||
@@ -99,6 +101,29 @@ test("upstream requests without a principal fail before a Pi runtime can be crea
|
|||||||
expect(created).toBe(false);
|
expect(created).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("GET /sessions with a query still requires session.use", async () => {
|
||||||
|
const app = Fastify();
|
||||||
|
app.addHook("preHandler", async (request) => {
|
||||||
|
request.principal = { issuer: "oidc", subject: "no-role", roles: [], permissions: [], isAdmin: false };
|
||||||
|
});
|
||||||
|
sessionRoutes(app, {
|
||||||
|
mgr: { get: () => undefined } as any,
|
||||||
|
tht: { sessionList: async () => [] } as any,
|
||||||
|
hub: {} as any,
|
||||||
|
getSettings: async () => ({}),
|
||||||
|
readiness: {} as any,
|
||||||
|
listModels: async () => [],
|
||||||
|
workspaceRegistry: { list: async () => [] } as any,
|
||||||
|
workspaceRuntimeSupport: () => true,
|
||||||
|
maintenanceBarrier: new MaintenanceBarrier(),
|
||||||
|
});
|
||||||
|
|
||||||
|
const response = await app.inject({ method: "GET", url: "/sessions?scope=mine" });
|
||||||
|
|
||||||
|
expect(response.statusCode).toBe(403);
|
||||||
|
expect(response.json()).toEqual({ code: "auth_forbidden", error: "This operation is not permitted" });
|
||||||
|
});
|
||||||
|
|
||||||
test("maintenance rejects new and resumed session admission without interrupting running sessions", async () => {
|
test("maintenance rejects new and resumed session admission without interrupting running sessions", async () => {
|
||||||
const maintenanceBarrier = new MaintenanceBarrier();
|
const maintenanceBarrier = new MaintenanceBarrier();
|
||||||
await maintenanceBarrier.activate();
|
await maintenanceBarrier.activate();
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
import { test, expect, vi } from "vitest";
|
import { test, expect, vi } from "vitest";
|
||||||
|
import Fastify from "fastify";
|
||||||
import { buildApp } from "../src/app.js";
|
import { buildApp } from "../src/app.js";
|
||||||
import { loadConfig } from "../src/config.js";
|
import { loadConfig } from "../src/config.js";
|
||||||
|
import { sqlRoutes } from "../src/routes/sql.js";
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// SQL routes
|
// SQL routes
|
||||||
@@ -29,6 +31,35 @@ test("POST /sessions/:id/sql/preview returns rows from injected thtRunner stub",
|
|||||||
expect(res.json()).toEqual(previewResult);
|
expect(res.json()).toEqual(previewResult);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("SQL lookup and execution derive harness admin compatibility from session.read_all", async () => {
|
||||||
|
const seen: boolean[] = [];
|
||||||
|
const app = Fastify();
|
||||||
|
app.addHook("preHandler", async (request) => {
|
||||||
|
request.principal = {
|
||||||
|
issuer: "portal", subject: "inconsistent", roles: ["user"], permissions: ["session.use"], isAdmin: true,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
const runner = {
|
||||||
|
sessionShow: async () => ({ id: "s1" }),
|
||||||
|
sqlPreview: async () => ({ columns: [], rows: [], execution_ms: 0, truncated: false }),
|
||||||
|
};
|
||||||
|
sqlRoutes(app, {
|
||||||
|
tht: {
|
||||||
|
withPrincipal: (principal: { isAdmin: boolean }) => {
|
||||||
|
seen.push(principal.isAdmin);
|
||||||
|
return runner;
|
||||||
|
},
|
||||||
|
} as any,
|
||||||
|
getSettings: async () => ({ workspace: "legacy" }),
|
||||||
|
workspaceRegistry: { list: async () => [] } as any,
|
||||||
|
});
|
||||||
|
|
||||||
|
const response = await app.inject({ method: "POST", url: "/sessions/s1/sql/preview", payload: {} });
|
||||||
|
|
||||||
|
expect(response.statusCode).toBe(200);
|
||||||
|
expect(seen).toEqual([false, false]);
|
||||||
|
});
|
||||||
|
|
||||||
test("POST /sessions/:id/sql/preview passes limit and offset to thtRunner", async () => {
|
test("POST /sessions/:id/sql/preview passes limit and offset to thtRunner", async () => {
|
||||||
let captured: any;
|
let captured: any;
|
||||||
const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), {
|
const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), {
|
||||||
|
|||||||
Reference in New Issue
Block a user