mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 00:02:37 +00:00
fix(startup): read the install-dir package ACL as SDDL so a repaired folder reads clean on Chinese/Japanese/Korean Windows (#22490)
* fix(startup): read the install-dir package ACL as SDDL so a repaired tree reads clean on every Windows language The Windows install-dir permission probe parsed icacls's display output and recognized the well-known package grants by their English names. On Chinese/Japanese/Korean Windows icacls keeps 'NT AUTHORITY' in English but translates the package names, so a tree the repair had just fixed read as poisoned AND reliable: the pre-window repair re-armed every launch, the dialog blamed the install, and the safe-graphics fallback was withheld. Read the DACL with 'icacls <target> /save <tmpfile>' instead, which writes SDDL (SIDs, AC alias) on every locale, and drop the English-name heuristic and its wellKnownNameCheckReliable flag. * fix(startup): reject incomplete saved Windows permission records * fix(windows): reject ACL exports from unsuccessful probes --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain>
This commit is contained in:
OrcaWin
m4air
@@ -84,7 +84,7 @@ function reportProbePoisoned(): { finishRepair: () => Promise<void> } {
|
||||
release = resolve
|
||||
})
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
{ status: 'ok', matchesPoisonSignature: true, wellKnownNameCheckReliable: true },
|
||||
{ status: 'ok', matchesPoisonSignature: true },
|
||||
{
|
||||
...recoveryOptions(),
|
||||
runProcessFn: (async () => {
|
||||
@@ -115,7 +115,7 @@ async function reportProbePoisonedWithSettledRepair(
|
||||
userDataPath?: string
|
||||
): Promise<void> {
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
{ status: 'ok', matchesPoisonSignature: true, wellKnownNameCheckReliable: true },
|
||||
{ status: 'ok', matchesPoisonSignature: true },
|
||||
{
|
||||
...recoveryOptions(userDataPath),
|
||||
runProcessFn: (async () => ({
|
||||
@@ -268,7 +268,7 @@ describe('handleGpuChildCrash vs the install-dir ACL verdict', () => {
|
||||
|
||||
// The reading lands poisoned: the claim was false, and engagement stays withheld.
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
{ status: 'ok', matchesPoisonSignature: true, wellKnownNameCheckReliable: true },
|
||||
{ status: 'ok', matchesPoisonSignature: true },
|
||||
recoveryOptions(userData.path)
|
||||
)
|
||||
await decisive
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
import { existsSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import type { CrashReportBreadcrumbData } from '../../shared/crash-reporting'
|
||||
import { describe, expect, it, beforeEach, vi } from 'vitest'
|
||||
import {
|
||||
probeWindowsInstallDirAcl,
|
||||
@@ -6,13 +9,10 @@ import {
|
||||
WINDOWS_INSTALL_DIR_ACL_BREADCRUMB,
|
||||
type WindowsInstallDirAclProbeOptions
|
||||
} from './windows-install-dir-acl-probe'
|
||||
import { isInstallDirAclPoisonVerdict } from './windows-install-dir-package-acl-repair'
|
||||
import {
|
||||
ALL_PACKAGES_ACE,
|
||||
ENGLISH_BASELINE_ACES,
|
||||
fakeIcaclsSpawn,
|
||||
FRENCH_BASELINE_ACES,
|
||||
FRENCH_RESTRICTED_PACKAGES_ACE,
|
||||
icaclsDacl,
|
||||
ORPHAN_PACKAGE_ACE,
|
||||
RESTRICTED_PACKAGES_ACE
|
||||
} from './windows-install-dir-acl.test-fixture'
|
||||
@@ -21,10 +21,6 @@ const INSTALL_DIR = 'C:\\Users\\neil\\AppData\\Local\\Programs\\orca'
|
||||
const ORPHAN = ORPHAN_PACKAGE_ACE
|
||||
const RESTRICTED_GRANT = RESTRICTED_PACKAGES_ACE
|
||||
|
||||
function dacl(target: string, firstAce: string, ...rest: string[]): string {
|
||||
return icaclsDacl(target, [firstAce, ...rest])
|
||||
}
|
||||
|
||||
const fakeSpawn = fakeIcaclsSpawn
|
||||
|
||||
function probe(options: WindowsInstallDirAclProbeOptions): Promise<Record<string, unknown>> {
|
||||
@@ -44,7 +40,7 @@ function probe(options: WindowsInstallDirAclProbeOptions): Promise<Record<string
|
||||
|
||||
/** Every target returns the same ACE set on top of the system baseline. */
|
||||
function probeWith(...aces: string[]): Promise<Record<string, unknown>> {
|
||||
return probe({ spawnFn: fakeSpawn((target) => dacl(target, aces[0], ...aces.slice(1))).spawnFn })
|
||||
return probe({ spawnFn: fakeSpawn(() => aces).spawnFn })
|
||||
}
|
||||
|
||||
describe('probeWindowsInstallDirAcl', () => {
|
||||
@@ -54,7 +50,7 @@ describe('probeWindowsInstallDirAcl', () => {
|
||||
|
||||
it('reports a clean DACL as unpoisoned', async () => {
|
||||
const data = await probe({
|
||||
spawnFn: fakeSpawn((target) => icaclsDacl(target, [], ENGLISH_BASELINE_ACES)).spawnFn
|
||||
spawnFn: fakeSpawn(() => []).spawnFn
|
||||
})
|
||||
expect(data.name).toBe(WINDOWS_INSTALL_DIR_ACL_BREADCRUMB)
|
||||
expect(data.status).toBe('ok')
|
||||
@@ -82,8 +78,8 @@ describe('probeWindowsInstallDirAcl', () => {
|
||||
// orphan alongside it launched clean on win32 10.0.26200 / Electron 43.4.1 — so
|
||||
// it is not the reproduced state, however useless -1 is to an LPAC token.
|
||||
it.each([
|
||||
['the localized-safe name form', ALL_PACKAGES_ACE],
|
||||
['the raw SID form', 'S-1-15-2-1:(OI)(CI)(RX)']
|
||||
['the AC alias', ALL_PACKAGES_ACE],
|
||||
['the raw SID form', '(A;OICI;0x1200a9;;;S-1-15-2-1)']
|
||||
])('clears the signature when only ALL APPLICATION PACKAGES grants (%s)', async (_l, ace) => {
|
||||
const data = await probeWith(ace, ORPHAN)
|
||||
expect(data.hasWellKnownPackageGrant).toBe(true)
|
||||
@@ -94,10 +90,10 @@ describe('probeWindowsInstallDirAcl', () => {
|
||||
// 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)']
|
||||
['deny', '(D;OICI;FA;;;AC)'],
|
||||
['inherit-only', '(A;OICIIO;GRGX;;;AC)'],
|
||||
['restricted deny', '(D;;FA;;;S-1-15-2-2)'],
|
||||
['restricted inherit-only', '(A;CIOIIO;GRGX;;;S-1-15-2-2)']
|
||||
])('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)
|
||||
@@ -108,67 +104,131 @@ describe('probeWindowsInstallDirAcl', () => {
|
||||
it('does not let a grant on one target mask its absence on another', async () => {
|
||||
const data = await probe({
|
||||
spawnFn: fakeSpawn((target) =>
|
||||
target.endsWith('ffmpeg.dll')
|
||||
? dacl(target, ORPHAN)
|
||||
: dacl(target, RESTRICTED_GRANT, ORPHAN)
|
||||
target.endsWith('ffmpeg.dll') ? [ORPHAN] : [RESTRICTED_GRANT, ORPHAN]
|
||||
).spawnFn
|
||||
})
|
||||
expect(data.hasWellKnownPackageGrant).toBe(true)
|
||||
expect(data.matchesPoisonSignature).toBe(true)
|
||||
})
|
||||
|
||||
it('reports whether the well-known name check could be trusted', async () => {
|
||||
const english = await probeWith(ORPHAN)
|
||||
expect(english.wellKnownNameCheckReliable).toBe(true)
|
||||
const localized = await new Promise<Record<string, unknown>>((resolve) => {
|
||||
resetWindowsInstallDirAclProbeForTest()
|
||||
probeWindowsInstallDirAcl({
|
||||
platform: 'win32',
|
||||
installDir: INSTALL_DIR,
|
||||
fileExists: () => false,
|
||||
// fr-FR install that already carries the restricted grant: the name check
|
||||
// cannot see it, so the signature is a false positive the flag must expose.
|
||||
spawnFn: fakeSpawn((target) =>
|
||||
icaclsDacl(target, [ORPHAN, FRENCH_RESTRICTED_PACKAGES_ACE], FRENCH_BASELINE_ACES)
|
||||
).spawnFn,
|
||||
recordBreadcrumb: (_name, d) => {
|
||||
resolve(d as Record<string, unknown>)
|
||||
return undefined
|
||||
}
|
||||
})
|
||||
// zh-CN/ja-JP/ko-KR icacls keeps "NT AUTHORITY" English but translates the package
|
||||
// names and summary. A tree the repair just fixed read as poisoned AND reliable there,
|
||||
// re-arming the pre-window repair and blaming the install on every launch.
|
||||
it('reads a repaired tree as clean when icacls translates package names', async () => {
|
||||
const zhDisplay = (target: string): string =>
|
||||
[
|
||||
`${target} S-1-15-2-999-999-999:(OI)(CI)(RX)`,
|
||||
' APPLICATION PACKAGE AUTHORITY\\所有受限制的应用程序包:(OI)(CI)(RX)',
|
||||
' NT AUTHORITY\\SYSTEM:(I)(OI)(CI)(F)',
|
||||
' BUILTIN\\Administrators:(I)(OI)(CI)(F)',
|
||||
'',
|
||||
'已成功处理 1 个文件; 处理 0 个文件时失败'
|
||||
].join('\r\n')
|
||||
let verdict: CrashReportBreadcrumbData = {}
|
||||
const data = await probe({
|
||||
spawnFn: fakeSpawn(() => [ORPHAN, RESTRICTED_GRANT], zhDisplay).spawnFn,
|
||||
onDone: (done) => (verdict = done)
|
||||
})
|
||||
expect(localized.matchesPoisonSignature).toBe(true)
|
||||
expect(localized.wellKnownNameCheckReliable).toBe(false)
|
||||
expect(isInstallDirAclPoisonVerdict(verdict)).toBe(false)
|
||||
expect(data.status).toBe('ok')
|
||||
expect(data.orphanPackageSidCount).toBe(1)
|
||||
expect(data.hasRestrictedPackageGrant).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)')
|
||||
const data = await probeWith('(A;OICI;0x1200a9;;;S-1-15-2-1234567890)')
|
||||
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)')
|
||||
const data = await probeWith('(A;;0x100020;;;S-1-15-3-65536-599108337-2355189375-1353122160)')
|
||||
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))
|
||||
const fake = fakeSpawn(() => [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))
|
||||
it('only saves the DACL to a temp file: no recursive or ACL-writing flag', async () => {
|
||||
const fake = fakeSpawn(() => [ORPHAN])
|
||||
await probe({ spawnFn: fake.spawnFn })
|
||||
for (const call of fake.calls) {
|
||||
expect(call.args).toHaveLength(1)
|
||||
expect(call.args).toHaveLength(3)
|
||||
expect(call.args[0]).not.toMatch(/^\//)
|
||||
expect(call.args[1]).toBe('/save')
|
||||
expect(call.args[2].startsWith(tmpdir())).toBe(true)
|
||||
expect(existsSync(call.args[2])).toBe(false)
|
||||
}
|
||||
})
|
||||
|
||||
// Verbatim `icacls <dir> /save` / `icacls <file> /save` output from win32 10.0.26200.
|
||||
it.each([
|
||||
[
|
||||
'a repaired module file',
|
||||
'ffmpeg.dll\r\nD:AI(A;;0x1200a9;;;S-1-15-2-2)(A;;0x1200a9;;;S-1-15-2-999-999-999)' +
|
||||
'(A;ID;0x1200a9;;;S-1-15-2-999-999-999)(A;ID;FA;;;SY)(A;ID;FA;;;BA)' +
|
||||
'(A;ID;FA;;;S-1-5-21-432636774-4279371817-3971399515-1001)\r\n',
|
||||
false
|
||||
],
|
||||
[
|
||||
'a poisoned dir whose AC ACE denies',
|
||||
'scan21acl\r\nD:AI(D;;0x100116;;;AC)(A;OICI;0x1200a9;;;S-1-15-2-999-999-999)' +
|
||||
'(A;OICIID;FA;;;SY)(A;OICIID;FA;;;BA)' +
|
||||
'(A;OICIID;FA;;;S-1-5-21-432636774-4279371817-3971399515-1001)\r\n',
|
||||
true
|
||||
]
|
||||
])('reads a real icacls /save capture: %s', async (_label, saved, poisoned) => {
|
||||
const data = await probe({
|
||||
fileExists: () => false,
|
||||
spawnFn: fakeSpawn(() => Buffer.from(saved, 'utf16le')).spawnFn
|
||||
})
|
||||
expect(data.orphanPackageSids).toBe('S-1-15-2-999-999-999')
|
||||
expect(data.matchesPoisonSignature).toBe(poisoned)
|
||||
})
|
||||
|
||||
it('keeps a conditional DACL unreadable instead of hiding a later package grant', async () => {
|
||||
const saved =
|
||||
'orca\r\nD:AI(A;OICI;0x1200a9;;;S-1-15-2-999-999-999)' +
|
||||
'(XA;OICI;FA;;;WD;(@User.Department == "Finance"))' +
|
||||
'(A;OICIID;0x1200a9;;;S-1-15-2-2)\r\n'
|
||||
let verdict: CrashReportBreadcrumbData = {}
|
||||
const data = await probe({
|
||||
fileExists: () => false,
|
||||
spawnFn: fakeSpawn(() => Buffer.from(saved, 'utf16le')).spawnFn,
|
||||
onDone: (done) => (verdict = done)
|
||||
})
|
||||
expect(data).toMatchObject({ status: 'failed', reason: 'all-targets-unreadable' })
|
||||
expect(data.matchesPoisonSignature).toBeUndefined()
|
||||
expect(isInstallDirAclPoisonVerdict(verdict)).toBe(false)
|
||||
})
|
||||
|
||||
it.each([1, null])('ignores a saved DACL when icacls exits with %s', async (exitCode) => {
|
||||
const fake = fakeSpawn(() => [ORPHAN], undefined, exitCode)
|
||||
let verdict: CrashReportBreadcrumbData = {}
|
||||
const data = await probe({
|
||||
fileExists: () => false,
|
||||
spawnFn: fake.spawnFn,
|
||||
onDone: (done) => (verdict = done)
|
||||
})
|
||||
expect(data).toMatchObject({ status: 'failed', reason: 'all-targets-unreadable' })
|
||||
expect(isInstallDirAclPoisonVerdict(verdict)).toBe(false)
|
||||
expect(fake.calls.every((call) => !existsSync(call.args[2]))).toBe(true)
|
||||
})
|
||||
|
||||
it('does not report malformed saved output as a clean DACL', async () => {
|
||||
const data = await probe({
|
||||
fileExists: () => false,
|
||||
spawnFn: fakeSpawn(() => Buffer.from('orca\r\n', 'utf16le')).spawnFn
|
||||
})
|
||||
expect(data).toMatchObject({ status: 'failed', reason: 'all-targets-unreadable' })
|
||||
expect(data.matchesPoisonSignature).toBeUndefined()
|
||||
})
|
||||
|
||||
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')
|
||||
@@ -180,7 +240,7 @@ describe('probeWindowsInstallDirAcl', () => {
|
||||
['darwin', { platform: 'darwin' as NodeJS.Platform }],
|
||||
['serve mode', { platform: 'win32' as NodeJS.Platform, isServeMode: true }]
|
||||
])('does no work on %s', async (_label, options) => {
|
||||
const fake = fakeSpawn((target) => dacl(target, ORPHAN))
|
||||
const fake = fakeSpawn(() => [ORPHAN])
|
||||
const record = vi.fn()
|
||||
const fileExists = vi.fn(() => true)
|
||||
probeWindowsInstallDirAcl({
|
||||
@@ -197,7 +257,7 @@ describe('probeWindowsInstallDirAcl', () => {
|
||||
})
|
||||
|
||||
it('runs once per process', async () => {
|
||||
const fake = fakeSpawn((target) => dacl(target, ORPHAN))
|
||||
const fake = fakeSpawn(() => [ORPHAN])
|
||||
await probe({ spawnFn: fake.spawnFn })
|
||||
const before = fake.calls.length
|
||||
probeWindowsInstallDirAcl({ platform: 'win32', installDir: INSTALL_DIR, spawnFn: fake.spawnFn })
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { spawn } from 'node:child_process'
|
||||
import { existsSync } from 'node:fs'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { existsSync, readFileSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { dirname, join } from 'node:path'
|
||||
import {
|
||||
sanitizeCrashReportString,
|
||||
@@ -36,23 +38,16 @@ const MODULE_SHORTLIST = ['ffmpeg.dll', 'libGLESv2.dll', 'libEGL.dll', 'icudtl.d
|
||||
|
||||
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
|
||||
// Why SDDL (`icacls /save`) rather than icacls's display: the display localizes
|
||||
// the well-known package names — and on zh/ja/ko even keeps "NT AUTHORITY" English
|
||||
// while doing so — so a repaired tree read as poisoned. SDDL prints SIDs on every locale.
|
||||
const PACKAGE_SID = /^S-1-15-2-[0-9-]+$/i
|
||||
/** ALL RESTRICTED APPLICATION PACKAGES: the grant the reproduced remedy added. */
|
||||
const RESTRICTED_PACKAGES_SID = 's-1-15-2-2'
|
||||
const WELL_KNOWN_PACKAGE_SIDS = new Set(['s-1-15-2-1', RESTRICTED_PACKAGES_SID])
|
||||
// icacls localizes these; the raw SID form is never printed for them. Orphan
|
||||
// detection stays SID-form and locale-independent, but this check is not — so we
|
||||
// report whether the output looked English at all, making a locale-induced
|
||||
// false positive recognizable instead of silent.
|
||||
const WELL_KNOWN_PACKAGE_NAMES = /ALL (RESTRICTED )?APPLICATION PACKAGES/i
|
||||
const RESTRICTED_PACKAGE_NAME = /ALL RESTRICTED APPLICATION PACKAGES/i
|
||||
// Not BUILTIN: fr-FR and es-ES print it verbatim while localizing the package
|
||||
// names, so it is evidence of nothing. SYSTEM's ACE is on every install tree.
|
||||
const ENGLISH_PRINCIPAL = /\b(NT AUTHORITY|APPLICATION PACKAGE AUTHORITY)\b/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
|
||||
const RESTRICTED_PACKAGES_SID = 'S-1-15-2-2'
|
||||
/** `AC` is SDDL's alias for ALL APPLICATION PACKAGES (S-1-15-2-1). */
|
||||
const WELL_KNOWN_PACKAGE_SIDS = new Set(['AC', 'S-1-15-2-1', RESTRICTED_PACKAGES_SID])
|
||||
/** (type;flags;rights;object;inheritedObject;sid[;condition]) */
|
||||
const SDDL_ACE = /\(([A-Z]+);([A-Z]*);[^;()]*;[^;()]*;[^;()]*;([^;()]+)[;)]/gi
|
||||
|
||||
export type WindowsInstallDirAclProbeOptions = {
|
||||
platform?: NodeJS.Platform
|
||||
@@ -69,80 +64,80 @@ type AclFacts = {
|
||||
orphanPackageSids: string[]
|
||||
hasWellKnownPackageGrant: boolean
|
||||
hasRestrictedPackageGrant: boolean
|
||||
sawEnglishPrincipal: boolean
|
||||
}
|
||||
|
||||
function readDacl(spawnFn: typeof spawn, target: string, deadlineMs: number): Promise<string> {
|
||||
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'],
|
||||
function readSavedDacl(spawnFn: typeof spawn, target: string, deadlineMs: number): Promise<string> {
|
||||
const saveFile = join(tmpdir(), `orca-install-acl-${randomUUID()}.txt`)
|
||||
return new Promise<string>((resolve) => {
|
||||
// /save only reads the target (it writes the temp file). 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, '/save', saveFile], {
|
||||
stdio: ['ignore', 'ignore', 'ignore'],
|
||||
windowsHide: true
|
||||
})
|
||||
let out = ''
|
||||
let settled = false
|
||||
const settle = (value: string): void => {
|
||||
const settle = (read: boolean): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
clearTimeout(timer)
|
||||
resolve(value)
|
||||
let out = ''
|
||||
try {
|
||||
// An empty read parses as a clean DACL, so a failed /save must stay ''.
|
||||
out = read ? readFileSync(saveFile, 'utf16le') : ''
|
||||
} catch {
|
||||
out = ''
|
||||
}
|
||||
try {
|
||||
// A killed icacls may still hold the file; a leftover temp file is harmless.
|
||||
rmSync(saveFile, { force: true })
|
||||
} catch {
|
||||
// Nothing to do.
|
||||
}
|
||||
resolve(out)
|
||||
}
|
||||
const timer = setTimeout(() => {
|
||||
child.kill()
|
||||
settle('')
|
||||
settle(false)
|
||||
}, 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))
|
||||
child.on('error', () => settle(false))
|
||||
child.on('close', (code) => settle(code === 0))
|
||||
})
|
||||
}
|
||||
|
||||
function collectAclFacts(daclOutput: string): AclFacts {
|
||||
function collectAclFacts(savedDacl: string): AclFacts | null {
|
||||
const orphanPackageSids: string[] = []
|
||||
let hasWellKnownPackageGrant = false
|
||||
let hasRestrictedPackageGrant = false
|
||||
let sawEnglishPrincipal = false
|
||||
for (const line of daclOutput.split(/\r?\n/)) {
|
||||
if (ENGLISH_PRINCIPAL.test(line)) {
|
||||
sawEnglishPrincipal = true
|
||||
}
|
||||
// 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) {
|
||||
// A partial DACL can hide a later grant; unsupported ACEs must stay unreadable.
|
||||
const dacl = /^D:[A-Z]*((?:\([^()]*\))*)(?:S:.*)?$/im.exec(savedDacl)
|
||||
if (!dacl) {
|
||||
return null
|
||||
}
|
||||
for (const [, type, flags, rawSid] of dacl[1].matchAll(SDDL_ACE)) {
|
||||
const sid = rawSid.toUpperCase()
|
||||
if (!WELL_KNOWN_PACKAGE_SIDS.has(sid)) {
|
||||
if (PACKAGE_SID.test(sid)) {
|
||||
orphanPackageSids.push(rawSid)
|
||||
}
|
||||
continue
|
||||
}
|
||||
const [, principal, flags] = ace
|
||||
const rawSid = RAW_PACKAGE_SID.exec(principal)?.[0]
|
||||
const sid = rawSid?.toLowerCase()
|
||||
if (rawSid && !WELL_KNOWN_PACKAGE_SIDS.has(rawSid.toLowerCase())) {
|
||||
orphanPackageSids.push(rawSid)
|
||||
continue
|
||||
}
|
||||
const isWellKnown = sid !== undefined || WELL_KNOWN_PACKAGE_NAMES.test(principal)
|
||||
if (!isWellKnown || NON_GRANTING_FLAGS.test(flags)) {
|
||||
// Why: an ACE that denies, or only propagates to children (IO), grants nothing
|
||||
// on this object — so it cannot satisfy an orphan the way the reproduced fix did.
|
||||
const flagTokens: string[] = flags.toUpperCase().match(/../g) ?? []
|
||||
const inheritOnly = flagTokens.includes('IO')
|
||||
if (type.toUpperCase() !== 'A' || inheritOnly) {
|
||||
continue
|
||||
}
|
||||
hasWellKnownPackageGrant = true
|
||||
// Reported, never the verdict: narrows which grant is present for triage.
|
||||
if (sid === RESTRICTED_PACKAGES_SID || (!sid && RESTRICTED_PACKAGE_NAME.test(principal))) {
|
||||
if (sid === RESTRICTED_PACKAGES_SID) {
|
||||
hasRestrictedPackageGrant = true
|
||||
}
|
||||
}
|
||||
return {
|
||||
orphanPackageSids,
|
||||
hasWellKnownPackageGrant,
|
||||
hasRestrictedPackageGrant,
|
||||
sawEnglishPrincipal
|
||||
}
|
||||
return { orphanPackageSids, hasWellKnownPackageGrant, hasRestrictedPackageGrant }
|
||||
}
|
||||
|
||||
function resolveTargets(installDir: string, fileExists: (path: string) => boolean): string[] {
|
||||
@@ -162,9 +157,9 @@ async function runProbe(options: WindowsInstallDirAclProbeOptions): Promise<void
|
||||
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) : '')
|
||||
outputs.push(remaining > 0 ? await readSavedDacl(spawnFn, target, remaining) : '')
|
||||
}
|
||||
const facts = outputs.map(collectAclFacts)
|
||||
const facts = outputs.map(collectAclFacts).filter((fact) => fact !== null)
|
||||
const orphans = [...new Set(facts.flatMap((f) => f.orphanPackageSids))]
|
||||
const hasWellKnownPackageGrant = facts.some((f) => f.hasWellKnownPackageGrant)
|
||||
const hasRestrictedPackageGrant = facts.some((f) => f.hasRestrictedPackageGrant)
|
||||
@@ -174,24 +169,22 @@ async function runProbe(options: WindowsInstallDirAclProbeOptions): Promise<void
|
||||
const poisoned = facts.some(
|
||||
(f) => f.orphanPackageSids.length > 0 && !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),
|
||||
// The verdict rides on this one: either well-known grant satisfies the orphan.
|
||||
hasWellKnownPackageGrant,
|
||||
// Diagnostic only — the -1-only shape launches clean on real hardware.
|
||||
hasRestrictedPackageGrant,
|
||||
// False positives are possible on a non-English Windows, where the
|
||||
// well-known ACE resolves to a localized name this cannot match.
|
||||
wellKnownNameCheckReliable: facts.some((f) => f.sawEnglishPrincipal),
|
||||
matchesPoisonSignature: poisoned
|
||||
}
|
||||
data =
|
||||
facts.length === 0
|
||||
? { 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),
|
||||
// The verdict rides on this one: either well-known grant satisfies the orphan.
|
||||
hasWellKnownPackageGrant,
|
||||
// Diagnostic only — the -1-only shape launches clean on real hardware.
|
||||
hasRestrictedPackageGrant,
|
||||
matchesPoisonSignature: poisoned
|
||||
}
|
||||
} catch (error) {
|
||||
data = { status: 'failed', reason: sanitizeCrashReportString(`probe: ${String(error)}`, 200) }
|
||||
}
|
||||
|
||||
@@ -32,9 +32,6 @@ import {
|
||||
import {
|
||||
ALL_PACKAGES_ACE,
|
||||
fakeIcaclsSpawn,
|
||||
FRENCH_BASELINE_ACES,
|
||||
FRENCH_RESTRICTED_PACKAGES_ACE,
|
||||
icaclsDacl,
|
||||
ORPHAN_PACKAGE_ACE,
|
||||
RESTRICTED_PACKAGES_ACE
|
||||
} from './windows-install-dir-acl.test-fixture'
|
||||
@@ -49,7 +46,7 @@ type Runner = (spec: ProcessSpec) => Promise<ProcessResult>
|
||||
* which decides whether icacls ever runs. Only the two process seams are faked.
|
||||
*/
|
||||
function probeThenRecover(
|
||||
dacl: (target: string) => string,
|
||||
savedAces: (target: string) => string[],
|
||||
options: { failRepair?: boolean } = {}
|
||||
): Promise<ProcessSpec[]> {
|
||||
const specs: ProcessSpec[] = []
|
||||
@@ -68,7 +65,7 @@ function probeThenRecover(
|
||||
platform: 'win32',
|
||||
installDir: INSTALL_DIR,
|
||||
fileExists: (path) => path.endsWith('ffmpeg.dll'),
|
||||
spawnFn: fakeIcaclsSpawn(dacl).spawnFn,
|
||||
spawnFn: fakeIcaclsSpawn(savedAces).spawnFn,
|
||||
recordBreadcrumb: () => undefined,
|
||||
onDone: (data) => {
|
||||
startWindowsInstallDirAclRepairIfPoisoned(data, {
|
||||
@@ -95,7 +92,7 @@ describe('startWindowsInstallDirAclRepairIfPoisoned', () => {
|
||||
})
|
||||
|
||||
it('repairs when the probe sees an orphan package ACE and no well-known grant', async () => {
|
||||
const specs = await probeThenRecover((target) => icaclsDacl(target, [ORPHAN_PACKAGE_ACE]))
|
||||
const specs = await probeThenRecover(() => [ORPHAN_PACKAGE_ACE])
|
||||
expect(specs.map((spec) => spec.args?.[2])).toEqual([
|
||||
'*S-1-15-2-2:(OI)(CI)(RX)',
|
||||
'*S-1-15-2-2:(RX)'
|
||||
@@ -106,27 +103,13 @@ describe('startWindowsInstallDirAclRepairIfPoisoned', () => {
|
||||
// win32 10.0.26200 / Electron 43.4.1, so it earns neither an ACL write nor the
|
||||
// accusing dialog copy.
|
||||
it('leaves an install whose package grant is ALL APPLICATION PACKAGES alone', async () => {
|
||||
const specs = await probeThenRecover((target) =>
|
||||
icaclsDacl(target, [ORPHAN_PACKAGE_ACE, ALL_PACKAGES_ACE])
|
||||
)
|
||||
const specs = await probeThenRecover(() => [ORPHAN_PACKAGE_ACE, ALL_PACKAGES_ACE])
|
||||
expect(specs).toHaveLength(0)
|
||||
expect(describeInstallDirAclPoison()).toBeNull()
|
||||
})
|
||||
|
||||
it('does not touch an install that already carries the restricted grant', async () => {
|
||||
const specs = await probeThenRecover((target) =>
|
||||
icaclsDacl(target, [ORPHAN_PACKAGE_ACE, RESTRICTED_PACKAGES_ACE])
|
||||
)
|
||||
expect(specs).toHaveLength(0)
|
||||
expect(describeInstallDirAclPoison()).toBeNull()
|
||||
})
|
||||
|
||||
// A localized icacls prints the grant under a name the probe cannot match, so
|
||||
// the signature is unproven: neither icacls nor the accusing dialog copy.
|
||||
it('does not act on a signature from a non-English icacls', async () => {
|
||||
const specs = await probeThenRecover((target) =>
|
||||
icaclsDacl(target, [ORPHAN_PACKAGE_ACE, FRENCH_RESTRICTED_PACKAGES_ACE], FRENCH_BASELINE_ACES)
|
||||
)
|
||||
const specs = await probeThenRecover(() => [ORPHAN_PACKAGE_ACE, RESTRICTED_PACKAGES_ACE])
|
||||
expect(specs).toHaveLength(0)
|
||||
expect(describeInstallDirAclPoison()).toBeNull()
|
||||
})
|
||||
@@ -169,7 +152,7 @@ describe('describeInstallDirAclPoison', () => {
|
||||
})
|
||||
|
||||
it('offers the copyable commands, and drops them once the repair lands', async () => {
|
||||
await probeThenRecover((target) => icaclsDacl(target, [ORPHAN_PACKAGE_ACE]))
|
||||
await probeThenRecover(() => [ORPHAN_PACKAGE_ACE])
|
||||
const repaired = describeInstallDirAclPoison()
|
||||
expect(repaired?.detail).toContain('Orca repaired the permissions')
|
||||
expect(repaired?.detail).not.toContain('Administrator Command Prompt')
|
||||
@@ -180,7 +163,7 @@ describe('describeInstallDirAclPoison', () => {
|
||||
})
|
||||
|
||||
it('walks a standard user through icacls when the repair could not write', async () => {
|
||||
await probeThenRecover((target) => icaclsDacl(target, [ORPHAN_PACKAGE_ACE]), {
|
||||
await probeThenRecover(() => [ORPHAN_PACKAGE_ACE], {
|
||||
failRepair: true
|
||||
})
|
||||
const failed = describeInstallDirAclPoison()
|
||||
@@ -190,7 +173,7 @@ describe('describeInstallDirAclPoison', () => {
|
||||
|
||||
it('reports the repair as in flight before icacls has answered', () => {
|
||||
startWindowsInstallDirAclRepairIfPoisoned(
|
||||
{ status: 'ok', matchesPoisonSignature: true, wellKnownNameCheckReliable: true },
|
||||
{ status: 'ok', matchesPoisonSignature: true },
|
||||
{
|
||||
platform: 'win32',
|
||||
installDir: INSTALL_DIR,
|
||||
@@ -206,8 +189,7 @@ describe('describeInstallDirAclPoison', () => {
|
||||
|
||||
const POISON_VERDICT: CrashReportBreadcrumbData = {
|
||||
status: 'ok',
|
||||
matchesPoisonSignature: true,
|
||||
wellKnownNameCheckReliable: true
|
||||
matchesPoisonSignature: true
|
||||
}
|
||||
const GPU_ENV = { appVersion: APP_VERSION, electronVersion: '43.4.1', platform: 'win32' } as const
|
||||
|
||||
@@ -897,7 +879,7 @@ describe('the probe-pending grace window', () => {
|
||||
platform: 'win32' as const,
|
||||
installDir: INSTALL_DIR,
|
||||
fileExists: () => false,
|
||||
spawnFn: fakeIcaclsSpawn((target) => icaclsDacl(target, [RESTRICTED_PACKAGES_ACE])).spawnFn,
|
||||
spawnFn: fakeIcaclsSpawn(() => [RESTRICTED_PACKAGES_ACE]).spawnFn,
|
||||
recordBreadcrumb: () => undefined
|
||||
}
|
||||
let settleVerdict: () => void = () => undefined
|
||||
|
||||
@@ -1,64 +1,69 @@
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { writeFileSync } from 'node:fs'
|
||||
import { basename } from 'node:path'
|
||||
import type { spawn } from 'node:child_process'
|
||||
|
||||
/** Shared icacls doubles for the install-dir DACL probe, repair, and recovery tests. */
|
||||
/**
|
||||
* Shared icacls doubles for the install-dir DACL probe, repair, and recovery tests.
|
||||
* ACEs are SDDL, the shape `icacls <target> /save <file>` writes (captured on
|
||||
* win32 10.0.26200: the leaf name, CRLF, then `D:AI(...)(...)`, UTF-16LE).
|
||||
*/
|
||||
|
||||
export const ORPHAN_PACKAGE_ACE = 'S-1-15-2-999-999-999:(OI)(CI)(RX)'
|
||||
export const RESTRICTED_PACKAGES_ACE =
|
||||
'APPLICATION PACKAGE AUTHORITY\\ALL RESTRICTED APPLICATION PACKAGES:(OI)(CI)(RX)'
|
||||
/** The Program Files default: present on healthy installs, which launch clean. */
|
||||
export const ALL_PACKAGES_ACE = 'APPLICATION PACKAGE AUTHORITY\\ALL APPLICATION PACKAGES:(RX)'
|
||||
export const ORPHAN_PACKAGE_ACE = '(A;OICI;0x1200a9;;;S-1-15-2-999-999-999)'
|
||||
/** ALL RESTRICTED APPLICATION PACKAGES; SDDL has no alias for it. */
|
||||
export const RESTRICTED_PACKAGES_ACE = '(A;OICI;0x1200a9;;;S-1-15-2-2)'
|
||||
/** ALL APPLICATION PACKAGES (`AC`), the Program Files default: healthy installs launch clean. */
|
||||
export const ALL_PACKAGES_ACE = '(A;;0x1200a9;;;AC)'
|
||||
|
||||
export const ENGLISH_BASELINE_ACES = [
|
||||
'NT AUTHORITY\\SYSTEM:(I)(OI)(CI)(F)',
|
||||
'BUILTIN\\Administrators:(I)(OI)(CI)(F)',
|
||||
'awin\\neil:(I)(OI)(CI)(F)'
|
||||
export const BASELINE_ACES = [
|
||||
'(A;OICIID;FA;;;SY)',
|
||||
'(A;OICIID;FA;;;BA)',
|
||||
'(A;OICIID;FA;;;S-1-5-21-432636774-4279371817-3971399515-1001)'
|
||||
]
|
||||
/** fr-FR icacls: no principal the English name check can recognize. */
|
||||
export const FRENCH_BASELINE_ACES = [
|
||||
'AUTORITE NT\\Systeme:(I)(OI)(CI)(F)',
|
||||
'BUILTIN\\Administrateurs:(I)(OI)(CI)(F)'
|
||||
]
|
||||
export const FRENCH_RESTRICTED_PACKAGES_ACE =
|
||||
"AUTORITE DE PACKAGE D'APPLICATION\\TOUS LES PACKAGES D'APPLICATION RESTREINTS:(RX)"
|
||||
|
||||
/** Real icacls shape: the echoed path is glued onto the first principal. */
|
||||
export function icaclsDacl(
|
||||
target: string,
|
||||
aces: string[],
|
||||
baseline: string[] = ENGLISH_BASELINE_ACES
|
||||
): string {
|
||||
const [first, ...rest] = [...aces, ...baseline]
|
||||
return [
|
||||
`${target} ${first}`,
|
||||
...rest.map((ace) => ` ${ace}`),
|
||||
'',
|
||||
'Successfully processed 1 files'
|
||||
].join('\r\n')
|
||||
/** The file `icacls <target> /save` writes for a target carrying these ACEs. */
|
||||
export function icaclsSavedAcl(target: string, aces: string[]): Buffer {
|
||||
const leaf = basename(target.replaceAll('\\', '/'))
|
||||
return Buffer.from(`${leaf}\r\nD:AI${[...aces, ...BASELINE_ACES].join('')}\r\n`, 'utf16le')
|
||||
}
|
||||
|
||||
/** `null` output makes the spawn fail, as an unreadable target does. */
|
||||
export function fakeIcaclsSpawn(output: (target: string) => string | null): {
|
||||
/**
|
||||
* `saved` returns the target's extra ACEs, or the raw saved file; `null` makes the
|
||||
* spawn fail, as an unreadable target does. `display` is what a bare
|
||||
* `icacls <target>` prints.
|
||||
*/
|
||||
export function fakeIcaclsSpawn(
|
||||
saved: (target: string) => string[] | Buffer | null,
|
||||
display: (target: string) => string = () => '',
|
||||
exitCode: number | null = 0
|
||||
): {
|
||||
spawnFn: typeof spawn
|
||||
calls: { file: string; args: string[] }[]
|
||||
} {
|
||||
const calls: { file: string; args: string[] }[] = []
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test double; the probe only touches kill/stdout/on of the child.
|
||||
const spawnFn = ((file: string, args: string[]) => {
|
||||
calls.push({ file, args })
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: both fields are assigned on the next two lines.
|
||||
const child = new EventEmitter() as EventEmitter & {
|
||||
stdout: EventEmitter
|
||||
kill: () => void
|
||||
}
|
||||
child.stdout = new EventEmitter()
|
||||
child.kill = () => undefined
|
||||
const out = output(args[0])
|
||||
const [target, verb, saveFile] = args
|
||||
const aces = saved(target)
|
||||
setImmediate(() => {
|
||||
if (out === null) {
|
||||
if (aces === null) {
|
||||
child.emit('error', new Error('ENOENT'))
|
||||
return
|
||||
}
|
||||
child.stdout.emit('data', Buffer.from(out, 'utf-8'))
|
||||
child.emit('close', 0)
|
||||
if (verb === '/save') {
|
||||
writeFileSync(saveFile, Buffer.isBuffer(aces) ? aces : icaclsSavedAcl(target, aces))
|
||||
} else {
|
||||
child.stdout.emit('data', Buffer.from(display(target), 'utf-8'))
|
||||
}
|
||||
child.emit('close', exitCode)
|
||||
})
|
||||
return child
|
||||
}) as unknown as typeof spawn
|
||||
|
||||
@@ -312,8 +312,7 @@ describe('isInstallDirAclPoisonVerdict', () => {
|
||||
expect(
|
||||
isInstallDirAclPoisonVerdict({
|
||||
status: 'ok',
|
||||
matchesPoisonSignature: true,
|
||||
wellKnownNameCheckReliable: true
|
||||
matchesPoisonSignature: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(
|
||||
@@ -324,16 +323,4 @@ describe('isInstallDirAclPoisonVerdict', () => {
|
||||
).toBe(false)
|
||||
expect(isInstallDirAclPoisonVerdict({ status: 'failed', reason: 'unreadable' })).toBe(false)
|
||||
})
|
||||
|
||||
// A localized icacls hides the well-known grants behind translated names, so the
|
||||
// signature there is unproven: repairing and blaming the install would be wrong.
|
||||
it('refuses a signature the probe could not name-check', () => {
|
||||
expect(
|
||||
isInstallDirAclPoisonVerdict({
|
||||
status: 'ok',
|
||||
matchesPoisonSignature: true,
|
||||
wellKnownNameCheckReliable: false
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -103,13 +103,11 @@ 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
|
||||
* under translated names the probe cannot match, so an unreliable name check is
|
||||
* not evidence of poison — acting on it would spawn icacls and tell a user with a
|
||||
* healthy install that their permissions are broken.
|
||||
* well-known package grant to satisfy it. The probe reads SDDL, so the verdict
|
||||
* holds on every Windows display language.
|
||||
*/
|
||||
export function isInstallDirAclPoisonVerdict(data: CrashReportBreadcrumbData): boolean {
|
||||
return data.matchesPoisonSignature === true && data.wellKnownNameCheckReliable !== false
|
||||
return data.matchesPoisonSignature === true
|
||||
}
|
||||
|
||||
/** The commands to hand a user whose account cannot write the install ACL. */
|
||||
|
||||
Reference in New Issue
Block a user