mirror of
https://github.com/stablyai/orca.git
synced 2026-11-02 10:08:13 +00:00
e8a18a03d5a22d362d1bbb54cf89fa16e2224efb
16
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  OrcaWinandOrca Worker
|
4559c250b3 |
fix(windows): restrict secure files in-process instead of spawning icacls (#28135)
* fix(windows): restrict secure files in-process instead of spawning icacls Every secure write on Windows spawned icacls.exe five times on the calling thread (the Electron main thread for most stores): verify, /reset, /inheritance:r /grant:r, verify, verify. Process creation under endpoint security costs ~0.6 s each, so each write froze the UI for seconds (#26027). The /reset + /inheritance:r pair is also what behavioural EDR scores as ransomware tradecraft (#25583). Add native/windows-security (@orca/windows-security): a small N-API addon over advapi32 that reads and writes DACLs in-process. It exposes the token user SID, a DACL read as SDDL, a DACL write (sync and on the thread pool), a create-with-DACL file write, and an additive grant. Policy and verification stay in TypeScript. - writeSecureFile creates each temp file already carrying the protected user/SYSTEM/Administrators DACL (CreateFileW + SECURITY_ATTRIBUTES), reads it back once, and renames it. No ACL is modified on the write path and no process starts. Before: 5-7 spawns, 94-138 ms per write. After: 0 spawns, ~0.5 ms. - A secure directory is verified once per process. It is rewritten only when verification fails, on the thread pool, because the write walks every descendant (~2 s on a 29k-entry userData). - The verifier follows OpenSSH's rule: only the user, SYSTEM and Administrators may be granted anything, and the path must be protected (or, for a file, inherit from a secure parent). DACLs written by the icacls-era hardening pass unchanged, so there is no migration. - If the addon cannot load, the write still happens and reports unrestricted, logged once. There is no icacls or PowerShell fallback. - grantDirAcl, the startup userData grant, and the install-dir probe and package repair use the same addon, so whoami.exe and icacls.exe are no longer spawned anywhere at runtime. - orcad's Windows slots ship the addon beside their bundles (built per arch, PE-machine checked). Its data-root privacy check fails closed, so without the addon a Windows orcad would not start. Fixes #26027 Fixes #25583 * refactor(secure-file): harden existing Windows files through the sync lane The read path called restrictWindowsPathSync directly and recorded success itself; the existing sync lane already does both. One call site fewer. * refactor(windows-security): one load, one rule, no fallthrough (review #28135) - The descriptor parser is anchored, so a conditional ACE (its body holds parentheses) or anything else it cannot read fails closed instead of verifying the ACEs before it. - findSecureDaclViolation is one flat rule: protected, allow rules only for the user, SYSTEM and Administrators (OpenSSH's ACE rule), and full control for the user. The inherits-from-a-secure-parent exception is gone: it accepted exactly what loosening the parent reaches. - The addon and the user SID load once, behind one wrapper that adds the extended-length prefix to every path and runs async DACL writes to one path in order, so a startup grant cannot land after a re-lock and restore the broader DACL. - With the addon loaded, a failed born-restricted create throws with the matching Node error code instead of falling back to a plain write. - A grant leaves a NULL DACL untouched instead of turning 'everyone' into 'only this SID'. WriteFile runs once and fails on a short write. - The artifact create-intent store writes through writeSecureFile and drops its own temp writer, directory LRU and synchronous directory re-lock. - Test seams the global fake makes redundant are gone. * fix(packaging): fail a Windows build whose workspace addon binary is missing or foreign-arch The @orca/windows-* workspace links exist on every host, so their presence proved nothing: a missing or wrong-architecture .node only surfaced at runtime, as unrestricted secure files or an orcad that refuses to start. beforePack now reads each addon's PE machine against the target arch. * refactor(windows-security): prune what the native ACL left dead (review #28135) - Sync-spawn ratchet: hardenSecurePath and the sync-flag exemption it alone used are gone, and the advice and baseline header no longer say secure writers run icacls. - The install-dir probe reads ACEs through parseSddlDacl, so an ACE it cannot parse fails the whole reading closed instead of being skipped; AC is a resolved alias there now. - The packaging check reads the addon binary through node_modules, the copy that ships. - No winCode-to-errno table: nothing reads .code off a failed secure write. - The load memo is not mutated; secure-path keeps its own log-once flag. - grantInstallDirPackageAcl is a plain function, and secure-file restricts a Windows directory and a POSIX path directly instead of through one two-platform helper. * fix(windows-security): resolve LA in real-ACL tests; tear a file whose close fails - The GitHub Windows runner is the built-in Administrator (RID 500), whose SID reads back as LA. Production already resolves it through localDomainSidOf; the win32 test helpers parsed without it. One shared readDaclOf resolves aliases for every real-ACL suite, and the verifier's unit test covers an LA-aliased directory and LA as a stranger. - createFileWithDacl treats a failed CloseHandle like a failed write (lost data on a network volume) and deletes the torn file. - Drop a duplicate test reset and a typo. * fix(windows-security): load only from inside the app; read DACLs as structure (review #28135) - Loader: orcad's staged binary first, then the workspace package only if its real path is inside the app (process.resourcesPath, or this checkout). A bare specifier otherwise walks every parent up to C:\node_modules, which any local user may create, and orcad holds credentials. - The addon reads a DACL as { protected, aces: [{ type, flags, mask, sid }] } via GetAce, with a SID only for plain allow/deny ACEs and null for a NULL DACL. findSecureDaclViolation and the install-dir probe check that structure, so conditional, callback and object ACEs fail closed. The SDDL parser, its alias table, LA/localDomainSid resolution and getDaclSddl are gone; SDDL is only the write format. - Install-dir tree pass: files flagless as before, but a directory only when its own DACL is protected (inheritably). Re-granting inheriting directories re-propagated each subtree (3.2 s for 1,093 directories). The root is no longer granted, or counted, twice. The two passes are plain functions behind a { root, tree } seam. - One namespacing (path.win32.toNamespacedPath) at the addon boundary, also the per-path ordering key; the comment states the real sync-grant invariant. - restrictWindowsDirectory(dir): Promise<boolean> replaces the callback lane; the reporter is guarded once, so the 'settle' stage is gone. - Sync-spawn ratchet: the secure writers no longer spawn or block, so they leave the gate (109 -> 61 uses). - Errors name the Win32 call and code only; a failed GetSecurityDescriptorControl fails the write instead of defaulting to unprotected. * test(secure-file): assert the durable POSIX write's outcome, not the old chmod order writeSecureFile now chmods the temp file to 0o600 before fsyncing it, so the fsync also covers the mode change and the file is owner-writable when opened. Assert what matters under umask 0o200: the write succeeds, the temp file is fsynced read-only, and the published file is exactly 0o600. * fix(windows-security): namespace paths with the host's toNamespacedPath path.win32.toNamespacedPath rewrote POSIX temp paths (/tmp/x -> \tmp\x) in the Linux runs of the Windows-logic unit tests, so the fake addon wrote to the wrong place. The addon only runs on Windows, where the host function is the win32 one; elsewhere it is the identity. The long-path assertion runs on Windows only. * test(windows-security): assert the install-dir long-path prefix on Windows only The prefix comes from the host's toNamespacedPath, which is the identity off Windows; the Linux run of this suite expected the win32 escape. * fix(windows-user-data-acl): grant the root even when its children cannot be listed (review #28135) A readdir failure on userData aborted the whole grant, so the root grant that could repair a DACL refusing the listing was never attempted. The listing failure is now recorded, the root is still granted, and the marker stays withheld so the next launch retries. * test(windows-security): wait for relock propagation, accept Entra ID SIDs (review #28135) - The large-relock test waited only for the parent to turn protected, which happens before propagation reaches the children, then read a child. It now waits on the parent and both ends of the child range. - The token-user test rejected S-1-12-1 (Entra ID) accounts, failing the suite on Entra-joined dev machines. --------- Co-authored-by: Orca Worker <orca-worker@localhost> |
||
 Neil
|
1f6ab26ec6 | fix(accounts): preserve competing keys during failed GLM saves | ||
|
0cbb01ef4b |
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> |
||
 Jinjing
|
7c93aed6dc |
Fix fsync of read-only files on POSIX (#14235)
* fix(files): fsync read-only files on POSIX * test(e2e): add golden E2E tests for POSIX profile index fsync Validates that profile index files are properly persisted on POSIX systems, including with restrictive umask settings. These are release-blocking golden tests for Linux and macOS. * test(terminal): wait for fish child ownership before stdin write Fish 4.8 withdraws DECSET 2031 before spawning the child, so the shell-contracts harness could send hello into an intermediate prompt and hang waiting for CHILD-READ. Wait for the child's CHILD-READY marker and answer split DA1/CPR/OSC queries across chunk boundaries. * test(e2e): verify profile index persists to disk with restrictive umask Strengthen the POSIX fsync test to verify the rebuilt index is actually written to disk and has correct permissions under a restrictive umask, not just cached in memory. |
||
    
|
e8044b1b30 |
fix(windows): restore fresh-profile startup after durable fsync (#14173)
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Co-authored-by: DHTheOne <238933622+DHTheOne@users.noreply.github.com> Co-authored-by: Anton Tupitsyn <70199858+PLUTONYY@users.noreply.github.com> Co-authored-by: 7loop <48346764+7loop@users.noreply.github.com> |
||
 Jinwoo Hong
|
536d17dca3 | fix(artifacts): recover committed cloud mutations (#13796) | ||
 NeilandOrca
|
879aad7dd6 |
oom(foundation): bound shared readers/limits + add BoundedMap primitive (#10299)
* oom(01): A1-shared-readers — reintroduce #10179 subset Files: 18 applied, 0 deleted (from |
||
|
NeilandOrca
|
aab112933e |
Revert "fix(memory): bound OOM-prone accumulators (#10179)" (#10255)
Co-authored-by: Orca <help@stably.ai> |
||
|
Neil
|
8f40ddf328 | fix(memory): bound OOM-prone accumulators (#10179) | ||
|
NeilandOrca
|
0989128287 |
refactor(comments): slim verbose comments in shared/cli/relay/preload (#9544)
Collapse multi-line explanatory comment blocks into single-line "why" statements
per AGENTS.md ("Document the Why, Briefly"): drop restatements of the code and
mechanism narration; keep the non-obvious reason, external refs, and directives.
Comments-only — verified no code changed via a Babel/esbuild comment-strip
token-equality gate against origin/main; typecheck and oxlint clean.
Area: shared, cli, relay, preload. 26 files changed, 1039 insertions(+), 3300 deletions(-).
Co-authored-by: Orca <help@stably.ai>
|
||
 Vladislav Meshkorudnyj
|
fa3ca498c2 |
fix(secure-file): re-harden on coarse-ctime filesystems
Store POSIX mode bits in secure-file hardening cache entries so permission drift is detected even when ctime granularity is coarse. Also clears inherited HISTFILE in the local PTY test harness for hermetic WSL history assertions, and adds a deterministic coarse-ctime regression test for directory and credential-file mode drift. |
||
|
NeilandOrca
|
46646d7ff1 |
chore(lint): upgrade oxlint to 1.71 + enable 7 new rules (autofixed backlog) (#6841)
* chore(lint): upgrade oxlint to 1.71 and enable 7 new rules Upgrade oxlint 1.67.0 -> 1.71.0 (1.72 was blocked by the repo's 3-day minimum-release-age supply-chain guard; nothing here needs it). The bump is a no-op on the existing config. Enable 3 error rules (backlog autofixed to zero in this commit) and 4 warn rules (surface signal without gating CI): error (autofixed, behavior-preserving): - unicorn/prefer-node-protocol (~1531 sites: bare builtin -> node:) - typescript/no-import-type-side-effects (~36: all-inline-type -> import type) - unicorn/no-array-reverse (19: copy-then-reverse -> toReversed) warn (real signal, current fires are test-only/correct): - unicorn/no-array-fill-with-reference-type (aliasing footgun guard) - typescript/no-unsafe-function-type (bans bare Function type) - unicorn/prefer-array-flat-map (map().flat() -> flatMap()) - unicorn/prefer-regexp-test (.match() in bool ctx -> .test()) mobile/.oxlintrc.json extends root, so it inherits all 7; the autofix ran from root and covered mobile/ too. Verification (all green): oxlint 0 errors (root+mobile+aux configs), oxfmt clean, typecheck (node+cli+web), vitest 22795 passed / 0 failed, builds (electron-vite + web + cli) succeed. node: rewrites confirmed to skip embedded SSH/CLI string payloads (AST-only); all toReversed sites verified to operate on fresh copies or write-once locals. * chore(lint): bump mobile oxlint to 1.71 so inherited rules parse mobile/ is a standalone pnpm project pinning its own oxlint@1.67, which lacks unicorn/no-array-fill-with-reference-type (needs >=1.70). Since mobile/.oxlintrc.json extends the root config, mobile CI's 'cd mobile && oxlint' failed to parse the new rule. Bump mobile to match root (1.71). Verified in mobile/: oxlint 0 errors, oxfmt --check clean, tsc --noEmit pass, vitest 978 passed / 0 failed. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai> |
||
 
|
4d6c291eff |
fix(windows): stop main-thread PowerShell ACL storm on env-store reads (#5011)
* fix(windows): stop main-thread PowerShell storm on env-store reads Two changes fix the v1.4.52+ Windows performance regression (#4901 regression against #4840) where 49 powershell.exe processes were spawned in 27 seconds during load, saturating the Electron main thread and causing black terminals and runtimeEnvironments:call timeouts. Root cause: `readEnvironmentStore` calls `hardenExistingSecureFile` on every read. The env-store parent directory's mtime churns constantly (every secure write updates it), so the mtime-keyed idempotency cache never matched → `bestEffortRestrictWindowsPath` (powershell, ~1-1.5s synchronous) fired on every call. After #4901, the remote-runtime tab-sync loop reads the store ~2×/s, turning sporadic mtime misses into a continuous main-thread storm. Fix 1 – path-cached directory hardening: add `hardenedDirectoryPathsThisProcess (Set<string>)` that caches directory hardening by PATH for the process lifetime. A directory's required ACL does not change when its mtime changes; only file hardening retains the metadata-keyed cache so post-rename inode changes are detected correctly. Fix 2 – async ACL application: replace `execFileSync(powershell.exe, ...)` with `execFile` (fire-and-forget). PowerShell cold-start is ~1-1.5s; the function is already named `bestEffortRestrictWindowsPath` so async/optimistic caching is correct. `applySecurePathRestriction` returns `true` optimistically on win32 so the cache entry is written before the background process completes. Tests: new regression tests verify the directory is hardened exactly once even when its mtime changes between calls, that unchanged files are not re-hardened, and that ACL application goes through async execFile (not execFileSync). * fix(windows): apply credential-file ACL synchronously on write path Follow-up rigor on the env-store PowerShell ACL storm fix (#5006). The read-path storm fix (path-cached async directory hardening + async file re-harden) is retained, but switching ALL ACL application to async opened a narrow Windows-only security window: because writeFileSync({mode}) is a no-op on Windows, writeSecureFile returned with the credential file still carrying the parent directory's inherited (broader) ACL for the ~1-1.5s PowerShell cold-start, affecting the e2ee keypair, device registry, and runtime env auth store. Fix: apply the credential FILE's ACL synchronously (execFileSync) on the infrequent write path, before the atomic rename publishes it, and cache the path as hardened only on confirmed success so a failed apply retries. Keep the DIRECTORY hardening async + path-cached for the process lifetime (that is what killed the #4901/#5006 main-thread storm). The read path's existing-file re-harden stays async + metadata-cached (fires at most once per file, no storm). Also: - Document the dir-path cache process-lifetime known limitation (deleted+ recreated dir not re-hardened until restart). - Remove the redundant double dir-cache write in writeSecureFile. - Add docs/windows-secure-file-acl-hardening.md describing the sync-file/ async-dir model and a manual Windows e2e test plan (the cross-platform Playwright harness runs on Linux and cannot reach the PowerShell path). Tests (src/shared/secure-file.test.ts, 13 passing): credential file hardened synchronously while dir stays async (no async file-ACL window); failed sync file-ACL apply is not cached and retries; dir hardened exactly once across many writes despite mtime churn; no PowerShell spawned on non-win32. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: keep POSIX secure directory hardening metadata-aware --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> |
||
 Lesley MurfinandJinwoo Hong
|
3d013e48e8 |
fix(windows): slow launch + keystroke stalls — main process synchronously spawns powershell to re-harden env-store ACLs (#4840)
* fix(windows): stop synchronous powershell ACL spawns blocking the main thread markEnvironmentUsed re-hardened the env-store on every runtime round-trip, spawning powershell.exe ~5x synchronously (~1-1.5s each) and freezing the Windows UI on launch and every keystroke. Throttle the non-security lastUsedAt write (60s window; runtimeId changes bypass) and make path hardening idempotent per process. ACL hardening is preserved; post-rename credential hardening still always runs. ~14.0s -> ~6.8s launch; main thread 95.7% blocked -> 99.0% idle. * Fix secure-file hardening cache retries --------- Co-authored-by: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> |
||
|
Jinjing
|
bcddccae2b | fix: harden shared platform helpers (#3877) | ||
|
Jinwoo HongandOrca
|
a22717bb35 |
Refactor runtime app architecture (#1878)
Co-authored-by: Orca <help@stably.ai> |