diff --git a/src/claude/intercept/local-ca-files.ts b/src/claude/intercept/local-ca-files.ts index bac8f737ba0..15883800ac7 100644 --- a/src/claude/intercept/local-ca-files.ts +++ b/src/claude/intercept/local-ca-files.ts @@ -11,6 +11,8 @@ const MAX_PEM_BYTES = 64 * 1024; const KEY = "ca.key"; const CERT = "ca.pem"; type FileMode = 0o600 | 0o644; +type AclLevel = "owner" | "inherited" | "private"; +type AclCheck = ((path: string, stat: BigIntStats, level?: AclLevel) => void) & { forget(stat: BigIntStats): void }; type FileHooks = { beforeRead?: (path: string) => void; beforeWrite?: (path: string) => void; beforePublish?: (path: string) => void }; let hooks: FileHooks = {}; /** Boundary fault injection for substitution and zero-secret-write assertions. */ @@ -28,34 +30,48 @@ function owned(stat: BigIntStats, directory = false, mode: FileMode = 0o600): vo if ((Number(stat.mode) & forbidden) !== 0) throw unsafe(); } } -function same(path: string, expected: BigIntStats, directory = false, mode: FileMode = 0o600): void { +function same(path: string, expected: BigIntStats, directory = false, mode: FileMode = 0o600): BigIntStats { const stat = lstatSync(path, { bigint: true }); owned(stat, directory, mode); if (stat.dev !== expected.dev || stat.ino !== expected.ino || stat.ino === 0n) throw unsafe(); + return stat; } -function inspect(path: string, mode: FileMode, sqliteSidecar = false): BigIntStats | null { +function inspect(path: string, mode: FileMode, checkAcl: AclCheck, sqliteSidecar = false): BigIntStats | null { let stat: BigIntStats; try { stat = lstatSync(path, { bigint: true }); } catch (error) { if (missing(error)) return null; throw error; } owned(stat, false, mode); - if (windowsSecretAclApplies()) assertLocalCaWindowsAcl(path, sqliteSidecar ? "inherited" : "private"); + if (windowsSecretAclApplies()) { + if (sqliteSidecar) checkAcl(path, stat, "inherited"); + else { + try { checkAcl(path, stat); } + catch { + // Legacy files may inherit only trusted grants; verify before narrowing. + checkAcl(path, stat, "inherited"); + same(path, stat, false, mode); + hardenSecretPath(path, { required: true }); + same(path, stat, false, mode); + checkAcl(path, stat); + } + } + } same(path, stat, false, mode); return stat; } -function protectNew(path: string, fd: number, mode: FileMode): void { +function protectNew(path: string, fd: number, mode: FileMode, checkAcl: AclCheck): void { const created = fstatSync(fd, { bigint: true }); owned(created, false, mode); if (windowsSecretAclApplies()) { - // Owner inspection precedes ACL mutation; inherited grants on a new EMPTY file - // are removed by the existing hardener before the strict inspection and write. - assertLocalCaWindowsAcl(path, "owner"); + // Only O_CREAT|O_EXCL entries reach here. Narrow the empty file we created, + // then strictly verify before writing; an Administrators default owner still fails closed. + same(path, created, false, mode); hardenSecretPath(path, { required: true }); - assertLocalCaWindowsAcl(path); + checkAcl(path, created); } else fchmodSync(fd, mode); same(path, created, false, mode); } -function readPinned(path: string, expected: BigIntStats, mode: FileMode, assertDirectory: () => void): string { +function readPinned(path: string, expected: BigIntStats, mode: FileMode, assertDirectory: () => void, checkAcl: AclCheck): string { hooks.beforeRead?.(path); assertDirectory(); same(path, expected, false, mode); @@ -64,7 +80,7 @@ function readPinned(path: string, expected: BigIntStats, mode: FileMode, assertD const opened = fstatSync(fd, { bigint: true }); owned(opened, false, mode); if (opened.dev !== expected.dev || opened.ino !== expected.ino || opened.size > BigInt(MAX_PEM_BYTES)) throw unsafe(); - if (windowsSecretAclApplies()) assertLocalCaWindowsAcl(path); + checkAcl(path, opened); same(path, opened, false, mode); const bytes = Buffer.alloc(MAX_PEM_BYTES + 1); let length = 0; @@ -87,9 +103,9 @@ export interface LocalCaFiles { writePair(pair: { certPem: string; keyPem: string }): void; } -function stage(path: string, contents: string, mode: FileMode, assertDirectory: () => void): { publish(): void; dispose(): void } { +function stage(path: string, contents: string, mode: FileMode, assertDirectory: () => void, checkAcl: AclCheck): { publish(): void; dispose(): void } { assertDirectory(); - const previous = inspect(path, mode); + const previous = inspect(path, mode, checkAcl); const temp = join(dirname(path), `.${basename(path)}.${process.pid}.${randomBytes(16).toString("hex")}.tmp`); const fd = openSync(temp, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | NOFOLLOW | NONBLOCK, mode); let created: BigIntStats; @@ -102,15 +118,15 @@ function stage(path: string, contents: string, mode: FileMode, assertDirectory: if (removed) return; try { same(temp, created, false, mode); unlinkSync(temp); removed = true; } catch (error) { if (missing(error)) removed = true; else throw error; } - if (removed) forgetEphemeralSecretPath(temp); + if (removed) { checkAcl.forget(created); forgetEphemeralSecretPath(temp); } }; try { - protectNew(temp, fd, mode); + protectNew(temp, fd, mode, checkAcl); assertDirectory(); hooks.beforeWrite?.(temp); same(temp, created, false, mode); assertDirectory(); - if (windowsSecretAclApplies()) assertLocalCaWindowsAcl(temp); + checkAcl(temp, created); writeFileSync(fd, contents, { encoding: "utf8" }); same(temp, created, false, mode); return { @@ -118,10 +134,11 @@ function stage(path: string, contents: string, mode: FileMode, assertDirectory: hooks.beforePublish?.(path); assertDirectory(); same(temp, created, false, mode); - if (windowsSecretAclApplies()) assertLocalCaWindowsAcl(temp); - const current = inspect(path, mode); + checkAcl(temp, created); + const current = inspect(path, mode, checkAcl); if (previous ? !current || previous.dev !== current.dev || previous.ino !== current.ino : current !== null) throw unsafe(); renameSync(temp, path); + if (current) checkAcl.forget(current); removed = true; forgetEphemeralSecretPath(temp); same(path, created, false, mode); @@ -142,6 +159,27 @@ export function withLocalCaPublication(dir: string, lockName: string, work: ( let created = false; try { mkdirSync(dir, { mode: 0o700 }); created = true; } catch (error) { if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; } + // Publication-local only: a private DACL can be changed only by the owner, + // SYSTEM or Administrators. Every memo hit still checks lstat identity via same(). + // bigint birthtimeNs adds discrimination for recycled SQLite sidecar file IDs; it is not a + // uniqueness guarantee (NTFS tunneling can restore creation times). Safety rests on the + // verified-private directory DACL. Creation time survives same-volume rename, so staged + // entries keep their memo. + const verified = new Map(); + const strength = { owner: 0, inherited: 1, private: 2 }; + const checkAcl: AclCheck = Object.assign((path: string, stat: BigIntStats, level: AclLevel = "private") => { + if (!windowsSecretAclApplies()) return; + const identity = same(path, stat, stat.isDirectory()); + const key = `${identity.dev}:${identity.ino}:${identity.birthtimeNs}`; + const prior = verified.get(key); + if (prior !== undefined && strength[prior] >= strength[level]) return; + assertLocalCaWindowsAcl(path, level); + same(path, stat, stat.isDirectory()); + verified.set(key, level); + }, { + // An identity this publication unlinked or replaced is retired; a recycled id must be re-inspected. + forget(stat: BigIntStats): void { verified.delete(`${stat.dev}:${stat.ino}:${stat.birthtimeNs}`); }, + }); const directory = lstatSync(dir, { bigint: true }); owned(directory, true); const dirFd = windowsSecretAclApplies() ? null : openSync(dir, constants.O_RDONLY | (constants.O_DIRECTORY ?? 0) | NOFOLLOW); @@ -152,30 +190,40 @@ export function withLocalCaPublication(dir: string, lockName: string, work: ( if (opened.dev !== directory.dev || opened.ino !== directory.ino) throw unsafe(); owned(opened, true); } else { - assertLocalCaWindowsAcl(dir, created ? "owner" : "private"); - hardenSecretDir(dir, { required: true }); - assertLocalCaWindowsAcl(dir); + let needsHardening = created; + // A successful mkdir created this directory; strict verification after narrowing + // still refuses an Administrators default owner before any CA access. + if (!created) { + try { checkAcl(dir, directory); } + catch { checkAcl(dir, directory, "inherited"); needsHardening = true; } + } + if (needsHardening) { + same(dir, directory, true); + hardenSecretDir(dir, { required: true }); + same(dir, directory, true); + checkAcl(dir, directory); + } } const assertDirectory = (): void => { same(dir, directory, true); }; assertDirectory(); const lockPath = join(dir, lockName); const assertEntries = (): void => { assertDirectory(); - inspect(join(dir, KEY), 0o600); - inspect(join(dir, CERT), 0o644); - inspect(lockPath, 0o600); + inspect(join(dir, KEY), 0o600, checkAcl); + inspect(join(dir, CERT), 0o644, checkAcl); + inspect(lockPath, 0o600, checkAcl); // SQLite creates sidecars itself, inheriting only the protected directory's // current-user ACE. Inspect every grant and owner; an inherited DACL is valid here. - for (const suffix of ["-journal", "-wal", "-shm"]) inspect(`${lockPath}${suffix}`, 0o600, true); + for (const suffix of ["-journal", "-wal", "-shm"]) inspect(`${lockPath}${suffix}`, 0o600, checkAcl, true); assertDirectory(); }; assertEntries(); try { lockFd = openSync(lockPath, constants.O_RDWR | constants.O_CREAT | constants.O_EXCL | NOFOLLOW | NONBLOCK, 0o600); - protectNew(lockPath, lockFd, 0o600); + protectNew(lockPath, lockFd, 0o600, checkAcl); } catch (error) { if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; - const existing = inspect(lockPath, 0o600); + const existing = inspect(lockPath, 0o600, checkAcl); if (!existing) throw unsafe(); lockFd = openSync(lockPath, constants.O_RDONLY | NOFOLLOW | NONBLOCK); const opened = fstatSync(lockFd, { bigint: true }); @@ -191,17 +239,17 @@ export function withLocalCaPublication(dir: string, lockName: string, work: ( const files: LocalCaFiles = { readPair() { assertEntries(); - const cert = inspect(join(dir, CERT), 0o644); - const key = inspect(join(dir, KEY), 0o600); + const cert = inspect(join(dir, CERT), 0o644, checkAcl); + const key = inspect(join(dir, KEY), 0o600, checkAcl); if (!cert || !key) return null; - return { certPem: readPinned(join(dir, CERT), cert, 0o644, assertDirectory), keyPem: readPinned(join(dir, KEY), key, 0o600, assertDirectory) }; + return { certPem: readPinned(join(dir, CERT), cert, 0o644, assertDirectory, checkAcl), keyPem: readPinned(join(dir, KEY), key, 0o600, assertDirectory, checkAcl) }; }, writePair(pair) { assertEntries(); - const key = stage(join(dir, KEY), pair.keyPem, 0o600, assertDirectory); + const key = stage(join(dir, KEY), pair.keyPem, 0o600, assertDirectory, checkAcl); let cert: ReturnType | undefined; try { - cert = stage(join(dir, CERT), pair.certPem, 0o644, assertDirectory); + cert = stage(join(dir, CERT), pair.certPem, 0o644, assertDirectory, checkAcl); assertEntries(); key.publish(); cert.publish(); diff --git a/src/claude/intercept/local-ca-windows.ts b/src/claude/intercept/local-ca-windows.ts index e9f3d32b176..74583e52f40 100644 --- a/src/claude/intercept/local-ca-windows.ts +++ b/src/claude/intercept/local-ca-windows.ts @@ -9,14 +9,24 @@ const SID = /^S-1-(?:\d+-)+\d+$/i; function nativeAclRunner(path: string, timeoutMs: number): IcaclsResult { // Encode the path as data, not PowerShell syntax. SID-form rules avoid localized account names. + // .NET only: Get-Acl/ConvertTo-Json/ForEach-Object are module-autoloaded cmdlets, and + // autoload resolves them through the per-profile module analysis cache. With a fresh or + // redirected LOCALAPPDATA that rebuild scans every PSModulePath module and can exceed the + // inspection budget (hosted Windows runners: 30 s timeouts during `ocx start`); it also lets + // a module earlier on PSModulePath shadow the cmdlet. SIDs, ints and booleans need no escaping; + // the JSON quote is [char]34 so the -Command argument carries no double quotes to re-parse. const encodedPath = Buffer.from(path, "utf8").toString("base64"); const script = [ "$ErrorActionPreference='Stop'", `$p=[Text.Encoding]::UTF8.GetString([Convert]::FromBase64String('${encodedPath}'))`, - "$a=Get-Acl -LiteralPath $p", "$s=[System.Security.Principal.SecurityIdentifier]", - "$r=@($a.GetAccessRules($true,$true,$s)|ForEach-Object {@{sid=$_.IdentityReference.Value;type=[int]$_.AccessControlType;rights=[long]$_.FileSystemRights}})", - "@{owner=$a.GetOwner($s).Value;protected=$a.AreAccessRulesProtected;rules=$r}|ConvertTo-Json -Depth 4 -Compress", + "$q=[char]34", + "$c=[System.Security.AccessControl.AccessControlSections]'Owner,Access'", + "if([System.IO.Directory]::Exists($p)){$a=[System.Security.AccessControl.DirectorySecurity]::new($p,$c)}else{$a=[System.Security.AccessControl.FileSecurity]::new($p,$c)}", + "$r=foreach($x in $a.GetAccessRules($true,$true,$s)){'{'+$q+'sid'+$q+':'+$q+$x.IdentityReference.Value+$q+','+$q+'type'+$q+':'+[int]$x.AccessControlType+','+$q+'rights'+$q+':'+[long]$x.FileSystemRights+'}'}", + "$o='{'+$q+'owner'+$q+':'+$q+$a.GetOwner($s).Value+$q+','+$q+'protected'+$q+':'+$(if($a.AreAccessRulesProtected){'true'}else{'false'})+','+$q+'rules'+$q+':['+(@($r) -join ',')+']}'", + "[Console]::Out.Write($o)", + "[Console]::Out.Flush()", ].join(";"); const result = Bun.spawnSync([resolveTrustedWindowsPowerShellExe(), "-NoLogo", "-NoProfile", "-NonInteractive", "-Command", script], { stdin: "ignore", stdout: "pipe", stderr: "ignore", timeout: timeoutMs, windowsHide: true, diff --git a/src/claude/intercept/picker-ca-persistence.ts b/src/claude/intercept/picker-ca-persistence.ts index 21e4e712d0c..c8414acdcce 100644 --- a/src/claude/intercept/picker-ca-persistence.ts +++ b/src/claude/intercept/picker-ca-persistence.ts @@ -1,5 +1,5 @@ import { X509Certificate } from "node:crypto"; -import { constants, closeSync, fstatSync, lstatSync, openSync, readSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { constants, closeSync, fstatSync, lstatSync, openSync, readSync, realpathSync, rmSync, writeFileSync, type Stats } from "node:fs"; import { join } from "node:path"; import { decodePickerCaCredential, openPickerCaCredential, pickerCaConfigId, readPickerCaCredential, @@ -21,11 +21,16 @@ export function canonicalPickerConfigDir(configDir: string): string { /** Public records still cannot follow symlinks or accept oversized/malformed recovery data. */ function readState(path: string, configId: string, initialization: boolean): Initialization | AuthorityMetadata | null { + let expected: Stats; + try { expected = lstatSync(path); } + catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; throw new Error("picker_ca_metadata_unsafe"); } + if (expected.isSymbolicLink()) throw new Error("picker_ca_metadata_unsafe"); let fd: number; try { fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); } catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; throw new Error("picker_ca_metadata_unsafe"); } try { const stat = fstatSync(fd); + if (stat.dev !== expected.dev || stat.ino !== expected.ino) throw new Error("picker_ca_metadata_unsafe"); if (!stat.isFile() || stat.nlink !== 1 || stat.size > MAX_PUBLIC_STATE_BYTES || (process.platform !== "win32" && stat.uid !== process.getuid!())) throw new Error(); const bytes = Buffer.alloc(MAX_PUBLIC_STATE_BYTES + 1); @@ -46,7 +51,10 @@ function readState(path: string, configId: string, initialization: boolean): Ini if (initialization && value.predecessor !== null && new X509Certificate(value.predecessor).toString() !== value.predecessor) throw new Error(); return value; - } catch { throw new Error("picker_ca_metadata_invalid"); } + } catch (error) { + if (error instanceof Error && error.message === "picker_ca_metadata_unsafe") throw error; + throw new Error("picker_ca_metadata_invalid"); + } finally { closeSync(fd); } } diff --git a/structure/clients/claude-desktop.md b/structure/clients/claude-desktop.md index 6f4d5957200..39d79a32da0 100644 --- a/structure/clients/claude-desktop.md +++ b/structure/clients/claude-desktop.md @@ -179,6 +179,7 @@ The shared CONNECT primitive accepts optional `allowedTargets` authorities. It s normalizes that list at startup; an empty list denies all, and other host/port pairs receive 403 before tunnel selection or dialing. Authentication and loopback refusal remain in force. Existing Claude consumers omit this option and retain blind forwarding; it enables no new integration or certificate trust. +Windows local-CA publication in `src/claude/intercept/local-ca-files.ts` hardens legacy inherited DACLs only after verifying the current owner and exclusively current-user, SYSTEM or Administrators grants; already private directories skip hardening. Newly created exclusive files and directories are hardened before strict owner/ACL verification and before CA access. ACL verification, including inherited SQLite sidecar ACLs, is memoized by bigint device/inode/birthtime within one publication; birthtime distinguishes recycled file IDs while preserving same-volume rename identity. Path and descriptor identity checks remain active on every access, and removed or replaced entries retire their memo. The authority primitive accepts `validityDays` from 1 through 3650 for short-lived callers; omitted values preserve the existing 3650-day CA lifetime. This parameter does not install trust or rotate an existing authority. On macOS, when the lifecycle passes `loadPickerRoutes` (the server always does), `startClaudeIntercept` also @@ -228,7 +229,7 @@ application-access dialogs are controlled by macOS; restart or upgrade does not `picker-ca-store.ts` owns the versioned OS credential service, canonical-config identity namespace, bounded exact-shape payload, full constrained CA profile, validity and P-256 private-key match validation. `picker-ca-persistence.ts` validates public `authority.json` and `authority-init.json` records under -the canonical CA lock. Initialization journals the config identity, new fingerprint and public +the canonical CA lock, rejecting symlinks and mismatched pre-open/path and descriptor identities. Initialization journals the config identity, new fingerprint and public predecessor before writing the credential, verifies readback, then commits metadata and publication; it removes the journal last. Recovery requires matching journal/store identity; missing initialized credentials, unavailable storage or inconsistent metadata fail closed without publishing a replacement. diff --git a/tests/claude-integration/claude-intercept-local-ca-windows.test.ts b/tests/claude-integration/claude-intercept-local-ca-windows.test.ts index 4bff69e155c..dc24f556284 100644 --- a/tests/claude-integration/claude-intercept-local-ca-windows.test.ts +++ b/tests/claude-integration/claude-intercept-local-ca-windows.test.ts @@ -1,10 +1,10 @@ import { afterEach, expect, spyOn, test } from "bun:test"; import * as filesystem from "node:fs"; -import { mkdtempSync, readFileSync, readdirSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readFileSync, readdirSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { basename, join } from "node:path"; import { claudeInterceptStateDir, ensureLocalInterceptCa } from "../../src/claude/intercept/local-ca"; -import { setLocalCaFileHooksForTests } from "../../src/claude/intercept/local-ca-files"; +import { setLocalCaFileHooksForTests, withLocalCaPublication } from "../../src/claude/intercept/local-ca-files"; import { setLocalCaWindowsAclRunnerForTests } from "../../src/claude/intercept/local-ca-windows"; import { resetHardenedStateForTests, setIcaclsRunnerForTests, setPlatformForTests, type IcaclsResult } from "../../src/lib/windows-secret-acl"; import { setWindowsPrincipalRunnerForTests } from "../../src/lib/windows-user-principal"; @@ -83,27 +83,32 @@ test("Windows local CA protects and verifies empty temps before every PEM write" }); for (const name of ["claude-intercept", "ca.key", "ca.pem", "ca-publication.sqlite"]) { - for (const failure of ["owner", "foreign read", "foreign tamper", "inspection error", "unprotected"] as const) { + for (const failure of ["owner", "foreign read", "foreign tamper", "inspection error", "unprotected after hardening"] as const) { test(`Windows local CA rejects ${failure} on ${name} before reads or writes`, () => { const f = setup(); const first = ensureLocalInterceptCa(f.root); f.writes.length = 0; f.reads.length = 0; f.secretWrites.length = 0; + f.hardened.clear(); + resetHardenedStateForTests(); const dir = claudeInterceptStateDir(f.root); + const target = name === "claude-intercept" ? dir : join(dir, name); const keyBefore = readFileSync(join(dir, "ca.key")); const certBefore = readFileSync(join(dir, "ca.pem")); setLocalCaWindowsAclRunnerForTests(path => { if (basename(path) !== name) return acl(); if (failure === "owner") return acl(FOREIGN); if (failure === "inspection error") throw new Error("native inspection unavailable"); - if (failure === "unprotected") return acl(CURRENT, undefined, false); + if (failure === "unprotected after hardening") return acl(CURRENT, undefined, false); return acl(CURRENT, [{ sid: CURRENT, type: 0, rights: FULL_CONTROL }, { sid: FOREIGN, type: 0, rights: failure === "foreign read" ? 1 : 262144 }]); }); expect(() => { ensureLocalInterceptCa(f.root); }).toThrow("Windows owner/ACL"); expect(f.writes).toEqual([]); expect(f.secretWrites).toEqual([]); expect(f.reads).toEqual([]); + // Only a trusted-grant DACL may be narrowed; foreign owners, grants and failed probes are never mutated. + if (failure !== "unprotected after hardening") expect(f.hardened.has(target)).toBe(false); expect(readFileSync(join(dir, "ca.key"))).toEqual(keyBefore); expect(readFileSync(join(dir, "ca.pem"))).toEqual(certBefore); expect(certBefore.toString()).toBe(first.certPem); @@ -111,7 +116,7 @@ for (const name of ["claude-intercept", "ca.key", "ca.pem", "ca-publication.sqli } } -for (const failure of ["foreign owner", "foreign grant", "hardener failure", "probe failure", "timeout", "malformed", "empty DACL"] as const) { +for (const failure of ["foreign owner", "Administrators default owner", "foreign grant", "hardener failure", "probe failure", "timeout", "malformed", "empty DACL"] as const) { test(`Windows ${failure} while protecting a new temp causes zero secret writes`, () => { const f = setup(); let tempSeen = false; @@ -120,6 +125,10 @@ for (const failure of ["foreign owner", "foreign grant", "hardener failure", "pr tempSeen = true; expect(statSync(path).size).toBe(0); if (failure === "foreign owner") return acl(FOREIGN); + if (failure === "Administrators default owner") { + expect(f.hardened.has(path)).toBe(true); + return acl("S-1-5-32-544"); + } if (failure === "foreign grant") return acl(CURRENT, [{ sid: FOREIGN, type: 0, rights: 2 }]); if (failure === "probe failure") return { success: false, exitCode: 1, timedOut: false, stdout: "native failure" }; if (failure === "timeout") return { success: false, exitCode: null, timedOut: true, stdout: "" }; @@ -171,7 +180,157 @@ test("Windows inspection failure on corrupt PEM never enters corruption regenera test("Windows effective SID lookup failure occurs before any secret write", () => { const f = setup(); setWindowsPrincipalRunnerForTests(() => ({ success: false, exitCode: 1, timedOut: false, stdout: "" })); - expect(() => { ensureLocalInterceptCa(f.root); }).toThrow("Windows owner/ACL"); + expect(() => { ensureLocalInterceptCa(f.root); }).toThrow("ACL hardening failed (EACLIDENTITY)"); expect(f.writes).toEqual([]); expect(f.secretWrites).toEqual([]); }); + +test("Windows refuses a newly created directory with an Administrators default owner after hardening", () => { + const f = setup(); + const dir = claudeInterceptStateDir(f.root); + setLocalCaWindowsAclRunnerForTests(path => { + expect(path).toBe(dir); + expect(f.hardened.has(dir)).toBe(true); + return acl("S-1-5-32-544"); + }); + expect(() => { ensureLocalInterceptCa(f.root); }).toThrow("Windows owner/ACL"); + expect(readdirSync(dir)).toEqual([]); + expect(f.secretWrites).toEqual([]); +}); + +// Platform, principal and icacls seams exercise the Windows branch on POSIX CI too. +test("Windows accepts and hardens an existing unprotected directory with only trusted grants", () => { + const f = setup(); + const dir = claudeInterceptStateDir(f.root); + mkdirSync(dir, { mode: 0o700 }); + let inspections = 0; + setLocalCaWindowsAclRunnerForTests(path => { + if (path !== dir) return acl(); + inspections++; + return acl(CURRENT, undefined, f.hardened.has(dir)); + }); + const ca = ensureLocalInterceptCa(f.root); + expect(inspections).toBe(3); // strict, inherited, strict after hardening + expect(f.hardened.has(dir)).toBe(true); + expect(readFileSync(join(dir, "ca.key"), "utf8")).toBe(ca.keyPem); +}); + +for (const foreign of ["owner", "grant"] as const) { + test(`Windows refuses an existing inherited directory with a foreign ${foreign} without hardening`, () => { + const f = setup(); + const dir = claudeInterceptStateDir(f.root); + mkdirSync(dir, { mode: 0o700 }); + setLocalCaWindowsAclRunnerForTests(() => acl(foreign === "owner" ? FOREIGN : CURRENT, + [{ sid: foreign === "grant" ? FOREIGN : CURRENT, type: 0, rights: FULL_CONTROL }], false)); + const mutations: string[] = []; + setIcaclsRunnerForTests(args => { mutations.push(args[0]!); return ok(); }); + expect(() => { ensureLocalInterceptCa(f.root); }).toThrow("Windows owner/ACL"); + expect(mutations).toEqual([]); + expect(readdirSync(dir)).toEqual([]); + expect(f.secretWrites).toEqual([]); + }); +} + +for (const name of ["ca.key", "ca.pem", "ca-publication.sqlite"]) { + test(`Windows hardens a legacy inherited ${name} and retains the authority`, () => { + const f = setup(); + const first = ensureLocalInterceptCa(f.root); + const path = join(claudeInterceptStateDir(f.root), name); + resetHardenedStateForTests(); + f.hardened.clear(); + f.writes.length = 0; + let inspections = 0; + setLocalCaWindowsAclRunnerForTests(target => { + if (target !== path) return acl(); + inspections++; + return acl(CURRENT, undefined, f.hardened.has(path)); + }); + const restored = ensureLocalInterceptCa(f.root); + expect(restored.certPem).toBe(first.certPem); + expect(restored.keyPem).toBe(first.keyPem); + expect(f.hardened.has(path)).toBe(true); + expect(inspections).toBe(3); + expect(f.writes).toEqual([]); + }); +} + +test("Windows memo verifies unchanged identities once per publication, including repeated pair reads", () => { + const f = setup(); + const first = ensureLocalInterceptCa(f.root); + const dir = claudeInterceptStateDir(f.root); + const inspections: string[] = []; + setLocalCaWindowsAclRunnerForTests(path => { inspections.push(path); return acl(); }); + withLocalCaPublication(dir, "ca-publication.sqlite", files => { + expect(files.readPair()).toEqual({ certPem: first.certPem, keyPem: first.keyPem }); + expect(files.readPair()).toEqual({ certPem: first.certPem, keyPem: first.keyPem }); + }); + // Native SQLite may retain a journal during the lease; POSIX seams may not. + expect(inspections.filter(path => !/sqlite-(journal|wal|shm)$/.test(path)).map(path => basename(path)).sort()) + .toEqual(["ca-publication.sqlite", "ca.key", "ca.pem", "claude-intercept"]); + expect(new Set(inspections).size).toBe(inspections.length); // each unchanged sidecar is inspected at most once too + inspections.length = 0; + expect(ensureLocalInterceptCa(f.root).keyPem).toBe(first.keyPem); + expect(inspections.filter(path => !/sqlite-(journal|wal|shm)$/.test(path))).toHaveLength(4); // no memo survives a publication + expect(new Set(inspections).size).toBe(inspections.length); +}); + +test("Windows memo follows staged identities across rename without another ACL inspection", () => { + const f = setup(); + const inspections: string[] = []; + setLocalCaWindowsAclRunnerForTests(path => { inspections.push(path); return acl(); }); + ensureLocalInterceptCa(f.root); + expect(inspections.filter(path => basename(path) === "claude-intercept")).toHaveLength(1); + expect(inspections.filter(path => basename(path) === "ca-publication.sqlite")).toHaveLength(1); + expect(inspections.filter(path => path.endsWith(".tmp"))).toHaveLength(2); // strict after hardening for each temp + expect(inspections.filter(path => ["ca.key", "ca.pem"].includes(basename(path)))).toEqual([]); +}); + +test("Windows inherited SQLite sidecar is inspected once per identity without requiring protection or hardening", () => { + const f = setup(); + ensureLocalInterceptCa(f.root); + const dir = claudeInterceptStateDir(f.root); + const sidecar = join(dir, "ca-publication.sqlite-shm"); + writeFileSync(sidecar, "fixture", { mode: 0o600 }); + let inspections = 0; + setLocalCaWindowsAclRunnerForTests(path => { + if (path !== sidecar) return acl(); + inspections++; + return acl(CURRENT, undefined, false); + }); + withLocalCaPublication(dir, "ca-publication.sqlite", files => { files.readPair(); files.readPair(); }); + expect(inspections).toBe(1); + expect(f.hardened.has(sidecar)).toBe(false); +}); + +test("Windows re-inspects a substituted key identity and rejects foreign grants before reading it", () => { + const f = setup(); + ensureLocalInterceptCa(f.root); + const dir = claudeInterceptStateDir(f.root); + const key = join(dir, "ca.key"); + const original = statSync(key, { bigint: true }); + const inspected: bigint[] = []; + let substituted = false; + setLocalCaWindowsAclRunnerForTests(path => { + if (path !== key) return acl(); + inspected.push(statSync(path, { bigint: true }).ino); + return substituted ? acl(CURRENT, [{ sid: FOREIGN, type: 0, rights: FULL_CONTROL }]) : acl(); + }); + withLocalCaPublication(dir, "ca-publication.sqlite", files => { + files.readPair(); + // Keep the original inode alive so this fixture cannot accidentally recycle it. + renameSync(key, join(f.root, "original-key")); + writeFileSync(key, "substituted fixture", { mode: 0o600 }); + substituted = true; + f.reads.length = 0; + expect(() => { files.readPair(); }).toThrow("Windows owner/ACL"); + expect(f.reads).toEqual([]); + // Restore before the publication's final entry guard. + renameSync(key, join(f.root, "substituted-key")); + renameSync(join(f.root, "original-key"), key); + substituted = false; + }); + expect(inspected).toHaveLength(3); // original private, replacement private + inherited + expect(inspected[0]).toBe(original.ino); + expect(inspected[1]).not.toBe(original.ino); + expect(inspected[2]).toBe(inspected[1]); +}); diff --git a/tests/claude-integration/claude-picker-ca-store.test.ts b/tests/claude-integration/claude-picker-ca-store.test.ts index 76aeb8fe366..6a12ab894ab 100644 --- a/tests/claude-integration/claude-picker-ca-store.test.ts +++ b/tests/claude-integration/claude-picker-ca-store.test.ts @@ -1,4 +1,5 @@ import { afterEach, expect, spyOn, test } from "bun:test"; +import * as filesystem from "node:fs"; import { generateKeyPairSync, X509Certificate } from "node:crypto"; import { createCertificateAuthority } from "../../src/claude/intercept/local-ca"; import { PICKER_CA_COMMON_NAME, PICKER_HOST } from "../../src/claude/intercept/picker-ca"; @@ -284,6 +285,35 @@ test("metadata mismatch and symlink recovery records fail before credential muta expect(fake.writes).toBe(1); }); +test("metadata replaced between lstat and open fails before credential access", () => { + const dir = root(); + const fake = memoryPickerCaStore(); + ensurePickerCa(dir, { persistent: true, rotation: "startup", store: fake.store }); + // The picker canonicalizes its config directory (macOS tmpdir is /var -> /private/var). + const path = join(filesystem.realpathSync(dir), "claude-picker", "authority.json"); + const original = readFileSync(path, "utf8"); + const open = filesystem.openSync; + let replaced = false; + const opening = spyOn(filesystem, "openSync").mockImplementation((target, flags, mode) => { + if (target === path && !replaced) { + filesystem.renameSync(path, join(dir, "original-metadata")); + writeFileSync(path, original, { mode: 0o600 }); + replaced = true; + } + return open(target, flags, mode); + }); + let storeCalls = 0; + try { + expect(() => { ensurePickerCa(dir, { persistent: true, rotation: "startup", store: (service, account) => { + storeCalls++; + return fake.store(service, account); + } }); }).toThrow("picker_ca_metadata_unsafe"); + expect(replaced).toBe(true); + expect(storeCalls).toBe(0); + expect(fake.writes).toBe(1); + } finally { opening.mockRestore(); } +}); + test("unsafe picker directory or lock cannot reach the credential store", () => { for (const pathKind of ["directory", "lock"] as const) { const dir = root(); diff --git a/tests/claude-integration/claude-picker-startup.test.ts b/tests/claude-integration/claude-picker-startup.test.ts index cbebdfc3cbc..355bcca97f8 100644 --- a/tests/claude-integration/claude-picker-startup.test.ts +++ b/tests/claude-integration/claude-picker-startup.test.ts @@ -265,7 +265,7 @@ test("startup preparation diagnostics distinguish cleanup and sanitize authority startProxy: async (_port, options) => startConnectProxy(0, options), }); handles.push(handle); - expect(warnings).toEqual([expected]); + expect(warnings.filter(line => line.includes("Claude Desktop picker"))).toEqual([expected]); expect(warnings.join(" ")).not.toContain("PRIVATE-KEY-DIAGNOSTIC"); expect(handle?.pickerReason).toBe("failed"); } finally { prepare.mockRestore(); warn.mockRestore(); } diff --git a/tests/cli/sibling-home-client-sync.test.ts b/tests/cli/sibling-home-client-sync.test.ts index 82289566c56..92f4623ea1d 100644 --- a/tests/cli/sibling-home-client-sync.test.ts +++ b/tests/cli/sibling-home-client-sync.test.ts @@ -93,7 +93,9 @@ async function finishChildOutput(child: ReturnType) { } async function waitForRuntime(path: string, child: ReturnType) { - const deadline = Date.now() + 15_000; + // Windows startup verifies the intercept CA ACLs (~2 s on hosted runners) and pre-existing + // runs already reached 13.6 s here; 25 s keeps a diagnostic watchdog inside the 30 s test budget. + const deadline = Date.now() + 25_000; while (Date.now() < deadline) { if (existsSync(path)) { try { @@ -111,7 +113,7 @@ async function waitForRuntime(path: string, child: ReturnType) } async function waitForClientStartup(child: ReturnType): Promise { - const deadline = Date.now() + 15_000; + const deadline = Date.now() + 25_000; while (Date.now() < deadline) { if (capturedChildren.get(child)!.snapshot().startupComplete) return; if (child.exitCode !== null) {