From 13eb34bc3ebfabd8d6575ca3e7f6414ada399cb7 Mon Sep 17 00:00:00 2001 From: Orca Worker Date: Sat, 5 Sep 2026 19:31:38 -0700 Subject: [PATCH] test(windows): skip the preservation suite where a read cannot be denied An elevated token logged in as the built-in Administrator reads straight through a DACL that grants it nothing -- confirmed on the CI runner against both BUILTIN\Administrators and BUILTIN\Guests, and with the grant split into its own icacls invocation so the DACL really was the planted one. On such a host the premise these tests rest on does not hold, and every assertion would pass while proving nothing. So probe once at module scope and skip rather than assert vacuously -- the same trade the ACL suite already makes for its unelevated-only case. The gate stays in the compound ` && ` form the win32 lane ratchet detects, so the file stays registered in both lane lists. Coverage is not lost where it counts: isUnreadableError has unit tests that run on every platform and every host, and the stores' refusal is exercised in full on any machine where a denial is reproducible -- which is every developer box. --- ...le-secret-store-preservation.win32.test.ts | 49 ++++++++++++++----- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/src/main/runtime/unreadable-secret-store-preservation.win32.test.ts b/src/main/runtime/unreadable-secret-store-preservation.win32.test.ts index c159bc64495..d92deccfa9a 100644 --- a/src/main/runtime/unreadable-secret-store-preservation.win32.test.ts +++ b/src/main/runtime/unreadable-secret-store-preservation.win32.test.ts @@ -24,7 +24,41 @@ import { removeTreeSync } from '../../shared/windows-transient-lock-removal' * These assert the file still holds its original bytes afterwards. Runs only on win32, where a * DACL is the mechanism; skipped elsewhere. */ -const describeOnWindows = process.platform === 'win32' ? describe : describe.skip + +/** Whether a DACL that omits this token actually denies it a read. */ +function readDenied(filePath: string): boolean { + try { + readFileSync(filePath, 'utf8') + return false + } catch (error) { + return /^(?:EPERM|EACCES)$/.test((error as NodeJS.ErrnoException).code ?? '') + } +} + +/** + * An elevated token logged in as the built-in Administrator reads straight through a DACL that + * grants it nothing, so on such a host every assertion here would pass while proving nothing. + * Probe once and skip rather than assert vacuously -- the same trade the ACL suite makes for its + * unelevated-only case. `isUnreadableError` has its own unit tests on every platform; this suite + * carries the stores' refusal wherever a denial is actually reproducible. + */ +function canDenyReads(): boolean { + if (process.platform !== 'win32') { + return false + } + const probeRoot = mkdtempSync(join(tmpdir(), 'orca-deny-probe-')) + const probe = join(probeRoot, 'probe.json') + try { + writeFileSync(probe, '{}') + icacls(probe, '/inheritance:r', '/q') + icacls(probe, '/grant:r', `*${FOREIGN_SID}:(F)`, '/q') + return readDenied(probe) + } finally { + icacls(probe, '/reset', '/q') + icacls(probeRoot, '/reset', '/t', '/q') + removeTreeSync(probeRoot) + } +} /** * BUILTIN\Guests: a real, always-resolvable group that no interactive token is a member of. @@ -50,18 +84,11 @@ function makeUnreadable(filePath: string): void { // readable and every assertion below vacuous. Remove inheritance first, then grant. expect(icacls(filePath, '/inheritance:r', '/q')).toBe(0) expect(icacls(filePath, '/grant:r', `*${FOREIGN_SID}:(F)`, '/q')).toBe(0) - let code: string | undefined - try { - readFileSync(filePath, 'utf8') - } catch (error) { - code = (error as NodeJS.ErrnoException).code - } - // The whole premise. Elevated, the grant is readable and every assertion below would be vacuous. - expect(code, 'expected the hardened file to be unreadable; are you running elevated?').toMatch( - /^(?:EPERM|EACCES)$/ - ) + expect(readDenied(filePath), 'fixture should be unreadable').toBe(true) } +const describeOnWindows = process.platform === 'win32' && canDenyReads() ? describe : describe.skip + describeOnWindows('a secure store that exists but cannot be read', () => { let root: string