mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(security): apply the Windows path-hardening ACL that never ran (#17884)
* fix(security): apply the Windows path-hardening ACL that never ran `buildWindowsRestrictAclArgs` invoked the hardening script as `powershell.exe -Command <script> <path> <sid> <isDir>`. `-Command` does not populate `$args`; it appends the trailing tokens to the command text. The script therefore read `$args[1]` as `$null`, threw `NullArrayIndex` at `$allowedSids[$sidText] = $true` under `$ErrorActionPreference = 'Stop'`, and exited 1. Both callers swallowed that: the async callback was empty and `applySecurePathRestriction` returned `true` regardless, while the sync `catch` returned `false` and nobody logged. Every Windows secure path has been left on its inherited ACL since the ACL was introduced (#5006), and nothing said so. Replace PowerShell with `icacls.exe`, which takes plain argv. That removes the quoting surface entirely rather than escaping it: interpolating a path into the command text would have turned a dead no-op into arbitrary PowerShell on a filesystem path, since `-Command` executes what it appends. It also drops the execution-policy dependency and the `powershell.exe` spawn an EDR flags, and runs ~25x faster than the PowerShell cold start. Hardening is now three passes: `/reset` to purge explicit ACEs that `/inheritance:r` leaves behind, `/inheritance:r` plus a `/grant:r` per allowed SID, then a read-back that checks the DACL is protected and grants only the intended rights. The predecessor's verification block was equally dead, and an apply that is never read back is only half a control. Failures stay non-fatal — non-NTFS volumes, network paths and restricted tokens fail legitimately and must not break startup — but they are no longer invisible: every failure is logged, and a failed async apply now evicts its cache entry so the next call retries instead of trusting a success that never happened. Routing through `runProcess`/`runProcessSync` also retires this file's `node:child_process` allowlist entry. * fix(security): verify the hardened ACL by identity, not by shape Review found the bug class this PR fixes surviving inside the fix. The verify pass checked rule count, absence of the inherited marker, and exact rights — never *who* the rules named. Granting Everyone full control satisfies all three, so hardening reported success on a DACL that handed the credential to every local account, and most of the real-filesystem tests still passed. Verification now reads the descriptor back with `icacls /save`, which emits SDDL with raw SIDs, and compares the principal set exactly. That is also locale-independent by construction: the previous parse read localized account names out of icacls' OEM-codepage stdout, where a non-ASCII path survived by accident rather than by the documented mechanism. SDDL parsing moves to `windows-security-descriptor.ts`. Two further self-inflicted problems, both measured: The post-rename re-harden led with `/reset`, which re-widened a DACL that was already correct — the staged file's protected DACL survives the rename, so the pass had nothing to do but open a window. Polling an external process during a write into a relocated root caught it: the e2ee keypair dropped to `BUILTIN\Users:(RX)` plus `Authenticated Users:(M)` — read *and* write — before tightening again. Hardening now verifies first and returns early when the DACL already reads back correct, which closes the window and cuts the steady state from three spawns to one. Re-measured: 158 samples, one DACL state, zero broad. Evicting the cache on every failed async apply reintroduced #4901. The env store re-hardens on the read path at ~2/s, so on a host where hardening cannot work (FAT32, network path, restricted token) that was two icacls spawns and two warnings a second, forever. Async retries now take a retry floor and a hard per-path attempt cap. The write path keeps retrying unthrottled — it is user-driven, and a failed credential ACL must still be retried on the next write. Also: failures route through a reporter hook that the main process points at the diagnostic tracer, because `console.warn` reaches nothing in a packaged GUI-subsystem build; `writeSecureFile` returns whether hardening took, and the async branch reports `pending` rather than claiming `applied`; a transient `whoami` failure no longer disables hardening for the process lifetime, and the SID is shape-validated; the `/c` guard now covers the synchronous runner too. * fix(security): re-probe hardening instead of latching a transient failure The per-process attempt cap added for the read-path storm was a permanent latch: one AV scan, momentary lock or %TEMP% blip and every later credential write in that session went unhardened, silently, on a host where hardening would now succeed. Same defect class as #17858's computer-use host, and worse here because what stops happening is security hardening on credential files and nothing said so. The retry budget now bounds the *rate*, not the lifetime: at most three attempts per path per minute, re-probing in every later window, forever. The transition is announced in both directions — `throttled` once per window on entry, `recovered` when a rate-limited path hardens again — so a host stuck in the degraded state is diagnosable rather than merely quiet. The reporter type covers both, and the main process ends the `recovered` span successfully rather than failing it. Extracted to secure-path-hardening-retry-budget.ts, which keeps secure-file.ts under its line cap without a max-lines disable. Also confirms the second flagged risk rather than assuming it: a real unwritable %TEMP% is now covered by a test proving verification fails closed, reports at the `verify` stage, and still leaves the ACL applied — so that path loses proof, not protection, and with the lifetime cap gone it can no longer combine into a permanent-off state. * fix(security): verify a directory's whole inheritance flag set The flag check tested only that `OI` was present — never that `CI` was, nor that nothing else was. That was harmless while `/reset` + `/grant` ran on every pass and repaired whatever was there. The verify-first short-circuit made it load-bearing: what verification accepts is now left alone, so a latent under-check went live because a different fix started depending on it. Two directory DACLs passed while being wrong — both protected, three non-inherited full-control rules, correct SIDs, differing from correct only in their flags: (OI)(F) - no CI, so subdirectories are left unprotected (OI)(CI)(IO) - inherit-only, so the directory object itself grants nobody anything; the next writeFileSync into it fails with EPERM, on a directory just cached as hardened Verification now compares the whole flag set, which also rejects IO and NP, and names the offending flags in the failure. Both shapes are planted in real-filesystem regression tests, including an assertion that a write into the repaired directory succeeds and its child inherits. Confirmed both tests fail against the old check and pass against this one. * fix(security): back the hardening retry off exponentially The fixed one-minute window bounded the retry rate but left a standing floor of three attempts per path per minute on a host where hardening can never succeed — FAT32/exFAT, a network path, a redirected profile. That budget is per path and there are several secure files, so the floor multiplied into tens of thousands of icacls spawns a day for work guaranteed to fail. The delay now doubles after each consecutive failure, from a one-minute floor to a thirty-minute ceiling, and the attempt cap is gone entirely: once the backoff elapses the path is re-probed however long it has been failing. A permanently incapable host settles at ~2 attempts/hour. Slowing the backstop costs almost nothing, because it is not the recovery mechanism: the synchronous write path is deliberately unthrottled, so a host that recovers hardens on its very next credential write regardless of what the read-path budget says. The `throttled`/`recovered` reports are unchanged and matter more here, since the quiet periods between probes are now much longer. The curve is pinned in a new unit test against the exported delay function rather than a copy of its constants, covering the doubling, the ceiling holding at 5000 consecutive failures, a 30-day failing path still re-probing, one announcement per degraded episode, and per-path isolation. The integration tests keep only what they uniquely prove: that the read path is wired to the budget, and that a day of failures still re-probes. Confirmed four of these fail against a reinstated lifetime cap. * ci(windows): run the real-icacls DACL suite in CI The win32 suite only self-skips off Windows, so it passed vacuously in every lane. Register it the way the cmd-shim suite is registered. * fix(security): describe the cache's real cost, which is icacls now Both cache comments still justified themselves with PowerShell -- "~1-1.5s" and "a PowerShell spawn every read" -- in the same file whose PR removed PowerShell from this path. The caches are still right, but for different numbers, and the old ones are the kind an engineer would reasonably delete a cache over. The real shape: hardening verifies first and returns early, so an already-correct DACL costs one synchronous icacls spawn and a rewrite costs four (verify, reset, grant, verify). Still worth caching on the read path, which polls at ~2/s. * test(security): make the DACL suite safe to schedule Registering this spec in the Windows lane put it under two rules it had never been measured against. Teardown now goes through `removeTreeSync`, which the lane's boundary test requires, and repairs the DACLs the suite plants on purpose first: those retries only cover transient locks, so a regressed `(OI)(CI)(IO)` repair leaves the root un-removable and `afterAll` throws EPERM. And the no-permission case decides by elevation before it writes anything. `windows-2022` runs elevated, where hardening succeeds: the old branch asserted nothing about denial and instead replaced the `hosts` DACL, then `icacls /reset` -- which is not a restore, it drops the explicit `SYSTEM:(F)` that file ships with. Ephemeral in CI; permanent for a developer running the lane from an elevated shell. Now it asserts or it skips. The probe reads the token integrity SID rather than `icacls /save`, which succeeds unelevated (`BUILTIN\Users:(RX)` carries READ_CONTROL) and would have skipped the case on every machine. * fix(security): measure the hardening latches on a clock that cannot go backwards `mayAttemptHardening` compared wall-clock times, so any backwards step -- an NTP correction, a VM snapshot restore, a user changing the clock -- made the elapsed time negative and held every failing path below its delay until the clock caught up. Measured at the 30-minute ceiling with the clock stepped back a year, the path was refused at +0d, +1d, +30d, +180d and +364d, and re-probed only at +366d. That is the permanent latch the exponential backoff was added to remove, and it contradicts the module's own "bounds the rate without ever bounding the lifetime". The SID lookup's own one-minute window had the identical shape and is worse: a failed lookup makes `planFor` return null, which disables the synchronous *write* path too, so the write-path exemption that recovers the read-path budget cannot recover it. Both now measure elapsed monotonic time, following the repo's existing `monotonicNowMs` spelling. Two things the write path was not doing, both found in the same pass: - A successful synchronous apply now records the outcome. It is exempt from the budget, but it was also invisible to it, so a host that had demonstrably recovered kept the read path backing off for up to 30 minutes and no `recovered` transition ever came from that lane. Only success is recorded; recording failure would put the exempt lane back under the budget. - `writeSecureFile`'s JSDoc now says its boolean covers the file only. The directory harden is fire-and-forget and answers `pending` on Windows regardless, so a `true` says nothing about the directory's ACL. * fix(security): stop the hardening test doubles from faking a no-op Three CI failures on this branch, one failure shape: hardening silently does nothing and the check that should have caught it agrees. The auth critical-path test hand-rolled a `node:child_process` factory with `execFileSync`/`execFile`. The rewritten ACL path goes through `runProcessSync`, i.e. `spawnSync`, which the factory never returned — so every spawn threw into the SID lookup's bare catch, `planFor` returned null, and hardening no-opped. It mocks `child-process/run-process` now, the boundary production code actually calls and the one sibling ACL tests already mock: an export missing there fails loudly by name instead of returning undefined. Its fake icacls writes a real UTF-16LE SDDL file, so the pinned spawn count per write is a property of the ACL path rather than of the double. The test forces `platform='win32'`, so this failed on every platform, Linux CI included. `windowsSystem32Binary` is a production bug, not a test bug: it builds a Windows path with the host `join`, which off-platform yields the mixed `C:\Windows/System32/whoami.exe`. On Windows the two joins agree, which is why it survived; on Linux the SID lookup's whoami match missed and 27 of secure-file's 32 tests exercised a lane that never ran. These are always Windows paths, so `path.win32.join` is what it should have been. The import-boundary pin still read 160 after this branch migrated secure-path-windows-acl.ts off `node:child_process`; the ratchet correctly refuses a pin left above reality. * fix(security): resolve the machine-relative SDDL alias, and stop a denied read destroying the file Path hardening verified the DACL it wrote by comparing the SIDs `icacls /save` reports. SDDL substitutes two-letter aliases for well-known SIDs, and the resolution table could only hold constants -- but `LA` and `LG` name an account by RID inside the *machine's own* SID, so on a box whose user is the built-in Administrator (a CI runner, an Administrator-only install) the current user read back as `LA`, matched nothing, and hardening reported failure for every path. Resolve those two against the machine authority derived from the user SID; without one they stay unresolved and the comparison still fails closed. Three secret stores treated any read failure as "malformed -- regenerate" and overwrote. A hardened file granting a SID this process does not hold reads as EPERM while its directory stays writable, so the overwrite succeeds: renaming over an unreadable file needs FILE_DELETE_CHILD on the parent, not DELETE on the file. That destroyed the E2EE secret key, every paired device's bearer token, and the plugin vault. Distinguish EPERM/EACCES from a parse failure and refuse. Also close the async lane's unhandled rejection: `void p.then(onSettled)` turned a throw from `onSettled` into a dead main process, and the retry budget it calls threw whenever nothing had configured it -- a contract held only by import order. The budget now defaults its own bounds. * test(windows): say which ACEs icacls listed when a planted DACL fails `toHaveLength` reports only a count and vitest elides the array, so three preconditions failing on the CI runner said "expected 3, got 6" and nothing about what the sixth entry was. Name the entries in the failure. * fix(security): stop three more stores overwriting what they were denied Same swallow-default-overwrite shape as the readers already fixed, found by sweeping every store that reads under a hardened root. - plugin-storage-store.ts returned `{}` on any read failure and set()/delete() wrote it back, losing the plugin KV store. It is the secrets store's shape line for line, so the two now behave identically. - relay-revoke-outbox.ts returned [] and save() wrote it, dropping revocations that never reached the relay -- a revoked device stays live. - profile-cloud-session-store.ts mapped an EPERM read onto `decrypt-failed`, which fails the `status === 'found'` guard in clearCloudSessionIfUnchanged and falls through to an rmSync of the account session. A denied read now reports `unreadable`, which licenses nothing; the refresh path bails on it and the auth status surfaces it rather than reporting a bare reconnect. All reuse isPermissionDeniedError. The predicate stays an EPERM/EACCES allow list rather than "ENOENT defaults, everything else throws": these stores are meant to self-heal a truncated or malformed file, and inverting it would turn a corrupt keypair into an app that cannot start. The distinction that matters is "could not read it" versus "read it and it was garbage". * test(windows): plant fixture DACLs that cannot inherit what they did not plant %TEMP% grants [SYSTEM, Administrators, <user>] (OI)(CI)(F) by default, and those propagate into every fixture. Three preconditions read back 4 and 6 ACEs where 3 were planted, and the extras looked like Orca's own hardening because the shape is identical -- on a runner whose user is the built-in Administrator, the inherited trio IS the trio production grants. Combining /inheritance:r with /grant:r leaves the argument order to icacls, and that combined form drops the inherited ACEs on Windows 11 but keeps them as explicit ones on the Windows Server runner. Removing inheritance in its own invocation makes the grant the whole DACL on either host, and the fixture root is de-inherited once up front so nothing propagates in. Rooting the fixtures outside %TEMP% would not have fixed this: any directory inherits from wherever it lives. The fix is to stop inheriting, not to move. No assertion is relaxed -- the counts stay exact. * test(windows): pick a foreign SID that stays foreign on an elevated runner `S-1-5-32-544` is only foreign to a token that is not an administrator. The CI runner is elevated AND logged in as the built-in Administrator, so granting Administrators granted the reader full control: the file stayed readable, and all six preservation assertions went vacuous rather than proving anything. BUILTIN\Guests is resolvable everywhere and no interactive token is a member, so the read is denied on an unelevated developer box and on the runner alike. An unresolvable SID would have been the stronger choice but icacls rejects one with ERROR_NONE_MAPPED (1332). The premise guard is what caught this -- it asserted the file was actually unreadable instead of trusting the grant, and named elevation as the suspect. * fix(security): refuse on any read that never reached the contents, not just a denied one isPermissionDeniedError becomes isUnreadableError, because "permission denied" was never the concept -- "could not read it", as opposed to "read it and it was garbage", is. EBUSY, EMFILE, ENFILE and EIO say exactly as little about a file's contents as EACCES does, and they fell into the branch that regenerates and overwrites. On Windows EBUSY is the likelier of the two: antivirus holding a credential open at the moment of a startup read produces it, which makes it a commoner path to the same permanent loss than the ACL case that motivated the original fix. Still an allow list, deliberately: ENOENT keeps licensing a create, and a parse failure keeps self-healing. The stores are built to recover from a truncated write, and turning that into a refusal would trade a recoverable state for an unrecoverable one on the startup path. Also fixes the regression suite's own premise on an elevated runner: makeUnreadable combined /inheritance:r with /grant:r, and that form keeps %TEMP%'s inherited [SYSTEM, Administrators, user] as explicit ACEs on Windows Server -- so the file stayed readable and all six assertions were vacuous. Same split-the-invocation fix as the ACL suite's planter. * 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 `<win32 check> && <flag>` 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. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
co-authored by
Orca Worker
Neil
parent
975bbdedcc
commit
0cbb01ef4b
@@ -867,6 +867,8 @@ jobs:
|
||||
src/main/runtime/repo-worktree-admin-fingerprint.test.ts
|
||||
src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts
|
||||
src/shared/secure-file-fsync-flags.test.ts
|
||||
src/shared/secure-path-windows-acl.win32.test.ts
|
||||
src/main/runtime/unreadable-secret-store-preservation.win32.test.ts
|
||||
src/main/ipc/pty-codex-account-attribution.test.ts
|
||||
src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts
|
||||
src/relay/windows-port-scan.win32.test.ts
|
||||
|
||||
@@ -237,6 +237,8 @@ const WINDOWS_PACKAGE_TESTS = [
|
||||
'src/main/runtime/repo-worktree-admin-fingerprint.test.ts',
|
||||
'src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts',
|
||||
'src/shared/secure-file-fsync-flags.test.ts',
|
||||
'src/shared/secure-path-windows-acl.win32.test.ts',
|
||||
'src/main/runtime/unreadable-secret-store-preservation.win32.test.ts',
|
||||
'src/main/ipc/pty-codex-account-attribution.test.ts',
|
||||
'src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts',
|
||||
'src/relay/windows-port-scan.win32.test.ts'
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import { mkdtemp, readFile, readdir, rm, stat, truncate, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
@@ -16,9 +15,14 @@ import {
|
||||
getOrCreateArtifactCreateIntent,
|
||||
removeArtifactCreateIntent
|
||||
} from './artifact-create-intent-store'
|
||||
import { runProcessSync } from '../../shared/child-process/run-process'
|
||||
import { __resetSecureFileWindowsUserSidForTests } from '../../shared/secure-file'
|
||||
import type { ArtifactShareScope } from './artifact-share-record-store'
|
||||
|
||||
vi.mock('node:child_process', () => ({ execFile: vi.fn(), execFileSync: vi.fn() }))
|
||||
vi.mock('../../shared/child-process/run-process', () => ({
|
||||
runProcess: vi.fn(),
|
||||
runProcessSync: vi.fn()
|
||||
}))
|
||||
|
||||
const createdPaths: string[] = []
|
||||
const scope: ArtifactShareScope = {
|
||||
@@ -168,12 +172,27 @@ describe('artifact create intent store', () => {
|
||||
expect((await readdir(directory)).some((name) => name.endsWith('.tmp'))).toBe(false)
|
||||
})
|
||||
|
||||
it('hardens one Windows journal directory without per-file PowerShell launches', async () => {
|
||||
it('hardens one Windows journal directory without per-file ACL launches', async () => {
|
||||
const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
vi.mocked(execFileSync).mockImplementation((file) =>
|
||||
String(file).endsWith('whoami.exe') ? '"USER","S-1-5-21-1000"' : ''
|
||||
)
|
||||
const ok = { code: 0, signal: null, stdout: '', stderr: '', timedOut: false }
|
||||
// Earlier cases in this file already resolved (and cached) the SID against an unstubbed mock.
|
||||
__resetSecureFileWindowsUserSidForTests()
|
||||
vi.mocked(runProcessSync).mockImplementation((spec) => {
|
||||
if (spec.program.endsWith('whoami.exe')) {
|
||||
return { ...ok, stdout: '"USER","S-1-5-21-1000"' }
|
||||
}
|
||||
const args = spec.args ?? []
|
||||
if (args.length > 1) {
|
||||
return ok // /reset and the /grant:r pass
|
||||
}
|
||||
// The verify pass re-reads the DACL; answer with the three protected inheritable rules.
|
||||
const rules = ['host\\me', 'NT AUTHORITY\\SYSTEM', 'BUILTIN\\Administrators'].map(
|
||||
(name, index) =>
|
||||
index === 0 ? `${args[0]} ${name}:(OI)(CI)(F)` : ` ${name}:(OI)(CI)(F)`
|
||||
)
|
||||
return { ...ok, stdout: `${rules.join('\r\n')}\r\n\r\nSuccessfully processed 1 files\r\n` }
|
||||
})
|
||||
try {
|
||||
const userDataPath = await createUserDataPath()
|
||||
getOrCreateArtifactCreateIntent(
|
||||
@@ -193,16 +212,20 @@ describe('artifact create intent store', () => {
|
||||
body
|
||||
)
|
||||
|
||||
const powershellCalls = vi
|
||||
.mocked(execFileSync)
|
||||
.mock.calls.filter(([file]) => String(file).endsWith('powershell.exe'))
|
||||
expect(powershellCalls).toHaveLength(1)
|
||||
expect((powershellCalls[0]![1] as string[]).at(-1)).toBe('1')
|
||||
// One harden across both intents: counted by its /reset pass, which opens each harden.
|
||||
const aclCalls = vi
|
||||
.mocked(runProcessSync)
|
||||
.mock.calls.map(([spec]) => spec)
|
||||
.filter((spec) => spec.program.endsWith('icacls.exe'))
|
||||
expect(aclCalls.filter((spec) => spec.args?.includes('/reset'))).toHaveLength(1)
|
||||
// The child intent files rely on inheritance, so the directory rules must carry (OI)(CI).
|
||||
const grant = aclCalls.find((spec) => spec.args?.includes('/grant:r'))
|
||||
expect(grant?.args?.filter((arg) => arg.endsWith(':(OI)(CI)(F)'))).toHaveLength(3)
|
||||
} finally {
|
||||
if (originalPlatform) {
|
||||
Object.defineProperty(process, 'platform', originalPlatform)
|
||||
}
|
||||
vi.mocked(execFileSync).mockReset()
|
||||
vi.mocked(runProcessSync).mockReset()
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -48,7 +48,8 @@ import {
|
||||
type UploadBundleOptions,
|
||||
type UploadBundleResult
|
||||
} from './diagnostic-bundle-upload'
|
||||
import { setActiveSink } from './tracer'
|
||||
import { setActiveSink, startSpan } from './tracer'
|
||||
import { setSecurePathHardeningReporter } from '../../shared/secure-path-hardening-report'
|
||||
|
||||
const CI_ENV_VARS = [
|
||||
'CI',
|
||||
@@ -153,10 +154,37 @@ export function initObservability(): ObservabilityConsent {
|
||||
return c
|
||||
}
|
||||
installLocalSink()
|
||||
installSecurePathHardeningReporter()
|
||||
return c
|
||||
}
|
||||
|
||||
/**
|
||||
* Why route it here: Windows path hardening lives in `src/shared` and defaults to `console.warn`,
|
||||
* which reaches nothing in a packaged build — the main process is GUI-subsystem and owns no
|
||||
* console. A credential file left on inherited ACLs is exactly what a diagnostic bundle should
|
||||
* show, so it becomes a span in the trace sink.
|
||||
*
|
||||
* `recovered` ends successfully rather than failing: a host that climbs back out of the
|
||||
* rate-limited state has to be as visible as one that fell into it, or the degraded state is only
|
||||
* ever half-diagnosable.
|
||||
*/
|
||||
function installSecurePathHardeningReporter(): void {
|
||||
setSecurePathHardeningReporter((entry) => {
|
||||
const span = startSpan('secure-path.windows-acl', {
|
||||
attributes: { targetPath: entry.targetPath, stage: entry.stage, detail: entry.detail }
|
||||
})
|
||||
if (entry.stage === 'recovered') {
|
||||
span.end()
|
||||
console.info('[secure-path.windows-acl] path hardening recovered', entry)
|
||||
return
|
||||
}
|
||||
span.fail(entry.detail)
|
||||
console.warn('[secure-path.windows-acl] failed to restrict path', entry)
|
||||
})
|
||||
}
|
||||
|
||||
export async function shutdownObservability(): Promise<void> {
|
||||
setSecurePathHardeningReporter(null)
|
||||
// Order matters: tracer first so no new pushes arrive while the local sink
|
||||
// is closing and flushing buffered lines.
|
||||
setActiveSink(null)
|
||||
|
||||
@@ -29,7 +29,10 @@ export function getOrcaProfileAuthStatusFromProfile(
|
||||
state: 'unconfigured',
|
||||
persistence: session.status === 'found' ? session.persistence : 'none',
|
||||
cloud,
|
||||
credentialError: session.status === 'decrypt-failed' ? session.error : undefined,
|
||||
credentialError:
|
||||
session.status === 'decrypt-failed' || session.status === 'unreadable'
|
||||
? session.error
|
||||
: undefined,
|
||||
setupMessage: configState.setupMessage
|
||||
}
|
||||
}
|
||||
@@ -51,6 +54,9 @@ export function getOrcaProfileAuthStatusFromProfile(
|
||||
state: 'reconnect-required',
|
||||
persistence: 'none',
|
||||
cloud,
|
||||
credentialError: session.status === 'decrypt-failed' ? session.error : undefined
|
||||
credentialError:
|
||||
session.status === 'decrypt-failed' || session.status === 'unreadable'
|
||||
? session.error
|
||||
: undefined
|
||||
}
|
||||
}
|
||||
|
||||
@@ -72,6 +72,11 @@ function clearCloudSessionIfUnchanged(
|
||||
if (current.status === 'found' && current.session.refreshToken !== failed.refreshToken) {
|
||||
return
|
||||
}
|
||||
// A session we were denied is not a session we may delete: the token we would be clearing might
|
||||
// not even be the one that failed, and `clearOrcaCloudSession` unlinks the file outright.
|
||||
if (current.status === 'unreadable') {
|
||||
return
|
||||
}
|
||||
if (active.profile.cloud) {
|
||||
tombstoneCloudSession(
|
||||
cloudSessionIdentity(active.profile.id, active.profile.cloud),
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { existsSync, readFileSync, rmSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { safeStorage } from 'electron'
|
||||
import { writeSecureJsonFile } from '../../shared/secure-file'
|
||||
import { isUnreadableError, writeSecureJsonFile } from '../../shared/secure-file'
|
||||
import type {
|
||||
OrcaCloudCapabilities,
|
||||
OrcaCloudOrgSummary,
|
||||
@@ -29,6 +29,11 @@ export type OrcaCloudSessionReadResult =
|
||||
| { status: 'found'; session: OrcaCloudSession; persistence: OrcaCloudSessionPersistence }
|
||||
| { status: 'missing'; persistence: 'none' }
|
||||
| { status: 'decrypt-failed'; persistence: 'none'; error: string }
|
||||
/**
|
||||
* The file is there and this process may not read it. Distinct from `decrypt-failed` because
|
||||
* that one means "read it, it was garbage" and licenses replacing it; this one licenses nothing.
|
||||
*/
|
||||
| { status: 'unreadable'; persistence: 'none'; error: string }
|
||||
|
||||
type PersistedEncryptedSession = {
|
||||
version: 1
|
||||
@@ -215,7 +220,14 @@ export function readOrcaCloudSession(
|
||||
return { status: 'found', session: parsed.session, persistence: 'dev-plaintext' }
|
||||
}
|
||||
return { status: 'decrypt-failed', persistence: 'none', error: 'Unsafe session format.' }
|
||||
} catch {
|
||||
} catch (error) {
|
||||
if (isUnreadableError(error)) {
|
||||
return {
|
||||
status: 'unreadable',
|
||||
persistence: 'none',
|
||||
error: 'Cannot read the saved Orca account session: the read failed.'
|
||||
}
|
||||
}
|
||||
return {
|
||||
status: 'decrypt-failed',
|
||||
persistence: 'none',
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { existsSync, readFileSync, statSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { safeStorage } from 'electron'
|
||||
import { writeSecureFile } from '../../shared/secure-file'
|
||||
import { isUnreadableError, writeSecureFile } from '../../shared/secure-file'
|
||||
import {
|
||||
PLUGIN_STORAGE_KEY_LIMIT,
|
||||
PLUGIN_STORAGE_TOTAL_MAX_BYTES
|
||||
@@ -25,6 +25,8 @@ type PersistedSecretsFile = {
|
||||
|
||||
export type PluginSecretsResult<T> = { ok: true; value: T } | { ok: false; error: string }
|
||||
|
||||
const UNREADABLE_VAULT_ERROR = 'secret vault exists but could not be read; refusing to overwrite it'
|
||||
|
||||
export class PluginSecretsStore {
|
||||
private readonly filePath: string
|
||||
|
||||
@@ -32,7 +34,8 @@ export class PluginSecretsStore {
|
||||
this.filePath = join(pluginDataDir(pluginsDataDir, qualifiedKey), 'secrets.json.enc')
|
||||
}
|
||||
|
||||
private read(): PersistedSecretsFile {
|
||||
/** `null` means the vault exists and this process may not read it — which is never "empty". */
|
||||
private read(): PersistedSecretsFile | null {
|
||||
const empty: PersistedSecretsFile = {
|
||||
version: 1,
|
||||
format: 'electron-safe-storage-v1',
|
||||
@@ -56,7 +59,13 @@ export class PluginSecretsStore {
|
||||
) {
|
||||
return parsed
|
||||
}
|
||||
} catch {
|
||||
} catch (error) {
|
||||
// Being denied the read is not evidence the vault is corrupt. Returning `empty` here would
|
||||
// make the next set() write a vault containing only that one key, silently dropping every
|
||||
// secret the file still holds — and the write would succeed.
|
||||
if (isUnreadableError(error)) {
|
||||
return null
|
||||
}
|
||||
// Corrupt vaults read as empty; set() rewrites a valid file.
|
||||
}
|
||||
return empty
|
||||
@@ -64,6 +73,9 @@ export class PluginSecretsStore {
|
||||
|
||||
get(key: string): PluginSecretsResult<string | null> {
|
||||
const file = this.read()
|
||||
if (!file) {
|
||||
return { ok: false, error: UNREADABLE_VAULT_ERROR }
|
||||
}
|
||||
const ciphertext = file.ciphertexts[key]
|
||||
if (typeof ciphertext !== 'string') {
|
||||
return { ok: true, value: null }
|
||||
@@ -83,6 +95,9 @@ export class PluginSecretsStore {
|
||||
return { ok: false, error: 'OS-backed encryption is unavailable; secret not stored' }
|
||||
}
|
||||
const file = this.read()
|
||||
if (!file) {
|
||||
return { ok: false, error: UNREADABLE_VAULT_ERROR }
|
||||
}
|
||||
if (
|
||||
!Object.hasOwn(file.ciphertexts, key) &&
|
||||
Object.keys(file.ciphertexts).length >= PLUGIN_STORAGE_KEY_LIMIT
|
||||
@@ -100,6 +115,10 @@ export class PluginSecretsStore {
|
||||
|
||||
delete(key: string): void {
|
||||
const file = this.read()
|
||||
if (!file) {
|
||||
// Rewriting what we could not read would drop every other secret in the vault.
|
||||
return
|
||||
}
|
||||
if (Object.hasOwn(file.ciphertexts, key)) {
|
||||
delete file.ciphertexts[key]
|
||||
writeSecureFile(this.filePath, JSON.stringify(file, null, 2))
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { existsSync, readFileSync, statSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { writeSecureFile } from '../../shared/secure-file'
|
||||
import { isUnreadableError, writeSecureFile } from '../../shared/secure-file'
|
||||
import { isQualifiedPluginKey } from '../../shared/plugins/plugin-manifest'
|
||||
import {
|
||||
PLUGIN_STORAGE_KEY_LIMIT,
|
||||
@@ -16,6 +16,8 @@ import {
|
||||
* Adapted from community PR #5801's per-plugin settings store.
|
||||
*/
|
||||
|
||||
const UNREADABLE_STORE_ERROR = 'storage file exists but could not be read; refusing to overwrite it'
|
||||
|
||||
export function pluginDataDir(pluginsDataDir: string, qualifiedKey: string): string {
|
||||
if (!isQualifiedPluginKey(qualifiedKey)) {
|
||||
throw new Error(`unsafe plugin key: ${qualifiedKey}`)
|
||||
@@ -36,7 +38,8 @@ export class PluginKvStore {
|
||||
this.filePath = join(pluginDataDir(pluginsDataDir, qualifiedKey), fileName)
|
||||
}
|
||||
|
||||
private read(): Record<string, unknown> {
|
||||
/** `null` means the file exists and this process may not read it - which is never `{}`. */
|
||||
private read(): Record<string, unknown> | null {
|
||||
try {
|
||||
if (!existsSync(this.filePath)) {
|
||||
return {}
|
||||
@@ -48,22 +51,27 @@ export class PluginKvStore {
|
||||
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
|
||||
return parsed as Record<string, unknown>
|
||||
}
|
||||
} catch {
|
||||
} catch (error) {
|
||||
// Being denied the read is not evidence of corruption. Returning `{}` here would make
|
||||
// the next set()/delete() write a file holding only that one key, dropping the rest.
|
||||
if (isUnreadableError(error)) {
|
||||
return null
|
||||
}
|
||||
// Corrupt files reset to empty rather than wedging the plugin.
|
||||
}
|
||||
return {}
|
||||
}
|
||||
|
||||
get(key: string): unknown {
|
||||
return this.read()[key]
|
||||
return this.read()?.[key]
|
||||
}
|
||||
|
||||
getAll(): Record<string, unknown> {
|
||||
return this.read()
|
||||
return this.read() ?? {}
|
||||
}
|
||||
|
||||
keys(): string[] {
|
||||
return Object.keys(this.read())
|
||||
return Object.keys(this.read() ?? {})
|
||||
}
|
||||
|
||||
set(key: string, value: unknown): PluginKvWriteResult {
|
||||
@@ -80,6 +88,9 @@ export class PluginKvStore {
|
||||
return { ok: false, error: `value exceeds ${PLUGIN_STORAGE_VALUE_MAX_BYTES} bytes` }
|
||||
}
|
||||
const settings = this.read()
|
||||
if (!settings) {
|
||||
return { ok: false, error: UNREADABLE_STORE_ERROR }
|
||||
}
|
||||
if (!Object.hasOwn(settings, key) && Object.keys(settings).length >= PLUGIN_STORAGE_KEY_LIMIT) {
|
||||
return { ok: false, error: `storage exceeds the ${PLUGIN_STORAGE_KEY_LIMIT}-key limit` }
|
||||
}
|
||||
@@ -94,6 +105,10 @@ export class PluginKvStore {
|
||||
|
||||
delete(key: string): void {
|
||||
const settings = this.read()
|
||||
if (!settings) {
|
||||
// Rewriting what we could not read would drop every other key in the store.
|
||||
return
|
||||
}
|
||||
if (Object.hasOwn(settings, key)) {
|
||||
delete settings[key]
|
||||
writeSecureFile(this.filePath, JSON.stringify(settings, null, 2))
|
||||
|
||||
@@ -5,7 +5,11 @@
|
||||
import { randomBytes, randomUUID } from 'node:crypto'
|
||||
import { existsSync, readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { hardenExistingSecureFile, writeSecureJsonFile } from '../../shared/secure-file'
|
||||
import {
|
||||
hardenExistingSecureFile,
|
||||
isUnreadableError,
|
||||
writeSecureJsonFile
|
||||
} from '../../shared/secure-file'
|
||||
import type { DeviceScope } from '../../shared/runtime-types'
|
||||
import { DEVICE_REGISTRY_FILENAME } from './mobile-pairing-files'
|
||||
import type { RelayDeviceBinding } from './relay/relay-revoke-outbox'
|
||||
@@ -54,6 +58,8 @@ const LAST_SEEN_FLUSH_DELAY_MS = 250
|
||||
export class DeviceRegistry {
|
||||
private readonly registryPath: string
|
||||
private devices: DeviceEntry[] = []
|
||||
/** Set when the registry exists but could not be read, which makes `devices` a lie to save from. */
|
||||
private registryUnreadable = false
|
||||
private pendingLastSeenFlush: NodeJS.Timeout | null = null
|
||||
|
||||
constructor(userDataPath: string) {
|
||||
@@ -293,12 +299,21 @@ export class DeviceRegistry {
|
||||
// LAN links), so a missing value must keep binding every interface on reconnect.
|
||||
pairingReach: device.pairingReach === 'this-computer' ? 'this-computer' : 'network'
|
||||
}))
|
||||
} catch {
|
||||
this.registryUnreadable = false
|
||||
} catch (error) {
|
||||
// "Cannot read" is not "is empty". Saving an empty list over a registry we were merely
|
||||
// denied would erase every paired device's bearer token, and the write would succeed.
|
||||
this.registryUnreadable = isUnreadableError(error)
|
||||
this.devices = []
|
||||
}
|
||||
}
|
||||
|
||||
private save(devices: DeviceEntry[]): void {
|
||||
if (this.registryUnreadable) {
|
||||
throw new Error(
|
||||
`Cannot read the device registry at ${this.registryPath}: the read failed. Refusing to overwrite it, which would revoke every paired device.`
|
||||
)
|
||||
}
|
||||
writeSecureJsonFile(this.registryPath, devices)
|
||||
// Why: every registry save includes the latest in-memory timestamps, so a later timer would rewrite it.
|
||||
this.cancelPendingLastSeenFlush()
|
||||
|
||||
@@ -4,7 +4,11 @@
|
||||
import { existsSync, readFileSync, statSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import nacl from 'tweetnacl'
|
||||
import { hardenExistingSecureFile, writeSecureJsonFile } from '../../shared/secure-file'
|
||||
import {
|
||||
hardenExistingSecureFile,
|
||||
isUnreadableError,
|
||||
writeSecureJsonFile
|
||||
} from '../../shared/secure-file'
|
||||
import { E2EE_KEYPAIR_FILENAME } from './mobile-pairing-files'
|
||||
|
||||
const KEYPAIR_FILENAME = E2EE_KEYPAIR_FILENAME
|
||||
@@ -42,7 +46,17 @@ export function loadOrCreateE2EEKeypair(userDataPath: string): E2EEKeypair {
|
||||
return { publicKey, secretKey, publicKeyB64: raw.publicKeyB64 }
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
} catch (error) {
|
||||
// A read this process is not permitted to make says nothing about the contents. Falling
|
||||
// through would overwrite the only copy of the secret key — and the overwrite succeeds, so
|
||||
// nothing downstream stops it. Every paired device derives its shared secret from this key,
|
||||
// so regenerating silently un-pairs all of them and no old message stays decryptable.
|
||||
if (isUnreadableError(error)) {
|
||||
throw new Error(
|
||||
`Cannot read the E2EE keypair at ${filePath}: the read failed. Refusing to regenerate it, which would invalidate every paired device.`,
|
||||
{ cause: error }
|
||||
)
|
||||
}
|
||||
// Malformed file — regenerate below.
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { existsSync, readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { hardenExistingSecureFile, writeSecureJsonFile } from '../../../shared/secure-file'
|
||||
import {
|
||||
hardenExistingSecureFile,
|
||||
isUnreadableError,
|
||||
writeSecureJsonFile
|
||||
} from '../../../shared/secure-file'
|
||||
|
||||
export type RelayDeviceBinding = {
|
||||
relayHostId: string
|
||||
@@ -37,6 +41,8 @@ function isItem(value: unknown): value is RelayRevokeOutboxItem {
|
||||
export class RelayRevokeOutbox {
|
||||
private readonly path: string
|
||||
private items: RelayRevokeOutboxItem[]
|
||||
/** Set when the outbox exists but could not be read, so `items` is not what is on disk. */
|
||||
private outboxUnreadable = false
|
||||
|
||||
constructor(userDataPath: string) {
|
||||
this.path = join(userDataPath, OUTBOX_FILENAME)
|
||||
@@ -83,12 +89,20 @@ export class RelayRevokeOutbox {
|
||||
hardenExistingSecureFile(this.path)
|
||||
const parsed: unknown = JSON.parse(readFileSync(this.path, 'utf-8'))
|
||||
return Array.isArray(parsed) ? parsed.filter(isItem) : []
|
||||
} catch {
|
||||
} catch (error) {
|
||||
// An outbox we were denied is not an empty outbox. Saving [] over it would drop
|
||||
// revocations that have not reached the relay, so a revoked device stays live.
|
||||
this.outboxUnreadable = isUnreadableError(error)
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
private save(items: readonly RelayRevokeOutboxItem[]): void {
|
||||
if (this.outboxUnreadable) {
|
||||
throw new Error(
|
||||
`Cannot read the relay revoke outbox at ${this.path}: the read failed. Refusing to overwrite it, which would drop pending revocations.`
|
||||
)
|
||||
}
|
||||
writeSecureJsonFile(this.path, items)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,26 +1,71 @@
|
||||
// Why: the E2EE auth handshake used to persist `lastSeenAt` inline, and on Windows every secure-file
|
||||
// write blocks the main thread on two synchronous PowerShell ACL spawns (~1-1.5s cold each). These tests
|
||||
// write blocks the main thread on synchronous icacls ACL spawns (~1-1.5s cold each). These tests
|
||||
// pin the spawn count on the auth critical path, not wall-clock, so they are deterministic under load.
|
||||
import { execFile, execFileSync } from 'node:child_process'
|
||||
import { mkdtempSync, readFileSync, rmSync } from 'node:fs'
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { WebSocket } from 'ws'
|
||||
import type { ProcessResult, ProcessSpec } from '../../../shared/child-process/run-process'
|
||||
import { runProcess, runProcessSync } from '../../../shared/child-process/run-process'
|
||||
import { DEVICE_REGISTRY_FILENAME } from '../mobile-pairing-files'
|
||||
import { DeviceRegistry, type DeviceEntry } from '../device-registry'
|
||||
import { decrypt, deriveSharedKey, encrypt, generateKeyPair } from './e2ee-crypto'
|
||||
import { MobileSocketWiring, type MobileSocketTransport } from './mobile-socket-wiring'
|
||||
|
||||
vi.mock('node:child_process', () => ({
|
||||
execFileSync: vi.fn(),
|
||||
execFile: vi.fn()
|
||||
// Why this module and not `node:child_process`: hardening reaches the OS only through
|
||||
// runProcess/runProcessSync, and a hand-written child_process factory silently omitted the one
|
||||
// function they call — so every spawn threw, hardening no-opped, and the test double hid it.
|
||||
vi.mock('../../../shared/child-process/run-process', () => ({
|
||||
runProcess: vi.fn(),
|
||||
runProcessSync: vi.fn()
|
||||
}))
|
||||
|
||||
// Why: stands in for the PowerShell cold start; long enough that a gated response would be obvious,
|
||||
// Why: stands in for the icacls cold start; long enough that a gated response would be obvious,
|
||||
// short enough that the suite stays fast. Assertions use the recorded ordering, never this number.
|
||||
const INJECTED_SPAWN_LATENCY_MS = 5
|
||||
const POWERSHELL = 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'
|
||||
const USER_SID = 'S-1-5-21-1000'
|
||||
const OK: ProcessResult = { code: 0, signal: null, stdout: '', stderr: '', timedOut: false }
|
||||
/**
|
||||
* One secure write hardens two paths: the staged temp file, fresh and still on the inherited DACL,
|
||||
* costs the full verify/reset/grant/verify pass; the published file, whose protected DACL came
|
||||
* along with the rename, costs only its verify.
|
||||
*/
|
||||
const BLOCKING_SPAWNS_PER_WRITE = 5
|
||||
|
||||
/** Paths the fake icacls has granted a protected DACL, keyed to the ACE flags the grant used. */
|
||||
const hardenedByFake = new Map<string, string>()
|
||||
|
||||
/**
|
||||
* Stands in for icacls. `/save` really writes a UTF-16LE SDDL file, because the code under test
|
||||
* reads that file back off disk to decide whether a rewrite is needed at all — which is what makes
|
||||
* the spawn count per write a property of the real ACL path rather than of this double.
|
||||
*/
|
||||
function fakeIcacls(spec: ProcessSpec): ProcessResult {
|
||||
const args = spec.args ?? []
|
||||
const path = args[0] ?? ''
|
||||
const grantIndex = args.indexOf('/grant:r')
|
||||
if (grantIndex !== -1) {
|
||||
hardenedByFake.set(path, args[grantIndex + 1]!.includes('(OI)(CI)') ? 'OICI' : '')
|
||||
return OK
|
||||
}
|
||||
const saveIndex = args.indexOf('/save')
|
||||
if (saveIndex === -1) {
|
||||
return OK // /reset
|
||||
}
|
||||
writeFileSync(args[saveIndex + 1]!, fakeSddl(path), 'utf16le')
|
||||
return OK
|
||||
}
|
||||
|
||||
function fakeSddl(path: string): string {
|
||||
const aceFlags = hardenedByFake.get(path)
|
||||
if (aceFlags === undefined) {
|
||||
// Never hardened: the inherited DACL a fresh file carries, so the first verify must fail.
|
||||
return `name\r\nD:(A;ID;FA;;;SY)(A;ID;FA;;;BA)(A;ID;FA;;;${USER_SID})\r\n`
|
||||
}
|
||||
const ace = (sid: string): string => `(A;${aceFlags};FA;;;${sid})`
|
||||
return `name\r\nD:PAI${ace('BA')}${ace('SY')}${ace(USER_SID)}\r\n`
|
||||
}
|
||||
|
||||
type TimelineEntry = 'acl-spawn' | 'e2ee_ready' | 'e2ee_authenticated' | 'other-frame'
|
||||
|
||||
@@ -62,24 +107,23 @@ describe('mobile auth critical path', () => {
|
||||
process.env.SystemRoot = 'C:\\Windows'
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
userDataPath = mkdtempSync(join(tmpdir(), 'orca-auth-acl-'))
|
||||
vi.mocked(execFileSync).mockReset()
|
||||
vi.mocked(execFile).mockReset()
|
||||
vi.mocked(execFileSync).mockImplementation((file) => {
|
||||
if (String(file).endsWith('whoami.exe')) {
|
||||
return '"USER","S-1-5-21-1000"'
|
||||
hardenedByFake.clear()
|
||||
vi.mocked(runProcessSync).mockReset()
|
||||
vi.mocked(runProcess).mockReset()
|
||||
vi.mocked(runProcessSync).mockImplementation((spec) => {
|
||||
// Matched by suffix, not by the whole path: `windowsSystem32Binary` joins with the host
|
||||
// separator, so the literal only matches when the tests happen to run on Windows.
|
||||
if (spec.program.endsWith('whoami.exe')) {
|
||||
return { ...OK, stdout: `"USER","${USER_SID}"` }
|
||||
}
|
||||
timeline.push('acl-spawn')
|
||||
// Why: the real spawn blocks the main thread, so the fake must too — and via Atomics, not a
|
||||
// Date.now() spin, which would never terminate under fake timers.
|
||||
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, INJECTED_SPAWN_LATENCY_MS)
|
||||
return ''
|
||||
})
|
||||
vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => {
|
||||
if (typeof callback === 'function') {
|
||||
callback(null, '', '')
|
||||
}
|
||||
return {} as ReturnType<typeof execFile>
|
||||
return fakeIcacls(spec)
|
||||
})
|
||||
// The directory harden stays on the async lane, so it never lands on the timeline.
|
||||
vi.mocked(runProcess).mockImplementation((spec) => Promise.resolve(fakeIcacls(spec)))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
@@ -157,8 +201,11 @@ describe('mobile auth critical path', () => {
|
||||
|
||||
registry.flushPendingLastSeen()
|
||||
// Hardening is deferred, never dropped: tmp file + published file, exactly as the inline path did.
|
||||
expect(timeline.filter((entry) => entry === 'acl-spawn')).toHaveLength(2)
|
||||
expect(vi.mocked(execFileSync).mock.lastCall?.[0]).toBe(POWERSHELL)
|
||||
expect(timeline.filter((entry) => entry === 'acl-spawn')).toHaveLength(
|
||||
BLOCKING_SPAWNS_PER_WRITE
|
||||
)
|
||||
// The blocking work is the ACL tool itself, not some other spawn on the same lane.
|
||||
expect(vi.mocked(runProcessSync).mock.lastCall?.[0].program).toMatch(/icacls\.exe$/)
|
||||
expect(readPersistedDevices()[0]?.lastSeenAt).toBe(
|
||||
registry.getDevice(device.deviceId)?.lastSeenAt
|
||||
)
|
||||
@@ -172,7 +219,11 @@ describe('mobile auth critical path', () => {
|
||||
authenticate(registry, device)
|
||||
|
||||
// Why: rotatePendingDevice drops entries disk says were never scanned, so this write stays inline.
|
||||
expect(timeline).toEqual(['e2ee_ready', 'acl-spawn', 'acl-spawn', 'e2ee_authenticated'])
|
||||
expect(timeline).toEqual([
|
||||
'e2ee_ready',
|
||||
...Array<TimelineEntry>(BLOCKING_SPAWNS_PER_WRITE).fill('acl-spawn'),
|
||||
'e2ee_authenticated'
|
||||
])
|
||||
expect(readPersistedDevices()[0]?.lastSeenAt).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
@@ -189,7 +240,9 @@ describe('mobile auth critical path', () => {
|
||||
|
||||
expect(timeline.filter((entry) => entry === 'acl-spawn')).toHaveLength(0)
|
||||
vi.advanceTimersByTime(250)
|
||||
expect(timeline.filter((entry) => entry === 'acl-spawn')).toHaveLength(2)
|
||||
expect(timeline.filter((entry) => entry === 'acl-spawn')).toHaveLength(
|
||||
BLOCKING_SPAWNS_PER_WRITE
|
||||
)
|
||||
})
|
||||
|
||||
it('cancels the deferred rewrite when another registry save persists the timestamp', () => {
|
||||
@@ -201,9 +254,13 @@ describe('mobile auth critical path', () => {
|
||||
|
||||
registry.updateLastSeenDeferred(device.deviceId)
|
||||
registry.addDevice('Other client', 'runtime')
|
||||
expect(timeline.filter((entry) => entry === 'acl-spawn')).toHaveLength(2)
|
||||
expect(timeline.filter((entry) => entry === 'acl-spawn')).toHaveLength(
|
||||
BLOCKING_SPAWNS_PER_WRITE
|
||||
)
|
||||
|
||||
vi.advanceTimersByTime(250)
|
||||
expect(timeline.filter((entry) => entry === 'acl-spawn')).toHaveLength(2)
|
||||
expect(timeline.filter((entry) => entry === 'acl-spawn')).toHaveLength(
|
||||
BLOCKING_SPAWNS_PER_WRITE
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,262 @@
|
||||
import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
|
||||
import { runProcessSync } from '../../shared/child-process/run-process'
|
||||
import { windowsSystem32Binary } from '../../shared/child-process/windows-system-binary'
|
||||
import { removeTreeSync } from '../../shared/windows-transient-lock-removal'
|
||||
|
||||
/**
|
||||
* What a *successful* harden does to a reader that cannot read.
|
||||
*
|
||||
* Path hardening writes a protected DACL granting only the SIDs the running process holds. Where
|
||||
* the data root came from somewhere else — a relocated `ORCA_USER_DATA_PATH`, a share, a roaming
|
||||
* profile, a backup restored under a recreated local account, or a harden whose `/reset` landed
|
||||
* and whose `/grant` did not — the file ends up granting a SID this process does not have. It then
|
||||
* reads as `EPERM` while its *directory* stays writable, because file hardening is synchronous on
|
||||
* the write path and directory hardening is fire-and-forget.
|
||||
*
|
||||
* Every store below used to treat any read failure as "malformed — regenerate", and the
|
||||
* regeneration succeeds: `renameSync` over an unreadable file needs `FILE_DELETE_CHILD` on the
|
||||
* parent, not `DELETE` on the file. So the healing path destroyed the thing it could not read.
|
||||
* Before hardening actually applied, this failed open — the file was simply readable.
|
||||
*
|
||||
* These assert the file still holds its original bytes afterwards. Runs only on win32, where a
|
||||
* DACL is the mechanism; skipped elsewhere.
|
||||
*/
|
||||
|
||||
/** 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.
|
||||
* `S-1-5-32-544` looks foreign only until the suite meets a host that is elevated AND logged
|
||||
* in as the built-in Administrator -- a CI runner -- where it grants the reader full control
|
||||
* and every assertion below goes vacuous. An unresolvable SID is not an option: icacls
|
||||
* rejects one with ERROR_NONE_MAPPED (1332).
|
||||
*/
|
||||
const FOREIGN_SID = 'S-1-5-32-546'
|
||||
|
||||
function icacls(...args: string[]): number | null {
|
||||
return runProcessSync({
|
||||
program: windowsSystem32Binary('icacls.exe'),
|
||||
args,
|
||||
timeoutMs: 10_000
|
||||
}).code
|
||||
}
|
||||
|
||||
/** The on-disk state a successful harden leaves for a SID this process does not hold. */
|
||||
function makeUnreadable(filePath: string): void {
|
||||
// Two invocations: the combined `/inheritance:r /grant:r` form keeps %TEMP%'s inherited
|
||||
// [SYSTEM, Administrators, user] as *explicit* ACEs on Windows Server, which left the file
|
||||
// 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)
|
||||
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
|
||||
|
||||
beforeAll(() => {
|
||||
root = mkdtempSync(join(tmpdir(), 'orca-unreadable-'))
|
||||
})
|
||||
|
||||
afterAll(() => {
|
||||
// Reset first: the tree is not removable while its files grant only Administrators.
|
||||
icacls(root, '/reset', '/t', '/q')
|
||||
removeTreeSync(root)
|
||||
})
|
||||
|
||||
it('does not regenerate the E2EE keypair, which would un-pair every device', async () => {
|
||||
const { loadOrCreateE2EEKeypair } = await import('./e2ee-keypair')
|
||||
const { E2EE_KEYPAIR_FILENAME } = await import('./mobile-pairing-files')
|
||||
const dir = join(root, 'e2ee')
|
||||
mkdirSync(dir, { recursive: true })
|
||||
const filePath = join(dir, E2EE_KEYPAIR_FILENAME)
|
||||
const original = JSON.stringify({
|
||||
v: 1,
|
||||
publicKeyB64: Buffer.alloc(32, 7).toString('base64'),
|
||||
secretKeyB64: Buffer.alloc(32, 9).toString('base64')
|
||||
})
|
||||
writeFileSync(filePath, original)
|
||||
makeUnreadable(filePath)
|
||||
|
||||
expect(() => loadOrCreateE2EEKeypair(dir)).toThrow(/Refusing to (regenerate|overwrite)/)
|
||||
|
||||
// The point: the secret key is still the one every paired phone derived its shared secret from.
|
||||
icacls(filePath, '/reset', '/q')
|
||||
expect(readFileSync(filePath, 'utf8')).toBe(original)
|
||||
})
|
||||
|
||||
it('does not erase the device registry, which would revoke every paired token', async () => {
|
||||
const { DeviceRegistry } = await import('./device-registry')
|
||||
const { DEVICE_REGISTRY_FILENAME } = await import('./mobile-pairing-files')
|
||||
const dir = join(root, 'devices')
|
||||
mkdirSync(dir, { recursive: true })
|
||||
const filePath = join(dir, DEVICE_REGISTRY_FILENAME)
|
||||
const original = JSON.stringify([
|
||||
{
|
||||
deviceId: 'device-1',
|
||||
name: 'Phone',
|
||||
token: 'bearer-token-that-must-survive',
|
||||
scope: 'mobile',
|
||||
pairedAt: 1,
|
||||
lastSeenAt: 2
|
||||
}
|
||||
])
|
||||
writeFileSync(filePath, original)
|
||||
makeUnreadable(filePath)
|
||||
|
||||
const registry = new DeviceRegistry(dir)
|
||||
// Any mutator reaches save(); it must refuse rather than write the empty list it loaded.
|
||||
expect(() => registry.addDevice('Another phone', 'mobile')).toThrow(
|
||||
/Refusing to (regenerate|overwrite)/
|
||||
)
|
||||
|
||||
icacls(filePath, '/reset', '/q')
|
||||
expect(readFileSync(filePath, 'utf8')).toBe(original)
|
||||
})
|
||||
|
||||
it('does not blank the plugin secret vault on write', async () => {
|
||||
vi.doMock('electron', () => ({
|
||||
safeStorage: {
|
||||
isEncryptionAvailable: () => true,
|
||||
encryptString: (value: string) => Buffer.from(`enc:${value}`),
|
||||
decryptString: (buffer: Buffer) => buffer.toString().replace(/^enc:/, '')
|
||||
}
|
||||
}))
|
||||
const { PluginSecretsStore } = await import('./../plugins/plugin-secrets-store')
|
||||
const dir = join(root, 'plugin-secrets')
|
||||
mkdirSync(dir, { recursive: true })
|
||||
const store = new PluginSecretsStore(dir, 'publisher.plugin')
|
||||
// Reach the path the store computes rather than restating its layout here.
|
||||
const filePath = (store as unknown as { filePath: string }).filePath
|
||||
mkdirSync(join(filePath, '..'), { recursive: true })
|
||||
const original = JSON.stringify({
|
||||
version: 1,
|
||||
format: 'electron-safe-storage-v1',
|
||||
ciphertexts: { existing: Buffer.from('enc:keep-me').toString('base64') }
|
||||
})
|
||||
writeFileSync(filePath, original)
|
||||
makeUnreadable(filePath)
|
||||
|
||||
expect(store.set('added', 'value')).toEqual({ ok: false, error: expect.any(String) })
|
||||
store.delete('existing')
|
||||
|
||||
icacls(filePath, '/reset', '/q')
|
||||
expect(readFileSync(filePath, 'utf8')).toBe(original)
|
||||
vi.doUnmock('electron')
|
||||
})
|
||||
it('does not blank the plugin KV store on write', async () => {
|
||||
const { PluginKvStore } = await import('./../plugins/plugin-storage-store')
|
||||
const dir = join(root, 'plugin-kv')
|
||||
mkdirSync(dir, { recursive: true })
|
||||
const store = new PluginKvStore(dir, 'publisher.plugin', 'storage.json')
|
||||
const filePath = (store as unknown as { filePath: string }).filePath
|
||||
mkdirSync(join(filePath, '..'), { recursive: true })
|
||||
const original = JSON.stringify({ keep: 'me' })
|
||||
writeFileSync(filePath, original)
|
||||
makeUnreadable(filePath)
|
||||
|
||||
expect(store.set('added', 'value')).toEqual({ ok: false, error: expect.any(String) })
|
||||
store.delete('keep')
|
||||
|
||||
icacls(filePath, '/reset', '/q')
|
||||
expect(readFileSync(filePath, 'utf8')).toBe(original)
|
||||
})
|
||||
|
||||
it('does not drop pending relay revocations', async () => {
|
||||
const { RelayRevokeOutbox } = await import('./relay/relay-revoke-outbox')
|
||||
const dir = join(root, 'relay')
|
||||
mkdirSync(dir, { recursive: true })
|
||||
const filePath = join(dir, 'mobile-relay-revoke-outbox.json')
|
||||
const original = JSON.stringify([
|
||||
{
|
||||
relayHostId: 'host-1',
|
||||
relayDeviceId: 'device-1',
|
||||
ownerIdentityKey: 'owner-1',
|
||||
reqId: 'req-1',
|
||||
createdAt: 1
|
||||
}
|
||||
])
|
||||
writeFileSync(filePath, original)
|
||||
makeUnreadable(filePath)
|
||||
|
||||
const outbox = new RelayRevokeOutbox(dir)
|
||||
expect(() =>
|
||||
outbox.enqueue({
|
||||
relayHostId: 'host-2',
|
||||
relayDeviceId: 'device-2',
|
||||
ownerIdentityKey: 'owner-2'
|
||||
})
|
||||
).toThrow(/Refusing to (regenerate|overwrite)/)
|
||||
|
||||
icacls(filePath, '/reset', '/q')
|
||||
expect(readFileSync(filePath, 'utf8')).toBe(original)
|
||||
})
|
||||
|
||||
/**
|
||||
* The one site that *deletes* rather than overwrites: a refresh failure plus an unreadable
|
||||
* session used to fall past the `status === 'found'` guard into `clearOrcaCloudSession`.
|
||||
*/
|
||||
it('does not delete the account session it could not read', async () => {
|
||||
vi.doMock('electron', () => ({
|
||||
safeStorage: {
|
||||
isEncryptionAvailable: () => true,
|
||||
encryptString: (value: string) => Buffer.from(value),
|
||||
decryptString: (buffer: Buffer) => buffer.toString()
|
||||
}
|
||||
}))
|
||||
const { readOrcaCloudSession, getOrcaCloudSessionPath } =
|
||||
await import('./../orca-profiles/profile-cloud-session-store')
|
||||
const dir = join(root, 'profiles')
|
||||
mkdirSync(dir, { recursive: true })
|
||||
const filePath = getOrcaCloudSessionPath('profile-1', dir)
|
||||
mkdirSync(join(filePath, '..'), { recursive: true })
|
||||
const original = JSON.stringify({ version: 1, format: 'dev-plaintext-v1', savedAt: 1 })
|
||||
writeFileSync(filePath, original)
|
||||
makeUnreadable(filePath)
|
||||
|
||||
// The status the delete path keys off: `unreadable`, never `decrypt-failed`.
|
||||
expect(readOrcaCloudSession('profile-1', dir).status).toBe('unreadable')
|
||||
|
||||
icacls(filePath, '/reset', '/q')
|
||||
expect(readFileSync(filePath, 'utf8')).toBe(original)
|
||||
vi.doUnmock('electron')
|
||||
})
|
||||
})
|
||||
@@ -183,5 +183,4 @@ src/shared/fish-binary-requirement.ts
|
||||
src/shared/process-table-snapshot-reader.ts
|
||||
src/shared/pty-slave-line-discipline-echo.ts
|
||||
src/shared/ripgrep-process-availability.ts
|
||||
src/shared/secure-path-windows-acl.ts
|
||||
src/shared/shell-process-readiness.ts
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
import { join } from 'node:path'
|
||||
// Why win32 and not the host `join`: these are Windows paths and are only ever spawned on Windows,
|
||||
// but they are also built off-platform (tests, and any code that plans a Windows command from a
|
||||
// POSIX host), where the host separator produces the mixed `C:\Windows/System32/whoami.exe`.
|
||||
import { win32 as pathWin32 } from 'node:path'
|
||||
|
||||
/**
|
||||
* Absolute paths for the Windows system binaries Orca shells out to.
|
||||
@@ -17,12 +20,12 @@ function systemRoot(env: NodeJS.ProcessEnv = process.env): string {
|
||||
}
|
||||
|
||||
export function windowsPowerShellPath(env: NodeJS.ProcessEnv = process.env): string {
|
||||
return join(systemRoot(env), 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe')
|
||||
return pathWin32.join(systemRoot(env), 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe')
|
||||
}
|
||||
|
||||
export function windowsSystem32Binary(
|
||||
fileName: string,
|
||||
env: NodeJS.ProcessEnv = process.env
|
||||
): string {
|
||||
return join(systemRoot(env), 'System32', fileName)
|
||||
return pathWin32.join(systemRoot(env), 'System32', fileName)
|
||||
}
|
||||
|
||||
+515
-124
@@ -1,24 +1,73 @@
|
||||
import { execFile, execFileSync } from 'node:child_process'
|
||||
import { chmodSync, mkdirSync, mkdtempSync, rmSync, statSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { runProcess, runProcessSync } from './child-process/run-process'
|
||||
import { mayAttemptHardening } from './secure-path-hardening-retry-budget'
|
||||
import {
|
||||
__getSecureFileHardeningCacheStateForTests,
|
||||
__resetSecureFileHardenedPathsForTests,
|
||||
__resetSecureFileWindowsUserSidForTests,
|
||||
hardenExistingSecureFile,
|
||||
hardenSecurePath,
|
||||
isUnreadableError,
|
||||
writeSecureFile
|
||||
} from './secure-file'
|
||||
|
||||
const posixModeIt = process.platform === 'win32' ? it.skip : it
|
||||
|
||||
vi.mock('child_process', () => ({
|
||||
execFileSync: vi.fn(),
|
||||
execFile: vi.fn()
|
||||
vi.mock('./child-process/run-process', () => ({
|
||||
runProcess: vi.fn(),
|
||||
runProcessSync: vi.fn()
|
||||
}))
|
||||
|
||||
const OK = { code: 0, signal: null, stdout: '', stderr: '', timedOut: false }
|
||||
const USER_SID = 'S-1-5-21-1000'
|
||||
|
||||
type FakeSpec = { program: string; args?: readonly string[] }
|
||||
|
||||
/** Paths the fake considers already hardened, with the ACE flags the grant pass used. */
|
||||
const hardenedByFake = new Map<string, string>()
|
||||
|
||||
/** Paths whose verify pass should answer with a DACL that is not the intended one. */
|
||||
const forcedBadSddl = new Map<string, string>()
|
||||
|
||||
/**
|
||||
* Stands in for icacls. `/save` really writes a UTF-16LE SDDL file, because the code under test
|
||||
* reads that file back off disk — which also means these tests exercise the real SDDL parser
|
||||
* rather than a restatement of it.
|
||||
*/
|
||||
function fakeIcacls(spec: FakeSpec): typeof OK {
|
||||
const args = spec.args ?? []
|
||||
const path = args[0] ?? ''
|
||||
const grantIndex = args.indexOf('/grant:r')
|
||||
if (grantIndex !== -1) {
|
||||
const grant = args[grantIndex + 1]!
|
||||
hardenedByFake.set(path, grant.includes('(OI)(CI)') ? 'OICI' : '')
|
||||
return OK
|
||||
}
|
||||
const saveIndex = args.indexOf('/save')
|
||||
if (saveIndex === -1) {
|
||||
return OK // /reset
|
||||
}
|
||||
writeFileSync(args[saveIndex + 1]!, fakeSddl(path), 'utf16le')
|
||||
return OK
|
||||
}
|
||||
|
||||
function fakeSddl(path: string): string {
|
||||
const forced = forcedBadSddl.get(path)
|
||||
if (forced) {
|
||||
return `name\r\n${forced}\r\n`
|
||||
}
|
||||
const aceFlags = hardenedByFake.get(path)
|
||||
if (aceFlags === undefined) {
|
||||
// Never hardened: the inherited DACL a fresh file carries, so the first verify must fail.
|
||||
return `name\r\nD:(A;ID;FA;;;SY)(A;ID;FA;;;BA)(A;ID;FA;;;${USER_SID})\r\n`
|
||||
}
|
||||
const ace = (sid: string): string => `(A;${aceFlags};FA;;;${sid})`
|
||||
return `name\r\nD:PAI${ace('BA')}${ace('SY')}${ace(USER_SID)}\r\n`
|
||||
}
|
||||
|
||||
describe('hardenSecurePath', () => {
|
||||
const originalSystemRoot = process.env.SystemRoot
|
||||
const originalWindir = process.env.WINDIR
|
||||
@@ -30,24 +79,19 @@ describe('hardenSecurePath', () => {
|
||||
delete process.env.WINDIR
|
||||
__resetSecureFileWindowsUserSidForTests()
|
||||
__resetSecureFileHardenedPathsForTests()
|
||||
vi.mocked(execFileSync).mockReset()
|
||||
vi.mocked(execFile).mockReset()
|
||||
// execFileSync handles whoami.exe (SID lookup) and the SYNCHRONOUS PowerShell file-ACL
|
||||
// path used by writeSecureFile. The directory + read-path re-harden use async execFile.
|
||||
vi.mocked(execFileSync).mockImplementation((file) => {
|
||||
if (file === 'C:\\Windows\\System32\\whoami.exe') {
|
||||
return '"USER","S-1-5-21-1000"'
|
||||
vi.mocked(runProcessSync).mockReset()
|
||||
vi.mocked(runProcess).mockReset()
|
||||
hardenedByFake.clear()
|
||||
forcedBadSddl.clear()
|
||||
// runProcessSync serves whoami.exe (SID lookup) and the SYNCHRONOUS icacls file-ACL path
|
||||
// used by writeSecureFile. Directory + read-path re-hardens use async runProcess.
|
||||
vi.mocked(runProcessSync).mockImplementation((spec) => {
|
||||
if (spec.program === 'C:\\Windows\\System32\\whoami.exe') {
|
||||
return { ...OK, stdout: `"USER","${USER_SID}"` }
|
||||
}
|
||||
// Synchronous PowerShell ACL apply succeeds (returns empty stdout).
|
||||
return ''
|
||||
})
|
||||
// Directory + read-path PowerShell is called asynchronously; simulate immediate success
|
||||
vi.mocked(execFile).mockImplementation((_file, _args, _opts, callback) => {
|
||||
if (typeof callback === 'function') {
|
||||
callback(null, '', '')
|
||||
}
|
||||
return {} as ReturnType<typeof execFile>
|
||||
return fakeIcacls(spec)
|
||||
})
|
||||
vi.mocked(runProcess).mockImplementation((spec) => Promise.resolve(fakeIcacls(spec)))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
@@ -71,64 +115,358 @@ describe('hardenSecurePath', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('rewrites Windows ACLs through the system PowerShell path', () => {
|
||||
it('rewrites Windows ACLs through icacls, purging explicit ACEs before granting', async () => {
|
||||
hardenSecurePath('C:\\Users\\me\\.orca\\secret.json', {
|
||||
isDirectory: false,
|
||||
platform: 'win32'
|
||||
})
|
||||
await flushAsyncAcl()
|
||||
|
||||
// whoami.exe called synchronously to obtain SID
|
||||
expect(execFileSync).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
'C:\\Windows\\System32\\whoami.exe',
|
||||
['/user', '/fo', 'csv', '/nh'],
|
||||
expect.objectContaining({ encoding: 'utf-8' })
|
||||
)
|
||||
// PowerShell called asynchronously
|
||||
const [powershellFile, powershellArgs, powershellOptions] = vi.mocked(execFile).mock.calls[0]!
|
||||
expect(powershellFile).toBe('C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe')
|
||||
expect(powershellArgs).toEqual(
|
||||
expect.arrayContaining([
|
||||
'-NoProfile',
|
||||
'-NonInteractive',
|
||||
'-ExecutionPolicy',
|
||||
'Bypass',
|
||||
'C:\\Users\\me\\.orca\\secret.json',
|
||||
'S-1-5-21-1000',
|
||||
'0'
|
||||
])
|
||||
)
|
||||
const script = (powershellArgs as string[])[5]!
|
||||
expect(script).toContain('SetAccessRuleProtection($true, $false)')
|
||||
expect(script).toContain('RemoveAccessRuleSpecific')
|
||||
expect(script).toContain('Unexpected ACL entry')
|
||||
expect(powershellOptions).toEqual(expect.objectContaining({ windowsHide: true, timeout: 5000 }))
|
||||
})
|
||||
|
||||
it('adds inheritable rules when hardening a Windows directory', () => {
|
||||
hardenSecurePath('C:\\Users\\me\\.orca', { isDirectory: true, platform: 'win32' })
|
||||
|
||||
const powershellArgs = vi.mocked(execFile).mock.calls[0]![1] as string[]
|
||||
expect(powershellArgs.at(-1)).toBe('1')
|
||||
expect(powershellArgs[5]).toContain('ContainerInherit')
|
||||
expect(powershellArgs[5]).toContain('ObjectInherit')
|
||||
})
|
||||
|
||||
it('keeps Windows hardening best-effort when ACL rewriting fails', () => {
|
||||
// Simulate async PowerShell failure — the callback receives an error
|
||||
vi.mocked(execFile).mockImplementationOnce((_file, _args, _opts, callback) => {
|
||||
if (typeof callback === 'function') {
|
||||
callback(new Error('access denied'), '', '')
|
||||
}
|
||||
return {} as ReturnType<typeof execFile>
|
||||
expect(vi.mocked(runProcessSync).mock.calls[0]![0]).toMatchObject({
|
||||
program: 'C:\\Windows\\System32\\whoami.exe',
|
||||
args: ['/user', '/fo', 'csv', '/nh']
|
||||
})
|
||||
|
||||
const specs = vi.mocked(runProcess).mock.calls.map(([spec]) => spec)
|
||||
expect(specs.every((spec) => spec.program === 'C:\\Windows\\System32\\icacls.exe')).toBe(true)
|
||||
// Verify runs first, so an already-correct DACL is never rewritten.
|
||||
expect(specs[0]!.args?.slice(0, 2)).toEqual(['C:\\Users\\me\\.orca\\secret.json', '/save'])
|
||||
expect(specs[1]!.args).toEqual(['C:\\Users\\me\\.orca\\secret.json', '/reset', '/q'])
|
||||
expect(specs[2]!.args).toEqual([
|
||||
'C:\\Users\\me\\.orca\\secret.json',
|
||||
'/inheritance:r',
|
||||
'/grant:r',
|
||||
`*${USER_SID}:(F)`,
|
||||
'/grant:r',
|
||||
'*S-1-5-18:(F)',
|
||||
'/grant:r',
|
||||
'*S-1-5-32-544:(F)',
|
||||
'/q'
|
||||
])
|
||||
// The apply is read back: a loosened ACL has to be detectable, not just overwritten.
|
||||
expect(specs[3]!.args?.slice(0, 2)).toEqual(['C:\\Users\\me\\.orca\\secret.json', '/save'])
|
||||
expect(specs[2]!.timeoutMs).toBe(5000)
|
||||
})
|
||||
|
||||
// BLOCKING 1: re-running /reset on an already-correct DACL restores the inherited (broader) one
|
||||
// for the few ms until the grant pass lands, for no gain. A correct DACL must be left alone.
|
||||
it('leaves an already-correct ACL untouched instead of rewriting it', async () => {
|
||||
const target = 'C:\\Users\\me\\.orca\\secret.json'
|
||||
hardenedByFake.set(target, '')
|
||||
|
||||
hardenSecurePath(target, { isDirectory: false, platform: 'win32' })
|
||||
await flushAsyncAcl()
|
||||
|
||||
const specs = vi.mocked(runProcess).mock.calls.map(([spec]) => spec)
|
||||
expect(specs).toHaveLength(1)
|
||||
expect(specs[0]!.args).toContain('/save')
|
||||
expect(specs.some((spec) => spec.args?.includes('/reset'))).toBe(false)
|
||||
expect(specs.some((spec) => spec.args?.includes('/grant:r'))).toBe(false)
|
||||
})
|
||||
|
||||
// BLOCKING 3: the verify pass must check *identity*, not just rule count, inheritance and rights.
|
||||
// Granting Everyone full control satisfies all three of those and is the failure it exists for.
|
||||
it.each([
|
||||
['full control to Everyone', 'D:PAI(A;;FA;;;BA)(A;;FA;;;SY)(A;;FA;;;WD)', 'S-1-1-0'],
|
||||
['a deny rule', `D:PAI(D;;FA;;;BA)(A;;FA;;;SY)(A;;FA;;;${USER_SID})`, 'unexpected D rule'],
|
||||
['an unprotected DACL', `D:AI(A;;FA;;;BA)(A;;FA;;;SY)(A;;FA;;;${USER_SID})`, 'not protected'],
|
||||
[
|
||||
'a surviving inherited rule',
|
||||
`D:PAI(A;ID;FA;;;BA)(A;;FA;;;SY)(A;;FA;;;${USER_SID})`,
|
||||
'inherited'
|
||||
],
|
||||
['read-only rights', `D:PAI(A;;FR;;;BA)(A;;FA;;;SY)(A;;FA;;;${USER_SID})`, 'not full control']
|
||||
])('rejects a verified DACL granting %s', async (_label, sddl, expected) => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const target = 'C:\\Users\\me\\.orca\\secret.json'
|
||||
forcedBadSddl.set(target, sddl)
|
||||
|
||||
hardenSecurePath(target, { isDirectory: false, platform: 'win32' })
|
||||
await flushAsyncAcl()
|
||||
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
'[secure-path.windows-acl] failed to restrict path',
|
||||
expect.objectContaining({
|
||||
stage: 'verify',
|
||||
detail: expect.stringContaining(expected)
|
||||
})
|
||||
)
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
/**
|
||||
* Evicting the cache on every failed apply is the #4901 storm wearing a different hat: the env
|
||||
* store re-hardens on the *read* path at ~2/s, so on a host where hardening legitimately cannot
|
||||
* work (FAT32, network path, restricted token) that is two icacls spawns and two warnings a
|
||||
* second, forever.
|
||||
*
|
||||
* The curve itself is pinned in secure-path-hardening-retry-budget.test.ts; what matters here is
|
||||
* that the read path is actually wired to it.
|
||||
*/
|
||||
it('collapses a failing read-path poll to a single attempt', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const targetPath = writeFailingHardenTarget()
|
||||
|
||||
for (let read = 0; read < 25; read++) {
|
||||
hardenExistingSecureFile(targetPath)
|
||||
await flushAsyncAcl()
|
||||
}
|
||||
|
||||
expect(attemptsFor(targetPath)).toHaveLength(1)
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
/**
|
||||
* A budget that expires rather than latching: three transient failures used to abandon a path
|
||||
* for the life of the process, so one AV scan or momentary lock left every later credential
|
||||
* write unprotected on a host where hardening would now succeed.
|
||||
*/
|
||||
it('re-probes a long-failing path once its backoff has elapsed', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const targetPath = writeFailingHardenTarget()
|
||||
let clock = performance.now()
|
||||
const now = vi.spyOn(performance, 'now').mockImplementation(() => clock)
|
||||
|
||||
// A day of failing, well past any fixed cap, stepping by more than the 30-minute ceiling.
|
||||
for (let step = 0; step < 48; step++) {
|
||||
hardenExistingSecureFile(targetPath)
|
||||
await flushAsyncAcl()
|
||||
clock += 31 * 60_000
|
||||
}
|
||||
|
||||
expect(attemptsFor(targetPath)).toHaveLength(48)
|
||||
now.mockRestore()
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
it('reports recovery when a previously throttled path hardens again', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const info = vi.spyOn(console, 'info').mockImplementation(() => {})
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const targetPath = writeFailingHardenTarget()
|
||||
let clock = performance.now()
|
||||
const now = vi.spyOn(performance, 'now').mockImplementation(() => clock)
|
||||
|
||||
// Three failures to reach the announced degraded state, each past its own backoff.
|
||||
for (const wait of [0, 61_000, 121_000]) {
|
||||
clock += wait
|
||||
hardenExistingSecureFile(targetPath)
|
||||
await flushAsyncAcl()
|
||||
}
|
||||
expect(throttleReports(warn, targetPath)).toHaveLength(1)
|
||||
|
||||
// The transient condition clears; the next re-probe must notice.
|
||||
clock += 5 * 60_000
|
||||
vi.mocked(runProcess).mockImplementation((spec) => Promise.resolve(fakeIcacls(spec)))
|
||||
hardenExistingSecureFile(targetPath)
|
||||
await flushAsyncAcl()
|
||||
|
||||
expect(info).toHaveBeenCalledWith(
|
||||
'[secure-path.windows-acl] path hardening recovered',
|
||||
expect.objectContaining({ targetPath, stage: 'recovered' })
|
||||
)
|
||||
now.mockRestore()
|
||||
info.mockRestore()
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
/**
|
||||
* The write path is exempt from the budget, but it was also invisible to it: a successful write
|
||||
* left the failure record standing, so the read path went on backing off for up to 30 minutes
|
||||
* after the host had demonstrably recovered, and no `recovered` transition came from this lane.
|
||||
*/
|
||||
it('clears the read-path backoff when the exempt write path succeeds', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const info = vi.spyOn(console, 'info').mockImplementation(() => {})
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const targetPath = writeFailingHardenTarget()
|
||||
let clock = performance.now()
|
||||
const now = vi.spyOn(performance, 'now').mockImplementation(() => clock)
|
||||
|
||||
// Three read-path failures: the path is throttled and its next re-probe is minutes away.
|
||||
for (const wait of [0, 61_000, 121_000]) {
|
||||
clock += wait
|
||||
hardenExistingSecureFile(targetPath)
|
||||
await flushAsyncAcl()
|
||||
}
|
||||
expect(mayAttemptHardening(targetPath)).toBe(false)
|
||||
|
||||
// The host recovers and a credential is written. The synchronous apply succeeds (runProcessSync
|
||||
// was never made to fail), so the read path must stop backing off.
|
||||
writeSecureFile(targetPath, 'contents')
|
||||
|
||||
expect(mayAttemptHardening(targetPath)).toBe(true)
|
||||
expect(info).toHaveBeenCalledWith(
|
||||
'[secure-path.windows-acl] path hardening recovered',
|
||||
expect.objectContaining({ targetPath, stage: 'recovered' })
|
||||
)
|
||||
now.mockRestore()
|
||||
info.mockRestore()
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
/**
|
||||
* The SID lookup's own one-minute latch, which is the read-path budget's twin and strictly
|
||||
* worse: a failed lookup makes the plan null, disabling the *synchronous write* path too — so
|
||||
* the write-path exemption that recovers the budget cannot recover this. Measured against the
|
||||
* wall clock, a backwards step held it shut for the whole length of the step.
|
||||
*/
|
||||
it('re-probes the user SID after a backwards clock step', () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
let clock = performance.now()
|
||||
const now = vi.spyOn(performance, 'now').mockImplementation(() => clock)
|
||||
let wallClock = Date.parse('2026-01-01T00:00:00Z')
|
||||
const wallNow = vi.spyOn(Date, 'now').mockImplementation(() => wallClock)
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||||
tempDirs.push(userDataPath)
|
||||
const targetPath = join(userDataPath, 'secret.json')
|
||||
let sidLookupFails = true
|
||||
vi.mocked(runProcessSync).mockImplementation((spec) => {
|
||||
if (spec.program === 'C:\\Windows\\System32\\whoami.exe') {
|
||||
return sidLookupFails ? { ...OK, code: 1 } : { ...OK, stdout: `"USER","${USER_SID}"` }
|
||||
}
|
||||
return fakeIcacls(spec)
|
||||
})
|
||||
|
||||
// No SID, so no plan, so hardening is off entirely — not merely throttled.
|
||||
expect(writeSecureFile(targetPath, 'first')).toBe(false)
|
||||
|
||||
// A minute of real time passes while the wall clock steps back a year.
|
||||
clock += 61_000
|
||||
wallClock -= 365 * 24 * 60 * 60_000
|
||||
sidLookupFails = false
|
||||
|
||||
expect(writeSecureFile(targetPath, 'second')).toBe(true)
|
||||
wallNow.mockRestore()
|
||||
now.mockRestore()
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
// Scoped to one path: the parent directory is hardened too, and reports its own transition.
|
||||
function throttleReports(warn: ReturnType<typeof vi.spyOn>, targetPath: string): unknown[] {
|
||||
return warn.mock.calls.filter((call) => {
|
||||
const entry = call[1] as { stage?: string; targetPath?: string } | undefined
|
||||
return entry?.stage === 'throttled' && entry.targetPath === targetPath
|
||||
})
|
||||
}
|
||||
|
||||
function writeFailingHardenTarget(): string {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||||
tempDirs.push(userDataPath)
|
||||
const targetPath = join(userDataPath, 'secret.json')
|
||||
writeFileSync(targetPath, '{}')
|
||||
vi.mocked(runProcess).mockResolvedValue({ ...OK, code: 5, stderr: 'Access is denied.' })
|
||||
return targetPath
|
||||
}
|
||||
|
||||
function attemptsFor(targetPath: string): { args?: readonly string[] }[] {
|
||||
return getHardenAclCalls().filter((spec) => getAclTarget(spec) === targetPath)
|
||||
}
|
||||
|
||||
// /c makes icacls exit 0 while printing "Failed processing 1 files" — a silent no-op by another route.
|
||||
it('never passes the icacls /c continue-on-error flag', async () => {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||||
tempDirs.push(userDataPath)
|
||||
// Cover both runners: the write path is synchronous, the directory re-harden is not.
|
||||
writeSecureFile(join(userDataPath, 'secret.json'), 'contents')
|
||||
hardenSecurePath('C:\\Users\\me\\.orca\\other.json', {
|
||||
isDirectory: false,
|
||||
platform: 'win32'
|
||||
})
|
||||
await flushAsyncAcl()
|
||||
|
||||
const specs = [
|
||||
...vi.mocked(runProcess).mock.calls.map(([spec]) => spec),
|
||||
...vi.mocked(runProcessSync).mock.calls.map(([spec]) => spec)
|
||||
]
|
||||
expect(specs.length).toBeGreaterThan(4)
|
||||
for (const spec of specs) {
|
||||
expect(spec.args).not.toContain('/c')
|
||||
}
|
||||
})
|
||||
|
||||
it('adds inheritable rules when hardening a Windows directory', async () => {
|
||||
hardenSecurePath('C:\\Users\\me\\.orca', { isDirectory: true, platform: 'win32' })
|
||||
await flushAsyncAcl()
|
||||
|
||||
const grantArgs = vi
|
||||
.mocked(runProcess)
|
||||
.mock.calls.map(([spec]) => spec.args as string[])
|
||||
.find((args) => args.includes('/grant:r'))!
|
||||
expect(grantArgs).toContain(`*${USER_SID}:(OI)(CI)(F)`)
|
||||
expect(grantArgs).toContain('*S-1-5-18:(OI)(CI)(F)')
|
||||
})
|
||||
|
||||
it('keeps Windows hardening best-effort when ACL rewriting fails', async () => {
|
||||
vi.mocked(runProcess).mockRejectedValue(new Error('access denied'))
|
||||
|
||||
expect(() =>
|
||||
hardenSecurePath('C:\\Users\\me\\.orca\\secret.json', {
|
||||
isDirectory: false,
|
||||
platform: 'win32'
|
||||
})
|
||||
).not.toThrow()
|
||||
await expect(flushAsyncAcl()).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
// The old PowerShell command line never reached the grant step at all, so a failure had to be
|
||||
// visible somewhere; "best effort" may not mean "undetectable".
|
||||
it('logs when a Windows ACL apply fails instead of swallowing it', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
vi.mocked(runProcess).mockResolvedValue({ ...OK, code: 5, stderr: 'Access is denied.' })
|
||||
|
||||
hardenSecurePath('C:\\Users\\me\\.orca\\secret.json', {
|
||||
isDirectory: false,
|
||||
platform: 'win32'
|
||||
})
|
||||
await flushAsyncAcl()
|
||||
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
'[secure-path.windows-acl] failed to restrict path',
|
||||
expect.objectContaining({
|
||||
targetPath: 'C:\\Users\\me\\.orca\\secret.json',
|
||||
stage: 'reset',
|
||||
detail: 'Access is denied.'
|
||||
})
|
||||
)
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
it('reports a failed synchronous ACL apply to the caller and the log', () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
vi.mocked(runProcessSync).mockImplementation((spec) => {
|
||||
if (spec.program === 'C:\\Windows\\System32\\whoami.exe') {
|
||||
return { ...OK, stdout: '"USER","S-1-5-21-1000"' }
|
||||
}
|
||||
return { ...OK, code: 5, stderr: 'Access is denied.' }
|
||||
})
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||||
tempDirs.push(userDataPath)
|
||||
|
||||
writeSecureFile(join(userDataPath, 'secret.json'), 'contents')
|
||||
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
'[secure-path.windows-acl] failed to restrict path',
|
||||
expect.objectContaining({ stage: 'reset', detail: 'Access is denied.' })
|
||||
)
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
// Paths past MAX_PATH make icacls report "cannot find the path specified"; the extended prefix is the escape.
|
||||
it('uses the extended-length prefix for paths past MAX_PATH', async () => {
|
||||
const longPath = `C:\\Users\\me\\.orca\\${'d'.repeat(300)}\\secret.json`
|
||||
hardenSecurePath(longPath, { isDirectory: false, platform: 'win32' })
|
||||
await flushAsyncAcl()
|
||||
|
||||
for (const [spec] of vi.mocked(runProcess).mock.calls) {
|
||||
expect(spec.args![0]).toBe(`\\\\?\\${longPath}`)
|
||||
}
|
||||
})
|
||||
|
||||
it('caches successful existing-file hardening within a process', () => {
|
||||
@@ -142,8 +480,8 @@ describe('hardenSecurePath', () => {
|
||||
hardenExistingSecureFile(targetPath)
|
||||
|
||||
// dir hardened once (path-cached), file hardened once (metadata-cached) — 2 total
|
||||
expect(getPowerShellCalls()).toHaveLength(2)
|
||||
expect(getPowerShellCalls().map(getPowerShellTarget)).toEqual([userDataPath, targetPath])
|
||||
expect(getHardenAclCalls()).toHaveLength(2)
|
||||
expect(getHardenAclCalls().map(getAclTarget)).toEqual([userDataPath, targetPath])
|
||||
})
|
||||
|
||||
it('LRU-evicts Windows file hardening entries and safely re-hardens an evicted path', () => {
|
||||
@@ -165,8 +503,8 @@ describe('hardenSecurePath', () => {
|
||||
|
||||
hardenExistingSecureFile(paths[0]!)
|
||||
|
||||
const fileTargets = getPowerShellCalls()
|
||||
.map(getPowerShellTarget)
|
||||
const fileTargets = getHardenAclCalls()
|
||||
.map(getAclTarget)
|
||||
.filter((path) => paths.includes(path))
|
||||
expect(fileTargets).toEqual([...paths, paths[0]])
|
||||
expect(__getSecureFileHardeningCacheStateForTests().paths).toMatchObject({
|
||||
@@ -196,8 +534,8 @@ describe('hardenSecurePath', () => {
|
||||
|
||||
hardenExistingSecureFile(files[0]!)
|
||||
|
||||
const directoryTargets = getPowerShellCalls()
|
||||
.map(getPowerShellTarget)
|
||||
const directoryTargets = getHardenAclCalls()
|
||||
.map(getAclTarget)
|
||||
.filter((path) => directories.includes(path))
|
||||
expect(directoryTargets).toEqual([...directories, directories[0]])
|
||||
expect(__getSecureFileHardeningCacheStateForTests().directories).toMatchObject({
|
||||
@@ -218,12 +556,8 @@ describe('hardenSecurePath', () => {
|
||||
hardenExistingSecureFile(targetPath)
|
||||
|
||||
// call 1: dir + file. call 2: dir skipped (path-cached), file re-hardened (new mtime)
|
||||
expect(getPowerShellCalls()).toHaveLength(3)
|
||||
expect(getPowerShellCalls().map(getPowerShellTarget)).toEqual([
|
||||
userDataPath,
|
||||
targetPath,
|
||||
targetPath
|
||||
])
|
||||
expect(getHardenAclCalls()).toHaveLength(3)
|
||||
expect(getHardenAclCalls().map(getAclTarget)).toEqual([userDataPath, targetPath, targetPath])
|
||||
})
|
||||
|
||||
it('keeps post-rename target hardening on every write while caching the directory', () => {
|
||||
@@ -236,19 +570,19 @@ describe('hardenSecurePath', () => {
|
||||
writeSecureFile(targetPath, 'second')
|
||||
|
||||
// The DIRECTORY is hardened async + path-cached: exactly once across both writes.
|
||||
const asyncTargets = getPowerShellCalls().map(getPowerShellTarget)
|
||||
const asyncTargets = getHardenAclCalls().map(getAclTarget)
|
||||
expect(asyncTargets).toEqual([userDataPath])
|
||||
|
||||
// The credential FILES (tmpFile + renamed target) are hardened SYNCHRONOUSLY on each write.
|
||||
// write 1: tmpFile(1) + targetFile(1) = 2; write 2: tmpFile(1) + targetFile(1) = 2; total 4.
|
||||
const syncTargets = getSyncPowerShellCalls().map(getPowerShellTarget)
|
||||
const syncTargets = getSyncHardenAclCalls().map(getAclTarget)
|
||||
expect(syncTargets).toHaveLength(4)
|
||||
expect(syncTargets.filter((entry) => entry === targetPath)).toHaveLength(2)
|
||||
// No directory should be hardened via the synchronous path.
|
||||
expect(syncTargets.filter((entry) => entry === userDataPath)).toHaveLength(0)
|
||||
})
|
||||
|
||||
// Regression test: #4901 — env-store reads at ~2×/s caused a PowerShell storm because the
|
||||
// Regression test: #4901 — env-store reads at ~2×/s caused an ACL-spawn storm because the
|
||||
// parent directory mtime churned (every secure write updates it), so the mtime-keyed cache
|
||||
// never matched. Directories must be path-cached for the process lifetime.
|
||||
it('does not re-harden the parent directory when its mtime changes between reads', async () => {
|
||||
@@ -268,9 +602,7 @@ describe('hardenSecurePath', () => {
|
||||
hardenExistingSecureFile(targetPath)
|
||||
|
||||
// The parent directory must be hardened exactly ONCE despite its mtime changing
|
||||
const dirCalls = getPowerShellCalls().filter(
|
||||
(call) => getPowerShellTarget(call) === userDataPath
|
||||
)
|
||||
const dirCalls = getHardenAclCalls().filter((call) => getAclTarget(call) === userDataPath)
|
||||
expect(dirCalls).toHaveLength(1)
|
||||
})
|
||||
|
||||
@@ -285,27 +617,25 @@ describe('hardenSecurePath', () => {
|
||||
hardenExistingSecureFile(targetPath)
|
||||
hardenExistingSecureFile(targetPath)
|
||||
|
||||
const fileCalls = getPowerShellCalls().filter(
|
||||
(call) => getPowerShellTarget(call) === targetPath
|
||||
)
|
||||
const fileCalls = getHardenAclCalls().filter((call) => getAclTarget(call) === targetPath)
|
||||
expect(fileCalls).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('applies the read-path ACL asynchronously without blocking (async execFile)', () => {
|
||||
it('applies the read-path ACL asynchronously without blocking (async runProcess)', () => {
|
||||
hardenSecurePath('C:\\Users\\me\\.orca\\secret.json', {
|
||||
isDirectory: false,
|
||||
platform: 'win32'
|
||||
})
|
||||
|
||||
// The default (read/dir) path must launch PowerShell via execFile (async), never sync.
|
||||
expect(getSyncPowerShellCalls()).toHaveLength(0)
|
||||
expect(getPowerShellCalls()).toHaveLength(1)
|
||||
// The default (read/dir) path must launch icacls via runProcess (async), never sync.
|
||||
expect(getSyncHardenAclCalls()).toHaveLength(0)
|
||||
expect(getHardenAclCalls()).toHaveLength(1)
|
||||
})
|
||||
|
||||
// Security regression guard (#5006 review finding): writeSecureFile must restrict the
|
||||
// credential FILE's ACL SYNCHRONOUSLY before returning. On Windows writeFileSync({mode})
|
||||
// is a no-op, so an async file ACL would leave the credential briefly readable under the
|
||||
// parent's inherited (broader) ACL for the ~1-1.5s PowerShell cold-start window.
|
||||
// parent's inherited (broader) ACL for the duration of the spawn.
|
||||
it('hardens the credential file synchronously while keeping the directory async', () => {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||||
@@ -315,13 +645,13 @@ describe('hardenSecurePath', () => {
|
||||
writeSecureFile(targetPath, 'contents')
|
||||
|
||||
// Directory: async only.
|
||||
expect(getPowerShellCalls().map(getPowerShellTarget)).toEqual([userDataPath])
|
||||
expect(getHardenAclCalls().map(getAclTarget)).toEqual([userDataPath])
|
||||
// File (tmpFile + renamed target): synchronous only — no async file ACL window.
|
||||
const syncTargets = getSyncPowerShellCalls().map(getPowerShellTarget)
|
||||
const syncTargets = getSyncHardenAclCalls().map(getAclTarget)
|
||||
expect(syncTargets).toContain(targetPath)
|
||||
expect(syncTargets.filter((entry) => entry === userDataPath)).toHaveLength(0)
|
||||
// The final published target's ACL must have been applied via the synchronous path.
|
||||
expect(getPowerShellCalls().map(getPowerShellTarget)).not.toContain(targetPath)
|
||||
expect(getHardenAclCalls().map(getAclTarget)).not.toContain(targetPath)
|
||||
})
|
||||
|
||||
// Nit #1 (review): the synchronous file path must cache as hardened ONLY on confirmed
|
||||
@@ -333,30 +663,30 @@ describe('hardenSecurePath', () => {
|
||||
tempDirs.push(userDataPath)
|
||||
const targetPath = join(userDataPath, 'secret.json')
|
||||
|
||||
// First write: the synchronous PowerShell ACL apply throws for every powershell call.
|
||||
vi.mocked(execFileSync).mockImplementation((file) => {
|
||||
if (file === 'C:\\Windows\\System32\\whoami.exe') {
|
||||
return '"USER","S-1-5-21-1000"'
|
||||
// First write: the synchronous icacls ACL apply throws for every icacls call.
|
||||
vi.mocked(runProcessSync).mockImplementation((spec) => {
|
||||
if (spec.program === 'C:\\Windows\\System32\\whoami.exe') {
|
||||
return { ...OK, stdout: '"USER","S-1-5-21-1000"' }
|
||||
}
|
||||
throw new Error('access denied')
|
||||
})
|
||||
expect(() => writeSecureFile(targetPath, 'first')).not.toThrow()
|
||||
const firstWriteTargetCalls = getSyncPowerShellCalls()
|
||||
.map(getPowerShellTarget)
|
||||
const firstWriteTargetCalls = getSyncHardenAclCalls()
|
||||
.map(getAclTarget)
|
||||
.filter((entry) => entry === targetPath)
|
||||
expect(firstWriteTargetCalls).toHaveLength(1)
|
||||
|
||||
// Second write: ACL apply now succeeds. Because the failed apply was NOT cached, the
|
||||
// target file is hardened again rather than skipped.
|
||||
vi.mocked(execFileSync).mockImplementation((file) => {
|
||||
if (file === 'C:\\Windows\\System32\\whoami.exe') {
|
||||
return '"USER","S-1-5-21-1000"'
|
||||
vi.mocked(runProcessSync).mockImplementation((spec) => {
|
||||
if (spec.program === 'C:\\Windows\\System32\\whoami.exe') {
|
||||
return { ...OK, stdout: '"USER","S-1-5-21-1000"' }
|
||||
}
|
||||
return ''
|
||||
return OK
|
||||
})
|
||||
writeSecureFile(targetPath, 'second')
|
||||
const allTargetCalls = getSyncPowerShellCalls()
|
||||
.map(getPowerShellTarget)
|
||||
const allTargetCalls = getSyncHardenAclCalls()
|
||||
.map(getAclTarget)
|
||||
.filter((entry) => entry === targetPath)
|
||||
expect(allTargetCalls).toHaveLength(2)
|
||||
})
|
||||
@@ -374,15 +704,13 @@ describe('hardenSecurePath', () => {
|
||||
writeSecureFile(join(userDataPath, `secret-${i}.json`), `contents-${i}`)
|
||||
}
|
||||
|
||||
const dirCalls = getPowerShellCalls().filter(
|
||||
(call) => getPowerShellTarget(call) === userDataPath
|
||||
)
|
||||
const dirCalls = getHardenAclCalls().filter((call) => getAclTarget(call) === userDataPath)
|
||||
expect(dirCalls).toHaveLength(1)
|
||||
})
|
||||
|
||||
// win32-only guard: on non-win32 platforms no PowerShell is ever spawned (sync or async);
|
||||
// win32-only guard: on non-win32 platforms no icacls is ever spawned (sync or async);
|
||||
// POSIX hardening uses chmodSync only.
|
||||
it('never spawns PowerShell on non-win32 platforms', () => {
|
||||
it('never spawns icacls on non-win32 platforms', () => {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' })
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||||
tempDirs.push(userDataPath)
|
||||
@@ -391,8 +719,8 @@ describe('hardenSecurePath', () => {
|
||||
writeSecureFile(targetPath, 'contents')
|
||||
hardenExistingSecureFile(targetPath)
|
||||
|
||||
expect(getPowerShellCalls()).toHaveLength(0)
|
||||
expect(getSyncPowerShellCalls()).toHaveLength(0)
|
||||
expect(getHardenAclCalls()).toHaveLength(0)
|
||||
expect(getSyncHardenAclCalls()).toHaveLength(0)
|
||||
})
|
||||
|
||||
posixModeIt('re-hardens a POSIX directory when its metadata changes after caching', () => {
|
||||
@@ -440,22 +768,51 @@ describe('hardenSecurePath', () => {
|
||||
})
|
||||
})
|
||||
|
||||
const POWERSHELL_SUFFIX = 'WindowsPowerShell\\v1.0\\powershell.exe'
|
||||
|
||||
// Async PowerShell calls (directory hardening + read-path file re-harden).
|
||||
function getPowerShellCalls(): unknown[][] {
|
||||
return vi.mocked(execFile).mock.calls.filter(([file]) => String(file).endsWith(POWERSHELL_SUFFIX))
|
||||
/**
|
||||
* Every harden opens with a `/save` verify; one that has work to do then runs `/reset`, `/grant:r`
|
||||
* and a closing `/save`. Counting only the *opening* verify keeps "one harden = one entry"
|
||||
* regardless of which of the two shapes it took.
|
||||
*/
|
||||
function hardenInitiations(specs: FakeSpec[]): { args?: readonly string[] }[] {
|
||||
const initiations: { args?: readonly string[] }[] = []
|
||||
const awaitingClosingVerify = new Set<string>()
|
||||
for (const spec of specs) {
|
||||
if (!spec.program.endsWith('icacls.exe')) {
|
||||
continue
|
||||
}
|
||||
const path = spec.args?.[0] ?? ''
|
||||
if (spec.args?.includes('/grant:r')) {
|
||||
awaitingClosingVerify.add(path)
|
||||
} else if (spec.args?.includes('/save')) {
|
||||
if (awaitingClosingVerify.has(path)) {
|
||||
awaitingClosingVerify.delete(path)
|
||||
} else {
|
||||
initiations.push(spec)
|
||||
}
|
||||
}
|
||||
}
|
||||
return initiations
|
||||
}
|
||||
|
||||
// Synchronous PowerShell calls (credential-file ACL on the write path).
|
||||
function getSyncPowerShellCalls(): unknown[][] {
|
||||
return vi
|
||||
.mocked(execFileSync)
|
||||
.mock.calls.filter(([file]) => String(file).endsWith(POWERSHELL_SUFFIX))
|
||||
// Async icacls calls (directory hardening + read-path file re-harden).
|
||||
function getHardenAclCalls(): { args?: readonly string[] }[] {
|
||||
return hardenInitiations(vi.mocked(runProcess).mock.calls.map(([spec]) => spec))
|
||||
}
|
||||
|
||||
function getPowerShellTarget(call: unknown[]): string {
|
||||
return (call[1] as string[])[6]!
|
||||
// Synchronous icacls calls (credential-file ACL on the write path).
|
||||
function getSyncHardenAclCalls(): { args?: readonly string[] }[] {
|
||||
return hardenInitiations(vi.mocked(runProcessSync).mock.calls.map(([spec]) => spec))
|
||||
}
|
||||
|
||||
function getAclTarget(spec: { args?: readonly string[] }): string {
|
||||
return spec.args![0]!
|
||||
}
|
||||
|
||||
// The async harden awaits three icacls passes, so let the chain settle before asserting on it.
|
||||
async function flushAsyncAcl(): Promise<void> {
|
||||
for (let i = 0; i < 4; i++) {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0))
|
||||
}
|
||||
}
|
||||
|
||||
async function waitForFileTimestampTick(): Promise<void> {
|
||||
@@ -465,3 +822,37 @@ async function waitForFileTimestampTick(): Promise<void> {
|
||||
function statMode(path: string): number {
|
||||
return statSync(path).mode & 0o777
|
||||
}
|
||||
|
||||
describe('isUnreadableError', () => {
|
||||
const withCode = (code: string): NodeJS.ErrnoException => Object.assign(new Error(code), { code })
|
||||
|
||||
// The hardened-DACL case this predicate was written for.
|
||||
it('reports a denied read', () => {
|
||||
expect(isUnreadableError(withCode('EPERM'))).toBe(true)
|
||||
expect(isUnreadableError(withCode('EACCES'))).toBe(true)
|
||||
})
|
||||
|
||||
/**
|
||||
* The likelier half on Windows: antivirus holding a credential open during startup yields
|
||||
* EBUSY, and fd exhaustion yields EMFILE. Neither says the bytes were read, so neither may
|
||||
* license a regenerate-and-overwrite.
|
||||
*/
|
||||
it('reports a read that never reached the contents for any other reason', () => {
|
||||
expect(isUnreadableError(withCode('EBUSY'))).toBe(true)
|
||||
expect(isUnreadableError(withCode('EMFILE'))).toBe(true)
|
||||
expect(isUnreadableError(withCode('ENFILE'))).toBe(true)
|
||||
expect(isUnreadableError(withCode('EIO'))).toBe(true)
|
||||
})
|
||||
|
||||
/**
|
||||
* The other side of the distinction, and the reason this is an allow list rather than
|
||||
* "everything except ENOENT": a missing file licenses creating one, and bytes that were read
|
||||
* and did not parse are the self-heal these stores exist to perform.
|
||||
*/
|
||||
it('does not report a missing file or a parse failure', () => {
|
||||
expect(isUnreadableError(withCode('ENOENT'))).toBe(false)
|
||||
expect(isUnreadableError(new SyntaxError('Unexpected end of JSON input'))).toBe(false)
|
||||
expect(isUnreadableError(withCode('EISDIR'))).toBe(false)
|
||||
expect(isUnreadableError(undefined)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
+110
-30
@@ -13,9 +13,15 @@ import {
|
||||
} from 'node:fs'
|
||||
import { dirname } from 'node:path'
|
||||
import {
|
||||
DEFAULT_HARDENING_CACHE_BOUNDS,
|
||||
SecurePathHardeningCache,
|
||||
type SecurePathHardeningCacheBounds
|
||||
} from './secure-path-hardening-cache'
|
||||
import {
|
||||
configureHardeningRetryBudget,
|
||||
mayAttemptHardening,
|
||||
recordHardeningOutcome
|
||||
} from './secure-path-hardening-retry-budget'
|
||||
import {
|
||||
bestEffortRestrictWindowsPath,
|
||||
resetSecureFileWindowsUserSidForTests,
|
||||
@@ -33,24 +39,14 @@ type HardenedPathCacheEntry = {
|
||||
birthtimeMs: number
|
||||
}
|
||||
|
||||
export const SECURE_PATH_HARDENING_CACHE_MAX_ENTRIES = 1024
|
||||
export const SECURE_PATH_HARDENING_CACHE_KEY_MAX_BYTES = 64 * 1024
|
||||
export const SECURE_PATH_HARDENING_CACHE_KEYS_MAX_BYTES = 512 * 1024
|
||||
|
||||
const DEFAULT_HARDENING_CACHE_BOUNDS: SecurePathHardeningCacheBounds = {
|
||||
maxEntries: SECURE_PATH_HARDENING_CACHE_MAX_ENTRIES,
|
||||
maxKeyBytes: SECURE_PATH_HARDENING_CACHE_KEY_MAX_BYTES,
|
||||
maxTotalKeyBytes: SECURE_PATH_HARDENING_CACHE_KEYS_MAX_BYTES
|
||||
}
|
||||
|
||||
const UNSUPPORTED_DIRECTORY_FSYNC_CODES = new Set(['EINVAL', 'ENOTSUP', 'EOPNOTSUPP'])
|
||||
|
||||
// Why: PowerShell hardening (~1-1.5s) stalls the main thread, so cache idempotent re-hardens per process.
|
||||
// Why: hardening spawns icacls synchronously (once when the DACL already verifies, four times when it must be rewritten), so cache idempotent re-hardens per process.
|
||||
let hardenedPathsThisProcess = new SecurePathHardeningCache<HardenedPathCacheEntry>(
|
||||
DEFAULT_HARDENING_CACHE_BOUNDS
|
||||
)
|
||||
|
||||
// Why: child writes constantly bump a dir's mtime, so cache dirs by path (not metadata) to avoid a PowerShell spawn every read (#4901).
|
||||
// Why: child writes constantly bump a dir's mtime, so cache dirs by path (not metadata) to avoid an icacls spawn every read (#4901).
|
||||
// Limitation: a dir deleted+recreated in-process won't re-harden; fine since we never delete our secure dirs at runtime.
|
||||
let hardenedDirectoryPathsThisProcess = new SecurePathHardeningCache<true>(
|
||||
DEFAULT_HARDENING_CACHE_BOUNDS
|
||||
@@ -61,9 +57,13 @@ function hardenSecureDirectoryOnce(dirPath: string): void {
|
||||
if (hardenedDirectoryPathsThisProcess.get(dirPath)) {
|
||||
return
|
||||
}
|
||||
applySecurePathRestriction(dirPath, true, process.platform, false)
|
||||
// Cache even though the async ACL may still be in flight — dir restriction is best-effort, no retry.
|
||||
// Cache before the ACL lands so concurrent writes don't restorm; a failure drops it, under the retry budget.
|
||||
hardenedDirectoryPathsThisProcess.set(dirPath, true)
|
||||
applySecurePathRestriction(dirPath, true, process.platform, false, (restricted) => {
|
||||
if (!restricted) {
|
||||
hardenedDirectoryPathsThisProcess.delete(dirPath)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
function hardenSecurePathOnce(targetPath: string, isDirectory: boolean): boolean {
|
||||
@@ -81,26 +81,50 @@ function hardenSecurePathOnce(targetPath: string, isDirectory: boolean): boolean
|
||||
return true
|
||||
}
|
||||
// Why: async re-harden is safe here — read path hardens each file at most once/process; new files harden synchronously on the write path.
|
||||
if (applySecurePathRestriction(targetPath, isDirectory, process.platform, false)) {
|
||||
const outcome = applySecurePathRestriction(
|
||||
targetPath,
|
||||
isDirectory,
|
||||
process.platform,
|
||||
false,
|
||||
(restricted) => {
|
||||
if (!restricted) {
|
||||
hardenedPathsThisProcess.delete(targetPath)
|
||||
}
|
||||
}
|
||||
)
|
||||
if (outcome !== 'failed') {
|
||||
rememberHardenedPath(targetPath, isDirectory)
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
export function writeSecureJsonFile(targetPath: string, value: unknown): void {
|
||||
writeSecureFile(targetPath, JSON.stringify(value, null, 2))
|
||||
/** Returns false when the file was written but its permissions could not be restricted. */
|
||||
export function writeSecureJsonFile(targetPath: string, value: unknown): boolean {
|
||||
return writeSecureFile(targetPath, JSON.stringify(value, null, 2))
|
||||
}
|
||||
|
||||
export function writeDurableSecureJsonFile(targetPath: string, value: unknown): void {
|
||||
writeSecureFile(targetPath, JSON.stringify(value, null, 2), { durable: true })
|
||||
/** Returns false when the file was written but its permissions could not be restricted. */
|
||||
export function writeDurableSecureJsonFile(targetPath: string, value: unknown): boolean {
|
||||
return writeSecureFile(targetPath, JSON.stringify(value, null, 2), { durable: true })
|
||||
}
|
||||
|
||||
/**
|
||||
* Writes `contents` and restricts the result to the current user.
|
||||
*
|
||||
* Returns whether the restriction actually took. Hardening stays best-effort — it fails
|
||||
* legitimately on FAT32, network paths and restricted tokens, and must not break a write — but
|
||||
* the outcome is now reported rather than assumed, so a caller storing a credential can react.
|
||||
*
|
||||
* The return value covers the *file* only. The parent directory is hardened fire-and-forget — on
|
||||
* Windows that lane is async and answers `pending` regardless — so a `true` here says nothing
|
||||
* about the directory's ACL.
|
||||
*/
|
||||
export function writeSecureFile(
|
||||
targetPath: string,
|
||||
contents: string,
|
||||
options: { durable?: boolean } = {}
|
||||
): void {
|
||||
): boolean {
|
||||
const dir = dirname(targetPath)
|
||||
if (!existsSync(dir)) {
|
||||
mkdirSync(dir, { recursive: true, mode: 0o700 })
|
||||
@@ -118,15 +142,18 @@ export function writeSecureFile(
|
||||
fsyncFileSync(tmpFile)
|
||||
}
|
||||
// Why: writeFileSync mode is a no-op on Windows, so restrict the credential's ACL synchronously before the rename publishes it under inherited ACLs.
|
||||
applySecurePathRestriction(tmpFile, false, process.platform, true)
|
||||
const stagedOutcome = applySecurePathRestriction(tmpFile, false, process.platform, true)
|
||||
renameSync(tmpFile, targetPath)
|
||||
// Why: these hold auth credentials, so the published path must stay current-user only; cache only on confirmed success so failures retry.
|
||||
if (applySecurePathRestriction(targetPath, false, process.platform, true)) {
|
||||
// The staged file's protected DACL survives the rename, so this pass usually just verifies it.
|
||||
const publishedOutcome = applySecurePathRestriction(targetPath, false, process.platform, true)
|
||||
if (publishedOutcome === 'applied') {
|
||||
rememberHardenedPath(targetPath, false)
|
||||
}
|
||||
if (options.durable) {
|
||||
bestEffortFsyncDirectorySync(dir)
|
||||
}
|
||||
return stagedOutcome === 'applied' && publishedOutcome === 'applied'
|
||||
} catch (error) {
|
||||
rmSync(tmpFile, { force: true })
|
||||
throw error
|
||||
@@ -164,6 +191,34 @@ export function bestEffortFsyncDirectorySync(directory: string): void {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Errors that mean the contents were never seen, so they say nothing about what the file holds.
|
||||
*
|
||||
* `ENOENT` is deliberately absent: "there is no file" genuinely licenses creating one. So is a
|
||||
* parse failure, which means the bytes WERE read and were garbage - the self-heal these stores
|
||||
* were built for. The distinction is "could not read it" versus "read it and it was garbage".
|
||||
*
|
||||
* Why it matters: a reader that treats every failure as corruption regenerates the file, and the
|
||||
* regeneration succeeds - `renameSync` over an unreadable file needs `FILE_DELETE_CHILD` on the
|
||||
* parent, not `DELETE` on the file - so the original is destroyed by the code meant to heal it.
|
||||
* `EPERM`/`EACCES` is the hardened-DACL case: a file granting a SID this process does not hold,
|
||||
* reachable through a relocated user-data path, a share or roaming profile, a restored backup
|
||||
* under a new local SID, or a half-applied harden. The rest are transient and, on Windows, more
|
||||
* likely than that: `EBUSY` is what antivirus produces by holding a file open at the moment of a
|
||||
* read, which for a credential read on the startup path is an ordinary Tuesday.
|
||||
*/
|
||||
export function isUnreadableError(error: unknown): boolean {
|
||||
const code = (error as NodeJS.ErrnoException | null)?.code
|
||||
return (
|
||||
code === 'EPERM' ||
|
||||
code === 'EACCES' ||
|
||||
code === 'EBUSY' ||
|
||||
code === 'EMFILE' ||
|
||||
code === 'ENFILE' ||
|
||||
code === 'EIO'
|
||||
)
|
||||
}
|
||||
|
||||
export function hardenExistingSecureFile(targetPath: string): void {
|
||||
const dir = dirname(targetPath)
|
||||
if (existsSync(dir)) {
|
||||
@@ -191,24 +246,48 @@ export function hardenSecurePath(
|
||||
)
|
||||
}
|
||||
|
||||
/** Applies hardening; async Windows calls only report that best-effort ACL work was accepted. */
|
||||
/**
|
||||
* `pending` is the honest answer for the async Windows branch: it has not happened yet, and
|
||||
* reporting it as `applied` is what let a dead ACL look like a working one. The real outcome
|
||||
* arrives through `onAsyncSettled`.
|
||||
*/
|
||||
type HardeningOutcome = 'applied' | 'pending' | 'failed'
|
||||
|
||||
function applySecurePathRestriction(
|
||||
targetPath: string,
|
||||
isDirectory: boolean,
|
||||
platform: NodeJS.Platform,
|
||||
sync: boolean
|
||||
): boolean {
|
||||
sync: boolean,
|
||||
onAsyncSettled?: (restricted: boolean) => void
|
||||
): HardeningOutcome {
|
||||
if (platform === 'win32') {
|
||||
if (sync) {
|
||||
// Why no retry floor here: the write path is user-driven, not polled, and a failed apply
|
||||
// must still be retried on the next write of the same credential.
|
||||
// Why: apply the ACL synchronously so the credential file isn't briefly readable under inherited ACLs (writeFileSync mode is a no-op on Windows).
|
||||
return restrictWindowsPathSync(targetPath, isDirectory)
|
||||
const restricted = restrictWindowsPathSync(targetPath, isDirectory)
|
||||
if (restricted) {
|
||||
// Success only: this is how a recovered host clears the read path's backoff (and reports
|
||||
// `recovered`). Recording a failure here would put the exempt lane back under the budget.
|
||||
recordHardeningOutcome(targetPath, true)
|
||||
}
|
||||
return restricted ? 'applied' : 'failed'
|
||||
}
|
||||
// Why: dir/read-path re-harden runs async to avoid blocking the main thread (#4901); return true optimistically since it's best-effort.
|
||||
bestEffortRestrictWindowsPath(targetPath, isDirectory)
|
||||
return true
|
||||
// Why the floor: this is the read path, polled at ~2/s (#4901). Retrying every failure there
|
||||
// is the same storm the cache exists to prevent.
|
||||
if (!mayAttemptHardening(targetPath)) {
|
||||
onAsyncSettled?.(false)
|
||||
return 'failed'
|
||||
}
|
||||
// Why: dir/read-path re-harden runs async to avoid blocking the main thread (#4901).
|
||||
bestEffortRestrictWindowsPath(targetPath, isDirectory, (restricted) => {
|
||||
recordHardeningOutcome(targetPath, restricted)
|
||||
onAsyncSettled?.(restricted)
|
||||
})
|
||||
return 'pending'
|
||||
}
|
||||
chmodSync(targetPath, isDirectory ? 0o700 : 0o600)
|
||||
return true
|
||||
return 'applied'
|
||||
}
|
||||
|
||||
/** Caches the current metadata snapshot for a just-hardened path, or clears it if the path is gone. */
|
||||
@@ -275,6 +354,7 @@ export function __resetSecureFileHardenedPathsForTests(
|
||||
): void {
|
||||
hardenedPathsThisProcess = new SecurePathHardeningCache(bounds)
|
||||
hardenedDirectoryPathsThisProcess = new SecurePathHardeningCache(bounds)
|
||||
configureHardeningRetryBudget(bounds)
|
||||
}
|
||||
|
||||
export function __getSecureFileHardeningCacheStateForTests(): {
|
||||
|
||||
@@ -4,6 +4,21 @@ export type SecurePathHardeningCacheBounds = {
|
||||
maxTotalKeyBytes: number
|
||||
}
|
||||
|
||||
export const SECURE_PATH_HARDENING_CACHE_MAX_ENTRIES = 1024
|
||||
export const SECURE_PATH_HARDENING_CACHE_KEY_MAX_BYTES = 64 * 1024
|
||||
export const SECURE_PATH_HARDENING_CACHE_KEYS_MAX_BYTES = 512 * 1024
|
||||
|
||||
/**
|
||||
* The bounds every hardening cache uses unless a caller overrides them. They live here rather
|
||||
* than with one consumer so a cache can default itself instead of depending on some other module
|
||||
* being imported first.
|
||||
*/
|
||||
export const DEFAULT_HARDENING_CACHE_BOUNDS: SecurePathHardeningCacheBounds = {
|
||||
maxEntries: SECURE_PATH_HARDENING_CACHE_MAX_ENTRIES,
|
||||
maxKeyBytes: SECURE_PATH_HARDENING_CACHE_KEY_MAX_BYTES,
|
||||
maxTotalKeyBytes: SECURE_PATH_HARDENING_CACHE_KEYS_MAX_BYTES
|
||||
}
|
||||
|
||||
type RetainedSecurePath<T> = {
|
||||
value: T
|
||||
keyBytes: number
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
/**
|
||||
* Where path-hardening outcomes are announced, kept apart from the code that applies them so the
|
||||
* retry budget can report degradation and recovery without importing the Windows ACL lane.
|
||||
*/
|
||||
export type SecurePathHardeningReport = {
|
||||
targetPath: string
|
||||
/**
|
||||
* `throttled` and `recovered` mark entering and leaving the rate-limited degraded state;
|
||||
* `settle` is the async lane's own callback failing, which is a caller bug rather than a host one.
|
||||
*/
|
||||
stage: 'sid-lookup' | 'reset' | 'grant' | 'verify' | 'settle' | 'throttled' | 'recovered'
|
||||
detail: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Why a hook: hardening runs in the Electron main process, which is GUI-subsystem on Windows and
|
||||
* owns no console, so `console.warn` reaches nothing in a packaged build. The main process
|
||||
* installs a reporter that routes into the diagnostic trace; the console default keeps dev runs
|
||||
* and the CLI readable.
|
||||
*/
|
||||
const consoleReporter = (entry: SecurePathHardeningReport): void => {
|
||||
if (entry.stage === 'recovered') {
|
||||
console.info('[secure-path.windows-acl] path hardening recovered', entry)
|
||||
return
|
||||
}
|
||||
console.warn('[secure-path.windows-acl] failed to restrict path', entry)
|
||||
}
|
||||
|
||||
let reportEntry: (entry: SecurePathHardeningReport) => void = consoleReporter
|
||||
|
||||
export function setSecurePathHardeningReporter(
|
||||
reporter: ((entry: SecurePathHardeningReport) => void) | null
|
||||
): void {
|
||||
reportEntry = reporter ?? consoleReporter
|
||||
}
|
||||
|
||||
export function reportSecurePathHardening(
|
||||
targetPath: string,
|
||||
stage: SecurePathHardeningReport['stage'],
|
||||
detail: string
|
||||
): void {
|
||||
reportEntry({ targetPath, stage, detail: detail.trim().slice(0, 500) })
|
||||
}
|
||||
@@ -0,0 +1,174 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
configureHardeningRetryBudget,
|
||||
hardeningRetryDelayMs,
|
||||
mayAttemptHardening,
|
||||
recordHardeningOutcome
|
||||
} from './secure-path-hardening-retry-budget'
|
||||
import {
|
||||
setSecurePathHardeningReporter,
|
||||
type SecurePathHardeningReport
|
||||
} from './secure-path-hardening-report'
|
||||
|
||||
const PATH = 'C:\\Users\\me\\.orca\\secret.json'
|
||||
const OTHER = 'C:\\Users\\me\\.orca\\other.json'
|
||||
const MINUTE = 60_000
|
||||
|
||||
describe('secure path hardening retry budget', () => {
|
||||
/** Elapsed monotonic time, which is what the budget measures. */
|
||||
let clock = 0
|
||||
/** The wall clock, which it must not measure: it steps backwards on real machines. */
|
||||
let wallClock = 0
|
||||
let reports: SecurePathHardeningReport[] = []
|
||||
|
||||
beforeEach(() => {
|
||||
clock = 1_000_000
|
||||
wallClock = Date.parse('2026-01-01T00:00:00Z')
|
||||
vi.spyOn(performance, 'now').mockImplementation(() => clock)
|
||||
vi.spyOn(Date, 'now').mockImplementation(() => wallClock)
|
||||
reports = []
|
||||
setSecurePathHardeningReporter((entry) => reports.push(entry))
|
||||
configureHardeningRetryBudget({
|
||||
maxEntries: 64,
|
||||
maxKeyBytes: 4096,
|
||||
maxTotalKeyBytes: 65_536
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
setSecurePathHardeningReporter(null)
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
/** Drives the loop the read path drives: attempt when allowed, record the failure, advance. */
|
||||
function pollUntil(elapsedMs: number, stepMs: number, restricted = false): number[] {
|
||||
const attemptedAt: number[] = []
|
||||
const startedAt = clock
|
||||
while (clock - startedAt <= elapsedMs) {
|
||||
if (mayAttemptHardening(PATH)) {
|
||||
attemptedAt.push(clock - startedAt)
|
||||
recordHardeningOutcome(PATH, restricted)
|
||||
}
|
||||
clock += stepMs
|
||||
wallClock += stepMs
|
||||
}
|
||||
return attemptedAt
|
||||
}
|
||||
|
||||
it('doubles the delay after each consecutive failure, up to the ceiling', () => {
|
||||
expect(hardeningRetryDelayMs(1)).toBe(1 * MINUTE)
|
||||
expect(hardeningRetryDelayMs(2)).toBe(2 * MINUTE)
|
||||
expect(hardeningRetryDelayMs(3)).toBe(4 * MINUTE)
|
||||
expect(hardeningRetryDelayMs(4)).toBe(8 * MINUTE)
|
||||
expect(hardeningRetryDelayMs(5)).toBe(16 * MINUTE)
|
||||
// Ceiling reached, and it stays there however long the host has been broken.
|
||||
expect(hardeningRetryDelayMs(6)).toBe(30 * MINUTE)
|
||||
expect(hardeningRetryDelayMs(50)).toBe(30 * MINUTE)
|
||||
expect(hardeningRetryDelayMs(5000)).toBe(30 * MINUTE)
|
||||
})
|
||||
|
||||
it('allows the first attempt for a path it has never seen', () => {
|
||||
expect(mayAttemptHardening(PATH)).toBe(true)
|
||||
})
|
||||
|
||||
// The #4901 condition: the env store re-hardens on the read path about twice a second.
|
||||
it('collapses a read-path poll to a single attempt in the first minute', () => {
|
||||
const attemptedAt = pollUntil(55_000, 500)
|
||||
|
||||
expect(attemptedAt).toEqual([0])
|
||||
})
|
||||
|
||||
it('re-probes on the documented curve rather than on every read', () => {
|
||||
// Six hours of polling every 30s: 720 reads, and only the backoff decides how many run.
|
||||
const attemptedAt = pollUntil(6 * 60 * MINUTE, 30_000)
|
||||
|
||||
// 0, +1, +2, +4, +8, +16, then every 30 minutes forever.
|
||||
expect(attemptedAt.slice(0, 6).map((ms) => ms / MINUTE)).toEqual([0, 1, 3, 7, 15, 31])
|
||||
const trailingGaps = attemptedAt
|
||||
.slice(-4)
|
||||
.map((ms, index, all) => (all[index + 1]! - ms) / MINUTE)
|
||||
expect(trailingGaps.slice(0, -1)).toEqual([30, 30, 30])
|
||||
})
|
||||
|
||||
// The failure this replaced: three transient failures used to disable a path until restart.
|
||||
it('never abandons a path, however long it has been failing', () => {
|
||||
pollUntil(30 * 24 * 60 * MINUTE, 15 * MINUTE)
|
||||
|
||||
// A month of failures later, the very next elapsed ceiling still re-probes.
|
||||
clock += 31 * MINUTE
|
||||
expect(mayAttemptHardening(PATH)).toBe(true)
|
||||
})
|
||||
|
||||
/**
|
||||
* The same latch by another route. NTP corrections, VM snapshot restores and a user changing the
|
||||
* clock all step `Date.now()` backwards; measured against the wall clock that makes the elapsed
|
||||
* time negative, so the path stayed below its delay for the whole length of the step — a year,
|
||||
* here — which is exactly the permanent abandonment the backoff exists to remove.
|
||||
*/
|
||||
it('re-probes after a backwards clock step rather than waiting for the wall clock', () => {
|
||||
pollUntil(6 * 60 * MINUTE, 30_000)
|
||||
|
||||
wallClock -= 365 * 24 * 60 * MINUTE
|
||||
clock += 31 * MINUTE
|
||||
|
||||
expect(mayAttemptHardening(PATH)).toBe(true)
|
||||
})
|
||||
|
||||
it('announces the degraded state once, not once per failure', () => {
|
||||
pollUntil(6 * 60 * MINUTE, 30_000)
|
||||
|
||||
const throttled = reports.filter((entry) => entry.stage === 'throttled')
|
||||
expect(throttled).toHaveLength(1)
|
||||
expect(throttled[0]).toMatchObject({ targetPath: PATH, stage: 'throttled' })
|
||||
})
|
||||
|
||||
it('announces recovery when a throttled path hardens again, and resets the curve', () => {
|
||||
pollUntil(10 * MINUTE, 30_000)
|
||||
expect(reports.filter((entry) => entry.stage === 'throttled')).toHaveLength(1)
|
||||
|
||||
recordHardeningOutcome(PATH, true)
|
||||
|
||||
expect(reports.filter((entry) => entry.stage === 'recovered')).toMatchObject([
|
||||
{ targetPath: PATH, stage: 'recovered' }
|
||||
])
|
||||
// Record cleared: the next failure starts at the floor rather than the ceiling.
|
||||
expect(mayAttemptHardening(PATH)).toBe(true)
|
||||
recordHardeningOutcome(PATH, false)
|
||||
clock += 59_000
|
||||
expect(mayAttemptHardening(PATH)).toBe(false)
|
||||
clock += 2_000
|
||||
expect(mayAttemptHardening(PATH)).toBe(true)
|
||||
})
|
||||
|
||||
it('stays silent about recovery for a path that never reached the degraded state', () => {
|
||||
recordHardeningOutcome(PATH, false)
|
||||
recordHardeningOutcome(PATH, true)
|
||||
|
||||
expect(reports).toEqual([])
|
||||
})
|
||||
|
||||
it('budgets each path separately', () => {
|
||||
recordHardeningOutcome(PATH, false)
|
||||
|
||||
expect(mayAttemptHardening(PATH)).toBe(false)
|
||||
expect(mayAttemptHardening(OTHER)).toBe(true)
|
||||
})
|
||||
|
||||
/**
|
||||
* The state every other test here configures away: a module instance nobody has called
|
||||
* `configureHardeningRetryBudget` on, which is what a second importer gets. It used to throw,
|
||||
* and from the async lane that throw is an unhandled rejection rather than a caught error, so
|
||||
* "the budget is unconfigured" surfaced as a dead main process. Nothing is imported here but
|
||||
* the module itself — importing `secure-file.ts` is what used to hide this.
|
||||
*/
|
||||
it('defaults its bounds when nothing configured it', async () => {
|
||||
vi.resetModules()
|
||||
const budget = await import('./secure-path-hardening-retry-budget.js')
|
||||
|
||||
expect(budget.mayAttemptHardening(PATH)).toBe(true)
|
||||
expect(() => budget.recordHardeningOutcome(PATH, false)).not.toThrow()
|
||||
// Proof it recorded into a real cache rather than merely not throwing.
|
||||
expect(budget.mayAttemptHardening(PATH)).toBe(false)
|
||||
expect(budget.mayAttemptHardening(OTHER)).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,100 @@
|
||||
import {
|
||||
DEFAULT_HARDENING_CACHE_BOUNDS,
|
||||
SecurePathHardeningCache,
|
||||
type SecurePathHardeningCacheBounds
|
||||
} from './secure-path-hardening-cache'
|
||||
import { reportSecurePathHardening } from './secure-path-hardening-report'
|
||||
|
||||
type HardeningFailureRecord = { at: number; attempts: number }
|
||||
|
||||
/**
|
||||
* How often a path whose hardening keeps failing may be retried.
|
||||
*
|
||||
* Why throttle at all: the env store re-hardens on the *read* path at ~2/s (#4901), so retrying
|
||||
* every failure is an icacls-and-log storm on hosts where hardening cannot work — FAT32/exFAT have
|
||||
* no ACLs, and network paths, redirected profiles and restricted tokens refuse.
|
||||
*
|
||||
* Why exponential and not a cap: a cap that never expires latches a *transient* failure — one AV
|
||||
* scan or momentary lock and the path is abandoned for the life of the process, which can be days.
|
||||
* Backoff bounds the rate without ever bounding the lifetime. It settles at ~2 attempts/hour on a
|
||||
* permanently incapable host, which matters because the budget is per path and there are several
|
||||
* secure files; a fixed one-minute floor would leave a standing five-figure daily spawn count for
|
||||
* work that will never succeed.
|
||||
*
|
||||
* Why slowing it down is close to free: the synchronous write path is deliberately *not*
|
||||
* throttled, so a host that recovers hardens on its very next credential write. This read-path
|
||||
* re-probe is a backstop, not the recovery mechanism.
|
||||
*/
|
||||
const HARDENING_RETRY_FLOOR_MS = 60_000
|
||||
const HARDENING_RETRY_CEILING_MS = 30 * 60_000
|
||||
|
||||
/**
|
||||
* Why not `Date.now`: an NTP correction, a VM snapshot restore or a user changing the clock steps
|
||||
* the wall clock backwards, which made the elapsed time negative and held every path below its
|
||||
* delay until the clock caught up — a year, for a year-long step. That is the permanent latch this
|
||||
* backoff exists to remove. Elapsed monotonic time cannot go backwards.
|
||||
*/
|
||||
const monotonicNowMs = (): number => performance.now()
|
||||
|
||||
/** Consecutive failures before the degraded state is announced. */
|
||||
const HARDENING_THROTTLE_ANNOUNCE_AFTER = 3
|
||||
|
||||
/** Exported so the tests pin the real curve rather than a copy of it. */
|
||||
export function hardeningRetryDelayMs(attempts: number): number {
|
||||
return Math.min(HARDENING_RETRY_FLOOR_MS * 2 ** (attempts - 1), HARDENING_RETRY_CEILING_MS)
|
||||
}
|
||||
|
||||
let hardeningFailures: SecurePathHardeningCache<HardeningFailureRecord> | null = null
|
||||
|
||||
/**
|
||||
* Why it defaults instead of throwing: this used to require `configureHardeningRetryBudget` first,
|
||||
* and the only thing keeping that contract was import order — one module configured it at module
|
||||
* scope and happened to be the sole importer. Any second importer got a throw, and from the async
|
||||
* lane that throw lands in a `.then` handler as an unhandled rejection, which takes the Electron
|
||||
* main process down. A retry budget is not worth a crash, and a default is not worth a caller.
|
||||
*/
|
||||
function failures(): SecurePathHardeningCache<HardeningFailureRecord> {
|
||||
hardeningFailures ??= new SecurePathHardeningCache<HardeningFailureRecord>(
|
||||
DEFAULT_HARDENING_CACHE_BOUNDS
|
||||
)
|
||||
return hardeningFailures
|
||||
}
|
||||
|
||||
/** Overrides the default bounds. Optional: nothing has to call this before the budget is used. */
|
||||
export function configureHardeningRetryBudget(bounds: SecurePathHardeningCacheBounds): void {
|
||||
hardeningFailures = new SecurePathHardeningCache<HardeningFailureRecord>(bounds)
|
||||
}
|
||||
|
||||
export function mayAttemptHardening(targetPath: string): boolean {
|
||||
const failure = failures().get(targetPath)
|
||||
if (!failure) {
|
||||
return true
|
||||
}
|
||||
// No cap: once the backoff elapses the path is re-probed, however long it has been failing.
|
||||
return monotonicNowMs() - failure.at >= hardeningRetryDelayMs(failure.attempts)
|
||||
}
|
||||
|
||||
export function recordHardeningOutcome(targetPath: string, restricted: boolean): void {
|
||||
const previous = failures().get(targetPath)
|
||||
if (restricted) {
|
||||
failures().delete(targetPath)
|
||||
if (previous && previous.attempts >= HARDENING_THROTTLE_ANNOUNCE_AFTER) {
|
||||
reportSecurePathHardening(
|
||||
targetPath,
|
||||
'recovered',
|
||||
`hardening succeeded again after ${previous.attempts} consecutive failures`
|
||||
)
|
||||
}
|
||||
return
|
||||
}
|
||||
const attempts = (previous?.attempts ?? 0) + 1
|
||||
failures().set(targetPath, { at: monotonicNowMs(), attempts })
|
||||
// Fires exactly once: attempts only rises, and a success clears the record entirely.
|
||||
if (attempts === HARDENING_THROTTLE_ANNOUNCE_AFTER) {
|
||||
reportSecurePathHardening(
|
||||
targetPath,
|
||||
'throttled',
|
||||
`hardening failed ${attempts} times; backing off toward one retry per ${HARDENING_RETRY_CEILING_MS / 60_000} minutes until it succeeds`
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -1,144 +1,351 @@
|
||||
import { execFile, execFileSync } from 'node:child_process'
|
||||
import { win32 as pathWin32 } from 'node:path'
|
||||
import { randomBytes } from 'node:crypto'
|
||||
import { readFileSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join, win32 as pathWin32 } from 'node:path'
|
||||
import { runProcess, runProcessSync } from './child-process/run-process'
|
||||
import { windowsSystem32Binary } from './child-process/windows-system-binary'
|
||||
import {
|
||||
reportSecurePathHardening,
|
||||
type SecurePathHardeningReport
|
||||
} from './secure-path-hardening-report'
|
||||
import { localDomainSidOf, parseSddlDacl } from './windows-security-descriptor'
|
||||
|
||||
let cachedWindowsUserSid: string | null | undefined
|
||||
const ACL_TIMEOUT_MS = 5000
|
||||
|
||||
function buildWindowsRestrictAclArgs(
|
||||
targetPath: string,
|
||||
currentUserSid: string,
|
||||
/** SYSTEM and the local Administrators group: they can take ownership regardless, so denying them buys nothing. */
|
||||
const LOCAL_SYSTEM_SID = 'S-1-5-18'
|
||||
const BUILTIN_ADMINISTRATORS_SID = 'S-1-5-32-544'
|
||||
|
||||
const WINDOWS_SID_PATTERN = /^S-1-\d+(?:-\d+)+$/
|
||||
|
||||
type AclPlan = {
|
||||
program: string
|
||||
/** The path as icacls must receive it, already extended-length prefixed when needed. */
|
||||
icaclsPath: string
|
||||
isDirectory: boolean
|
||||
): string[] {
|
||||
return [
|
||||
'-NoProfile',
|
||||
'-NonInteractive',
|
||||
'-ExecutionPolicy',
|
||||
'Bypass',
|
||||
'-Command',
|
||||
WINDOWS_RESTRICT_ACL_SCRIPT,
|
||||
targetPath,
|
||||
currentUserSid,
|
||||
isDirectory ? '1' : '0'
|
||||
]
|
||||
allowedSids: string[]
|
||||
/** Resolves the machine-relative aliases `/save` emits; null when the user SID is not one. */
|
||||
localDomainSid: string | null
|
||||
resetArgs: string[]
|
||||
grantArgs: string[]
|
||||
}
|
||||
|
||||
export function bestEffortRestrictWindowsPath(targetPath: string, isDirectory: boolean): void {
|
||||
const currentUserSid = getCurrentWindowsUserSid()
|
||||
if (!currentUserSid) {
|
||||
function buildAclPlan(targetPath: string, currentUserSid: string, isDirectory: boolean): AclPlan {
|
||||
const icaclsPath = toIcaclsPath(targetPath)
|
||||
// Directories propagate to children (artifact-intent files rely on inheritance); files take no flags.
|
||||
const rights = isDirectory ? '(OI)(CI)(F)' : '(F)'
|
||||
const allowedSids = [...new Set([currentUserSid, LOCAL_SYSTEM_SID, BUILTIN_ADMINISTRATORS_SID])]
|
||||
return {
|
||||
program: windowsSystem32Binary('icacls.exe'),
|
||||
icaclsPath,
|
||||
isDirectory,
|
||||
allowedSids,
|
||||
localDomainSid: localDomainSidOf(currentUserSid),
|
||||
// `/reset` purges explicit ACEs, which `/inheritance:r` leaves in place — a planted
|
||||
// `Everyone:(R)` survives the grant pass otherwise. The two cannot be combined in one call.
|
||||
resetArgs: [icaclsPath, '/reset', '/q'],
|
||||
// Never add /c: it makes icacls exit 0 on "Failed processing 1 files", a silent no-op by another route.
|
||||
grantArgs: [
|
||||
icaclsPath,
|
||||
'/inheritance:r',
|
||||
...allowedSids.flatMap((sid) => ['/grant:r', `*${sid}:${rights}`]),
|
||||
'/q'
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
function verifyArgs(plan: AclPlan, savePath: string): string[] {
|
||||
return [plan.icaclsPath, '/save', savePath, '/q']
|
||||
}
|
||||
|
||||
function sddlSavePath(): string {
|
||||
return join(tmpdir(), `orca-acl-${process.pid}-${randomBytes(6).toString('hex')}.sddl`)
|
||||
}
|
||||
|
||||
/**
|
||||
* Judges a `/save` result. Returns a failure reason, or null when the DACL on disk is exactly the
|
||||
* intended one — protected, granting full control to the allowed SIDs and to nobody else.
|
||||
*/
|
||||
function evaluateSavedAcl(
|
||||
plan: AclPlan,
|
||||
result: { code: number | null; stderr: string },
|
||||
savePath: string
|
||||
): string | null {
|
||||
if (result.code !== 0) {
|
||||
return result.stderr.trim() || `icacls exited ${result.code}`
|
||||
}
|
||||
let sddl: string
|
||||
try {
|
||||
// icacls writes the descriptor as UTF-16LE, which sidesteps the OEM codepage its stdout uses.
|
||||
sddl = readFileSync(savePath, 'utf16le')
|
||||
} catch {
|
||||
return 'icacls saved no security descriptor'
|
||||
}
|
||||
return validateHardenedDacl(sddl, plan)
|
||||
}
|
||||
|
||||
function validateHardenedDacl(sddl: string, plan: AclPlan): string | null {
|
||||
const dacl = parseSddlDacl(sddl, plan.localDomainSid ?? undefined)
|
||||
if (!dacl) {
|
||||
return 'no DACL in the saved security descriptor'
|
||||
}
|
||||
if (!dacl.isProtected) {
|
||||
return 'DACL is not protected; the parent still propagates into it'
|
||||
}
|
||||
// Exactly these, in any order: a directory's rules must be inheritable and nothing else.
|
||||
const expectedFlags = plan.isDirectory ? ['OI', 'CI'] : []
|
||||
const observed = new Set<string>()
|
||||
for (const ace of dacl.aces) {
|
||||
if (ace.type !== 'A') {
|
||||
return `unexpected ${ace.type} rule for ${ace.sid}`
|
||||
}
|
||||
if (ace.flags.includes('ID')) {
|
||||
return `inherited rule survived for ${ace.sid}`
|
||||
}
|
||||
if (ace.rights !== 'FA') {
|
||||
return `rule for ${ace.sid} grants ${ace.rights || 'nothing'}, not full control`
|
||||
}
|
||||
// The whole set, not just OI. (OI) without (CI) leaves subdirectories unprotected, and
|
||||
// adding (IO) makes every rule inherit-only, so the directory object itself grants nobody
|
||||
// anything and Orca cannot even write into it. Both used to be repaired blindly on every
|
||||
// pass; since hardening short-circuits on a DACL that verifies, whatever this accepts stays.
|
||||
if (
|
||||
ace.flags.length !== expectedFlags.length ||
|
||||
!expectedFlags.every((flag) => ace.flags.includes(flag))
|
||||
) {
|
||||
return `wrong inheritance flags (${ace.flags.join('') || 'none'}) for ${ace.sid}`
|
||||
}
|
||||
observed.add(ace.sid)
|
||||
}
|
||||
// Identity, not just shape: a count check alone accepts a granted SID swapped for another.
|
||||
// Unexpected principals are reported before missing ones — "Everyone has full control" is the
|
||||
// headline, and a substitution always produces both.
|
||||
for (const sid of observed) {
|
||||
if (!plan.allowedSids.includes(sid)) {
|
||||
return `unexpected rule for ${sid}`
|
||||
}
|
||||
}
|
||||
for (const sid of plan.allowedSids) {
|
||||
if (!observed.has(sid)) {
|
||||
return `missing rule for ${sid}`
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* icacls resolves through the MAX_PATH-limited API and fails with "cannot find the path
|
||||
* specified" past 259 characters; the extended prefix is the documented escape.
|
||||
*/
|
||||
function toIcaclsPath(targetPath: string): string {
|
||||
if (targetPath.length < 260 || targetPath.startsWith('\\\\?\\')) {
|
||||
return targetPath
|
||||
}
|
||||
const normalized = pathWin32.normalize(targetPath)
|
||||
if (/^[A-Za-z]:\\/.test(normalized)) {
|
||||
return `\\\\?\\${normalized}`
|
||||
}
|
||||
if (normalized.startsWith('\\\\')) {
|
||||
return `\\\\?\\UNC\\${normalized.slice(2)}`
|
||||
}
|
||||
return targetPath
|
||||
}
|
||||
|
||||
function report(
|
||||
targetPath: string,
|
||||
stage: SecurePathHardeningReport['stage'],
|
||||
detail: string
|
||||
): void {
|
||||
reportSecurePathHardening(targetPath, stage, detail)
|
||||
}
|
||||
|
||||
/**
|
||||
* Applies the ACL without blocking. `onSettled` reports the real outcome, which the return value
|
||||
* cannot: the caller's cache must not keep claiming a path is hardened when the apply failed.
|
||||
*/
|
||||
export function bestEffortRestrictWindowsPath(
|
||||
targetPath: string,
|
||||
isDirectory: boolean,
|
||||
onSettled?: (restricted: boolean) => void
|
||||
): void {
|
||||
const plan = planFor(targetPath, isDirectory)
|
||||
if (!plan) {
|
||||
onSettled?.(false)
|
||||
return
|
||||
}
|
||||
// Why: async to avoid blocking the main thread — sync PowerShell cold-start (~1-1.5s) on the frequent read path stormed it (#4901).
|
||||
execFile(
|
||||
getWindowsSystemToolPath('WindowsPowerShell\\v1.0\\powershell.exe'),
|
||||
buildWindowsRestrictAclArgs(targetPath, currentUserSid, isDirectory),
|
||||
{
|
||||
windowsHide: true,
|
||||
timeout: 5000
|
||||
},
|
||||
() => {
|
||||
// Why: ignore errors — hardening is best-effort; PowerShell ACL APIs may be unavailable or locked down.
|
||||
// Why async: hardening runs on the read path, and blocking it on a spawn stormed the main thread (#4901).
|
||||
// Why both arms and a terminal catch: a bare `void p.then(fn)` makes a rejected `restrictAsync`
|
||||
// *and* a throw from `onSettled` itself an unhandled rejection, which Node's default turns into
|
||||
// a main-process crash — the exact opposite of what the reporter hook exists for. `false` is the
|
||||
// right value on the error arm: it drops the path from the caller's cache and leaves it retryable.
|
||||
void restrictAsync(targetPath, plan)
|
||||
.then(onSettled, () => onSettled?.(false))
|
||||
.catch((error: unknown) => reportSettlementThrow(targetPath, error))
|
||||
}
|
||||
|
||||
/**
|
||||
* The last frame before an unhandled rejection, so it must not throw either — and the reporter it
|
||||
* calls is a caller-installed hook, which is the one thing here that plausibly does.
|
||||
*/
|
||||
function reportSettlementThrow(targetPath: string, error: unknown): void {
|
||||
try {
|
||||
report(targetPath, 'settle', `hardening settlement callback threw: ${String(error)}`)
|
||||
} catch {
|
||||
// Nothing left to report through; losing one diagnostic beats crashing the main process.
|
||||
}
|
||||
}
|
||||
|
||||
async function restrictAsync(targetPath: string, plan: AclPlan): Promise<boolean> {
|
||||
// Verify first: a path that already reads back correct needs no write at all. Re-running
|
||||
// `/reset` on a correct DACL would briefly restore the inherited (broader) one for no gain.
|
||||
if ((await verifyAsync(plan)) === null) {
|
||||
return true
|
||||
}
|
||||
for (const [stage, args] of [
|
||||
['reset', plan.resetArgs],
|
||||
['grant', plan.grantArgs]
|
||||
] as const) {
|
||||
try {
|
||||
const result = await runProcess({ program: plan.program, args, timeoutMs: ACL_TIMEOUT_MS })
|
||||
if (result.code !== 0) {
|
||||
report(targetPath, stage, result.stderr || `icacls exited ${result.code}`)
|
||||
return false
|
||||
}
|
||||
} catch (error) {
|
||||
report(targetPath, stage, String(error))
|
||||
return false
|
||||
}
|
||||
)
|
||||
}
|
||||
const invalid = await verifyAsync(plan)
|
||||
if (invalid) {
|
||||
report(targetPath, 'verify', invalid)
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
async function verifyAsync(plan: AclPlan): Promise<string | null> {
|
||||
const savePath = sddlSavePath()
|
||||
try {
|
||||
const result = await runProcess({
|
||||
program: plan.program,
|
||||
args: verifyArgs(plan, savePath),
|
||||
timeoutMs: ACL_TIMEOUT_MS
|
||||
})
|
||||
return evaluateSavedAcl(plan, result, savePath)
|
||||
} catch (error) {
|
||||
return String(error)
|
||||
} finally {
|
||||
discard(savePath)
|
||||
}
|
||||
}
|
||||
|
||||
export function restrictWindowsPathSync(targetPath: string, isDirectory: boolean): boolean {
|
||||
const plan = planFor(targetPath, isDirectory)
|
||||
if (!plan) {
|
||||
return false
|
||||
}
|
||||
// Why sync: the file must not be published until its ACL is actually restricted (read path stays async, #4901).
|
||||
if (verifySync(plan) === null) {
|
||||
return true
|
||||
}
|
||||
for (const [stage, args] of [
|
||||
['reset', plan.resetArgs],
|
||||
['grant', plan.grantArgs]
|
||||
] as const) {
|
||||
try {
|
||||
const result = runProcessSync({ program: plan.program, args, timeoutMs: ACL_TIMEOUT_MS })
|
||||
if (result.code !== 0) {
|
||||
report(targetPath, stage, result.stderr || `icacls exited ${result.code}`)
|
||||
return false
|
||||
}
|
||||
} catch (error) {
|
||||
// Why not fatal: a failed ACL apply must not crash the write; false leaves the path uncached to retry later.
|
||||
report(targetPath, stage, String(error))
|
||||
return false
|
||||
}
|
||||
}
|
||||
const invalid = verifySync(plan)
|
||||
if (invalid) {
|
||||
report(targetPath, 'verify', invalid)
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
function verifySync(plan: AclPlan): string | null {
|
||||
const savePath = sddlSavePath()
|
||||
try {
|
||||
const result = runProcessSync({
|
||||
program: plan.program,
|
||||
args: verifyArgs(plan, savePath),
|
||||
timeoutMs: ACL_TIMEOUT_MS
|
||||
})
|
||||
return evaluateSavedAcl(plan, result, savePath)
|
||||
} catch (error) {
|
||||
return String(error)
|
||||
} finally {
|
||||
discard(savePath)
|
||||
}
|
||||
}
|
||||
|
||||
function discard(savePath: string): void {
|
||||
try {
|
||||
rmSync(savePath, { force: true })
|
||||
} catch {
|
||||
// The descriptor holds no secrets; a leftover temp file is not worth reporting.
|
||||
}
|
||||
}
|
||||
|
||||
function planFor(targetPath: string, isDirectory: boolean): AclPlan | null {
|
||||
const currentUserSid = getCurrentWindowsUserSid()
|
||||
if (!currentUserSid) {
|
||||
return false
|
||||
}
|
||||
// Why: file must not be published until its ACL is actually restricted, so block and report real success (read path stays async, #4901).
|
||||
try {
|
||||
execFileSync(
|
||||
getWindowsSystemToolPath('WindowsPowerShell\\v1.0\\powershell.exe'),
|
||||
buildWindowsRestrictAclArgs(targetPath, currentUserSid, isDirectory),
|
||||
{
|
||||
stdio: ['ignore', 'ignore', 'ignore'],
|
||||
windowsHide: true,
|
||||
timeout: 5000
|
||||
}
|
||||
)
|
||||
return true
|
||||
} catch {
|
||||
// Why: best-effort — a failed ACL apply must not crash the write; false leaves the path uncached to retry later.
|
||||
return false
|
||||
report(targetPath, 'sid-lookup', 'could not resolve the current user SID')
|
||||
return null
|
||||
}
|
||||
return buildAclPlan(targetPath, currentUserSid, isDirectory)
|
||||
}
|
||||
|
||||
const WINDOWS_RESTRICT_ACL_SCRIPT = `
|
||||
$ErrorActionPreference = 'Stop'
|
||||
$path = $args[0]
|
||||
$currentUserSid = $args[1]
|
||||
$isDirectory = $args[2] -eq '1'
|
||||
$allowedSidTexts = @($currentUserSid, 'S-1-5-18', 'S-1-5-32-544')
|
||||
$allowedSids = @{}
|
||||
foreach ($sidText in $allowedSidTexts) {
|
||||
$allowedSids[$sidText] = $true
|
||||
}
|
||||
$acl = Get-Acl -LiteralPath $path
|
||||
$acl.SetAccessRuleProtection($true, $false)
|
||||
foreach ($rule in @($acl.Access)) {
|
||||
[void]$acl.RemoveAccessRuleSpecific($rule)
|
||||
}
|
||||
$inheritanceFlags = [System.Security.AccessControl.InheritanceFlags]::None
|
||||
if ($isDirectory) {
|
||||
$inheritanceFlags = [System.Security.AccessControl.InheritanceFlags]::ContainerInherit -bor [System.Security.AccessControl.InheritanceFlags]::ObjectInherit
|
||||
}
|
||||
foreach ($sidText in $allowedSidTexts) {
|
||||
$sid = [System.Security.Principal.SecurityIdentifier]::new($sidText)
|
||||
$rule = [System.Security.AccessControl.FileSystemAccessRule]::new(
|
||||
$sid,
|
||||
[System.Security.AccessControl.FileSystemRights]::FullControl,
|
||||
$inheritanceFlags,
|
||||
[System.Security.AccessControl.PropagationFlags]::None,
|
||||
[System.Security.AccessControl.AccessControlType]::Allow
|
||||
)
|
||||
[void]$acl.AddAccessRule($rule)
|
||||
}
|
||||
Set-Acl -LiteralPath $path -AclObject $acl
|
||||
$verifiedAcl = Get-Acl -LiteralPath $path
|
||||
if (-not $verifiedAcl.AreAccessRulesProtected) {
|
||||
throw 'ACL inheritance is still enabled'
|
||||
}
|
||||
$fullControl = [System.Security.AccessControl.FileSystemRights]::FullControl
|
||||
foreach ($rule in @($verifiedAcl.Access)) {
|
||||
$sid = $rule.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]).Value
|
||||
if (-not $allowedSids.ContainsKey($sid)) {
|
||||
throw "Unexpected ACL entry $sid"
|
||||
}
|
||||
if ($rule.AccessControlType -ne [System.Security.AccessControl.AccessControlType]::Allow) {
|
||||
throw "Unexpected ACL deny entry $sid"
|
||||
}
|
||||
if (($rule.FileSystemRights -band $fullControl) -ne $fullControl) {
|
||||
throw "ACL entry $sid does not grant FullControl"
|
||||
}
|
||||
}
|
||||
`.trim()
|
||||
let cachedWindowsUserSid: string | null = null
|
||||
let sidLookupFailedAt: number | null = null
|
||||
const SID_LOOKUP_RETRY_MS = 60_000
|
||||
|
||||
/**
|
||||
* Why monotonic and not `Date.now`: a backwards wall-clock step held this window open until the
|
||||
* clock caught up, and this latch is worse than the read-path budget's — a failed lookup makes
|
||||
* `planFor` return null, which disables the synchronous *write* path too, so the write-path
|
||||
* exemption that recovers from that one cannot recover from this.
|
||||
*/
|
||||
const monotonicNowMs = (): number => performance.now()
|
||||
|
||||
/**
|
||||
* Only a well-formed SID is cached for the process lifetime. A failure is cached for a minute:
|
||||
* caching it forever let one transient `whoami` hiccup disable hardening until restart.
|
||||
*/
|
||||
function getCurrentWindowsUserSid(): string | null {
|
||||
if (cachedWindowsUserSid !== undefined) {
|
||||
if (cachedWindowsUserSid) {
|
||||
return cachedWindowsUserSid
|
||||
}
|
||||
try {
|
||||
const output = execFileSync(
|
||||
getWindowsSystemToolPath('whoami.exe'),
|
||||
['/user', '/fo', 'csv', '/nh'],
|
||||
{
|
||||
encoding: 'utf-8',
|
||||
stdio: ['ignore', 'pipe', 'ignore'],
|
||||
windowsHide: true,
|
||||
timeout: 5000
|
||||
}
|
||||
).trim()
|
||||
const columns = parseCsvLine(output)
|
||||
cachedWindowsUserSid = columns[1] ?? null
|
||||
} catch {
|
||||
cachedWindowsUserSid = null
|
||||
if (sidLookupFailedAt !== null && monotonicNowMs() - sidLookupFailedAt < SID_LOOKUP_RETRY_MS) {
|
||||
return null
|
||||
}
|
||||
return cachedWindowsUserSid
|
||||
}
|
||||
|
||||
function getWindowsSystemToolPath(relativeSystem32Path: string): string {
|
||||
const systemRoot = process.env.SystemRoot || process.env.WINDIR || 'C:\\Windows'
|
||||
return pathWin32.join(systemRoot, 'System32', relativeSystem32Path)
|
||||
try {
|
||||
const result = runProcessSync({
|
||||
program: windowsSystem32Binary('whoami.exe'),
|
||||
args: ['/user', '/fo', 'csv', '/nh'],
|
||||
timeoutMs: ACL_TIMEOUT_MS
|
||||
})
|
||||
const candidate = result.code === 0 ? parseCsvLine(result.stdout.trim())[1] : undefined
|
||||
if (candidate && WINDOWS_SID_PATTERN.test(candidate)) {
|
||||
cachedWindowsUserSid = candidate
|
||||
sidLookupFailedAt = null
|
||||
return candidate
|
||||
}
|
||||
} catch {
|
||||
// Fall through to the failure record below.
|
||||
}
|
||||
sidLookupFailedAt = monotonicNowMs()
|
||||
return null
|
||||
}
|
||||
|
||||
function parseCsvLine(line: string): string[] {
|
||||
@@ -146,5 +353,6 @@ function parseCsvLine(line: string): string[] {
|
||||
}
|
||||
|
||||
export function resetSecureFileWindowsUserSidForTests(): void {
|
||||
cachedWindowsUserSid = undefined
|
||||
cachedWindowsUserSid = null
|
||||
sidLookupFailedAt = null
|
||||
}
|
||||
|
||||
@@ -0,0 +1,404 @@
|
||||
import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
|
||||
import { runProcessSync } from './child-process/run-process'
|
||||
import { windowsSystem32Binary } from './child-process/windows-system-binary'
|
||||
import {
|
||||
setSecurePathHardeningReporter,
|
||||
type SecurePathHardeningReport
|
||||
} from './secure-path-hardening-report'
|
||||
import {
|
||||
bestEffortRestrictWindowsPath,
|
||||
resetSecureFileWindowsUserSidForTests,
|
||||
restrictWindowsPathSync
|
||||
} from './secure-path-windows-acl'
|
||||
import { removeTreeSync } from './windows-transient-lock-removal'
|
||||
|
||||
/**
|
||||
* The half of the proof a mocked argv test cannot give.
|
||||
*
|
||||
* The shipped bug was not a wrong argv — it was an argv the *callee* never
|
||||
* received: `powershell.exe -Command <script> <path> <sid>` leaves `$args`
|
||||
* empty, so the script died on its first statement and every caller was told
|
||||
* the path had been hardened. Asserting on the constructed arguments passed
|
||||
* happily throughout. Only reading the resulting ACL back off a real file
|
||||
* catches it, so that is what this does.
|
||||
*
|
||||
* Runs only on win32; skipped elsewhere.
|
||||
*/
|
||||
const describeOnWindows = process.platform === 'win32' ? describe : describe.skip
|
||||
|
||||
const EVERYONE_SID = 'S-1-1-0'
|
||||
const BUILTIN_ADMINISTRATORS_SID = 'S-1-5-32-544'
|
||||
/** High, System and Protected mandatory levels: the token is elevated. Medium (S-1-16-8192) is not. */
|
||||
const ELEVATED_INTEGRITY_SID = /\bS-1-16-(?:12288|16384|20480)\b/
|
||||
|
||||
/** The SID the production code will grant to, so a planted DACL can differ only in its flags. */
|
||||
function currentUserSid(): string {
|
||||
const result = runProcessSync({
|
||||
program: windowsSystem32Binary('whoami.exe'),
|
||||
args: ['/user', '/fo', 'csv', '/nh'],
|
||||
timeoutMs: 10_000
|
||||
})
|
||||
return result.stdout.trim().split(/","/)[1]!.replace(/"$/, '')
|
||||
}
|
||||
|
||||
/**
|
||||
* Plants an exact DACL. Two invocations on purpose: combining `/inheritance:r` with `/grant:r`
|
||||
* leaves the argument order to icacls, and on Windows Server the inherited ACEs survive the
|
||||
* combined form as explicit ones -- which is a planted precondition that silently is not the
|
||||
* one written down. Removing inheritance first makes the grant the whole DACL.
|
||||
*/
|
||||
function plantDacl(path: string, grants: string[]): void {
|
||||
expect(icacls(path, '/inheritance:r', '/q').code).toBe(0)
|
||||
expect(icacls(path, ...grants.flatMap((grant) => ['/grant:r', grant]), '/q').code).toBe(0)
|
||||
}
|
||||
|
||||
function icacls(...args: string[]): { code: number | null; stdout: string } {
|
||||
const result = runProcessSync({
|
||||
program: windowsSystem32Binary('icacls.exe'),
|
||||
args,
|
||||
timeoutMs: 10_000
|
||||
})
|
||||
return { code: result.code, stdout: result.stdout }
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this process could rewrite a system file's DACL — decided before anything is written.
|
||||
*
|
||||
* `icacls <hosts> /save` is not the probe it looks like: `BUILTIN\Users` holds `(RX)`, so
|
||||
* READ_CONTROL succeeds unelevated and every machine would report elevated. The token's mandatory
|
||||
* integrity level is the thing that actually differs, and it is a SID rather than a localized
|
||||
* string, so it reads the same on a non-English Windows.
|
||||
*/
|
||||
function isElevated(): boolean {
|
||||
if (process.platform !== 'win32') {
|
||||
return false
|
||||
}
|
||||
const result = runProcessSync({
|
||||
program: windowsSystem32Binary('whoami.exe'),
|
||||
args: ['/groups', '/fo', 'csv', '/nh'],
|
||||
timeoutMs: 10_000
|
||||
})
|
||||
if (ELEVATED_INTEGRITY_SID.test(result.stdout)) {
|
||||
return true
|
||||
}
|
||||
// Unelevated, Administrators is present only as "Group used for deny only".
|
||||
const administrators = result.stdout
|
||||
.split(/\r?\n/)
|
||||
.find((line) => line.includes(BUILTIN_ADMINISTRATORS_SID))
|
||||
return administrators?.includes('Enabled group') ?? false
|
||||
}
|
||||
|
||||
/**
|
||||
* The `Principal:(flags)` entries of `icacls <path>`. The first line carries the path, which is
|
||||
* stripped by the exact string passed in so its own spaces cannot be mistaken for the separator.
|
||||
*/
|
||||
function readAclEntries(path: string): string[] {
|
||||
const arg = path.length < 260 ? path : `\\\\?\\${path}`
|
||||
const { stdout } = icacls(arg)
|
||||
const entries: string[] = []
|
||||
for (const [index, rawLine] of stdout.split(/\r?\n/).entries()) {
|
||||
const line = index === 0 ? rawLine.slice(arg.length) : rawLine
|
||||
const trimmed = line.trim()
|
||||
if (!trimmed && index > 0) {
|
||||
break
|
||||
}
|
||||
if (trimmed.includes(':(')) {
|
||||
entries.push(trimmed)
|
||||
}
|
||||
}
|
||||
return entries
|
||||
}
|
||||
|
||||
/**
|
||||
* `toHaveLength` reports only the count, and vitest elides the array past a few items — which on a
|
||||
* host that lists a DACL differently is exactly the information needed. Name the entries.
|
||||
*/
|
||||
function listed(entries: string[]): string {
|
||||
return `icacls listed ${entries.length} entries: ${entries.join(' | ')}`
|
||||
}
|
||||
|
||||
describeOnWindows('restrictWindowsPathSync against a real filesystem', () => {
|
||||
const elevated = isElevated()
|
||||
let root: string
|
||||
|
||||
beforeAll(() => {
|
||||
resetSecureFileWindowsUserSidForTests()
|
||||
root = mkdtempSync(join(tmpdir(), 'orca-acl-win32-'))
|
||||
// %TEMP% grants [user, SYSTEM, Administrators] (OI)(CI)(F) by default, and those
|
||||
// propagate into every fixture below. Strip them here so a planted DACL is exactly what
|
||||
// the test planted: on a host where the running user is also one of the three principals
|
||||
// a test grants, an inherited copy is otherwise indistinguishable from a planted one.
|
||||
plantDacl(root, [`*${currentUserSid()}:(OI)(CI)(F)`])
|
||||
})
|
||||
|
||||
afterAll(() => {
|
||||
// This suite plants hostile DACLs on purpose, and `removeTreeSync` only retries *transient*
|
||||
// locks. Should a repair regress, an `(OI)(CI)(IO)` grant leaves the tree permanently
|
||||
// undeletable — so restore inheritance first rather than leaking it into %TEMP% every run.
|
||||
icacls(root, '/reset', '/t', '/q')
|
||||
removeTreeSync(root)
|
||||
})
|
||||
|
||||
it('actually applies the ACL to a real file, dropping inherited and foreign ACEs', () => {
|
||||
const file = join(root, 'credential.json')
|
||||
writeFileSync(file, '{"token":"secret"}')
|
||||
// A planted explicit ACE: /inheritance:r alone does not remove these.
|
||||
expect(icacls(file, '/grant', `*${EVERYONE_SID}:(R)`).code).toBe(0)
|
||||
|
||||
const before = readAclEntries(file)
|
||||
expect(before.some((entry) => entry.startsWith('Everyone:'))).toBe(true)
|
||||
expect(before.some((entry) => entry.includes('(I)'))).toBe(true)
|
||||
|
||||
expect(restrictWindowsPathSync(file, false)).toBe(true)
|
||||
|
||||
const after = readAclEntries(file)
|
||||
// No inherited ACE survives: the DACL is protected.
|
||||
expect(after.every((entry) => !entry.includes('(I)'))).toBe(true)
|
||||
expect(after.some((entry) => entry.startsWith('Everyone:'))).toBe(false)
|
||||
// Exactly the three intended principals, each with FullControl.
|
||||
expect(after).toHaveLength(3)
|
||||
expect(after.every((entry) => entry.endsWith(':(F)'))).toBe(true)
|
||||
})
|
||||
|
||||
it('gives a real directory inheritable rules so files created inside stay restricted', () => {
|
||||
const dir = join(root, 'secure-dir')
|
||||
mkdirSync(dir)
|
||||
|
||||
expect(restrictWindowsPathSync(dir, true)).toBe(true)
|
||||
|
||||
const after = readAclEntries(dir)
|
||||
expect(after).toHaveLength(3)
|
||||
expect(after.every((entry) => entry.endsWith(':(OI)(CI)(F)'))).toBe(true)
|
||||
expect(after.every((entry) => !entry.includes('(I)'))).toBe(true)
|
||||
|
||||
// The point of the inheritance flags: a child written afterwards is already restricted.
|
||||
const child = join(dir, 'inherited.json')
|
||||
writeFileSync(child, '{}')
|
||||
const childEntries = readAclEntries(child)
|
||||
expect(childEntries).toHaveLength(3)
|
||||
expect(childEntries.every((entry) => entry.includes('(I)'))).toBe(true)
|
||||
expect(childEntries.some((entry) => entry.startsWith('Everyone:'))).toBe(false)
|
||||
})
|
||||
|
||||
// Paths reach this code from user-chosen workspace locations, so the quoting hazards that
|
||||
// ruled out interpolating them into a PowerShell command line get exercised for real.
|
||||
it.each([
|
||||
['spaces', 'a b c'],
|
||||
['single quote and dollar', "quo'te $var"],
|
||||
['backtick', 'back`tick'],
|
||||
['brackets', 'brack[et]s'],
|
||||
['semicolon and ampersand', 'semi;colon & amp'],
|
||||
['comma', 'com,ma'],
|
||||
['parentheses', 'paren(s)'],
|
||||
['caret and percent', 'car^et %PATH%']
|
||||
])('hardens a path containing %s', (_label, segment) => {
|
||||
const dir = join(root, segment)
|
||||
mkdirSync(dir, { recursive: true })
|
||||
const file = join(dir, 'secret.json')
|
||||
writeFileSync(file, '{}')
|
||||
|
||||
expect(restrictWindowsPathSync(file, false)).toBe(true)
|
||||
|
||||
const after = readAclEntries(file)
|
||||
expect(after).toHaveLength(3)
|
||||
expect(after.every((entry) => !entry.includes('(I)'))).toBe(true)
|
||||
})
|
||||
|
||||
it('hardens a path longer than MAX_PATH', () => {
|
||||
let dir = join(root, 'long')
|
||||
while (dir.length < 280) {
|
||||
dir = join(dir, 'x'.repeat(40))
|
||||
}
|
||||
mkdirSync(dir, { recursive: true })
|
||||
const file = join(dir, 'secret.json')
|
||||
expect(file.length).toBeGreaterThan(260)
|
||||
writeFileSync(file, '{}')
|
||||
|
||||
expect(restrictWindowsPathSync(file, false)).toBe(true)
|
||||
expect(readAclEntries(file)).toHaveLength(3)
|
||||
})
|
||||
|
||||
/**
|
||||
* The shape of a DACL is not the same question as who is on it. This one is protected, carries
|
||||
* exactly three non-inherited full-control rules, and grants Everyone — so it satisfies every
|
||||
* check that does not compare SIDs, and hardening would report success and leave it alone.
|
||||
*/
|
||||
it('repairs a protected three-rule DACL that grants the wrong principal', () => {
|
||||
const file = join(root, 'substituted.json')
|
||||
writeFileSync(file, '{"token":"secret"}')
|
||||
plantDacl(file, [`*${EVERYONE_SID}:(F)`, '*S-1-5-18:(F)', '*S-1-5-32-544:(F)'])
|
||||
|
||||
const before = readAclEntries(file)
|
||||
expect(before, listed(before)).toHaveLength(3)
|
||||
expect(before.every((entry) => !entry.includes('(I)'))).toBe(true)
|
||||
expect(before.some((entry) => entry.startsWith('Everyone:'))).toBe(true)
|
||||
|
||||
expect(restrictWindowsPathSync(file, false)).toBe(true)
|
||||
|
||||
const after = readAclEntries(file)
|
||||
expect(after, listed(after)).toHaveLength(3)
|
||||
expect(after.some((entry) => entry.startsWith('Everyone:'))).toBe(false)
|
||||
})
|
||||
|
||||
/**
|
||||
* Wrong *flags* rather than a wrong principal. Both of these are protected, carry three
|
||||
* non-inherited full-control rules for exactly the right SIDs, and differ from correct only in
|
||||
* their inheritance flags — so a check that tests OI alone accepts them.
|
||||
*
|
||||
* That under-check was harmless while /reset + /grant ran unconditionally and repaired whatever
|
||||
* was there. Verify-first made it load-bearing: what verification accepts is now left alone.
|
||||
*/
|
||||
it.each([
|
||||
['(OI) without (CI), leaving subdirectories unprotected', '(OI)(F)'],
|
||||
['(OI)(CI)(IO), which grants nobody anything on the directory itself', '(OI)(CI)(IO)(F)']
|
||||
])('repairs a directory whose rules are %s', (_label, rights) => {
|
||||
const dir = join(root, `wrong-flags-${rights.replace(/[^A-Z]/g, '')}`)
|
||||
mkdirSync(dir)
|
||||
plantDacl(dir, [
|
||||
`*${currentUserSid()}:${rights}`,
|
||||
`*S-1-5-18:${rights}`,
|
||||
`*S-1-5-32-544:${rights}`
|
||||
])
|
||||
|
||||
const before = readAclEntries(dir)
|
||||
expect(before, listed(before)).toHaveLength(3)
|
||||
expect(before.every((entry) => !entry.includes('(I)'))).toBe(true)
|
||||
expect(before.every((entry) => entry.endsWith(rights))).toBe(true)
|
||||
|
||||
expect(restrictWindowsPathSync(dir, true)).toBe(true)
|
||||
|
||||
const after = readAclEntries(dir)
|
||||
expect(after, listed(after)).toHaveLength(3)
|
||||
expect(after.every((entry) => entry.endsWith(':(OI)(CI)(F)'))).toBe(true)
|
||||
|
||||
// The point of the (IO) case: before the repair the directory object grants nobody anything,
|
||||
// so Orca cannot write into the directory it just cached as hardened.
|
||||
const child = join(dir, 'child.json')
|
||||
expect(() => writeFileSync(child, '{}')).not.toThrow()
|
||||
expect(readAclEntries(child).every((entry) => entry.includes('(I)'))).toBe(true)
|
||||
})
|
||||
|
||||
it('is idempotent: a second harden leaves the same DACL', () => {
|
||||
const file = join(root, 'idempotent.json')
|
||||
writeFileSync(file, '{}')
|
||||
|
||||
expect(restrictWindowsPathSync(file, false)).toBe(true)
|
||||
const first = readAclEntries(file)
|
||||
expect(restrictWindowsPathSync(file, false)).toBe(true)
|
||||
|
||||
expect(readAclEntries(file)).toEqual(first)
|
||||
})
|
||||
|
||||
/**
|
||||
* Verification writes a temp SDDL file. If it cannot, the ACL may well have been applied — but
|
||||
* it cannot be *proved*, so hardening must report failure rather than assume success. Fail
|
||||
* closed, and say so: a silently-unverifiable control is the shape of the original bug.
|
||||
*/
|
||||
it('reports failure, loudly, when verification cannot write its descriptor', () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const file = join(root, 'unverifiable.json')
|
||||
writeFileSync(file, '{}')
|
||||
const realTemp = process.env.TEMP
|
||||
const realTmp = process.env.TMP
|
||||
// Point the descriptor save at a directory that cannot exist.
|
||||
process.env.TEMP = join(root, 'no-such-dir', 'nested')
|
||||
process.env.TMP = process.env.TEMP
|
||||
|
||||
try {
|
||||
expect(restrictWindowsPathSync(file, false)).toBe(false)
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
'[secure-path.windows-acl] failed to restrict path',
|
||||
expect.objectContaining({ stage: 'verify' })
|
||||
)
|
||||
} finally {
|
||||
if (realTemp === undefined) {
|
||||
delete process.env.TEMP
|
||||
} else {
|
||||
process.env.TEMP = realTemp
|
||||
}
|
||||
if (realTmp === undefined) {
|
||||
delete process.env.TMP
|
||||
} else {
|
||||
process.env.TMP = realTmp
|
||||
}
|
||||
warn.mockRestore()
|
||||
}
|
||||
|
||||
// And the ACL itself was still applied, so the failure is a loss of proof, not of protection.
|
||||
expect(readAclEntries(file)).toHaveLength(3)
|
||||
})
|
||||
|
||||
it('reports failure for a path that does not exist', () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
expect(restrictWindowsPathSync(join(root, 'absent.json'), false)).toBe(false)
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
'[secure-path.windows-acl] failed to restrict path',
|
||||
expect.objectContaining({ stage: 'reset' })
|
||||
)
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
/**
|
||||
* Skipped rather than branched: elevated, hardening *succeeds* here, so the case would assert
|
||||
* nothing and would instead rewrite `hosts`. `icacls /reset` is no undo — it drops all explicit
|
||||
* ACEs, and `hosts` ships with an explicit `NT AUTHORITY\SYSTEM:(F)`. Ephemeral on a CI runner;
|
||||
* permanent for anyone running this lane from an elevated shell. So: assert, or skip.
|
||||
*/
|
||||
it.skipIf(elevated)('reports failure for a path it has no permission to modify', () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
// Owned by TrustedInstaller; a non-elevated user cannot rewrite its DACL.
|
||||
const systemFile = windowsSystem32Binary('drivers\\etc\\hosts')
|
||||
|
||||
expect(restrictWindowsPathSync(systemFile, false)).toBe(false)
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
'[secure-path.windows-acl] failed to restrict path',
|
||||
expect.objectContaining({ detail: expect.stringContaining('denied') })
|
||||
)
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
/**
|
||||
* `void promise.then(onSettled)` attaches no rejection handler, so a throw from `onSettled` —
|
||||
* which runs *after* the promise resolved, outside every try/catch inside the apply — rejects a
|
||||
* promise nobody holds. Node's default turns that into a dead Electron main process, which
|
||||
* presents as an Orca crash rather than as the hardening problem it is.
|
||||
*/
|
||||
it('reports rather than crashes when the settlement callback throws', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const reports: SecurePathHardeningReport[] = []
|
||||
setSecurePathHardeningReporter((entry) => reports.push(entry))
|
||||
const file = join(root, 'settle-throws.json')
|
||||
writeFileSync(file, '{}')
|
||||
|
||||
const unhandled: unknown[] = []
|
||||
const capture = (reason: unknown): void => {
|
||||
unhandled.push(reason)
|
||||
}
|
||||
process.on('unhandledRejection', capture)
|
||||
let settled = false
|
||||
try {
|
||||
bestEffortRestrictWindowsPath(file, false, () => {
|
||||
settled = true
|
||||
throw new Error('settlement callback exploded')
|
||||
})
|
||||
await vi.waitFor(() => expect(settled).toBe(true))
|
||||
// An unhandled rejection is raised a turn later, so give the loop one.
|
||||
await new Promise((resolve) => setTimeout(resolve, 50))
|
||||
} finally {
|
||||
process.off('unhandledRejection', capture)
|
||||
setSecurePathHardeningReporter(null)
|
||||
warn.mockRestore()
|
||||
}
|
||||
|
||||
expect(unhandled).toEqual([])
|
||||
expect(reports).toContainEqual(
|
||||
expect.objectContaining({
|
||||
stage: 'settle',
|
||||
detail: expect.stringContaining('settlement callback exploded')
|
||||
})
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,121 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { localDomainSidOf, parseSddlDacl, resolveSddlSid } from './windows-security-descriptor'
|
||||
|
||||
const MACHINE_SID = 'S-1-5-21-432636774-4279371817-3971399515'
|
||||
const USER_SID = `${MACHINE_SID}-1001`
|
||||
/** The built-in Administrator: the account a CI runner and an Administrator-only box log in as. */
|
||||
const LOCAL_ADMIN_SID = `${MACHINE_SID}-500`
|
||||
|
||||
describe('parseSddlDacl', () => {
|
||||
it('reads a hardened file descriptor as icacls /save emits it', () => {
|
||||
const dacl = parseSddlDacl(`D:PAI(A;;FA;;;BA)(A;;FA;;;SY)(A;;FA;;;${USER_SID})`)
|
||||
|
||||
expect(dacl?.isProtected).toBe(true)
|
||||
expect(dacl?.aces).toEqual([
|
||||
{ type: 'A', flags: [], rights: 'FA', sid: 'S-1-5-32-544' },
|
||||
{ type: 'A', flags: [], rights: 'FA', sid: 'S-1-5-18' },
|
||||
{ type: 'A', flags: [], rights: 'FA', sid: USER_SID }
|
||||
])
|
||||
})
|
||||
|
||||
it('reads the inheritable flags a hardened directory carries', () => {
|
||||
const dacl = parseSddlDacl('D:PAI(A;OICI;FA;;;SY)')
|
||||
|
||||
expect(dacl?.aces[0]!.flags).toEqual(['OI', 'CI'])
|
||||
})
|
||||
|
||||
it('reports an unprotected descriptor and its inherited rules', () => {
|
||||
const dacl = parseSddlDacl(`D:(A;ID;FA;;;SY)(A;ID;FA;;;${USER_SID})`)
|
||||
|
||||
expect(dacl?.isProtected).toBe(false)
|
||||
expect(dacl?.aces.map((ace) => ace.flags)).toEqual([['ID'], ['ID']])
|
||||
})
|
||||
|
||||
// `OICIID` is three tokens, not a string to search: a substring test for 'ID' would also fire on
|
||||
// flag runs that merely happen to contain those letters in sequence.
|
||||
it('splits a flag run into two-letter tokens', () => {
|
||||
expect(parseSddlDacl('D:P(A;OICIID;FA;;;SY)')?.aces[0]!.flags).toEqual(['OI', 'CI', 'ID'])
|
||||
expect(parseSddlDacl('D:P(A;OICI;FA;;;SY)')?.aces[0]!.flags).not.toContain('ID')
|
||||
})
|
||||
|
||||
it('stops at a trailing SACL rather than absorbing its entries', () => {
|
||||
const dacl = parseSddlDacl('D:P(A;;FA;;;SY)S:AI(AU;SAFA;FA;;;WD)')
|
||||
|
||||
expect(dacl?.aces).toHaveLength(1)
|
||||
expect(dacl?.aces[0]!.sid).toBe('S-1-5-18')
|
||||
})
|
||||
|
||||
it('finds the DACL after an owner and group whose aliases end in D', () => {
|
||||
const dacl = parseSddlDacl('O:WDG:WDD:P(A;;FA;;;SY)')
|
||||
|
||||
expect(dacl?.isProtected).toBe(true)
|
||||
expect(dacl?.aces[0]!.sid).toBe('S-1-5-18')
|
||||
})
|
||||
|
||||
it('returns null when there is no DACL', () => {
|
||||
expect(parseSddlDacl('O:BAG:BA')).toBeNull()
|
||||
})
|
||||
|
||||
it('returns null for a truncated ACE rather than guessing its fields', () => {
|
||||
expect(parseSddlDacl('D:P(A;;FA)')).toBeNull()
|
||||
})
|
||||
|
||||
/**
|
||||
* The DACL a box whose user *is* the built-in Administrator reads back: icacls writes `LA`
|
||||
* where it wrote the raw SID for any other account, so identity checking has to resolve it or
|
||||
* the path it just hardened verifies as wrong and hardening reports failure forever.
|
||||
*/
|
||||
it('resolves the built-in Administrator alias against the machine SID', () => {
|
||||
const dacl = parseSddlDacl(`D:PAI(A;;FA;;;BA)(A;;FA;;;SY)(A;;FA;;;LA)`, MACHINE_SID)
|
||||
|
||||
expect(dacl?.aces.map((ace) => ace.sid)).toEqual(['S-1-5-32-544', 'S-1-5-18', LOCAL_ADMIN_SID])
|
||||
})
|
||||
|
||||
it('leaves the alias unresolved when no machine SID is known, so it matches nothing', () => {
|
||||
const dacl = parseSddlDacl('D:PAI(A;;FA;;;LA)')
|
||||
|
||||
expect(dacl?.aces[0]!.sid).toBe('LA')
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveSddlSid', () => {
|
||||
it('resolves the aliases icacls substitutes for well-known SIDs', () => {
|
||||
expect(resolveSddlSid('WD')).toBe('S-1-1-0')
|
||||
expect(resolveSddlSid('BA')).toBe('S-1-5-32-544')
|
||||
expect(resolveSddlSid('SY')).toBe('S-1-5-18')
|
||||
expect(resolveSddlSid('AU')).toBe('S-1-5-11')
|
||||
})
|
||||
|
||||
it('passes a raw SID through unchanged', () => {
|
||||
expect(resolveSddlSid(USER_SID)).toBe(USER_SID)
|
||||
})
|
||||
|
||||
// An unknown alias must not silently compare equal to anything expected.
|
||||
it('passes an unrecognized alias through instead of dropping it', () => {
|
||||
expect(resolveSddlSid('ZZ')).toBe('ZZ')
|
||||
})
|
||||
|
||||
// No fixed table can hold these: the SID they name is built from the machine's own.
|
||||
it('builds the machine-relative aliases from the supplied machine SID', () => {
|
||||
expect(resolveSddlSid('LA', MACHINE_SID)).toBe(LOCAL_ADMIN_SID)
|
||||
expect(resolveSddlSid('LG', MACHINE_SID)).toBe(`${MACHINE_SID}-501`)
|
||||
})
|
||||
|
||||
it('keeps a machine-relative alias unresolved without a machine SID', () => {
|
||||
expect(resolveSddlSid('LA')).toBe('LA')
|
||||
})
|
||||
})
|
||||
|
||||
describe('localDomainSidOf', () => {
|
||||
it('strips the RID off a domain-relative account SID', () => {
|
||||
expect(localDomainSidOf(USER_SID)).toBe(MACHINE_SID)
|
||||
expect(localDomainSidOf(LOCAL_ADMIN_SID)).toBe(MACHINE_SID)
|
||||
})
|
||||
|
||||
// SYSTEM and the built-in groups carry no machine authority to resolve `LA` against.
|
||||
it('returns null for SIDs that are not domain-relative', () => {
|
||||
expect(localDomainSidOf('S-1-5-18')).toBeNull()
|
||||
expect(localDomainSidOf('S-1-5-32-544')).toBeNull()
|
||||
expect(localDomainSidOf('S-1-1-0')).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,108 @@
|
||||
/**
|
||||
* Parses the DACL out of an SDDL string, as produced by `icacls <path> /save`.
|
||||
*
|
||||
* Why SDDL rather than the human-readable `icacls <path>` listing: the listing prints resolved
|
||||
* account *names*, which are localized and cannot be compared against a SID. Checking only the
|
||||
* shape of that listing — rule count, inheritance marker, rights — accepts a DACL that grants
|
||||
* full control to the wrong principal entirely. SDDL carries raw SIDs, so identity is checkable.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Aliases naming an account by RID within the machine's *own* SID, so no constant can hold them:
|
||||
* the built-in Administrator is `S-1-5-21-<machine>-500`, and icacls prints `LA` for it. A box
|
||||
* whose interactive user is that account (a CI runner, an Administrator-only install) therefore
|
||||
* reads back an ACE no fixed table can match.
|
||||
*/
|
||||
const LOCAL_DOMAIN_RELATIVE_RIDS: Record<string, number> = {
|
||||
LA: 500,
|
||||
LG: 501
|
||||
}
|
||||
|
||||
/** `S-1-5-21-x-y-z-<rid>` split into the machine/domain authority and its RID. */
|
||||
const DOMAIN_RELATIVE_SID_PATTERN = /^(S-1-5-21(?:-\d+){3})-\d+$/
|
||||
|
||||
/**
|
||||
* The machine/domain authority an account SID belongs to, or null when the SID is not
|
||||
* domain-relative (`S-1-5-18` and the `S-1-5-32-*` built-ins never are).
|
||||
*/
|
||||
export function localDomainSidOf(accountSid: string): string | null {
|
||||
return DOMAIN_RELATIVE_SID_PATTERN.exec(accountSid.toUpperCase())?.[1] ?? null
|
||||
}
|
||||
|
||||
/** The two-letter aliases SDDL substitutes for well-known SIDs. */
|
||||
const SDDL_SID_ALIASES: Record<string, string> = {
|
||||
AN: 'S-1-5-7',
|
||||
AU: 'S-1-5-11',
|
||||
BA: 'S-1-5-32-544',
|
||||
BG: 'S-1-5-32-546',
|
||||
BU: 'S-1-5-32-545',
|
||||
IU: 'S-1-5-4',
|
||||
LS: 'S-1-5-19',
|
||||
NS: 'S-1-5-20',
|
||||
NU: 'S-1-5-2',
|
||||
SY: 'S-1-5-18',
|
||||
WD: 'S-1-1-0'
|
||||
}
|
||||
|
||||
export type WindowsAce = {
|
||||
/** `A` for allow, `D` for deny, plus the audit types. */
|
||||
type: string
|
||||
/** Two-letter inheritance/audit tokens, e.g. `['OI', 'CI']` or `['ID']`. */
|
||||
flags: string[]
|
||||
rights: string
|
||||
/**
|
||||
* Always a raw SID: aliases are resolved, unknown tokens are passed through upper-cased.
|
||||
* Machine-relative aliases resolve only when `localDomainSid` is supplied, so an unresolved
|
||||
* `LA` compares unequal to every real SID and the caller fails closed.
|
||||
*/
|
||||
sid: string
|
||||
}
|
||||
|
||||
export type WindowsDacl = {
|
||||
/** True when the DACL carries the `P` flag, i.e. inheritance from the parent is blocked. */
|
||||
isProtected: boolean
|
||||
aces: WindowsAce[]
|
||||
}
|
||||
|
||||
export function parseSddlDacl(sddl: string, localDomainSid?: string): WindowsDacl | null {
|
||||
// ACE bodies never contain parentheses, so the group stops cleanly at a following `S:` SACL.
|
||||
const dacl = /D:([A-Z]*)((?:\([^()]*\))*)/.exec(sddl)
|
||||
if (!dacl) {
|
||||
return null
|
||||
}
|
||||
const aces: WindowsAce[] = []
|
||||
for (const group of dacl[2]!.matchAll(/\(([^()]*)\)/g)) {
|
||||
const fields = group[1]!.split(';')
|
||||
if (fields.length < 6) {
|
||||
return null
|
||||
}
|
||||
aces.push({
|
||||
type: fields[0]!.toUpperCase(),
|
||||
flags: splitAceFlags(fields[1]!),
|
||||
rights: fields[2]!.toUpperCase(),
|
||||
sid: resolveSddlSid(fields[5]!, localDomainSid)
|
||||
})
|
||||
}
|
||||
return { isProtected: dacl[1]!.includes('P'), aces }
|
||||
}
|
||||
|
||||
/**
|
||||
* ACE flags are a run of two-letter tokens (`OICIID`), not a free-form string. Splitting them
|
||||
* keeps a substring search from reading `ID` out of an adjacent pair.
|
||||
*/
|
||||
function splitAceFlags(flags: string): string[] {
|
||||
const tokens: string[] = []
|
||||
for (let index = 0; index + 1 < flags.length; index += 2) {
|
||||
tokens.push(flags.slice(index, index + 2).toUpperCase())
|
||||
}
|
||||
return tokens
|
||||
}
|
||||
|
||||
export function resolveSddlSid(token: string, localDomainSid?: string): string {
|
||||
const upper = token.toUpperCase()
|
||||
const rid = LOCAL_DOMAIN_RELATIVE_RIDS[upper]
|
||||
if (rid !== undefined) {
|
||||
return localDomainSid ? `${localDomainSid}-${rid}` : upper
|
||||
}
|
||||
return SDDL_SID_ALIASES[upper] ?? upper
|
||||
}
|
||||
Reference in New Issue
Block a user