fix(auth): address Task 13 deployment review findings
This commit is contained in:
@@ -82,6 +82,12 @@ export function authenticateSession(deps: AuthDependencies): preHandlerHookHandl
|
||||
const snapshot = captureAuthConfigSnapshot(request, deps.authentication);
|
||||
if (isPublicRoute(request)) return;
|
||||
|
||||
const operator = loopbackMaintenancePrincipal(request);
|
||||
if (operator) {
|
||||
request.principal = operator;
|
||||
return;
|
||||
}
|
||||
|
||||
if (legacy) {
|
||||
await legacy(request, reply);
|
||||
if (reply.sent || !STATE_CHANGING_METHODS.has(request.method)) return;
|
||||
@@ -138,6 +144,15 @@ export function authenticateSession(deps: AuthDependencies): preHandlerHookHandl
|
||||
};
|
||||
}
|
||||
|
||||
function loopbackMaintenancePrincipal(request: FastifyRequest): PrincipalContext | undefined {
|
||||
if (request.ip !== "127.0.0.1" && request.ip !== "::1" && request.ip !== "::ffff:127.0.0.1") return undefined;
|
||||
if (singleHeader(request.headers["x-thoth-principal-issuer"]) !== "tht"
|
||||
|| singleHeader(request.headers["x-thoth-principal-subject"]) !== "tht-maintenance"
|
||||
|| singleHeader(request.headers["x-thoth-principal-display-name"]) !== "Tht maintenance"
|
||||
|| singleHeader(request.headers["x-thoth-is-admin"]) !== "1") return undefined;
|
||||
return upstreamPrincipal(request.headers);
|
||||
}
|
||||
|
||||
export function requireCsrf(request: FastifyRequest, reply: FastifyReply): true | FastifyReply {
|
||||
const expectedOrigin = request.authPublicOrigin;
|
||||
const token = request.authSessionToken;
|
||||
|
||||
@@ -173,7 +173,7 @@ function validateFilename(filename: string, allowClaim = false, allowOidcSlot =
|
||||
}
|
||||
|
||||
function safeThtExecutable(value: string | undefined, pathStyle: AuthStoragePathStyle): string {
|
||||
const executable = value ?? process.env.THT_BIN ?? "tht";
|
||||
const executable = value ?? process.env.THT_AUTH_STORAGE_BIN ?? process.env.THT_BIN ?? "tht";
|
||||
if (typeof executable !== "string" || executable.length === 0 || /[\u0000-\u001f\u007f]/.test(executable)) throw invalid();
|
||||
if (executable === "tht" || (pathStyle === "windows" && executable === "tht.exe")) return executable;
|
||||
const paths = pathStyle === "windows" ? win32 : posix;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { buildApp, type AppWithAuthSessionStore } from "./app.js";
|
||||
import { loadConfig } from "./config.js";
|
||||
import { formatStartupFailure } from "./startup-error.js";
|
||||
const config = loadConfig(process.env);
|
||||
const app = buildApp(config) as AppWithAuthSessionStore;
|
||||
|
||||
@@ -17,7 +18,7 @@ async function start(): Promise<void> {
|
||||
console.log(`backend listening on ${address}`);
|
||||
}
|
||||
|
||||
void start().catch(() => {
|
||||
console.error("backend startup failed");
|
||||
void start().catch((error: unknown) => {
|
||||
console.error(formatStartupFailure(error));
|
||||
process.exitCode = 1;
|
||||
});
|
||||
|
||||
@@ -0,0 +1,12 @@
|
||||
const STARTUP_CAUSES = new Set([
|
||||
"auth_config_invalid",
|
||||
"auth_session_store_invalid",
|
||||
"workspace_registry_invalid",
|
||||
]);
|
||||
|
||||
/** Return one bounded machine cause; never include the original error text or stack. */
|
||||
export function formatStartupFailure(error: unknown): string {
|
||||
const message = error instanceof Error ? error.message : "";
|
||||
const cause = STARTUP_CAUSES.has(message) ? message : "startup_unknown";
|
||||
return `backend startup failed: ${cause}`;
|
||||
}
|
||||
@@ -184,6 +184,43 @@ test("the session boundary exposes only exact health and authentication protocol
|
||||
expect((await app.inject({ method: "GET", url: "/auth/configured" })).statusCode).toBe(401);
|
||||
});
|
||||
|
||||
test("the session boundary retains the exact loopback tht maintenance identity in configured auth modes", async () => {
|
||||
const app = Fastify();
|
||||
app.addHook("preHandler", authenticateSession({ mode: "local" }));
|
||||
app.get("/private", async (request) => getPrincipal(request));
|
||||
app.post("/private", async (request) => getPrincipal(request));
|
||||
const headers = {
|
||||
"x-thoth-principal-issuer": "tht",
|
||||
"x-thoth-principal-subject": "tht-maintenance",
|
||||
"x-thoth-principal-display-name": "Tht maintenance",
|
||||
"x-thoth-is-admin": "1",
|
||||
};
|
||||
|
||||
for (const method of ["GET", "POST"] as const) {
|
||||
const response = await app.inject({ method, url: "/private", headers, remoteAddress: "127.0.0.1" });
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(response.json()).toMatchObject({ issuer: "tht", subject: "tht-maintenance", isAdmin: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("the session boundary rejects tht maintenance headers outside exact loopback provenance", async () => {
|
||||
const app = Fastify();
|
||||
app.addHook("preHandler", authenticateSession({ mode: "local" }));
|
||||
app.get("/private", async (request) => getPrincipal(request));
|
||||
const exact = {
|
||||
"x-thoth-principal-issuer": "tht",
|
||||
"x-thoth-principal-subject": "tht-maintenance",
|
||||
"x-thoth-principal-display-name": "Tht maintenance",
|
||||
"x-thoth-is-admin": "1",
|
||||
};
|
||||
|
||||
expect((await app.inject({ method: "GET", url: "/private", headers: exact, remoteAddress: "172.30.0.9" })).statusCode).toBe(503);
|
||||
expect((await app.inject({
|
||||
method: "GET", url: "/private", remoteAddress: "127.0.0.1",
|
||||
headers: { ...exact, "x-thoth-principal-subject": "not-maintenance" },
|
||||
})).statusCode).toBe(503);
|
||||
});
|
||||
|
||||
test("the session boundary touches a valid cookie session through the bounded Task 7 store operation", async () => {
|
||||
const sessions = {
|
||||
resolve: vi.fn(async () => ({
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { formatStartupFailure } from "../src/startup-error.js";
|
||||
|
||||
describe("formatStartupFailure", () => {
|
||||
it.each([
|
||||
[new Error("auth_session_store_invalid"), "backend startup failed: auth_session_store_invalid"],
|
||||
[new Error("auth_config_invalid"), "backend startup failed: auth_config_invalid"],
|
||||
[new Error("workspace_registry_invalid"), "backend startup failed: workspace_registry_invalid"],
|
||||
])("emits only an allowlisted startup cause", (error, expected) => {
|
||||
expect(formatStartupFailure(error)).toBe(expected);
|
||||
});
|
||||
|
||||
it("collapses unknown errors without exposing their message, stack, token, or path", () => {
|
||||
const error = new Error(
|
||||
"EACCES password=plain-secret token=token-secret at /run/secrets/private-token",
|
||||
);
|
||||
error.stack = "Error: raw failure\n at /app/backend/dist/server.js:42:1";
|
||||
|
||||
const formatted = formatStartupFailure(error);
|
||||
|
||||
expect(formatted).toBe("backend startup failed: startup_unknown");
|
||||
for (const leaked of [
|
||||
"EACCES", "plain-secret", "token-secret", "/run/secrets", "raw failure", "server.js",
|
||||
]) {
|
||||
expect(formatted).not.toContain(leaked);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -93,6 +93,29 @@ function bridgeForChild(child: FakeBridgeChild, pathStyle: "windows" | "posix" =
|
||||
}
|
||||
|
||||
describe("Windows auth-storage bridge", () => {
|
||||
test("uses a dedicated native storage executable without replacing the harness tht", async () => {
|
||||
vi.stubEnv("THT_BIN", "/opt/venv/bin/tht");
|
||||
vi.stubEnv("THT_AUTH_STORAGE_BIN", "/usr/local/bin/tht-auth-storage");
|
||||
const calls: Array<{ executable: string }> = [];
|
||||
const bridge = createPosixAuthStorageBridge({
|
||||
invoke: async (call) => {
|
||||
calls.push(call);
|
||||
return {
|
||||
code: 0,
|
||||
stdout: Buffer.from('{"version":1,"ok":true,"prepared":true}\n'),
|
||||
stderr: Buffer.alloc(0),
|
||||
};
|
||||
},
|
||||
});
|
||||
|
||||
await bridge.ensureLayout("/data/auth");
|
||||
|
||||
expect(calls).toHaveLength(1);
|
||||
expect(calls[0]!.executable).toBe("/usr/local/bin/tht-auth-storage");
|
||||
expect(process.env.THT_BIN).toBe("/opt/venv/bin/tht");
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
test("uses the same bounded hidden bridge to ensure a POSIX session layout", async () => {
|
||||
const calls: Array<{ executable: string; args: readonly string[]; input: Buffer; timeoutMs: number }> = [];
|
||||
const bridge = createPosixAuthStorageBridge({
|
||||
|
||||
Reference in New Issue
Block a user