mirror of
https://github.com/stablyai/orca.git
synced 2026-09-23 16:02:24 +00:00
0cbb01ef4b
* 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>
859 lines
35 KiB
TypeScript
859 lines
35 KiB
TypeScript
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/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
|
||
const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||
const tempDirs: string[] = []
|
||
|
||
beforeEach(() => {
|
||
process.env.SystemRoot = 'C:\\Windows'
|
||
delete process.env.WINDIR
|
||
__resetSecureFileWindowsUserSidForTests()
|
||
__resetSecureFileHardenedPathsForTests()
|
||
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}"` }
|
||
}
|
||
return fakeIcacls(spec)
|
||
})
|
||
vi.mocked(runProcess).mockImplementation((spec) => Promise.resolve(fakeIcacls(spec)))
|
||
})
|
||
|
||
afterEach(() => {
|
||
if (originalSystemRoot === undefined) {
|
||
delete process.env.SystemRoot
|
||
} else {
|
||
process.env.SystemRoot = originalSystemRoot
|
||
}
|
||
if (originalWindir === undefined) {
|
||
delete process.env.WINDIR
|
||
} else {
|
||
process.env.WINDIR = originalWindir
|
||
}
|
||
__resetSecureFileWindowsUserSidForTests()
|
||
__resetSecureFileHardenedPathsForTests()
|
||
if (originalPlatform) {
|
||
Object.defineProperty(process, 'platform', originalPlatform)
|
||
}
|
||
for (const dir of tempDirs.splice(0)) {
|
||
rmSync(dir, { recursive: true, force: true })
|
||
}
|
||
})
|
||
|
||
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(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', () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
const targetPath = join(userDataPath, 'secret.json')
|
||
writeFileSync(targetPath, '{}')
|
||
|
||
hardenExistingSecureFile(targetPath)
|
||
hardenExistingSecureFile(targetPath)
|
||
|
||
// dir hardened once (path-cached), file hardened once (metadata-cached) — 2 total
|
||
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', () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||
__resetSecureFileHardenedPathsForTests({
|
||
maxEntries: 2,
|
||
maxKeyBytes: 4096,
|
||
maxTotalKeyBytes: 8192
|
||
})
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
const paths = ['first.json', 'second.json', 'third.json'].map((name) =>
|
||
join(userDataPath, name)
|
||
)
|
||
for (const path of paths) {
|
||
writeFileSync(path, '{}')
|
||
hardenExistingSecureFile(path)
|
||
}
|
||
|
||
hardenExistingSecureFile(paths[0]!)
|
||
|
||
const fileTargets = getHardenAclCalls()
|
||
.map(getAclTarget)
|
||
.filter((path) => paths.includes(path))
|
||
expect(fileTargets).toEqual([...paths, paths[0]])
|
||
expect(__getSecureFileHardeningCacheStateForTests().paths).toMatchObject({
|
||
entries: 2
|
||
})
|
||
})
|
||
|
||
it('LRU-evicts Windows directory hardening entries instead of retaining every path', () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||
__resetSecureFileHardenedPathsForTests({
|
||
maxEntries: 2,
|
||
maxKeyBytes: 4096,
|
||
maxTotalKeyBytes: 8192
|
||
})
|
||
const root = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(root)
|
||
const directories = ['first', 'second', 'third'].map((name) => join(root, name))
|
||
const files = directories.map((dir) => {
|
||
mkdirSync(dir)
|
||
const file = join(dir, 'secret.json')
|
||
writeFileSync(file, '{}')
|
||
return file
|
||
})
|
||
for (const file of files) {
|
||
hardenExistingSecureFile(file)
|
||
}
|
||
|
||
hardenExistingSecureFile(files[0]!)
|
||
|
||
const directoryTargets = getHardenAclCalls()
|
||
.map(getAclTarget)
|
||
.filter((path) => directories.includes(path))
|
||
expect(directoryTargets).toEqual([...directories, directories[0]])
|
||
expect(__getSecureFileHardeningCacheStateForTests().directories).toMatchObject({
|
||
entries: 2
|
||
})
|
||
})
|
||
|
||
it('re-hardens an existing file when its metadata changes after caching', async () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
const targetPath = join(userDataPath, 'secret.json')
|
||
writeFileSync(targetPath, '{}')
|
||
|
||
hardenExistingSecureFile(targetPath)
|
||
await waitForFileTimestampTick()
|
||
writeFileSync(targetPath, '{"changed":true}')
|
||
hardenExistingSecureFile(targetPath)
|
||
|
||
// call 1: dir + file. call 2: dir skipped (path-cached), file re-hardened (new mtime)
|
||
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', () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
const targetPath = join(userDataPath, 'secret.json')
|
||
|
||
writeSecureFile(targetPath, 'first')
|
||
writeSecureFile(targetPath, 'second')
|
||
|
||
// The DIRECTORY is hardened async + path-cached: exactly once across both writes.
|
||
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 = 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 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 () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
const targetPath = join(userDataPath, 'secret.json')
|
||
writeFileSync(targetPath, '{}')
|
||
|
||
// Simulate the env-store read loop: hardenExistingSecureFile called many times while
|
||
// another part of Orca writes to the same directory (changing its mtime).
|
||
hardenExistingSecureFile(targetPath)
|
||
await waitForFileTimestampTick()
|
||
// Simulate a write to another file in the same dir (changes dir mtime)
|
||
writeFileSync(join(userDataPath, 'other.json'), '{}')
|
||
hardenExistingSecureFile(targetPath)
|
||
hardenExistingSecureFile(targetPath)
|
||
|
||
// The parent directory must be hardened exactly ONCE despite its mtime changing
|
||
const dirCalls = getHardenAclCalls().filter((call) => getAclTarget(call) === userDataPath)
|
||
expect(dirCalls).toHaveLength(1)
|
||
})
|
||
|
||
it('does not re-harden an unchanged file on repeated reads', () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
const targetPath = join(userDataPath, 'secret.json')
|
||
writeFileSync(targetPath, '{}')
|
||
|
||
hardenExistingSecureFile(targetPath)
|
||
hardenExistingSecureFile(targetPath)
|
||
hardenExistingSecureFile(targetPath)
|
||
|
||
const fileCalls = getHardenAclCalls().filter((call) => getAclTarget(call) === targetPath)
|
||
expect(fileCalls).toHaveLength(1)
|
||
})
|
||
|
||
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 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 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-'))
|
||
tempDirs.push(userDataPath)
|
||
const targetPath = join(userDataPath, 'secret.json')
|
||
|
||
writeSecureFile(targetPath, 'contents')
|
||
|
||
// Directory: async only.
|
||
expect(getHardenAclCalls().map(getAclTarget)).toEqual([userDataPath])
|
||
// File (tmpFile + renamed target): synchronous only — no async file ACL window.
|
||
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(getHardenAclCalls().map(getAclTarget)).not.toContain(targetPath)
|
||
})
|
||
|
||
// Nit #1 (review): the synchronous file path must cache as hardened ONLY on confirmed
|
||
// success, so a failed ACL apply is retried on the next write instead of being silently
|
||
// trusted.
|
||
it('retries the credential-file ACL on the next write when the sync apply fails', () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
const targetPath = join(userDataPath, 'secret.json')
|
||
|
||
// 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 = 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(runProcessSync).mockImplementation((spec) => {
|
||
if (spec.program === 'C:\\Windows\\System32\\whoami.exe') {
|
||
return { ...OK, stdout: '"USER","S-1-5-21-1000"' }
|
||
}
|
||
return OK
|
||
})
|
||
writeSecureFile(targetPath, 'second')
|
||
const allTargetCalls = getSyncHardenAclCalls()
|
||
.map(getAclTarget)
|
||
.filter((entry) => entry === targetPath)
|
||
expect(allTargetCalls).toHaveLength(2)
|
||
})
|
||
|
||
// Nit #2 (review) / hardening: the process-lifetime directory cache hardens a directory
|
||
// exactly once even when its mtime churns across many writes (the #4901 storm condition,
|
||
// exercised through the write path rather than the read path).
|
||
it('hardens the directory exactly once across many writes despite mtime churn', () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
|
||
for (let i = 0; i < 5; i++) {
|
||
// Each write changes the directory's mtime (a new file lands in it).
|
||
writeSecureFile(join(userDataPath, `secret-${i}.json`), `contents-${i}`)
|
||
}
|
||
|
||
const dirCalls = getHardenAclCalls().filter((call) => getAclTarget(call) === userDataPath)
|
||
expect(dirCalls).toHaveLength(1)
|
||
})
|
||
|
||
// win32-only guard: on non-win32 platforms no icacls is ever spawned (sync or async);
|
||
// POSIX hardening uses chmodSync only.
|
||
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)
|
||
const targetPath = join(userDataPath, 'secret.json')
|
||
|
||
writeSecureFile(targetPath, 'contents')
|
||
hardenExistingSecureFile(targetPath)
|
||
|
||
expect(getHardenAclCalls()).toHaveLength(0)
|
||
expect(getSyncHardenAclCalls()).toHaveLength(0)
|
||
})
|
||
|
||
posixModeIt('re-hardens a POSIX directory when its metadata changes after caching', () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' })
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
const targetPath = join(userDataPath, 'secret.json')
|
||
writeFileSync(targetPath, '{}')
|
||
|
||
hardenExistingSecureFile(targetPath)
|
||
expect(statMode(userDataPath)).toBe(0o700)
|
||
|
||
chmodSync(userDataPath, 0o755)
|
||
hardenExistingSecureFile(targetPath)
|
||
|
||
expect(statMode(userDataPath)).toBe(0o700)
|
||
})
|
||
|
||
posixModeIt('LRU-bounds POSIX hardening entries while keeping recent paths cached', () => {
|
||
Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' })
|
||
__resetSecureFileHardenedPathsForTests({
|
||
maxEntries: 2,
|
||
maxKeyBytes: 4096,
|
||
maxTotalKeyBytes: 8192
|
||
})
|
||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-secure-file-'))
|
||
tempDirs.push(userDataPath)
|
||
const firstPath = join(userDataPath, 'first.json')
|
||
const secondPath = join(userDataPath, 'second.json')
|
||
writeFileSync(firstPath, '{}')
|
||
writeFileSync(secondPath, '{}')
|
||
|
||
hardenExistingSecureFile(firstPath)
|
||
hardenExistingSecureFile(secondPath)
|
||
expect(__getSecureFileHardeningCacheStateForTests().paths.paths).toEqual([
|
||
userDataPath,
|
||
secondPath
|
||
])
|
||
|
||
hardenExistingSecureFile(firstPath)
|
||
expect(__getSecureFileHardeningCacheStateForTests().paths.paths).toEqual([
|
||
userDataPath,
|
||
firstPath
|
||
])
|
||
})
|
||
})
|
||
|
||
/**
|
||
* 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
|
||
}
|
||
|
||
// 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))
|
||
}
|
||
|
||
// 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> {
|
||
await new Promise((resolve) => setTimeout(resolve, 20))
|
||
}
|
||
|
||
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)
|
||
})
|
||
})
|