From 3e7bb113131f68ac5d101771736f375cef447bc9 Mon Sep 17 00:00:00 2001 From: mptyl Date: Mon, 17 Aug 2026 07:46:43 +0200 Subject: [PATCH] fix(auth): isolate bounded OIDC cleanup --- backend/src/auth/oidc-client.ts | 17 +- backend/src/auth/session-store.ts | 317 +++++++++--------- backend/src/auth/windows-auth-storage.ts | 25 +- backend/test/auth-session-store.test.ts | 144 ++++++++ backend/test/oidc-client.test.ts | 45 +++ backend/test/windows-auth-storage.test.ts | 33 ++ tools/tht/internal/authstorage/storage.go | 33 +- .../tht/internal/authstorage/storage_test.go | 19 ++ 8 files changed, 456 insertions(+), 177 deletions(-) diff --git a/backend/src/auth/oidc-client.ts b/backend/src/auth/oidc-client.ts index 8a12dadf..00f60608 100644 --- a/backend/src/auth/oidc-client.ts +++ b/backend/src/auth/oidc-client.ts @@ -173,7 +173,7 @@ async function readWithAbort( signal.throwIfAborted(); return await new Promise((resolve, reject) => { const aborted = () => { - void reader.cancel().catch(() => undefined); + cancelReaderBestEffort(reader); reject(signal.reason); }; signal.addEventListener("abort", aborted, { once: true }); @@ -181,6 +181,14 @@ async function readWithAbort( }); } +function cancelReaderBestEffort(reader: ReadableStreamDefaultReader): void { + try { + void reader.cancel().catch(() => undefined); + } catch { + // Cancellation is advisory; the bounded fetch timeout/rejection remains authoritative. + } +} + async function boundedJwksBody(response: Response, signal: AbortSignal): Promise { if (!response.body) throw new OidcProtocolError(); const reader = response.body.getReader(); @@ -204,8 +212,11 @@ async function boundedJwksBody(response: Response, signal: AbortSignal): Promise } completed = true; } finally { - if (!completed) await reader.cancel().catch(() => undefined); - try { reader.releaseLock(); } catch { /* cancellation already made the response unusable */ } + try { + if (!completed) cancelReaderBestEffort(reader); + } finally { + try { reader.releaseLock(); } catch { /* cancellation already made the response unusable */ } + } } signal.throwIfAborted(); try { diff --git a/backend/src/auth/session-store.ts b/backend/src/auth/session-store.ts index 5d4a5e84..91bbea47 100644 --- a/backend/src/auth/session-store.ts +++ b/backend/src/auth/session-store.ts @@ -10,8 +10,8 @@ import { linkSync, mkdirSync, openSync, + opendirSync, readSync, - readdirSync, realpathSync, renameSync, unlinkSync, @@ -40,6 +40,8 @@ const MAX_OIDC_SLOT_RECORD_BYTES = 512; const MAX_TTL_MS = 365 * 24 * 60 * 60 * 1000; const OIDC_STATE_TTL_MS = 10 * 60 * 1000; const OIDC_STATE_CAPACITY = 64; +const MAX_OIDC_STORAGE_ENTRIES = OIDC_STATE_CAPACITY * 3; +const MAX_SESSION_PRUNE_ENTRIES = 512; const TOUCH_INTERVAL_MS = 5 * 60 * 1000; const CSRF_CONTEXT = Buffer.from("thothii-csrf-v1", "utf8"); const EMPTY_HKDF_SALT = Buffer.alloc(0); @@ -374,6 +376,33 @@ function storageDirectories(root: string): StorageDirectories { return { root, sessions, oidc }; } +function boundedDirectoryNames(directory: string, maximumEntries: number): string[] { + if (!Number.isInteger(maximumEntries) || maximumEntries < 1) throw invalid(); + let handle: ReturnType | undefined; + try { + const before = directoryIdentity(directory); + handle = opendirSync(directory); + const names: string[] = []; + while (names.length <= maximumEntries) { + const entry = handle.readSync(); + if (entry === null) { + handle.closeSync(); + handle = undefined; + if (!sameDirectoryIdentity(before, directoryIdentity(directory))) throw invalid(); + return names; + } + names.push(entry.name); + } + throw invalid(); + } catch { + throw invalid(); + } finally { + if (handle !== undefined) { + try { handle.closeSync(); } catch { /* the operation is already fail-closed */ } + } + } +} + function openDirectory(directory: string): number | undefined { if (process.platform === "win32") return undefined; return openSync(directory, constants.O_RDONLY | (constants.O_DIRECTORY ?? 0) @@ -814,14 +843,9 @@ export function createFileAuthSessionStore( async function oidcStorageEntries(): Promise { const entries = process.platform === "win32" - ? (await requiredWindowsStorage().list(root, "oidc")).map((entry) => entry.name) - : (() => { - try { - return readdirSync(storageDirectories(root).oidc); - } catch { - throw invalid(); - } - })(); + ? (await requiredWindowsStorage().list(root, "oidc", MAX_OIDC_STORAGE_ENTRIES)).map((entry) => entry.name) + : boundedDirectoryNames(storageDirectories(root).oidc, MAX_OIDC_STORAGE_ENTRIES); + if (entries.length > MAX_OIDC_STORAGE_ENTRIES) throw invalid(); if (entries.some((entry) => !DIGEST_FILENAME_PATTERN.test(entry) && !CLAIM_FILENAME_PATTERN.test(entry) && oidcSlotIndex(entry) === undefined)) throw invalid(); return entries; @@ -1076,6 +1100,121 @@ export function createFileAuthSessionStore( }); } + async function pruneOidcStates(nowMs: number): Promise { + if (process.platform === "win32") { + const bridge = requiredWindowsStorage(); + const oidcEntries = await bridge.list(root, "oidc", MAX_OIDC_STORAGE_ENTRIES); + if (oidcEntries.length > MAX_OIDC_STORAGE_ENTRIES) throw invalid(); + const stateNames = new Set(oidcEntries + .filter((entry) => DIGEST_FILENAME_PATTERN.test(entry.name)) + .map((entry) => entry.name)); + const claimEntries = new Map(oidcEntries + .filter((entry) => CLAIM_FILENAME_PATTERN.test(entry.name)) + .map((entry) => [entry.name, entry])); + const slotEntries = oidcEntries.filter((entry) => oidcSlotIndex(entry.name) !== undefined); + if (stateNames.size + claimEntries.size + slotEntries.length !== oidcEntries.length) throw invalid(); + let removed = 0; + for (const filename of stateNames) { + const claim = claimFilename(filename); + const contents = claimEntries.has(claim) + ? await bridge.readClaim(root, filename) + : await bridge.read(root, "oidc", filename); + if (!contents) continue; + const record = parseWindowsRecord(contents, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord); + if (oidcStateExpired(record, nowMs)) { + const didRemove = claimEntries.has(claim) + ? await bridge.removeClaim(root, filename) + : await bridge.remove(root, "oidc", filename); + if (didRemove) removed += 1; + } + } + for (const [claim, entry] of claimEntries) { + const filename = `${claim.slice(0, -".claim".length)}.json`; + if (stateNames.has(filename)) continue; + if (nowMs >= entry.modifiedUnixMs + OIDC_STATE_TTL_MS + && await bridge.remove(root, "oidc", claim)) removed += 1; + } + for (const entry of slotEntries) { + const contents = await bridge.read(root, "oidc", entry.name); + if (!contents) continue; + const slot = parseWindowsRecord(contents, MAX_OIDC_SLOT_RECORD_BYTES, parseOidcSlotRecord); + if (nowMs < Date.parse(slot.expiresAt)) continue; + const claim = claimFilename(slot.stateFilename); + const stateContents = claimEntries.has(claim) + ? await bridge.readClaim(root, slot.stateFilename) + : await bridge.read(root, "oidc", slot.stateFilename); + if (stateContents) { + const state = parseWindowsRecord(stateContents, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord); + if (!oidcStateExpired(state, nowMs)) throw invalid(); + const didRemove = claimEntries.has(claim) + ? await bridge.removeClaim(root, slot.stateFilename) + : await bridge.remove(root, "oidc", slot.stateFilename); + if (didRemove) removed += 1; + } + if (!await bridge.remove(root, "oidc", entry.name)) throw invalid(); + } + return removed; + } + + const directory = storageDirectories(root).oidc; + const oidcEntries = boundedDirectoryNames(directory, MAX_OIDC_STORAGE_ENTRIES); + const stateFilenames = new Set(oidcEntries.filter((entry) => DIGEST_FILENAME_PATTERN.test(entry))); + let removed = 0; + for (const filename of stateFilenames) { + await withLock(lockKey(root, "oidc", filename), async () => { + const claim = inspectOidcStateClaim(directory, filename); + if (claim === "orphan") return; + if (claim) { + if (oidcStateExpired(claim.state.value, nowMs)) { + removeClaimedOidcState(directory, filename, claim); + removed += 1; + } + return; + } + const trusted = readTrusted(directory, filename, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord); + if (trusted && oidcStateExpired(trusted.value, nowMs) + && removeTrusted(directory, filename, trusted.identity)) removed += 1; + }); + } + for (const claimedFilename of oidcEntries) { + if (!CLAIM_FILENAME_PATTERN.test(claimedFilename)) continue; + const filename = `${claimedFilename.slice(0, -".claim".length)}.json`; + if (stateFilenames.has(filename)) continue; + await withLock(lockKey(root, "oidc", filename), async () => { + const claim = inspectOidcStateClaim(directory, filename); + if (claim !== "orphan") return; + const claimIdentity = fileIdentity(lstatSync(filePath(directory, claimedFilename)) as Stats); + if (nowMs >= claimIdentity.mtimeMs + OIDC_STATE_TTL_MS + && removeTrusted(directory, claimedFilename, claimIdentity)) removed += 1; + }); + } + const slots = await storedOidcSlots(oidcEntries.filter((entry) => oidcSlotIndex(entry) !== undefined)); + for (const slot of slots) { + if (nowMs < Date.parse(slot.record.expiresAt)) continue; + await withLock(lockKey(root, "oidc", slot.record.stateFilename), async () => { + const claim = inspectOidcStateClaim(directory, slot.record.stateFilename); + if (claim && claim !== "orphan") { + if (!oidcStateExpired(claim.state.value, nowMs)) throw invalid(); + removeClaimedOidcState(directory, slot.record.stateFilename, claim); + removed += 1; + } else if (!claim) { + const trusted = readTrusted( + directory, + slot.record.stateFilename, + MAX_OIDC_STATE_RECORD_BYTES, + parseOidcStateRecord, + ); + if (trusted) { + if (!oidcStateExpired(trusted.value, nowMs)) throw invalid(); + if (removeTrusted(directory, slot.record.stateFilename, trusted.identity)) removed += 1; + } + } + await removeOidcSlot(slot); + }); + } + return removed; + } + async function createOidcState(input: OidcStateCreateInput, now = new Date()): Promise { const nowMs = dateMilliseconds(now); let validated: z.infer; @@ -1087,7 +1226,7 @@ export function createFileAuthSessionStore( const expiresMs = nowMs + OIDC_STATE_TTL_MS; if (!Number.isSafeInteger(expiresMs)) throw invalid(); return withLock(lockKey(root, "oidc", "capacity"), async () => { - await prune(now); + await pruneOidcStates(nowMs); for (let attempt = 0; attempt < 8; attempt += 1) { const state = randomBytes(TOKEN_BYTES).toString("base64url"); const filename = digestFilename(state); @@ -1147,8 +1286,8 @@ export function createFileAuthSessionStore( const nowMs = dateMilliseconds(now); if (process.platform === "win32") { const bridge = requiredWindowsStorage(); - const sessionEntries = await bridge.list(root, "sessions"); - const oidcEntries = await bridge.list(root, "oidc"); + const sessionEntries = await bridge.list(root, "sessions", MAX_SESSION_PRUNE_ENTRIES); + if (sessionEntries.length > MAX_SESSION_PRUNE_ENTRIES) throw invalid(); let removed = 0; for (const entry of sessionEntries) { if (!DIGEST_FILENAME_PATTERN.test(entry.name)) throw invalid(); @@ -1157,156 +1296,20 @@ export function createFileAuthSessionStore( const record = parseWindowsRecord(contents, MAX_SESSION_RECORD_BYTES, parseSessionRecord); if (sessionExpired(record, nowMs) && await bridge.remove(root, "sessions", entry.name)) removed += 1; } - const stateNames = new Set(oidcEntries.filter((entry) => DIGEST_FILENAME_PATTERN.test(entry.name)).map((entry) => entry.name)); - const claimEntries = new Map(oidcEntries - .filter((entry) => CLAIM_FILENAME_PATTERN.test(entry.name)) - .map((entry) => [entry.name, entry])); - const slotEntries = oidcEntries.filter((entry) => oidcSlotIndex(entry.name) !== undefined); - if (stateNames.size + claimEntries.size + slotEntries.length !== oidcEntries.length) throw invalid(); - for (const filename of stateNames) { - const claim = claimFilename(filename); - const contents = claimEntries.has(claim) - ? await bridge.readClaim(root, filename) - : await bridge.read(root, "oidc", filename); - if (!contents) continue; - const record = parseWindowsRecord(contents, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord); - if (oidcStateExpired(record, nowMs)) { - const didRemove = claimEntries.has(claim) - ? await bridge.removeClaim(root, filename) - : await bridge.remove(root, "oidc", filename); - if (didRemove) removed += 1; - } - } - for (const [claim, entry] of claimEntries) { - const filename = `${claim.slice(0, -".claim".length)}.json`; - if (stateNames.has(filename)) continue; - if (nowMs >= entry.modifiedUnixMs + OIDC_STATE_TTL_MS - && await bridge.remove(root, "oidc", claim)) removed += 1; - } - for (const entry of slotEntries) { - const contents = await bridge.read(root, "oidc", entry.name); - if (!contents) continue; - const slot = parseWindowsRecord(contents, MAX_OIDC_SLOT_RECORD_BYTES, parseOidcSlotRecord); - if (nowMs < Date.parse(slot.expiresAt)) continue; - const claim = claimFilename(slot.stateFilename); - const stateContents = claimEntries.has(claim) - ? await bridge.readClaim(root, slot.stateFilename) - : await bridge.read(root, "oidc", slot.stateFilename); - if (stateContents) { - const state = parseWindowsRecord(stateContents, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord); - if (!oidcStateExpired(state, nowMs)) throw invalid(); - const didRemove = claimEntries.has(claim) - ? await bridge.removeClaim(root, slot.stateFilename) - : await bridge.remove(root, "oidc", slot.stateFilename); - if (didRemove) removed += 1; - } - if (!await bridge.remove(root, "oidc", entry.name)) throw invalid(); - } - return removed; + return removed + await pruneOidcStates(nowMs); } const directories = storageDirectories(root); let removed = 0; - const pruneDirectory = async ( - directory: string, - kind: "sessions" | "oidc", - maximumBytes: number, - parse: (source: string) => AuthSessionRecord | OidcStateRecord, - expired: (record: AuthSessionRecord | OidcStateRecord, timestamp: number) => boolean, - ): Promise => { - let entries: string[]; - try { - entries = readdirSync(directory); - } catch { - throw invalid(); - } - for (const filename of entries) { - if (!DIGEST_FILENAME_PATTERN.test(filename)) continue; - await withLock(lockKey(root, kind, filename), async () => { - const trusted = readTrusted(directory, filename, maximumBytes, parse); - if (trusted && expired(trusted.value, nowMs)) { - if (removeTrusted(directory, filename, trusted.identity)) removed += 1; - } - }); - } - }; - await pruneDirectory( - directories.sessions, - "sessions", - MAX_SESSION_RECORD_BYTES, - parseSessionRecord, - (record, timestamp) => sessionExpired(record as AuthSessionRecord, timestamp), - ); - let oidcEntries: string[]; - try { - oidcEntries = readdirSync(directories.oidc); - } catch { - throw invalid(); - } - const stateFilenames = new Set(oidcEntries.filter((entry) => DIGEST_FILENAME_PATTERN.test(entry))); - for (const filename of stateFilenames) { - await withLock(lockKey(root, "oidc", filename), async () => { - const claim = inspectOidcStateClaim(directories.oidc, filename); - if (claim === "orphan") return; - if (claim) { - if (oidcStateExpired(claim.state.value, nowMs)) { - removeClaimedOidcState(directories.oidc, filename, claim); - removed += 1; - } - return; - } - const trusted = readTrusted( - directories.oidc, - filename, - MAX_OIDC_STATE_RECORD_BYTES, - parseOidcStateRecord, - ); - if (trusted && oidcStateExpired(trusted.value, nowMs) - && removeTrusted(directories.oidc, filename, trusted.identity)) { - removed += 1; - } + const sessionEntries = boundedDirectoryNames(directories.sessions, MAX_SESSION_PRUNE_ENTRIES); + for (const filename of sessionEntries) { + if (!DIGEST_FILENAME_PATTERN.test(filename)) throw invalid(); + await withLock(lockKey(root, "sessions", filename), async () => { + const trusted = readTrusted(directories.sessions, filename, MAX_SESSION_RECORD_BYTES, parseSessionRecord); + if (trusted && sessionExpired(trusted.value, nowMs) + && removeTrusted(directories.sessions, filename, trusted.identity)) removed += 1; }); } - for (const claimedFilename of oidcEntries) { - if (!CLAIM_FILENAME_PATTERN.test(claimedFilename)) continue; - const filename = `${claimedFilename.slice(0, -".claim".length)}.json`; - if (stateFilenames.has(filename)) continue; - await withLock(lockKey(root, "oidc", filename), async () => { - const claim = inspectOidcStateClaim(directories.oidc, filename); - if (claim !== "orphan") return; - const claimIdentity = fileIdentity(lstatSync(filePath(directories.oidc, claimedFilename)) as Stats); - // A hard link retains the source mtime. It is therefore a conservative, bounded - // expiry marker even if a process crashed between the two unlink operations. - if (nowMs >= claimIdentity.mtimeMs + OIDC_STATE_TTL_MS - && removeTrusted(directories.oidc, claimedFilename, claimIdentity)) { - removed += 1; - } - }); - } - const slots = await storedOidcSlots(oidcEntries.filter((entry) => oidcSlotIndex(entry) !== undefined)); - for (const slot of slots) { - if (nowMs < Date.parse(slot.record.expiresAt)) continue; - await withLock(lockKey(root, "oidc", slot.record.stateFilename), async () => { - const claim = inspectOidcStateClaim(directories.oidc, slot.record.stateFilename); - if (claim && claim !== "orphan") { - if (!oidcStateExpired(claim.state.value, nowMs)) throw invalid(); - removeClaimedOidcState(directories.oidc, slot.record.stateFilename, claim); - removed += 1; - } else if (!claim) { - const trusted = readTrusted( - directories.oidc, - slot.record.stateFilename, - MAX_OIDC_STATE_RECORD_BYTES, - parseOidcStateRecord, - ); - if (trusted) { - if (!oidcStateExpired(trusted.value, nowMs)) throw invalid(); - if (removeTrusted(directories.oidc, slot.record.stateFilename, trusted.identity)) removed += 1; - } - } - await removeOidcSlot(slot); - }); - } - return removed; + return removed + await pruneOidcStates(nowMs); } return { diff --git a/backend/src/auth/windows-auth-storage.ts b/backend/src/auth/windows-auth-storage.ts index c9c00332..4c42a8bc 100644 --- a/backend/src/auth/windows-auth-storage.ts +++ b/backend/src/auth/windows-auth-storage.ts @@ -8,7 +8,8 @@ const MAX_PROTOCOL_BYTES = 64 * 1024; const MAX_RESPONSE_BYTES = 64 * 1024; const MAX_SESSION_BYTES = 16 * 1024; const MAX_OIDC_BYTES = 8 * 1024; -const MAX_ENTRIES = 256; +const DEFAULT_MAX_ENTRIES = 256; +const MAX_ENTRIES = 512; const TIMEOUT_MS = 5_000; const DIGEST_FILENAME = /^[a-f0-9]{64}\.json$/; const CLAIM_FILENAME = /^[a-f0-9]{64}\.claim$/; @@ -29,7 +30,11 @@ export interface WindowsAuthStorageBridge { read(root: string, directory: WindowsAuthStorageDirectory, filename: string): Promise; replace(root: string, directory: WindowsAuthStorageDirectory, filename: string, contents: Buffer): Promise; remove(root: string, directory: WindowsAuthStorageDirectory, filename: string): Promise; - list(root: string, directory: WindowsAuthStorageDirectory): Promise; + list( + root: string, + directory: WindowsAuthStorageDirectory, + maximumEntries?: number, + ): Promise; claimConsume(root: string, filename: string): Promise; readClaim(root: string, filename: string): Promise; removeClaim(root: string, filename: string): Promise; @@ -98,6 +103,7 @@ interface BridgeRequest { directory: WindowsAuthStorageDirectory; filename?: string; contentBase64?: string; + maximumEntries?: number; } function directoryMaximum(directory: WindowsAuthStorageDirectory): number { @@ -151,7 +157,10 @@ function encodedRequest(request: BridgeRequest): Buffer { validateRoot(request.root); if (request.operation === "list") { if (request.filename !== undefined || request.contentBase64 !== undefined) throw invalid(); + if (request.maximumEntries !== undefined && (!Number.isInteger(request.maximumEntries) + || request.maximumEntries < 1 || request.maximumEntries > MAX_ENTRIES)) throw invalid(); } else { + if (request.maximumEntries !== undefined) throw invalid(); if (request.filename === undefined) throw invalid(); const allowClaim = request.operation === "remove" && request.directory === "oidc"; const allowOidcSlot = request.directory === "oidc" @@ -350,9 +359,17 @@ export function createWindowsAuthStorageBridge(options: WindowsAuthStorageBridge const response = await request(recordRequest("remove", root, directory, filename)); return response.removed === true; }, - async list(root, directory) { - const response = await request({ version: PROTOCOL_VERSION, operation: "list", root, directory }); + async list(root, directory, maximumEntries = DEFAULT_MAX_ENTRIES) { + if (!Number.isInteger(maximumEntries) || maximumEntries < 1 || maximumEntries > MAX_ENTRIES) throw invalid(); + const response = await request({ + version: PROTOCOL_VERSION, + operation: "list", + root, + directory, + maximumEntries, + }); if (response.entries === undefined) throw invalid(); + if (response.entries.length > maximumEntries) throw invalid(); for (const entry of response.entries) { if (!DIGEST_FILENAME.test(entry.name) && !(directory === "oidc" && (CLAIM_FILENAME.test(entry.name) || OIDC_SLOT_FILENAME.test(entry.name)))) throw invalid(); diff --git a/backend/test/auth-session-store.test.ts b/backend/test/auth-session-store.test.ts index d6d3fbb3..7560bdd6 100644 --- a/backend/test/auth-session-store.test.ts +++ b/backend/test/auth-session-store.test.ts @@ -25,6 +25,7 @@ const fsHooks = vi.hoisted(() => ({ afterWrite: undefined as undefined | (() => void), beforeLstat: undefined as undefined | ((path: string) => boolean), afterLstat: undefined as undefined | ((path: string) => boolean), + beforeReaddir: undefined as undefined | ((path: string) => void), transformLstat: undefined as undefined | ((path: string, info: import("node:fs").Stats) => import("node:fs").Stats), })); @@ -55,6 +56,10 @@ vi.mock("node:fs", async (importOriginal) => { if (callback?.(String(args[0]))) fsHooks.afterLstat = undefined; return result; }, + readdirSync: (...args: Parameters) => { + fsHooks.beforeReaddir?.(String(args[0])); + return actual.readdirSync(...args); + }, }; }); import { @@ -87,6 +92,7 @@ afterEach(() => { fsHooks.afterWrite = undefined; fsHooks.beforeLstat = undefined; fsHooks.afterLstat = undefined; + fsHooks.beforeReaddir = undefined; fsHooks.transformLstat = undefined; for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); }); @@ -377,6 +383,75 @@ describe("file-backed auth session store", () => { expect(existsSync(digestPath(storageRoot, "oidc", admitted.state))).toBe(true); }); + test.skipIf(process.platform === "win32")("starts OIDC without scanning more than 256 ordinary session records", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const liveSession = await create(store); + for (let index = 0; index < 300; index += 1) { + writeFileSync(join(storageRoot, "sessions", `${index.toString(16).padStart(64, "0")}.json`), "{}", { + encoding: "utf8", + mode: 0o600, + }); + } + + const oidc = await store.createOidcState(oidcInput("n".repeat(43), "v".repeat(43)), base); + + expect(oidc.state).toMatch(/^[A-Za-z0-9_-]{43}$/); + expect(existsSync(digestPath(storageRoot, "sessions", liveSession.token))).toBe(true); + }); + + test.skipIf(process.platform === "win32")("uses bounded directory reads for ordinary and OIDC pruning", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const expiredSession = await create(store, { idleTtlMs: 60_000, absoluteTtlMs: 60_000 }); + const expiredOidc = await store.createOidcState(oidcInput("n".repeat(43), "v".repeat(43)), base); + fsHooks.beforeReaddir = (path) => { + if (path === join(storageRoot, "sessions") || path === join(storageRoot, "oidc")) { + throw new Error("unbounded directory read"); + } + }; + + await expect(store.prune(new Date(base.getTime() + 11 * 60_000))).resolves.toBe(2); + expect(existsSync(digestPath(storageRoot, "sessions", expiredSession.token))).toBe(false); + expect(existsSync(digestPath(storageRoot, "oidc", expiredOidc.state))).toBe(false); + }); + + test.skipIf(process.platform === "win32")("rejects an overfull OIDC directory without deleting a live state", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const live = await store.createOidcState(oidcInput("n".repeat(43), "v".repeat(43)), base); + for (let index = 0; index < 193; index += 1) { + writeFileSync(join(storageRoot, "oidc", `${(index + 1_000).toString(16).padStart(64, "0")}.json`), "{}", { + encoding: "utf8", + mode: 0o600, + }); + } + + await expect(store.createOidcState(oidcInput("x".repeat(43), "y".repeat(43)), base)) + .rejects.toThrow("auth_session_store_invalid"); + expect(existsSync(digestPath(storageRoot, "oidc", live.state))).toBe(true); + }); + + test("OIDC-only cleanup removes expired state while preserving a live transaction", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const expired = await store.createOidcState(oidcInput("e".repeat(43), "f".repeat(43)), base); + const live = await store.createOidcState( + oidcInput("l".repeat(43), "m".repeat(43)), + new Date(base.getTime() + 60_000), + ); + + const admitted = await store.createOidcState( + oidcInput("n".repeat(43), "v".repeat(43)), + new Date(base.getTime() + 10 * 60_000), + ); + + expect(existsSync(digestPath(storageRoot, "oidc", expired.state))).toBe(false); + expect(existsSync(digestPath(storageRoot, "oidc", admitted.state))).toBe(true); + await expect(store.consumeOidcState(live.state, new Date(base.getTime() + 10 * 60_000))) + .resolves.toMatchObject({ nonce: "l".repeat(43) }); + }); + test("bounds OIDC state creation across concurrent Node processes", async () => { const storageRoot = root(); const creators = await Promise.all([ @@ -703,6 +778,75 @@ describe("file-backed auth session store", () => { } }); + test("starts OIDC on Windows without listing more than 256 ordinary session records", async () => { + const records = new Map(); + for (let index = 0; index < 300; index += 1) { + records.set(`sessions/${index.toString(16).padStart(64, "0")}.json`, Buffer.from("{}")); + } + const key = (directory: string, filename: string) => `${directory}/${filename}`; + const listed: string[] = []; + const bridge = { + create: async (_root: string, directory: string, filename: string, contents: Buffer) => { + const entry = key(directory, filename); + if (records.has(entry)) return false; + records.set(entry, Buffer.from(contents)); + return true; + }, + read: async (_root: string, directory: string, filename: string) => records.get(key(directory, filename)), + remove: async (_root: string, directory: string, filename: string) => records.delete(key(directory, filename)), + list: async (_root: string, directory: string) => { + listed.push(directory); + if (directory === "sessions") throw new Error("ordinary sessions must not be listed during OIDC start"); + return [...records.keys()] + .filter((entry) => entry.startsWith("oidc/")) + .map((entry) => ({ name: entry.slice("oidc/".length), modifiedUnixMs: base.getTime() })); + }, + }; + 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", undefined, { + windowsStorageBridge: bridge as never, + }); + + await expect(store.createOidcState(oidcInput("n".repeat(43), "v".repeat(43)), base)) + .resolves.toMatchObject({ record: { nonce: "n".repeat(43) } }); + expect(listed).toEqual(["oidc", "oidc"]); + expect([...records.keys()].filter((entry) => entry.startsWith("sessions/"))).toHaveLength(300); + } finally { + Object.defineProperty(process, "platform", originalPlatform); + } + }); + + test("rejects an overfull Windows OIDC directory before reading or deleting live entries", async () => { + const entries = Array.from({ length: 193 }, (_unused, index) => ({ + name: `${index.toString(16).padStart(64, "0")}.json`, + modifiedUnixMs: base.getTime(), + })); + const read = vi.fn(async () => undefined); + const remove = vi.fn(async () => false); + 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", undefined, { + windowsStorageBridge: { + list: async () => entries, + read, + remove, + } as never, + }); + + await expect(store.createOidcState(oidcInput("n".repeat(43), "v".repeat(43)), base)) + .rejects.toThrow("auth_session_store_invalid"); + expect(read).not.toHaveBeenCalled(); + expect(remove).not.toHaveBeenCalled(); + } finally { + Object.defineProperty(process, "platform", originalPlatform); + } + }); + test("prunes nonempty Windows directories through the Go lower-camel list DTO", async () => { const records = new Map(); const key = (directory: string, entry: string) => `${directory}/${entry}`; diff --git a/backend/test/oidc-client.test.ts b/backend/test/oidc-client.test.ts index 3af76ff2..12cec828 100644 --- a/backend/test/oidc-client.test.ts +++ b/backend/test/oidc-client.test.ts @@ -241,6 +241,51 @@ test("cancels and releases a non-success JWKS response body", async () => { expect(body.locked).toBe(false); }); +test("rejects promptly and releases the JWKS reader when stream cancellation never settles", async () => { + let cancelled = false; + const body = new ReadableStream({ + pull() { /* a non-success response is rejected before any body read */ }, + cancel() { + cancelled = true; + return new Promise(() => { /* deliberately never settles */ }); + }, + }); + const subject = protocol({ jwksResponse: () => new Response(body, { status: 503 }) }); + const completion = callback(subject).then(() => "resolved" as const, () => "rejected" as const); + const outcome = await Promise.race([ + completion, + new Promise<"timed-out">((resolve) => setTimeout(() => resolve("timed-out"), 100)), + ]); + + expect(outcome).toBe("rejected"); + expect(cancelled).toBe(true); + expect(body.locked).toBe(false); +}); + +test("preserves the JWKS body timeout when cancellation never settles", async () => { + let cancelled = false; + const body = new ReadableStream({ + pull() { return new Promise(() => { /* body read deliberately hangs */ }); }, + cancel() { + cancelled = true; + return new Promise(() => { /* deliberately never settles */ }); + }, + }); + const subject = protocol({ + jwksTimeoutMs: 20, + jwksResponse: () => new Response(body), + }); + const completion = callback(subject).then(() => "resolved" as const, () => "rejected" as const); + const outcome = await Promise.race([ + completion, + new Promise<"timed-out">((resolve) => setTimeout(() => resolve("timed-out"), 100)), + ]); + + expect(outcome).toBe("rejected"); + expect(cancelled).toBe(true); + expect(body.locked).toBe(false); +}); + test("stops streaming a JWKS response as soon as the byte limit is exceeded", async () => { let pulls = 0; let cancelled = false; diff --git a/backend/test/windows-auth-storage.test.ts b/backend/test/windows-auth-storage.test.ts index fb9a39bc..434d7f78 100644 --- a/backend/test/windows-auth-storage.test.ts +++ b/backend/test/windows-auth-storage.test.ts @@ -199,6 +199,39 @@ describe("Windows auth-storage bridge", () => { ]); }); + test("passes an explicit bounded directory limit to the Go helper", async () => { + const invoke = vi.fn(async () => ({ + code: 0, + stdout: Buffer.from('{"version":1,"ok":true,"entries":[]}\n'), + stderr: Buffer.alloc(0), + })); + const bridge = createWindowsAuthStorageBridge({ thtExecutable: "C:\\tht.exe", invoke }); + + await expect(bridge.list(root, "oidc", 192)).resolves.toEqual([]); + expect(JSON.parse(invoke.mock.calls[0][0].input.toString("utf8"))).toMatchObject({ + operation: "list", + directory: "oidc", + maximumEntries: 192, + }); + }); + + test("accepts a bounded ordinary-session page larger than the legacy 256-entry limit", async () => { + const entries = Array.from({ length: 300 }, (_unused, index) => ({ + name: `${index.toString(16).padStart(64, "0")}.json`, + modifiedUnixMs: 1_893_456_245_000, + })); + const bridge = createWindowsAuthStorageBridge({ + thtExecutable: "C:\\tht.exe", + invoke: async () => ({ + code: 0, + stdout: Buffer.from(`${JSON.stringify({ version: 1, ok: true, entries })}\n`), + stderr: Buffer.alloc(0), + }), + }); + + await expect(bridge.list(root, "sessions", 300)).resolves.toHaveLength(300); + }); + test("parses the Go helper's required empty entries array", async () => { const bridge = createWindowsAuthStorageBridge({ thtExecutable: "C:\\tht.exe", diff --git a/tools/tht/internal/authstorage/storage.go b/tools/tht/internal/authstorage/storage.go index 427b3111..d5a7034e 100644 --- a/tools/tht/internal/authstorage/storage.go +++ b/tools/tht/internal/authstorage/storage.go @@ -22,7 +22,8 @@ const ( maximumProtocolBytes = 64 * 1024 maximumSessionBytes = 16 * 1024 maximumOIDCStateBytes = 8 * 1024 - maximumEntries = 256 + defaultMaximumEntries = 256 + maximumEntries = 512 ) var ( @@ -33,12 +34,13 @@ var ( ) type request struct { - Version int `json:"version"` - Operation string `json:"operation"` - Root string `json:"root"` - Directory string `json:"directory"` - Filename string `json:"filename,omitempty"` - ContentBase64 string `json:"contentBase64,omitempty"` + Version int `json:"version"` + Operation string `json:"operation"` + Root string `json:"root"` + Directory string `json:"directory"` + Filename string `json:"filename,omitempty"` + ContentBase64 string `json:"contentBase64,omitempty"` + MaximumEntries int `json:"maximumEntries,omitempty"` } type response struct { @@ -137,7 +139,11 @@ func execute(input request) (response, error) { } return response{Version: protocolVersion, OK: true, Removed: removed}, nil case "list": - entries, err := safeio.ListCanonicalPrivateDirectory(directory, maximumEntries) + limit := input.MaximumEntries + if limit == 0 { + limit = defaultMaximumEntries + } + entries, err := safeio.ListCanonicalPrivateDirectory(directory, limit) if err != nil { return response{}, errInvalid } @@ -175,17 +181,18 @@ func execute(input request) (response, error) { func validOperationShape(input request) bool { noContents := input.ContentBase64 == "" + noMaximumEntries := input.MaximumEntries == 0 switch input.Operation { case "create", "replace": - return digestFilename.MatchString(input.Filename) || (input.Operation == "create" && input.Directory == "oidc" && oidcSlotFilename.MatchString(input.Filename)) + return noMaximumEntries && (digestFilename.MatchString(input.Filename) || (input.Operation == "create" && input.Directory == "oidc" && oidcSlotFilename.MatchString(input.Filename))) case "read": - return noContents && (digestFilename.MatchString(input.Filename) || (input.Directory == "oidc" && oidcSlotFilename.MatchString(input.Filename))) + return noContents && noMaximumEntries && (digestFilename.MatchString(input.Filename) || (input.Directory == "oidc" && oidcSlotFilename.MatchString(input.Filename))) case "remove": - return noContents && (digestFilename.MatchString(input.Filename) || (input.Directory == "oidc" && (claimFilename.MatchString(input.Filename) || oidcSlotFilename.MatchString(input.Filename)))) + return noContents && noMaximumEntries && (digestFilename.MatchString(input.Filename) || (input.Directory == "oidc" && (claimFilename.MatchString(input.Filename) || oidcSlotFilename.MatchString(input.Filename)))) case "list": - return input.Filename == "" && noContents + return input.Filename == "" && noContents && input.MaximumEntries >= 0 && input.MaximumEntries <= maximumEntries case "claim-consume", "read-claim", "remove-claim": - return input.Directory == "oidc" && noContents && digestFilename.MatchString(input.Filename) + return input.Directory == "oidc" && noContents && noMaximumEntries && digestFilename.MatchString(input.Filename) default: return false } diff --git a/tools/tht/internal/authstorage/storage_test.go b/tools/tht/internal/authstorage/storage_test.go index 434cdfe0..bbb1cae1 100644 --- a/tools/tht/internal/authstorage/storage_test.go +++ b/tools/tht/internal/authstorage/storage_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/base64" "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -15,6 +16,24 @@ import ( "github.com/aritmolab/thothii/tools/tht/internal/testsupport" ) +func TestProtocolListUsesACallerSuppliedBound(t *testing.T) { + root := filepath.Join(privateTestRoot(t), "auth") + for index := 0; index < 300; index++ { + filename := fmt.Sprintf("%064x.json", index) + runRequest(t, request{Version: 1, Operation: "create", Root: root, Directory: "sessions", Filename: filename, ContentBase64: base64.StdEncoding.EncodeToString([]byte("record"))}) + } + + listed := runRequest(t, request{Version: 1, Operation: "list", Root: root, Directory: "sessions", MaximumEntries: 300}) + if listed.Entries == nil || len(*listed.Entries) != 300 { + t.Fatalf("bounded list size = %d, want 300", len(*listed.Entries)) + } + filename := fmt.Sprintf("%064x.json", 300) + runRequest(t, request{Version: 1, Operation: "create", Root: root, Directory: "sessions", Filename: filename, ContentBase64: base64.StdEncoding.EncodeToString([]byte("record"))}) + runRejected(t, request{Version: 1, Operation: "list", Root: root, Directory: "sessions", MaximumEntries: 300}) + runRejected(t, request{Version: 1, Operation: "list", Root: root, Directory: "sessions", MaximumEntries: maximumEntries + 1}) + runRejected(t, request{Version: 1, Operation: "read", Root: root, Directory: "sessions", Filename: filename, MaximumEntries: 1}) +} + func TestProtocolCreatesReadsReplacesListsAndRemovesPrivateRecord(t *testing.T) { root := filepath.Join(privateTestRoot(t), "auth") filename := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.json"