mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 16:02:56 +00:00
fix(windows): repair the poisoned install-dir ACL before the window, not after
The install-dir LPAC ACL poison (electron/electron#51761) still costs every affected machine at least one crash: the probe that detects it is setImmediate-deferred and answers 0.9-3.0s in, while createMainWindow runs synchronously in the same frame and its renderer dies at init 48-1373ms later. - Persist the poison verdict the moment the probe reports it, and await the repair (bounded at 20s) before any window is created on a launch that already carries the marker. - Do not engage the GPU safe-graphics fallback while the install-dir ACL verdict is poisoned or still outstanding. Safe graphics does not rescue a poisoned tree, and --in-process-gpu removes the GPU child, erasing the sibling-death evidence that identifies the shape (4 field reports landed in 'misc' this way). - Clear the safe-graphics marker once the repair lands, so a repaired machine stops launching software-rendered for the rest of that build. - Give the repair marker a bounded retry budget: it was written on failure and matched regardless of outcome, so one transient failure pinned a machine to 'marker-hit' for the life of that version.
This commit is contained in:
@@ -16,6 +16,7 @@ import { promptForGpuFallbackRestart } from '../crash-reporting/gpu-fallback-res
|
||||
import { engageGpuFallbackAfterCrashBurst } from '../crash-reporting/gpu-fallback-engagement'
|
||||
import { recordCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store'
|
||||
import { recordDurableCrashBreadcrumb } from '../crash-reporting/durable-crash-breadcrumb'
|
||||
import { isInstallDirAclSuspect } from './windows-install-dir-acl-recovery'
|
||||
import { mainProcessState as state, gpuFallbackEnvironment } from './main-process-state'
|
||||
import { createGpuAccelerationAboutPanelOptions } from '../menu/gpu-acceleration-about-panel'
|
||||
|
||||
@@ -124,6 +125,12 @@ export async function handleGpuChildCrash(
|
||||
if (state.gpuFallbackActiveThisLaunch || state.isQuitting || state.isServeMode) {
|
||||
return
|
||||
}
|
||||
// Why: a poisoned install DACL kills the GPU child exactly like a bad driver, but
|
||||
// safe graphics does not rescue it and --in-process-gpu removes the GPU child, so
|
||||
// every later crash loses the sibling deaths that identify the real cause.
|
||||
if (isInstallDirAclSuspect()) {
|
||||
return
|
||||
}
|
||||
const result = state.gpuCrashFallbackTracker.recordGpuCrash(crashedAt)
|
||||
if (!result.shouldEngageFallback) {
|
||||
return
|
||||
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
import { prepareCodexRuntimeHomeForLaunch } from './codex-launch-preparation'
|
||||
import { prepareCodexSessionResumeForLaunch } from './codex-session-resume-launch'
|
||||
import { startWindowsDesktopBeforeShellPathReady } from './windows-desktop-shell-path-startup'
|
||||
import { repairKnownPoisonedInstallDirBeforeWindow } from './windows-install-dir-acl-recovery'
|
||||
import { registerServeSignalHandlers } from './serve-signal-handlers'
|
||||
import { settleServeDesktopActivation } from './serve-desktop-activation'
|
||||
import {
|
||||
@@ -292,6 +293,17 @@ export async function initializeMainProcessRuntimeLaunch(
|
||||
// Why published: the renderer's git-environment barrier must fence on the same
|
||||
// generation the terminal startup services wait for, not a later re-read.
|
||||
state.shellPathReady = shellPathReady
|
||||
// Why before any window: the poisoned install DACL kills the renderer at init, and
|
||||
// the probe that detects it cannot finish before createMainWindow. Bounded, and a
|
||||
// no-op (one absent-file read) unless a previous launch already recorded the verdict.
|
||||
const aclGate = await repairKnownPoisonedInstallDirBeforeWindow({
|
||||
isServeMode: state.isServeMode || serveOptions !== null,
|
||||
userDataPath: app.getPath('userData'),
|
||||
appVersion: app.getVersion()
|
||||
})
|
||||
if (aclGate !== 'not-marked' && aclGate !== 'skipped') {
|
||||
logStartupMilestone('install-dir-acl-repair-blocking-done', { mode: aclGate })
|
||||
}
|
||||
let desktopWindow: BrowserWindow | null = null
|
||||
if (process.platform === 'win32' && app.isPackaged && !serveOptions) {
|
||||
const desktopStartup = startWindowsDesktopBeforeShellPathReady({
|
||||
|
||||
@@ -10,7 +10,10 @@ import { resolveConsent } from '../telemetry/consent'
|
||||
import { trackAppOpenedOnce } from '../telemetry/client'
|
||||
import { ensureWindowsUserDataAclGrant } from './windows-user-data-acl'
|
||||
import { probeWindowsInstallDirAcl } from './windows-install-dir-acl-probe'
|
||||
import { startWindowsInstallDirAclRepairIfPoisoned } from './windows-install-dir-acl-recovery'
|
||||
import {
|
||||
noteWindowsInstallDirAclProbePending,
|
||||
startWindowsInstallDirAclRepairIfPoisoned
|
||||
} from './windows-install-dir-acl-recovery'
|
||||
import { logStartupMilestone } from './startup-diagnostics'
|
||||
import { notifyMainWindowBecameVisible } from '../window/main-window-visibility'
|
||||
import { setTrayAttention } from '../tray/system-tray'
|
||||
@@ -74,6 +77,9 @@ export function openMainWindow(options: { revealOnDidFinishLoad?: boolean } = {}
|
||||
})
|
||||
// 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.
|
||||
if (!state.isServeMode) {
|
||||
noteWindowsInstallDirAclProbePending()
|
||||
}
|
||||
probeWindowsInstallDirAcl({
|
||||
isServeMode: state.isServeMode,
|
||||
onDone: (data) =>
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
|
||||
/**
|
||||
* "This install directory was found poisoned and has not been proven healthy since."
|
||||
*
|
||||
* Why a separate marker from `windows-install-dir-acl-repair.json`: that one is
|
||||
* written after an attempt finishes, so a launch the poison kills mid-repair
|
||||
* leaves no state at all and the next launch repeats the whole late-repair dance.
|
||||
* This one is written the moment the probe's verdict lands, and it is the only
|
||||
* thing that lets a later launch know it is poisoned *before* it creates a window
|
||||
* — the probe itself cannot answer that early. Same tiny synchronous-JSON shape
|
||||
* as `gpu-fallback-marker.ts`, for the same reason.
|
||||
*/
|
||||
|
||||
export const WINDOWS_INSTALL_DIR_ACL_POISON_MARKER_FILE = 'windows-install-dir-acl-poison.json'
|
||||
export const WINDOWS_INSTALL_DIR_ACL_POISON_SCHEME_VERSION = 1
|
||||
|
||||
type PoisonMarker = {
|
||||
schemeVersion: number
|
||||
installDir: string
|
||||
appVersion: string
|
||||
detectedAt: number
|
||||
}
|
||||
|
||||
function markerPath(userDataPath: string): string {
|
||||
return join(userDataPath, WINDOWS_INSTALL_DIR_ACL_POISON_MARKER_FILE)
|
||||
}
|
||||
|
||||
/** Keyed on both: a reinstall elsewhere or an update ships files with a fresh DACL. */
|
||||
export function hasInstallDirAclPoisonMarker(
|
||||
userDataPath: string,
|
||||
installDir: string,
|
||||
appVersion: string
|
||||
): boolean {
|
||||
try {
|
||||
const parsed = JSON.parse(readFileSync(markerPath(userDataPath), 'utf-8')) as
|
||||
| Partial<PoisonMarker>
|
||||
| undefined
|
||||
return (
|
||||
parsed?.schemeVersion === WINDOWS_INSTALL_DIR_ACL_POISON_SCHEME_VERSION &&
|
||||
parsed.installDir === installDir &&
|
||||
parsed.appVersion === appVersion
|
||||
)
|
||||
} catch {
|
||||
return false // missing or corrupt -> treat the install as healthy
|
||||
}
|
||||
}
|
||||
|
||||
export function writeInstallDirAclPoisonMarker(
|
||||
userDataPath: string,
|
||||
installDir: string,
|
||||
appVersion: string
|
||||
): void {
|
||||
const marker: PoisonMarker = {
|
||||
schemeVersion: WINDOWS_INSTALL_DIR_ACL_POISON_SCHEME_VERSION,
|
||||
installDir,
|
||||
appVersion,
|
||||
detectedAt: Date.now()
|
||||
}
|
||||
try {
|
||||
if (!existsSync(userDataPath)) {
|
||||
mkdirSync(userDataPath, { recursive: true })
|
||||
}
|
||||
writeFileSync(markerPath(userDataPath), JSON.stringify(marker))
|
||||
} catch {
|
||||
// Best effort: without it the next launch just falls back to today's late repair.
|
||||
}
|
||||
}
|
||||
|
||||
export function clearInstallDirAclPoisonMarker(userDataPath: string): void {
|
||||
try {
|
||||
rmSync(markerPath(userDataPath), { force: true })
|
||||
} catch {
|
||||
// Best effort; a stale marker only costs one redundant icacls pass.
|
||||
}
|
||||
}
|
||||
@@ -1,16 +1,23 @@
|
||||
import { mkdtempSync } from 'node:fs'
|
||||
import { mkdtempSync, readFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { beforeEach, describe, expect, it } from 'vitest'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { ProcessResult, ProcessSpec } from '../../shared/child-process/run-process'
|
||||
import type { CrashReportBreadcrumbData } from '../../shared/crash-reporting'
|
||||
import { readActiveGpuFallbackMarker, writeGpuFallbackMarker } from './gpu-fallback-marker'
|
||||
import {
|
||||
probeWindowsInstallDirAcl,
|
||||
resetWindowsInstallDirAclProbeForTest
|
||||
} from './windows-install-dir-acl-probe'
|
||||
import { hasInstallDirAclPoisonMarker } from './windows-install-dir-acl-poison-marker'
|
||||
import {
|
||||
describeInstallDirAclPoison,
|
||||
isInstallDirAclSuspect,
|
||||
noteWindowsInstallDirAclProbePending,
|
||||
repairKnownPoisonedInstallDirBeforeWindow,
|
||||
resetWindowsInstallDirAclRecoveryForTest,
|
||||
startWindowsInstallDirAclRepairIfPoisoned
|
||||
startWindowsInstallDirAclRepairIfPoisoned,
|
||||
type WindowsInstallDirAclRecoveryOptions
|
||||
} from './windows-install-dir-acl-recovery'
|
||||
import { resetWindowsInstallDirAclRepairForTest } from './windows-install-dir-package-acl-repair'
|
||||
import {
|
||||
@@ -26,6 +33,8 @@ import {
|
||||
const INSTALL_DIR = 'C:\\Users\\neil\\AppData\\Local\\Programs\\orca'
|
||||
const APP_VERSION = '1.4.184'
|
||||
|
||||
type Runner = (spec: ProcessSpec) => Promise<ProcessResult>
|
||||
|
||||
/**
|
||||
* Drives the production path: the real probe hands its verdict to the real gate,
|
||||
* which decides whether icacls ever runs. Only the two process seams are faked.
|
||||
@@ -185,3 +194,268 @@ describe('describeInstallDirAclPoison', () => {
|
||||
expect(describeInstallDirAclPoison()?.detail).toContain('repairing the permissions now')
|
||||
})
|
||||
})
|
||||
|
||||
const POISON_VERDICT: CrashReportBreadcrumbData = {
|
||||
status: 'ok',
|
||||
matchesPoisonSignature: true,
|
||||
wellKnownNameCheckReliable: true
|
||||
}
|
||||
const GPU_ENV = { appVersion: APP_VERSION, electronVersion: '43.4.1', platform: 'win32' } as const
|
||||
|
||||
function recoveryOptions(userDataPath: string, run: Runner): WindowsInstallDirAclRecoveryOptions {
|
||||
return {
|
||||
platform: 'win32',
|
||||
installDir: INSTALL_DIR,
|
||||
appVersion: APP_VERSION,
|
||||
userDataPath,
|
||||
runProcessFn: run as never,
|
||||
recordBreadcrumb: () => undefined
|
||||
}
|
||||
}
|
||||
|
||||
/** icacls' real success summary, as the repair's parser expects it. */
|
||||
const okRun: Runner = async () => ({
|
||||
code: 0,
|
||||
signal: null,
|
||||
stdout: 'Successfully processed 3200 files; Failed processing 0 files',
|
||||
stderr: '',
|
||||
timedOut: false
|
||||
})
|
||||
|
||||
describe('install-dir ACL repair vs the GPU safe-graphics marker', () => {
|
||||
beforeEach(() => {
|
||||
resetWindowsInstallDirAclProbeForTest()
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
resetWindowsInstallDirAclRecoveryForTest()
|
||||
})
|
||||
|
||||
it('clears the sticky safe-graphics marker once the real cause is repaired', async () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-acl-gpu-'))
|
||||
// The machine is in the reproduced state: the poisoned install DACL killed the
|
||||
// GPU child three times, so Orca latched safe graphics for this build.
|
||||
writeGpuFallbackMarker(
|
||||
userDataPath,
|
||||
{ engagedAt: Date.now(), crashesInWindow: 3, userConfirmed: false },
|
||||
GPU_ENV
|
||||
)
|
||||
expect(readActiveGpuFallbackMarker(userDataPath, GPU_ENV)).not.toBeNull()
|
||||
|
||||
await new Promise<void>((resolve) => {
|
||||
startWindowsInstallDirAclRepairIfPoisoned(POISON_VERDICT, {
|
||||
...recoveryOptions(userDataPath, okRun),
|
||||
// Settles after the repair's own setImmediate hop and its two icacls passes.
|
||||
recordBreadcrumb: () => {
|
||||
setTimeout(resolve, 0)
|
||||
return undefined
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
expect(describeInstallDirAclPoison()?.detail).toContain('repaired the permissions')
|
||||
// The GPU child deaths were never a driver fault, so safe graphics — and the
|
||||
// --in-process-gpu launch that hides the next crash's evidence — must not outlive the repair.
|
||||
expect(readActiveGpuFallbackMarker(userDataPath, GPU_ENV)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('isInstallDirAclSuspect', () => {
|
||||
beforeEach(() => {
|
||||
resetWindowsInstallDirAclProbeForTest()
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
resetWindowsInstallDirAclRecoveryForTest()
|
||||
})
|
||||
|
||||
it('is false when nothing has suggested the install DACL is involved', () => {
|
||||
expect(isInstallDirAclSuspect()).toBe(false)
|
||||
})
|
||||
|
||||
// The GPU child dies ~74ms in and the probe answers 0.9-3.0s later, so "no verdict
|
||||
// yet" is the entire window in which the misdiagnosis happens.
|
||||
it('holds while the probe verdict is outstanding, and releases on a clean verdict', () => {
|
||||
noteWindowsInstallDirAclProbePending()
|
||||
expect(isInstallDirAclSuspect()).toBe(true)
|
||||
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
{ status: 'ok', matchesPoisonSignature: false },
|
||||
recoveryOptions(mkdtempSync(join(tmpdir(), 'orca-acl-suspect-')), okRun)
|
||||
)
|
||||
expect(isInstallDirAclSuspect()).toBe(false)
|
||||
})
|
||||
|
||||
it('releases once the wait exceeds the grace window, so a silent probe cannot pin it', () => {
|
||||
noteWindowsInstallDirAclProbePending()
|
||||
expect(isInstallDirAclSuspect(Date.now() + 14_000)).toBe(true)
|
||||
expect(isInstallDirAclSuspect(Date.now() + 16_000)).toBe(false)
|
||||
})
|
||||
|
||||
it('holds through a repair that failed, and releases once one succeeds', async () => {
|
||||
const failing: Runner = async () => ({
|
||||
code: 5,
|
||||
signal: null,
|
||||
stdout: '',
|
||||
stderr: 'Access is denied.',
|
||||
timedOut: false
|
||||
})
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-acl-suspect-'))
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
POISON_VERDICT,
|
||||
recoveryOptions(userDataPath, failing)
|
||||
)
|
||||
expect(isInstallDirAclSuspect()).toBe(true)
|
||||
await vi.waitFor(() => expect(describeInstallDirAclPoison()?.detail).toContain('could not'))
|
||||
// Still suspect: the tree is proven poisoned, and safe graphics does not rescue it.
|
||||
expect(isInstallDirAclSuspect()).toBe(true)
|
||||
|
||||
resetWindowsInstallDirAclRecoveryForTest()
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
POISON_VERDICT,
|
||||
recoveryOptions(mkdtempSync(join(tmpdir(), 'orca-acl-suspect-')), okRun)
|
||||
)
|
||||
await vi.waitFor(() => expect(isInstallDirAclSuspect()).toBe(false))
|
||||
})
|
||||
})
|
||||
|
||||
describe('repairKnownPoisonedInstallDirBeforeWindow', () => {
|
||||
beforeEach(() => {
|
||||
resetWindowsInstallDirAclProbeForTest()
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
resetWindowsInstallDirAclRecoveryForTest()
|
||||
})
|
||||
|
||||
it('costs a healthy machine one absent-file read and no icacls', async () => {
|
||||
const specs: ProcessSpec[] = []
|
||||
const run: Runner = async (spec) => {
|
||||
specs.push(spec)
|
||||
return okRun(spec)
|
||||
}
|
||||
const mode = await repairKnownPoisonedInstallDirBeforeWindow(
|
||||
recoveryOptions(mkdtempSync(join(tmpdir(), 'orca-acl-gate-')), run)
|
||||
)
|
||||
expect(mode).toBe('not-marked')
|
||||
expect(specs).toHaveLength(0)
|
||||
})
|
||||
|
||||
// The crash this fixes: launch 1 detects the poison but createMainWindow already
|
||||
// ran, so the renderer is dead before icacls is spawned. Launch 2 must not repeat it.
|
||||
it('repairs a launch that a previous one recorded as poisoned, before returning', async () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-acl-gate-'))
|
||||
// Launch 1: the probe reports poison and the app dies mid-repair.
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
POISON_VERDICT,
|
||||
recoveryOptions(userDataPath, (() => new Promise<never>(() => undefined)) as Runner)
|
||||
)
|
||||
expect(hasInstallDirAclPoisonMarker(userDataPath, INSTALL_DIR, APP_VERSION)).toBe(true)
|
||||
|
||||
// Launch 2.
|
||||
resetWindowsInstallDirAclRecoveryForTest()
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
writeGpuFallbackMarker(
|
||||
userDataPath,
|
||||
{ engagedAt: Date.now(), crashesInWindow: 3, userConfirmed: false },
|
||||
GPU_ENV
|
||||
)
|
||||
const specs: ProcessSpec[] = []
|
||||
const run: Runner = async (spec) => {
|
||||
specs.push(spec)
|
||||
return okRun(spec)
|
||||
}
|
||||
const mode = await repairKnownPoisonedInstallDirBeforeWindow(recoveryOptions(userDataPath, run))
|
||||
expect(mode).toBe('repaired')
|
||||
// Both passes have already run by the time the window may be created.
|
||||
expect(specs.map((spec) => spec.args?.[2])).toEqual([
|
||||
'*S-1-15-2-2:(OI)(CI)(RX)',
|
||||
'*S-1-15-2-2:(RX)'
|
||||
])
|
||||
expect(hasInstallDirAclPoisonMarker(userDataPath, INSTALL_DIR, APP_VERSION)).toBe(false)
|
||||
expect(readActiveGpuFallbackMarker(userDataPath, GPU_ENV)).toBeNull()
|
||||
})
|
||||
|
||||
it('gives up on its budget rather than holding the window open forever', async () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-acl-gate-'))
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
POISON_VERDICT,
|
||||
recoveryOptions(userDataPath, (() => new Promise<never>(() => undefined)) as Runner)
|
||||
)
|
||||
resetWindowsInstallDirAclRecoveryForTest()
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
|
||||
const mode = await repairKnownPoisonedInstallDirBeforeWindow({
|
||||
...recoveryOptions(userDataPath, (() => new Promise<never>(() => undefined)) as Runner),
|
||||
timeoutMs: 20
|
||||
})
|
||||
expect(mode).toBe('timeout')
|
||||
})
|
||||
|
||||
it('is a no-op off win32 and in serve mode', async () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-acl-gate-'))
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
POISON_VERDICT,
|
||||
recoveryOptions(userDataPath, (() => new Promise<never>(() => undefined)) as Runner)
|
||||
)
|
||||
resetWindowsInstallDirAclRecoveryForTest()
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
|
||||
expect(
|
||||
await repairKnownPoisonedInstallDirBeforeWindow({
|
||||
...recoveryOptions(userDataPath, okRun),
|
||||
platform: 'darwin'
|
||||
})
|
||||
).toBe('skipped')
|
||||
expect(
|
||||
await repairKnownPoisonedInstallDirBeforeWindow({
|
||||
...recoveryOptions(userDataPath, okRun),
|
||||
isServeMode: true
|
||||
})
|
||||
).toBe('skipped')
|
||||
})
|
||||
|
||||
it('retires the marker when a later probe reports the install clean', () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-acl-gate-'))
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
POISON_VERDICT,
|
||||
recoveryOptions(userDataPath, (() => new Promise<never>(() => undefined)) as Runner)
|
||||
)
|
||||
expect(hasInstallDirAclPoisonMarker(userDataPath, INSTALL_DIR, APP_VERSION)).toBe(true)
|
||||
|
||||
resetWindowsInstallDirAclRecoveryForTest()
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
{ status: 'ok', matchesPoisonSignature: false },
|
||||
recoveryOptions(userDataPath, okRun)
|
||||
)
|
||||
expect(hasInstallDirAclPoisonMarker(userDataPath, INSTALL_DIR, APP_VERSION)).toBe(false)
|
||||
})
|
||||
|
||||
// An unreadable DACL is not evidence of health; forgetting the verdict there would
|
||||
// hand the next launch straight back to the crash it already recorded.
|
||||
it('keeps the marker when the probe could not read the DACL', () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-acl-gate-'))
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
POISON_VERDICT,
|
||||
recoveryOptions(userDataPath, (() => new Promise<never>(() => undefined)) as Runner)
|
||||
)
|
||||
resetWindowsInstallDirAclRecoveryForTest()
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
{ status: 'failed', reason: 'all-targets-unreadable' },
|
||||
recoveryOptions(userDataPath, okRun)
|
||||
)
|
||||
expect(hasInstallDirAclPoisonMarker(userDataPath, INSTALL_DIR, APP_VERSION)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Why a source assertion: gpu-lifecycle's transitive import graph reaches the real
|
||||
* `electron` binding, so the guard cannot be driven in-process. This pins the one
|
||||
* thing that matters — the ACL verdict is consulted before the crash is counted
|
||||
* towards the burst that latches safe graphics.
|
||||
*/
|
||||
describe('handleGpuChildCrash call site', () => {
|
||||
it('consults the install-dir ACL verdict before counting the crash', () => {
|
||||
const source = readFileSync(join(__dirname, 'gpu-lifecycle.ts'), 'utf8')
|
||||
const start = source.indexOf('export async function handleGpuChildCrash')
|
||||
const countIndex = source.indexOf('recordGpuCrash(', start)
|
||||
expect(start).toBeGreaterThanOrEqual(0)
|
||||
expect(countIndex).toBeGreaterThan(start)
|
||||
expect(source.slice(start, countIndex)).toContain('isInstallDirAclSuspect()')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
import { dirname } from 'node:path'
|
||||
import type { CrashReportBreadcrumbData } from '../../shared/crash-reporting'
|
||||
import { logStartupMilestone } from './startup-diagnostics'
|
||||
import { clearGpuFallbackMarker } from './gpu-fallback-marker'
|
||||
import {
|
||||
clearInstallDirAclPoisonMarker,
|
||||
hasInstallDirAclPoisonMarker,
|
||||
writeInstallDirAclPoisonMarker
|
||||
} from './windows-install-dir-acl-poison-marker'
|
||||
import {
|
||||
buildInstallDirAclRepairCommands,
|
||||
isInstallDirAclPoisonVerdict,
|
||||
@@ -25,10 +31,62 @@ export type WindowsInstallDirAclRecoveryOptions = Omit<WindowsInstallDirAclRepai
|
||||
|
||||
type RepairStage = WindowsInstallDirAclRepairResult['mode'] | 'pending'
|
||||
|
||||
/** Long enough for the ~4-13s repair measured on real hosts, short enough to still be a launch. */
|
||||
const BLOCKING_REPAIR_BUDGET_MS = 20_000
|
||||
/** A probe that never answers must not suppress the driver fallback for the session. */
|
||||
const PROBE_VERDICT_GRACE_MS = 15_000
|
||||
|
||||
let poison: { installDir: string; stage: RepairStage } | null = null
|
||||
let probePendingSince: number | null = null
|
||||
|
||||
export function resetWindowsInstallDirAclRecoveryForTest(): void {
|
||||
poison = null
|
||||
probePendingSince = null
|
||||
}
|
||||
|
||||
/** Call when the install-DACL probe is dispatched: its verdict is not in yet. */
|
||||
export function noteWindowsInstallDirAclProbePending(): void {
|
||||
probePendingSince = Date.now()
|
||||
}
|
||||
|
||||
/**
|
||||
* True while a sandboxed-child death could be the install DACL rather than the
|
||||
* graphics driver. Safe graphics does not rescue a poisoned tree — it still kills
|
||||
* the renderer — and it removes the GPU child, erasing the sibling-death evidence
|
||||
* that is the only way to recognise the shape in a crash report.
|
||||
*/
|
||||
export function isInstallDirAclSuspect(now: number = Date.now()): boolean {
|
||||
if (poison) {
|
||||
return poison.stage !== 'repaired'
|
||||
}
|
||||
return probePendingSince !== null && now - probePendingSince < PROBE_VERDICT_GRACE_MS
|
||||
}
|
||||
|
||||
function startRepair(
|
||||
installDir: string,
|
||||
options: WindowsInstallDirAclRecoveryOptions,
|
||||
onDone?: (result: WindowsInstallDirAclRepairResult) => void
|
||||
): void {
|
||||
poison = { installDir, stage: 'pending' }
|
||||
writeInstallDirAclPoisonMarker(options.userDataPath, installDir, options.appVersion)
|
||||
repairWindowsInstallDirPackageAcl({
|
||||
...options,
|
||||
installDir,
|
||||
onDone: (result) => {
|
||||
poison = { installDir, stage: result.mode }
|
||||
logStartupMilestone('install-dir-acl-repair-done', { mode: result.mode })
|
||||
if (result.mode === 'repaired') {
|
||||
clearInstallDirAclPoisonMarker(options.userDataPath)
|
||||
// The GPU child deaths were never a driver fault, so safe graphics — and the
|
||||
// --in-process-gpu launch that hides the next crash's evidence — must not outlive the repair.
|
||||
clearGpuFallbackMarker(options.userDataPath)
|
||||
}
|
||||
if (result.mode === 'failed') {
|
||||
console.warn('[win32-acl] install dir package ACL repair failed:', result.reason)
|
||||
}
|
||||
onDone?.(result)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/** The probe's `onDone`: no-op unless the machine is in the reproduced state. */
|
||||
@@ -36,21 +94,52 @@ export function startWindowsInstallDirAclRepairIfPoisoned(
|
||||
data: CrashReportBreadcrumbData,
|
||||
options: WindowsInstallDirAclRecoveryOptions
|
||||
): void {
|
||||
probePendingSince = null
|
||||
if (!isInstallDirAclPoisonVerdict(data)) {
|
||||
// Only a positive clean reading retires the marker; an unreadable DACL proves nothing.
|
||||
if (data.matchesPoisonSignature === false) {
|
||||
clearInstallDirAclPoisonMarker(options.userDataPath)
|
||||
}
|
||||
return
|
||||
}
|
||||
// The blocking pre-window gate may already own this launch's repair; restarting it
|
||||
// would reset the verdict to 'pending' against a repair that can no longer report.
|
||||
if (poison) {
|
||||
return
|
||||
}
|
||||
startRepair(options.installDir ?? dirname(process.execPath), options)
|
||||
}
|
||||
|
||||
/**
|
||||
* Pre-window gate for a machine a previous launch already found poisoned.
|
||||
*
|
||||
* Why blocking, and why only here: the probe is `setImmediate`-deferred and takes
|
||||
* 0.9-3.0s on the affected hosts, while the renderer it has to save is spawned
|
||||
* synchronously by `createMainWindow` and dies at init 48-1373ms in. The
|
||||
* persisted verdict is what buys that knowledge for free — a healthy machine
|
||||
* reads one absent file and pays nothing.
|
||||
*/
|
||||
export async function repairKnownPoisonedInstallDirBeforeWindow(
|
||||
options: WindowsInstallDirAclRecoveryOptions & { timeoutMs?: number }
|
||||
): Promise<'not-marked' | 'skipped' | WindowsInstallDirAclRepairResult['mode'] | 'timeout'> {
|
||||
if ((options.platform ?? process.platform) !== 'win32' || options.isServeMode === true) {
|
||||
return 'skipped'
|
||||
}
|
||||
const installDir = options.installDir ?? dirname(process.execPath)
|
||||
poison = { installDir, stage: 'pending' }
|
||||
repairWindowsInstallDirPackageAcl({
|
||||
...options,
|
||||
installDir,
|
||||
onDone: (result) => {
|
||||
poison = { installDir, stage: result.mode }
|
||||
logStartupMilestone('install-dir-acl-repair-done', { mode: result.mode })
|
||||
if (result.mode === 'failed') {
|
||||
console.warn('[win32-acl] install dir package ACL repair failed:', result.reason)
|
||||
}
|
||||
}
|
||||
if (!hasInstallDirAclPoisonMarker(options.userDataPath, installDir, options.appVersion)) {
|
||||
return 'not-marked'
|
||||
}
|
||||
logStartupMilestone('install-dir-acl-repair-blocking-start')
|
||||
return await new Promise((resolve) => {
|
||||
const timer = setTimeout(
|
||||
() => resolve('timeout'),
|
||||
options.timeoutMs ?? BLOCKING_REPAIR_BUDGET_MS
|
||||
)
|
||||
timer.unref?.()
|
||||
startRepair(installDir, options, (result) => {
|
||||
clearTimeout(timer)
|
||||
resolve(result.mode)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -233,6 +233,38 @@ describe('repairWindowsInstallDirPackageAcl', () => {
|
||||
expect(marker.outcome).toBe('failed')
|
||||
})
|
||||
|
||||
// The bricking mechanism: a marker was written on failure and matched regardless of
|
||||
// outcome, so one Defender-locked file or one timeout pinned the machine to
|
||||
// 'marker-hit' — repair permanently skipped — for the life of that version.
|
||||
it('retries a failed repair on later launches, then stops once the budget is spent', async () => {
|
||||
const userDataPath = userDataDir()
|
||||
const failing = fakeRunner(() => ({ code: 5, stderr: 'Access is denied.' }))
|
||||
for (let attempt = 0; attempt < 3; attempt++) {
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
expect((await repair({ userDataPath, run: failing.run })).result.mode).toBe('failed')
|
||||
}
|
||||
expect(failing.specs).toHaveLength(6)
|
||||
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
const spent = fakeRunner()
|
||||
const { result } = await repair({ userDataPath, run: spent.run })
|
||||
expect(result).toEqual({ mode: 'marker-hit' })
|
||||
expect(spent.specs).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('stops retrying immediately once a repair has succeeded', async () => {
|
||||
const userDataPath = userDataDir()
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
await repair({ userDataPath, run: fakeRunner(() => ({ code: 5 })).run })
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
expect((await repair({ userDataPath })).result).toEqual({ mode: 'repaired' })
|
||||
|
||||
resetWindowsInstallDirAclRepairForTest()
|
||||
const after = fakeRunner()
|
||||
expect((await repair({ userDataPath, run: after.run })).result).toEqual({ mode: 'marker-hit' })
|
||||
expect(after.specs).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('is a no-op off win32 and in serve mode', async () => {
|
||||
const off = fakeRunner()
|
||||
repairWindowsInstallDirPackageAcl({
|
||||
|
||||
@@ -81,8 +81,17 @@ type RepairMarker = {
|
||||
appVersion: string
|
||||
attemptedAt: number
|
||||
outcome: string
|
||||
/** Absent on schemeVersion-1 markers written before the retry budget existed. */
|
||||
attempts?: number
|
||||
}
|
||||
|
||||
// Why bounded rather than one-and-done: the failure modes are not all permanent.
|
||||
// A Defender-locked file, a timeout or a contended volume fails one launch and
|
||||
// succeeds the next, and pinning on the first failure leaves the machine blank
|
||||
// forever for that version. Three is enough to stop a standard-user Program Files
|
||||
// install — which can never win — from re-spawning icacls on every launch.
|
||||
const MAX_REPAIR_ATTEMPTS = 3
|
||||
|
||||
/**
|
||||
* The probe's verdict is the only trigger: an orphan package ACE with no
|
||||
* well-known package grant to satisfy it. A localized icacls prints those grants
|
||||
@@ -106,31 +115,43 @@ function markerPath(userDataPath: string): string {
|
||||
return join(userDataPath, WINDOWS_INSTALL_DIR_ACL_REPAIR_MARKER_FILE)
|
||||
}
|
||||
|
||||
function hasMarkerFor(args: WindowsInstallDirAclRepairArgs): boolean {
|
||||
/** The marker for this exact install and version, or null. */
|
||||
function readMarkerFor(args: WindowsInstallDirAclRepairArgs): Partial<RepairMarker> | null {
|
||||
try {
|
||||
const parsed = JSON.parse(readFileSync(markerPath(args.userDataPath), 'utf-8')) as
|
||||
| Partial<RepairMarker>
|
||||
| undefined
|
||||
return (
|
||||
parsed?.schemeVersion === WINDOWS_INSTALL_DIR_ACL_REPAIR_SCHEME_VERSION &&
|
||||
parsed.installDir === args.installDir &&
|
||||
parsed.appVersion === args.appVersion
|
||||
)
|
||||
if (
|
||||
parsed?.schemeVersion !== WINDOWS_INSTALL_DIR_ACL_REPAIR_SCHEME_VERSION ||
|
||||
parsed.installDir !== args.installDir ||
|
||||
parsed.appVersion !== args.appVersion
|
||||
) {
|
||||
return null
|
||||
}
|
||||
return parsed
|
||||
} catch {
|
||||
return false // missing or corrupt -> attempt again
|
||||
return null // missing or corrupt -> attempt again
|
||||
}
|
||||
}
|
||||
|
||||
// Why write it on failure too: a standard-user Program Files install can never
|
||||
// win, and re-spawning icacls on every launch forever buys nothing. Reinstall or
|
||||
// update changes the key and retries.
|
||||
function hasMarkerFor(args: WindowsInstallDirAclRepairArgs): boolean {
|
||||
const marker = readMarkerFor(args)
|
||||
if (!marker) {
|
||||
return false
|
||||
}
|
||||
return marker.outcome === 'repaired' || (marker.attempts ?? 0) >= MAX_REPAIR_ATTEMPTS
|
||||
}
|
||||
|
||||
// Why write it on failure too: re-spawning icacls on every launch forever buys
|
||||
// nothing, so failures spend the retry budget. Reinstall or update changes the key.
|
||||
function writeMarker(args: WindowsInstallDirAclRepairArgs, outcome: string): void {
|
||||
const marker: RepairMarker = {
|
||||
schemeVersion: WINDOWS_INSTALL_DIR_ACL_REPAIR_SCHEME_VERSION,
|
||||
installDir: args.installDir ?? '',
|
||||
appVersion: args.appVersion,
|
||||
attemptedAt: Date.now(),
|
||||
outcome
|
||||
outcome,
|
||||
attempts: (readMarkerFor(args)?.attempts ?? 0) + 1
|
||||
}
|
||||
if (!existsSync(args.userDataPath)) {
|
||||
mkdirSync(args.userDataPath, { recursive: true })
|
||||
|
||||
Reference in New Issue
Block a user