fix(auth): tighten bridge claim protocol

This commit is contained in:
2026-08-16 22:44:44 +02:00
parent 7d9ca13f1d
commit 8477a69a29
7 changed files with 333 additions and 40 deletions
+3
View File
@@ -609,6 +609,9 @@ function claimOidcState(directory: string, filename: string): OidcStateClaim | u
before = fileIdentity(lstatSync(statePath) as Stats);
} catch (error) {
if (isNotFound(error)) return undefined;
// A winner can install its hard-link after the first claim check and before this state
// identity check. That process owns the state; this contender must fail closed as used.
if (oidcClaimExists(directory, filename)) return undefined;
throw invalid();
}
try {
+33 -9
View File
@@ -70,6 +70,8 @@ export interface WindowsAuthStorageBridgeOptions {
thtExecutable?: string;
/** Test-only child-launch seam; production uses the fixed no-shell Node child-process launcher. */
spawnChild?: WindowsAuthStorageSpawn;
/** Test-only input scheduling seam for real child-process lifecycle tests. */
beforeInputForTest?: () => Promise<void>;
}
const responseSchema = z.strictObject({
@@ -117,8 +119,8 @@ function validateRoot(root: string): void {
|| !win32.isAbsolute(root) || win32.normalize(root) !== root) throw invalid();
}
function validateFilename(filename: string, claim = false): void {
if (typeof filename !== "string" || !(claim ? CLAIM_FILENAME : DIGEST_FILENAME).test(filename)) throw invalid();
function validateFilename(filename: string, allowClaim = false): void {
if (typeof filename !== "string" || (!DIGEST_FILENAME.test(filename) && !(allowClaim && CLAIM_FILENAME.test(filename)))) throw invalid();
}
function safeThtExecutable(value: string | undefined): string {
@@ -144,7 +146,16 @@ function parseResponse(result: WindowsAuthStorageInvocationResult): BridgeRespon
function encodedRequest(request: BridgeRequest): Buffer {
validateRoot(request.root);
if (request.filename !== undefined) validateFilename(request.filename);
if (request.operation === "list") {
if (request.filename !== undefined || request.contentBase64 !== undefined) throw invalid();
} else {
if (request.filename === undefined) throw invalid();
const allowClaim = request.operation === "remove" && request.directory === "oidc";
validateFilename(request.filename, allowClaim);
if (request.contentBase64 !== undefined && request.operation !== "create" && request.operation !== "replace") throw invalid();
}
if ((request.operation === "claim-consume" || request.operation === "read-claim" || request.operation === "remove-claim")
&& request.directory !== "oidc") throw invalid();
if (request.contentBase64 !== undefined) canonicalBase64(request.contentBase64, directoryMaximum(request.directory));
const encoded = Buffer.from(JSON.stringify(request), "utf8");
if (encoded.length === 0 || encoded.length > MAX_PROTOCOL_BYTES) throw invalid();
@@ -165,6 +176,7 @@ const spawnTht: WindowsAuthStorageSpawn = (executable, args, options) => spawn(e
async function invokeTht(
invocation: WindowsAuthStorageInvocation,
spawnChild: WindowsAuthStorageSpawn = spawnTht,
beforeInputForTest?: () => Promise<void>,
): Promise<WindowsAuthStorageInvocationResult> {
return new Promise((resolve, reject) => {
let settled = false;
@@ -260,10 +272,18 @@ async function invokeTht(
stderr.push(Buffer.from(chunk));
});
stdin.once("error", abort);
try {
stdin.end(invocation.input);
} catch {
abort();
const writeInput = (): void => {
if (aborted || settled) return;
try {
stdin.end(invocation.input);
} catch {
abort();
}
};
if (beforeInputForTest === undefined) {
writeInput();
} else {
void Promise.resolve().then(beforeInputForTest).then(writeInput, abort);
}
});
}
@@ -279,7 +299,11 @@ function contentFrom(response: BridgeResponse, maximum: number): Buffer | undefi
export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridgeOptions = {}): WindowsAuthStorageBridge {
const executable = safeThtExecutable(options.thtExecutable);
const invoke = options.invoke ?? ((invocation: WindowsAuthStorageInvocation) => invokeTht(invocation, options.spawnChild));
const invoke = options.invoke ?? ((invocation: WindowsAuthStorageInvocation) => invokeTht(
invocation,
options.spawnChild,
options.beforeInputForTest,
));
const request = async (value: BridgeRequest): Promise<BridgeResponse> => {
try {
const response = await invoke({
@@ -325,7 +349,7 @@ export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridge
const response = await request({ version: PROTOCOL_VERSION, operation: "list", root, directory });
if (response.entries === undefined) throw invalid();
for (const entry of response.entries) {
if (!DIGEST_FILENAME.test(entry.name) && !CLAIM_FILENAME.test(entry.name)) throw invalid();
if (!DIGEST_FILENAME.test(entry.name) && !(directory === "oidc" && CLAIM_FILENAME.test(entry.name))) throw invalid();
}
return response.entries.map((entry) => ({ name: entry.name, modifiedUnixMs: entry.modifiedUnixMs }));
},
+87
View File
@@ -22,6 +22,7 @@ import { afterEach, describe, expect, test, vi } from "vitest";
const fsHooks = vi.hoisted(() => ({
afterRead: undefined as undefined | (() => void),
afterWrite: undefined as undefined | (() => void),
beforeLstat: undefined as undefined | ((path: string) => boolean),
afterLstat: undefined as undefined | ((path: string) => boolean),
transformLstat: undefined as undefined | ((path: string, info: import("node:fs").Stats) => import("node:fs").Stats),
}));
@@ -45,6 +46,8 @@ vi.mock("node:fs", async (importOriginal) => {
return result;
},
lstatSync: (...args: Parameters<typeof actual.lstatSync>) => {
const before = fsHooks.beforeLstat;
if (before?.(String(args[0]))) fsHooks.beforeLstat = undefined;
const original = actual.lstatSync(...args);
const result = fsHooks.transformLstat?.(String(args[0]), original) ?? original;
const callback = fsHooks.afterLstat;
@@ -69,6 +72,7 @@ const validLocalUser = { enabled: true, authRevision: 7, roles: ["admin"] as con
afterEach(() => {
fsHooks.afterRead = undefined;
fsHooks.afterWrite = undefined;
fsHooks.beforeLstat = undefined;
fsHooks.afterLstat = undefined;
fsHooks.transformLstat = undefined;
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
@@ -297,6 +301,23 @@ describe("file-backed auth session store", () => {
expect(existsSync(statePath)).toBe(true);
});
test("treats a competing OIDC claim installed between availability and state checks as unavailable", async () => {
const storageRoot = root();
const store = validStore(storageRoot);
const created = await store.createOidcState({ nonce: "n".repeat(43), codeVerifier: "v".repeat(43), returnTo: "/" }, base);
const statePath = digestPath(storageRoot, "oidc", created.state);
const stateClaimPath = claimPath(storageRoot, created.state);
fsHooks.beforeLstat = (observed) => {
if (observed !== statePath) return false;
linkSync(statePath, stateClaimPath);
return true;
};
await expect(store.consumeOidcState(created.state)).resolves.toBeUndefined();
expect(existsSync(statePath)).toBe(true);
expect(existsSync(stateClaimPath)).toBe(true);
});
test("prunes an expired OIDC state abandoned after an atomic claim", async () => {
const storageRoot = root();
const store = validStore(storageRoot);
@@ -639,6 +660,72 @@ describe("file-backed auth session store", () => {
}
});
test("prunes empty Windows directories through the required Go entries array", async () => {
const bridge = createWindowsAuthStorageBridge({
thtExecutable: "C:\\tht.exe",
invoke: async () => ({
code: 0,
stdout: Buffer.from('{"version":1,"ok":true,"entries":[]}\n'),
stderr: Buffer.alloc(0),
}),
});
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
if (!originalPlatform) throw new Error("platform descriptor unavailable");
Object.defineProperty(process, "platform", { configurable: true, value: "win32" });
try {
const store = createFileAuthSessionStore("C:\\ProgramData\\ThothII\\auth", {
currentAuthConfigRevision: () => revision,
findLocalUser: async () => validLocalUser,
}, { windowsStorageBridge: bridge });
await expect(store.prune(base)).resolves.toBe(0);
} finally {
Object.defineProperty(process, "platform", originalPlatform);
}
});
test("prunes a stale Windows OIDC orphan claim through the Go DTO path", async () => {
const claim = `${"d".repeat(64)}.claim`;
let removed = false;
const bridge = createWindowsAuthStorageBridge({
thtExecutable: "C:\\tht.exe",
invoke: async ({ input }) => {
const request = JSON.parse(input.toString("utf8")) as { operation: string; directory: string; filename?: string };
const response = (value: Record<string, unknown>) => ({
code: 0,
stdout: Buffer.from(`${JSON.stringify({ version: 1, ok: true, ...value })}\n`),
stderr: Buffer.alloc(0),
});
if (request.operation === "list") {
return response({
entries: request.directory === "oidc"
? [{ name: claim, modifiedUnixMs: base.getTime() }]
: [],
});
}
if (request.operation === "remove" && request.directory === "oidc" && request.filename === claim) {
removed = true;
return response({ removed: true });
}
throw new Error("unexpected bridge request");
},
});
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
if (!originalPlatform) throw new Error("platform descriptor unavailable");
Object.defineProperty(process, "platform", { configurable: true, value: "win32" });
try {
const store = createFileAuthSessionStore("C:\\ProgramData\\ThothII\\auth", {
currentAuthConfigRevision: () => revision,
findLocalUser: async () => validLocalUser,
}, { windowsStorageBridge: bridge });
await expect(store.prune(new Date(base.getTime() + 11 * 60_000))).resolves.toBe(1);
expect(removed).toBe(true);
} finally {
Object.defineProperty(process, "platform", originalPlatform);
}
});
test("revokes on config, local-user, revision, enabled, or role mismatch before returning", async () => {
const storageRoot = root();
let currentRevision = revision;
@@ -0,0 +1,21 @@
import { appendFileSync, closeSync, writeFileSync } from "node:fs";
const [, , mode, marker] = process.argv;
writeFileSync(marker, `started:${process.pid}\n`);
process.on("exit", () => appendFileSync(marker, "exited\n"));
process.on("SIGTERM", () => {
appendFileSync(marker, "terminated\n");
process.exit(0);
});
if (mode === "stdin") {
closeSync(0);
appendFileSync(marker, "stdin-closed\n");
} else if (mode === "stdout") {
process.stdout.write(Buffer.alloc(64 * 1024 + 1));
} else if (mode === "stderr") {
process.stderr.write(Buffer.alloc(64 * 1024 + 1));
}
setInterval(() => {}, 1_000);
+113 -1
View File
@@ -1,10 +1,63 @@
import { appendFileSync, chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import { spawn } from "node:child_process";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { fileURLToPath } from "node:url";
import { EventEmitter } from "node:events";
import { PassThrough } from "node:stream";
import { describe, expect, test, vi } from "vitest";
import { afterEach, describe, expect, test, vi } from "vitest";
import { createWindowsAuthStorageBridge } from "../src/auth/windows-auth-storage.js";
const root = "C:\\ProgramData\\ThothII\\auth";
const filename = "a".repeat(64) + ".json";
const realChildFixture = fileURLToPath(new URL("./fixtures/windows-auth-storage-real-child.mjs", import.meta.url));
const fixtureRoots: string[] = [];
function shellQuote(value: string): string {
return `'${value.replaceAll("'", `'\\''`)}'`;
}
async function waitForMarker(marker: string, expected: string): Promise<void> {
const deadline = Date.now() + 3_000;
while (Date.now() < deadline) {
if (existsSync(marker) && readFileSync(marker, "utf8").includes(expected)) return;
await new Promise<void>((resolve) => setTimeout(resolve, 10));
}
throw new Error(`real helper marker did not contain ${expected}`);
}
function realChildBridge(mode: "timeout" | "stdout" | "stderr" | "stdin") {
const directory = mkdtempSync(join(tmpdir(), "thothii-auth-bridge-child-"));
fixtureRoots.push(directory);
const marker = join(directory, "marker.txt");
const launcher = join(directory, "tht.exe");
writeFileSync(launcher, `#!/bin/sh\nexec ${shellQuote(process.execPath)} ${shellQuote(realChildFixture)} ${shellQuote(mode)} ${shellQuote(marker)} "$@"\n`, { mode: 0o700 });
chmodSync(launcher, 0o700);
return {
marker,
bridge: createWindowsAuthStorageBridge({
thtExecutable: "C:\\tht.exe",
spawnChild: (_executable, args, options) => spawn(launcher, [...args], options),
...(mode === "stdin" ? {
beforeInputForTest: async () => {
await waitForMarker(marker, "stdin-closed");
appendFileSync(marker, "before-input\n");
},
} : {}),
} as never),
};
}
afterEach(() => {
for (const directory of fixtureRoots.splice(0)) {
const marker = join(directory, "marker.txt");
try {
const pid = Number(/^started:(\d+)$/m.exec(readFileSync(marker, "utf8"))?.[1]);
if (Number.isSafeInteger(pid) && pid > 0) process.kill(pid, "SIGKILL");
} catch { /* the test-owned child already exited or did not start */ }
rmSync(directory, { recursive: true, force: true });
}
});
class FakeBridgeChild extends EventEmitter {
readonly stdin = new PassThrough();
@@ -110,6 +163,52 @@ describe("Windows auth-storage bridge", () => {
]);
});
test("parses the Go helper's required empty entries array", async () => {
const bridge = createWindowsAuthStorageBridge({
thtExecutable: "C:\\tht.exe",
invoke: async () => ({
code: 0,
stdout: Buffer.from('{"version":1,"ok":true,"entries":[]}\n'),
stderr: Buffer.alloc(0),
}),
});
await expect(bridge.list(root, "sessions")).resolves.toEqual([]);
});
test("allows only canonical OIDC claim removal and rejects claims elsewhere", async () => {
const claim = `${"b".repeat(64)}.claim`;
const invoke = vi.fn(async () => ({
code: 0,
stdout: Buffer.from('{"version":1,"ok":true,"removed":true}\n'),
stderr: Buffer.alloc(0),
}));
const bridge = createWindowsAuthStorageBridge({ thtExecutable: "C:\\tht.exe", invoke });
await expect(bridge.remove(root, "oidc", claim)).resolves.toBe(true);
await expect(bridge.remove(root, "sessions", claim)).rejects.toThrow("auth_session_store_invalid");
await expect(bridge.read(root, "oidc", claim)).rejects.toThrow("auth_session_store_invalid");
await expect(bridge.create(root, "oidc", claim, Buffer.from("record"))).rejects.toThrow("auth_session_store_invalid");
await expect(bridge.replace(root, "oidc", claim, Buffer.from("record"))).rejects.toThrow("auth_session_store_invalid");
await expect(bridge.remove(root, "oidc", `../${claim}`)).rejects.toThrow("auth_session_store_invalid");
await expect(bridge.remove(root, "oidc", `${claim}.bak`)).rejects.toThrow("auth_session_store_invalid");
expect(invoke).toHaveBeenCalledTimes(1);
});
test("rejects OIDC claim entries returned for a sessions list", async () => {
const claim = `${"c".repeat(64)}.claim`;
const bridge = createWindowsAuthStorageBridge({
thtExecutable: "C:\\tht.exe",
invoke: async () => ({
code: 0,
stdout: Buffer.from(`{"version":1,"ok":true,"entries":[{"name":"${claim}","modifiedUnixMs":1}]}\n`),
stderr: Buffer.alloc(0),
}),
});
await expect(bridge.list(root, "sessions")).rejects.toThrow("auth_session_store_invalid");
});
test("aborts a stdin-closed looping helper on timeout and waits for close", async () => {
vi.useFakeTimers();
const child = new FakeBridgeChild();
@@ -189,4 +288,17 @@ describe("Windows auth-storage bridge", () => {
await expect(bridge.list(root, "sessions")).rejects.toThrow("auth_session_store_invalid");
});
test.each(["timeout", "stdout", "stderr", "stdin"] as const)("kills a real %s helper process through the production spawn path", async (mode) => {
const { bridge, marker } = realChildBridge(mode);
const startedAt = Date.now();
const pending = bridge.list(root, "sessions");
const outcome = pending.then(() => undefined, (error: unknown) => error);
await waitForMarker(marker, "started");
if (mode === "stdin") await waitForMarker(marker, "before-input");
await expect(outcome).resolves.toMatchObject({ message: "auth_session_store_invalid" });
await waitForMarker(marker, "terminated");
if (mode === "stdin") expect(Date.now() - startedAt).toBeLessThan(2_000);
}, 10_000);
});