From 0289d72818bc47c686a10681cc6febe1dac46b4c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:15:26 -0700 Subject: [PATCH] feat(crash-reporting): read-only Windows install-dir DACL probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records whether the install tree carries an orphan S-1-15-2-* package ACE with no S-1-15-2-1/-2 to satisfy it — the state that reproduces the 0x80000003 GPU/renderer init crash 10/10 (electron/electron#51761). Diagnostic only: never writes an ACL, never changes behavior. --- src/main/index.ts | 4 + .../windows-install-dir-acl-probe.test.ts | 188 ++++++++++++++++++ .../startup/windows-install-dir-acl-probe.ts | 187 +++++++++++++++++ 3 files changed, 379 insertions(+) create mode 100644 src/main/startup/windows-install-dir-acl-probe.test.ts create mode 100644 src/main/startup/windows-install-dir-acl-probe.ts diff --git a/src/main/index.ts b/src/main/index.ts index 90aed9840ef..640adcbf232 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -190,6 +190,7 @@ import { logStartupMilestone } from './startup/startup-diagnostics' import { ensureWindowsUserDataAclGrant } from './startup/windows-user-data-acl' +import { probeWindowsInstallDirAcl } from './startup/windows-install-dir-acl-probe' import { neutralizeLegacyTerminalShimDir } from './pty/legacy-terminal-shim-dir' import { shouldQuitWhenAllWindowsClosed } from './startup/window-all-closed-quit-policy' import { @@ -1374,6 +1375,9 @@ function openMainWindow(options: { revealOnDidFinishLoad?: boolean } = {}): Brow } } }) + // Why here: read-only, and the install DACL is the one thing a 0x80000003 + // child death cannot tell us about itself. See electron/electron#51761. + probeWindowsInstallDirAcl({ isServeMode }) } const window = createMainWindow(store, { diff --git a/src/main/startup/windows-install-dir-acl-probe.test.ts b/src/main/startup/windows-install-dir-acl-probe.test.ts new file mode 100644 index 00000000000..e36c6e6a548 --- /dev/null +++ b/src/main/startup/windows-install-dir-acl-probe.test.ts @@ -0,0 +1,188 @@ +import { EventEmitter } from 'node:events' +import { join } from 'node:path' +import { describe, expect, it, beforeEach, vi } from 'vitest' +import { + probeWindowsInstallDirAcl, + resetWindowsInstallDirAclProbeForTest, + WINDOWS_INSTALL_DIR_ACL_BREADCRUMB, + type WindowsInstallDirAclProbeOptions +} from './windows-install-dir-acl-probe' + +const INSTALL_DIR = 'C:\\Users\\neil\\AppData\\Local\\Programs\\orca' +const SYSTEM_ACES = [ + ' NT AUTHORITY\\SYSTEM:(I)(OI)(CI)(F)', + ' BUILTIN\\Administrators:(I)(OI)(CI)(F)', + ' awin\\neil:(I)(OI)(CI)(F)' +] + +/** Real icacls shape: the echoed path is glued onto the first principal. */ +function dacl(target: string, firstAce: string, ...rest: string[]): string { + return [ + `${target} ${firstAce}`, + ...rest, + ...SYSTEM_ACES, + '', + 'Successfully processed 1 files' + ].join('\r\n') +} + +const ORPHAN = 'S-1-15-2-999-999-999:(OI)(CI)(RX)' +const RESTRICTED_GRANT = + 'APPLICATION PACKAGE AUTHORITY\\ALL RESTRICTED APPLICATION PACKAGES:(OI)(CI)(RX)' + +function fakeSpawn(output: (target: string) => string | null): { + spawnFn: WindowsInstallDirAclProbeOptions['spawnFn'] + calls: { file: string; args: string[] }[] +} { + const calls: { file: string; args: string[] }[] = [] + const spawnFn = ((file: string, args: string[]) => { + calls.push({ file, args }) + const child = new EventEmitter() as EventEmitter & { + stdout: EventEmitter + kill: () => void + } + child.stdout = new EventEmitter() + child.kill = () => undefined + const out = output(args[0]) + setImmediate(() => { + if (out === null) { + child.emit('error', new Error('ENOENT')) + return + } + child.stdout.emit('data', Buffer.from(out, 'utf-8')) + child.emit('close', 0) + }) + return child + }) as unknown as WindowsInstallDirAclProbeOptions['spawnFn'] + return { spawnFn, calls } +} + +function probe(options: WindowsInstallDirAclProbeOptions): Promise> { + return new Promise((resolve) => { + probeWindowsInstallDirAcl({ + platform: 'win32', + installDir: INSTALL_DIR, + fileExists: (path) => path.endsWith('ffmpeg.dll'), + ...options, + recordBreadcrumb: (name, data) => { + resolve({ ...(data as Record), name }) + return undefined + } + }) + }) +} + +/** Every target returns the same ACE set on top of the system baseline. */ +function probeWith(...aces: string[]): Promise> { + return probe({ spawnFn: fakeSpawn((target) => dacl(target, aces[0], ...aces.slice(1))).spawnFn }) +} + +describe('probeWindowsInstallDirAcl', () => { + beforeEach(() => { + resetWindowsInstallDirAclProbeForTest() + }) + + it('reports a clean DACL as unpoisoned', async () => { + const data = await probe({ + spawnFn: fakeSpawn((target) => + [`${target} NT AUTHORITY\\SYSTEM:(I)(OI)(CI)(F)`, ...SYSTEM_ACES.slice(1)].join('\r\n') + ).spawnFn + }) + expect(data.name).toBe(WINDOWS_INSTALL_DIR_ACL_BREADCRUMB) + expect(data.status).toBe('ok') + expect(data.orphanPackageSidCount).toBe(0) + expect(data.matchesPoisonSignature).toBe(false) + }) + + it('flags an orphan package SID with no well-known grant', async () => { + const data = await probeWith(ORPHAN) + expect(data.matchesPoisonSignature).toBe(true) + expect(data.orphanPackageSidCount).toBe(1) + expect(data.orphanPackageSids).toBe('S-1-15-2-999-999-999') + expect(data.hasWellKnownPackageGrant).toBe(false) + }) + + it('clears the signature once a well-known package ACE grants access', async () => { + const data = await probeWith(RESTRICTED_GRANT, ORPHAN) + expect(data.hasWellKnownPackageGrant).toBe(true) + expect(data.orphanPackageSidCount).toBe(1) + expect(data.matchesPoisonSignature).toBe(false) + }) + + // The reproduced remedy was an additive *grant*; an ACE that grants nothing on + // the object cannot satisfy the orphan, so it must not clear the signature. + it.each([ + ['deny', 'APPLICATION PACKAGE AUTHORITY\\ALL APPLICATION PACKAGES:(DENY)(OI)(CI)(F)'], + ['inherit-only', 'APPLICATION PACKAGE AUTHORITY\\ALL APPLICATION PACKAGES:(OI)(CI)(IO)(GR,GE)'], + ['raw-sid deny', 'S-1-15-2-2:(DENY)(F)'], + ['raw-sid inherit-only', 'S-1-15-2-1:(OI)(CI)(IO)(GR,GE)'] + ])('does not let a %s well-known ACE satisfy an orphan', async (_label, ace) => { + const data = await probeWith(ace, ORPHAN) + expect(data.hasWellKnownPackageGrant).toBe(false) + expect(data.matchesPoisonSignature).toBe(true) + }) + + it('matches the well-known SIDs exactly, not by prefix', async () => { + const data = await probeWith('S-1-15-2-1234567890:(OI)(CI)(RX)') + expect(data.orphanPackageSidCount).toBe(1) + expect(data.hasWellKnownPackageGrant).toBe(false) + expect(data.matchesPoisonSignature).toBe(true) + }) + + it('ignores capability SIDs, which are a different family and harmless', async () => { + const data = await probeWith('S-1-15-3-65536-599108337-2355189375-1353122160:(S,X)') + expect(data.orphanPackageSidCount).toBe(0) + expect(data.matchesPoisonSignature).toBe(false) + }) + + it('probes a content file, not just the directory object', async () => { + const fake = fakeSpawn((target) => dacl(target, ORPHAN)) + await probe({ spawnFn: fake.spawnFn }) + expect(fake.calls.map((c) => c.args[0])).toEqual([INSTALL_DIR, join(INSTALL_DIR, 'ffmpeg.dll')]) + }) + + it('never passes a recursive or write flag', async () => { + const fake = fakeSpawn((target) => dacl(target, ORPHAN)) + await probe({ spawnFn: fake.spawnFn }) + for (const call of fake.calls) { + expect(call.args).toHaveLength(1) + expect(call.args[0]).not.toMatch(/^\//) + } + }) + + it('records a failure instead of throwing when icacls cannot be read', async () => { + const data = await probe({ spawnFn: fakeSpawn(() => null).spawnFn }) + expect(data.status).toBe('failed') + expect(data.reason).toBe('all-targets-unreadable') + expect(data.matchesPoisonSignature).toBeUndefined() + }) + + it.each([ + ['darwin', { platform: 'darwin' as NodeJS.Platform }], + ['serve mode', { isServeMode: true }] + ])('does no work on %s', async (_label, options) => { + const fake = fakeSpawn((target) => dacl(target, ORPHAN)) + const record = vi.fn() + const fileExists = vi.fn(() => true) + probeWindowsInstallDirAcl({ + installDir: INSTALL_DIR, + spawnFn: fake.spawnFn, + fileExists, + recordBreadcrumb: record, + ...options + }) + await new Promise((resolve) => setImmediate(resolve)) + expect(fake.calls).toHaveLength(0) + expect(fileExists).not.toHaveBeenCalled() + expect(record).not.toHaveBeenCalled() + }) + + it('runs once per process', async () => { + const fake = fakeSpawn((target) => dacl(target, ORPHAN)) + await probe({ spawnFn: fake.spawnFn }) + const before = fake.calls.length + probeWindowsInstallDirAcl({ platform: 'win32', installDir: INSTALL_DIR, spawnFn: fake.spawnFn }) + await new Promise((resolve) => setImmediate(resolve)) + expect(fake.calls).toHaveLength(before) + }) +}) diff --git a/src/main/startup/windows-install-dir-acl-probe.ts b/src/main/startup/windows-install-dir-acl-probe.ts new file mode 100644 index 00000000000..e37c60d110d --- /dev/null +++ b/src/main/startup/windows-install-dir-acl-probe.ts @@ -0,0 +1,187 @@ +import { spawn } from 'node:child_process' +import { existsSync } from 'node:fs' +import { dirname, join } from 'node:path' +import { + sanitizeCrashReportString, + type CrashReportBreadcrumbData +} from '../../shared/crash-reporting' +import { recordCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store' +import { getIcaclsExePath } from '../win32-utils' + +/** + * Read-only DACL probe for the win32 install directory. + * + * Why: six 1.4.184 reports show the GPU and renderer children both dying at init + * with 0x80000003 and nothing to distinguish them from any other CHECK. An install + * tree carrying an orphan S-1-15-2-* package ACE with no S-1-15-2-1/-2 to satisfy + * it reproduces exactly that signature (10/10 launches), and an additive grant of + * S-1-15-2-2 clears it — see electron/electron#51761. This records whether a + * machine is in that state so the next crash report answers the question itself. + * + * Diagnostic only: it never writes an ACL and never changes behavior. + */ + +export const WINDOWS_INSTALL_DIR_ACL_BREADCRUMB = 'windows_install_dir_acl' + +// Why a shortlist rather than a readdir: the reproduced failure is a per-file +// content read, so the directory's own DACL is not sufficient evidence — but any +// one shipped module answers it, and existsSync costs nothing. +const MODULE_SHORTLIST = ['ffmpeg.dll', 'libGLESv2.dll', 'libEGL.dll', 'icudtl.dat'] + +const PROBE_BUDGET_MS = 5_000 + +/** Raw S-1-15-2-* means icacls could not resolve it — locale-independent. */ +const RAW_PACKAGE_SID = /\bS-1-15-2-[0-9-]+\b/i +const WELL_KNOWN_PACKAGE_SIDS = new Set(['s-1-15-2-1', 's-1-15-2-2']) +// icacls localizes these; the raw SID form is never printed for them. +const WELL_KNOWN_PACKAGE_NAMES = /ALL (RESTRICTED )?APPLICATION PACKAGES/i +// Why: an ACE that denies, or only propagates to children, grants nothing on this +// object — so it cannot satisfy an orphan the way the reproduced fix did. +const NON_GRANTING_FLAGS = /\((?:DENY|IO)\)/i + +export type WindowsInstallDirAclProbeOptions = { + platform?: NodeJS.Platform + isServeMode?: boolean + installDir?: string + /** Test seams. */ + spawnFn?: typeof spawn + fileExists?: (path: string) => boolean + recordBreadcrumb?: typeof recordCrashBreadcrumb + onDone?: (data: CrashReportBreadcrumbData) => void +} + +type AclFacts = { + orphanPackageSids: string[] + hasWellKnownPackageGrant: boolean +} + +function readDacl(spawnFn: typeof spawn, target: string, deadlineMs: number): Promise { + return new Promise((resolve) => { + // No flags: icacls with a bare path only reads. Never /T — a recursive walk on + // a real profile measured 62s and timed out (see windows-user-data-acl.ts). + const child = spawnFn(getIcaclsExePath(), [target], { + stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true + }) + let out = '' + let settled = false + const settle = (value: string): void => { + if (settled) { + return + } + settled = true + clearTimeout(timer) + resolve(value) + } + const timer = setTimeout(() => { + child.kill() + settle('') + }, deadlineMs) + timer.unref?.() + child.stdout?.on('data', (chunk: Buffer) => { + out += chunk.toString('utf-8') + }) + child.on('error', () => settle('')) + // 'close' not 'exit': exit can fire before stdout drains, and an empty read + // would parse as a clean DACL — a false negative in the only case we care about. + child.on('close', () => settle(out)) + }) +} + +function collectAclFacts(daclOutput: string): AclFacts { + const orphanPackageSids: string[] = [] + let hasWellKnownPackageGrant = false + for (const line of daclOutput.split(/\r?\n/)) { + // icacls glues the echoed path onto the first principal with no separator, so + // match the principal:(flags) tail rather than trying to split the line. + const ace = /([^\s:][^:]*):(\([^\s]*\))\s*$/.exec(line.trim()) + if (!ace) { + continue + } + const [, principal, flags] = ace + const sid = RAW_PACKAGE_SID.exec(principal)?.[0] + if (sid && !WELL_KNOWN_PACKAGE_SIDS.has(sid.toLowerCase())) { + orphanPackageSids.push(sid) + continue + } + const isWellKnown = sid !== undefined || WELL_KNOWN_PACKAGE_NAMES.test(principal) + if (isWellKnown && !NON_GRANTING_FLAGS.test(flags)) { + hasWellKnownPackageGrant = true + } + } + return { orphanPackageSids, hasWellKnownPackageGrant } +} + +function resolveTargets(installDir: string, fileExists: (path: string) => boolean): string[] { + const moduleFile = MODULE_SHORTLIST.map((name) => join(installDir, name)).find(fileExists) + return moduleFile ? [installDir, moduleFile] : [installDir] +} + +async function runProbe(options: WindowsInstallDirAclProbeOptions): Promise { + const record = options.recordBreadcrumb ?? recordCrashBreadcrumb + let data: CrashReportBreadcrumbData + try { + const installDir = options.installDir ?? dirname(process.execPath) + const targets = resolveTargets(installDir, options.fileExists ?? existsSync) + const spawnFn = options.spawnFn ?? spawn + const startedAt = Date.now() + const outputs: string[] = [] + for (const target of targets) { + const remaining = PROBE_BUDGET_MS - (Date.now() - startedAt) + // Why one shared budget: two targets must never cost two full timeouts. + outputs.push(remaining > 0 ? await readDacl(spawnFn, target, remaining) : '') + } + const facts = outputs.map(collectAclFacts) + const orphans = [...new Set(facts.flatMap((f) => f.orphanPackageSids))] + const hasWellKnownPackageGrant = facts.some((f) => f.hasWellKnownPackageGrant) + data = outputs.every((out) => out === '') + ? { status: 'failed', reason: 'all-targets-unreadable' } + : { + status: 'ok', + probedTargetCount: targets.length, + orphanPackageSidCount: orphans.length, + // Capped: correlating the same orphan across reports is what would + // identify the tool that left it, which is the point of recording it. + orphanPackageSids: sanitizeCrashReportString(orphans.slice(0, 3).join(','), 200), + hasWellKnownPackageGrant, + matchesPoisonSignature: orphans.length > 0 && !hasWellKnownPackageGrant + } + } catch (error) { + data = { status: 'failed', reason: sanitizeCrashReportString(`probe: ${String(error)}`, 200) } + } + record(WINDOWS_INSTALL_DIR_ACL_BREADCRUMB, data) + options.onDone?.(data) +} + +// Why once per process: the install DACL cannot usefully change mid-session, and +// openMainWindow re-runs on re-activation. +let probeStarted = false + +export function resetWindowsInstallDirAclProbeForTest(): void { + probeStarted = false +} + +/** + * Fire-and-forget; returns before any spawn. win32 only — no spawn and no fs I/O + * anywhere else. Records into the retained breadcrumb ring, which is what + * process-gone-recorder snapshots into every crash report. + */ +export function probeWindowsInstallDirAcl(options: WindowsInstallDirAclProbeOptions = {}): void { + if ((options.platform ?? process.platform) !== 'win32' || options.isServeMode === true) { + return + } + if (probeStarted) { + return + } + probeStarted = true + // Why the try: this runs at the caller's module scope before app.whenReady, so + // anything thrown here would abort main-process init and no window would ever + // be created. A diagnostic must never be able to do that. + try { + setImmediate(() => { + void runProbe(options).catch(() => undefined) + }) + } catch { + // Nothing left to report to that would not throw again. + } +}