diff --git a/config/scripts/ensure-native-runtime.mjs b/config/scripts/ensure-native-runtime.mjs index fd1d0385cfb..d77dcf1f707 100644 --- a/config/scripts/ensure-native-runtime.mjs +++ b/config/scripts/ensure-native-runtime.mjs @@ -295,8 +295,8 @@ function loadNativeModule(moduleName) { return } if (moduleName === '@orca/windows-security') { - // Why: the package defers loading its .node addon until the first call. - require(moduleName).currentUserSid() + // Why: the addon loads on first call; the newest export also rejects a stale build that loads. + require(moduleName).clearStdHandleInheritance() return } if (moduleName === 'node-pty') { diff --git a/config/scripts/ensure-native-runtime.test.mjs b/config/scripts/ensure-native-runtime.test.mjs index 4514fab0eae..20ebbfab40f 100644 --- a/config/scripts/ensure-native-runtime.test.mjs +++ b/config/scripts/ensure-native-runtime.test.mjs @@ -423,7 +423,10 @@ function writeFakeWindowsRegistry(projectDir, { requiresMarker = false } = {}) { ) const securityDir = join(projectDir, 'node_modules', '@orca', 'windows-security') mkdirSync(securityDir, { recursive: true }) - writeFileSync(join(securityDir, 'index.js'), "exports.currentUserSid = () => 'S-1-5-18'\n") + writeFileSync( + join(securityDir, 'index.js'), + "exports.currentUserSid = () => 'S-1-5-18'; exports.clearStdHandleInheritance = () => []\n" + ) } function writeNodePtyPatchFile(projectDir) { diff --git a/config/scripts/pr-package-test-paths.mjs b/config/scripts/pr-package-test-paths.mjs index 9a82fa1d3d7..1e6261e4cb4 100644 --- a/config/scripts/pr-package-test-paths.mjs +++ b/config/scripts/pr-package-test-paths.mjs @@ -95,6 +95,7 @@ export const WINDOWS_BOUNDARY_TESTS = [ 'src/main/orca-profiles/profile-index-store.test.ts', 'src/main/startup/windows-install-dir-acl-repair.win32.test.ts', 'src/main/startup/windows-app-data-path.test.ts', + 'src/main/startup/windows-std-handle-inheritance.win32.test.ts', 'src/main/runtime/repo-worktree-admin-fingerprint.test.ts', 'src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts', 'src/shared/secure-file-fsync-flags.test.ts', diff --git a/config/scripts/rebuild-native-deps-test-fixtures.mjs b/config/scripts/rebuild-native-deps-test-fixtures.mjs index 1d346bfd674..4066d0c62ac 100644 --- a/config/scripts/rebuild-native-deps-test-fixtures.mjs +++ b/config/scripts/rebuild-native-deps-test-fixtures.mjs @@ -400,7 +400,10 @@ export function writeFakeWindowsRegistry(projectDir) { export function writeFakeWindowsSecurity(projectDir) { const securityDir = join(projectDir, 'node_modules', '@orca', 'windows-security') mkdirSync(securityDir, { recursive: true }) - writeFileSync(join(securityDir, 'index.js'), "exports.currentUserSid = () => 'S-1-5-18'\n") + writeFileSync( + join(securityDir, 'index.js'), + "exports.currentUserSid = () => 'S-1-5-18'; exports.clearStdHandleInheritance = () => []\n" + ) } // Healthy patched addon must report CreationTime; no exports represents upstream tarball prebuild to the probe. diff --git a/config/scripts/rebuild-native-deps.mjs b/config/scripts/rebuild-native-deps.mjs index 3f974b5a28d..36c23e9ef98 100644 --- a/config/scripts/rebuild-native-deps.mjs +++ b/config/scripts/rebuild-native-deps.mjs @@ -577,8 +577,8 @@ function loadNativeModule(moduleName) { return } if (moduleName === '@orca/windows-security') { - // Why: the package defers loading its .node addon until the first call. - projectRequire(moduleName).currentUserSid() + // Why: the addon loads on first call; the newest export also rejects a stale build that loads. + projectRequire(moduleName).clearStdHandleInheritance() return } if (moduleName === 'node-pty') { diff --git a/native/windows-security/index.d.ts b/native/windows-security/index.d.ts index 298ff1a8f4e..c64267337de 100644 --- a/native/windows-security/index.d.ts +++ b/native/windows-security/index.d.ts @@ -51,3 +51,11 @@ export declare function grantAccessAsync( accessMask: number, options: GrantAccessOptions ): Promise + +export type StdHandleInheritanceFailure = { stream: 'stdin' | 'stdout' | 'stderr'; code: number } + +/** + * Makes the process's std handles non-inheritable, skipping absent ones. Never throws; returns the + * handles SetHandleInformation rejected (e.g. a legacy console pseudo-handle) with their Win32 code. + */ +export declare function clearStdHandleInheritance(): StdHandleInheritanceFailure[] diff --git a/native/windows-security/index.js b/native/windows-security/index.js index d96c2ecfe01..b7cc64f82bb 100644 --- a/native/windows-security/index.js +++ b/native/windows-security/index.js @@ -17,5 +17,6 @@ module.exports = { createFileWithDacl: (path, sddl, data) => getAddon().createFileWithDacl(path, sddl, data), grantAccess: (path, sid, mask, options) => getAddon().grantAccess(path, sid, mask, options), grantAccessAsync: (path, sid, mask, options) => - getAddon().grantAccessAsync(path, sid, mask, options) + getAddon().grantAccessAsync(path, sid, mask, options), + clearStdHandleInheritance: () => getAddon().clearStdHandleInheritance() } diff --git a/native/windows-security/src/addon.cc b/native/windows-security/src/addon.cc index 76edd13bffd..649d29628c1 100644 --- a/native/windows-security/src/addon.cc +++ b/native/windows-security/src/addon.cc @@ -384,6 +384,34 @@ Napi::Value GrantAccessAsync(const Napi::CallbackInfo& info) { return promise; } +// Clear HANDLE_FLAG_INHERIT on the std handles so a spawn with bInheritHandles=TRUE can't leak them +// to a long-lived child; libuv duplicates an inheritable copy for an explicit stdio 'inherit'. +// Returns only failures, as [{ stream, code }]; absent handles are skipped. +Napi::Value ClearStdHandleInheritance(const Napi::CallbackInfo& info) { + auto env = info.Env(); + struct Stream { + DWORD id; + const char* name; + }; + const Stream streams[] = {{STD_INPUT_HANDLE, "stdin"}, + {STD_OUTPUT_HANDLE, "stdout"}, + {STD_ERROR_HANDLE, "stderr"}}; + auto failures = Napi::Array::New(env); + for (const auto& stream : streams) { + HANDLE handle = GetStdHandle(stream.id); + if (handle == nullptr || handle == INVALID_HANDLE_VALUE) { + continue; + } + if (!SetHandleInformation(handle, HANDLE_FLAG_INHERIT, 0)) { + auto failure = Napi::Object::New(env); + failure.Set("stream", stream.name); + failure.Set("code", static_cast(GetLastError())); + failures.Set(failures.Length(), failure); + } + } + return failures; +} + } // namespace Napi::Object Init(Napi::Env env, Napi::Object exports) { @@ -394,6 +422,7 @@ Napi::Object Init(Napi::Env env, Napi::Object exports) { exports.Set("createFileWithDacl", Napi::Function::New(env, CreateFileWithDaclJs)); exports.Set("grantAccess", Napi::Function::New(env, GrantAccessJs)); exports.Set("grantAccessAsync", Napi::Function::New(env, GrantAccessAsync)); + exports.Set("clearStdHandleInheritance", Napi::Function::New(env, ClearStdHandleInheritance)); return exports; } diff --git a/src/main/index.ts b/src/main/index.ts index c153fe7d745..0cd411e0582 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -40,6 +40,7 @@ import { wslHostsServing } from './wsl/wsl-host-gate' import { configureWslHostsGateFromDisk } from './wsl/wsl-host-gate-startup' import { ignoreInheritedWslHostsGateState } from '../shared/wsl-host-internal-gate' import { registerOrcaProtocolClient } from './startup/orca-protocol-registration' +import { clearWindowsStdHandleInheritance } from './startup/windows-std-handle-inheritance' function openMainWindow(options: { revealOnDidFinishLoad?: boolean } = {}): BrowserWindow { return openMainWindowController(options) @@ -107,6 +108,8 @@ const handleMacAppActivation = createMacAppActivationHandler({ requestActivation: requestDesktopActivation }) +// Before anything spawns: no child may inherit Orca's std handles. +clearWindowsStdHandleInheritance() // A relaunch or installer inherits the parent's published WSL gate state; this process recomputes. ignoreInheritedWslHostsGateState() // The uninstaller's holder release and hook removal run instead of the app, never alongside it. diff --git a/src/main/startup/windows-install-dir-package-acl-grant.test.ts b/src/main/startup/windows-install-dir-package-acl-grant.test.ts index 88a1370cec4..d170d08cf5a 100644 --- a/src/main/startup/windows-install-dir-package-acl-grant.test.ts +++ b/src/main/startup/windows-install-dir-package-acl-grant.test.ts @@ -42,6 +42,7 @@ function installFakeAddon( setDaclSddlAsync: unexpected, createFileWithDacl: unexpected, grantAccess: unexpected, + clearStdHandleInheritance: unexpected, grantAccessAsync: async (path, sid, accessMask, options) => { calls.push({ path: plain(path), sid, accessMask, options }) if (fail(plain(path))) { @@ -189,6 +190,7 @@ describe('install-dir package grant passes', () => { setDaclSddlAsync: unexpected, createFileWithDacl: unexpected, grantAccess: unexpected, + clearStdHandleInheritance: unexpected, grantAccessAsync: async (path) => { seen.push(path) } diff --git a/src/main/startup/windows-std-handle-inheritance.test.ts b/src/main/startup/windows-std-handle-inheritance.test.ts new file mode 100644 index 00000000000..e4d01492384 --- /dev/null +++ b/src/main/startup/windows-std-handle-inheritance.test.ts @@ -0,0 +1,69 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +const { loadWindowsSecurity } = vi.hoisted(() => ({ loadWindowsSecurity: vi.fn() })) +vi.mock('../../shared/windows-security-addon', () => ({ loadWindowsSecurity })) + +import { clearWindowsStdHandleInheritance } from './windows-std-handle-inheritance' + +function addonWith(clearStdHandleInheritance: () => unknown) { + return { userSid: 'S-1-5-18', addon: { clearStdHandleInheritance } } +} + +describe('clearWindowsStdHandleInheritance', () => { + afterEach(() => { + vi.restoreAllMocks() + loadWindowsSecurity.mockReset() + }) + + it('is a no-op off Windows', () => { + clearWindowsStdHandleInheritance('darwin') + clearWindowsStdHandleInheritance('linux') + + expect(loadWindowsSecurity).not.toHaveBeenCalled() + }) + + it('clears the std handles through the addon on Windows', () => { + const clear = vi.fn(() => []) + loadWindowsSecurity.mockReturnValue(addonWith(clear)) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + clearWindowsStdHandleInheritance('win32') + + expect(clear).toHaveBeenCalledOnce() + expect(warn).not.toHaveBeenCalled() + }) + + it('logs each handle the addon could not clear', () => { + loadWindowsSecurity.mockReturnValue(addonWith(() => [{ stream: 'stdout', code: 6 }])) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + clearWindowsStdHandleInheritance('win32') + + expect(warn).toHaveBeenCalledWith('[startup] stdout stays inheritable; Win32 error 6') + }) + + it('logs instead of throwing when the addon is unavailable', () => { + loadWindowsSecurity.mockReturnValue({ error: 'missing binary' }) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + expect(() => clearWindowsStdHandleInheritance('win32')).not.toThrow() + expect(warn).toHaveBeenCalledWith( + '[startup] std handles stay inheritable; addon unavailable: missing binary' + ) + }) + + it('logs instead of throwing when the addon call throws, e.g. a stale binary', () => { + loadWindowsSecurity.mockReturnValue( + addonWith(() => { + throw new TypeError('clearStdHandleInheritance is not a function') + }) + ) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + expect(() => clearWindowsStdHandleInheritance('win32')).not.toThrow() + expect(warn).toHaveBeenCalledWith( + '[startup] std handles stay inheritable:', + expect.any(TypeError) + ) + }) +}) diff --git a/src/main/startup/windows-std-handle-inheritance.ts b/src/main/startup/windows-std-handle-inheritance.ts new file mode 100644 index 00000000000..224068740f3 --- /dev/null +++ b/src/main/startup/windows-std-handle-inheritance.ts @@ -0,0 +1,23 @@ +import { loadWindowsSecurity } from '../../shared/windows-security-addon' + +/** libuv spawns with bInheritHandles=TRUE, so an inheritable std handle reaches every child, and a + * long-lived one (the WSL holder, the daemon) keeps a launcher's pipe open after Orca exits. */ +export function clearWindowsStdHandleInheritance( + platform: NodeJS.Platform = process.platform +): void { + if (platform !== 'win32') { + return + } + try { + const security = loadWindowsSecurity() + if ('error' in security) { + console.warn(`[startup] std handles stay inheritable; addon unavailable: ${security.error}`) + return + } + for (const failure of security.addon.clearStdHandleInheritance()) { + console.warn(`[startup] ${failure.stream} stays inheritable; Win32 error ${failure.code}`) + } + } catch (error) { + console.warn('[startup] std handles stay inheritable:', error) + } +} diff --git a/src/main/startup/windows-std-handle-inheritance.win32.test.ts b/src/main/startup/windows-std-handle-inheritance.win32.test.ts new file mode 100644 index 00000000000..4f940c8d516 --- /dev/null +++ b/src/main/startup/windows-std-handle-inheritance.win32.test.ts @@ -0,0 +1,103 @@ +import { once } from 'node:events' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { afterAll, describe, expect, it } from 'vitest' +import { spawnProcess } from '@orca/process-host' + +const describeOnWindows = process.platform === 'win32' ? describe : describe.skip +const ADDON = resolve(__dirname, '..', '..', '..', 'native', 'windows-security', 'index.js') +const root = mkdtempSync(join(tmpdir(), 'orca-std-handles-')) +const entry = join(root, 'main.cjs') + +/** Optionally clears, writes through a stdio:'inherit' grandchild, then leaves a detached + * stdio:'ignore' holder behind, like the WSL holder or the daemon. */ +writeFileSync( + entry, + ` +const { spawn, spawnSync } = require('node:child_process') +const { writeFileSync } = require('node:fs') +const [mode, holderMs, pidFile] = process.argv.slice(-3) +if (mode === 'clear') { + const failures = require(${JSON.stringify(ADDON)}).clearStdHandleInheritance() + if (failures.length > 0) { writeFileSync(pidFile, JSON.stringify(failures)); process.exit(3) } +} +const env = { ...process.env, ELECTRON_RUN_AS_NODE: '1' } +spawnSync(process.execPath, ['-e', 'process.stdout.write("inherited-output\\\\n")'], { stdio: 'inherit', env, windowsHide: true }) +const holder = spawn(process.execPath, ['-e', 'setTimeout(() => {}, ' + holderMs + ')'], { stdio: 'ignore', detached: true, env, windowsHide: true }) +holder.unref() +writeFileSync(pidFile, String(holder.pid)) +if (process.type === 'browser') require('electron').app.exit(0) +` +) + +afterAll(() => rmSync(root, { recursive: true, force: true })) + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch { + return false + } +} + +/** Electron main is Orca's real shape: it keeps std handles inheritable (kEnableStdioInheritance), + * where plain Node clears them itself at startup. */ +async function runChild(host: 'electron' | 'node', mode: 'clear' | 'keep', holderMs: number) { + const electron: unknown = createRequire(import.meta.url)('electron') + if (typeof electron !== 'string') { + throw new Error('Electron executable is unavailable') + } + const env: NodeJS.ProcessEnv = { ...process.env, ORCA_BACKGROUND_LAUNCH: '1' } + delete env.ELECTRON_RUN_AS_NODE + const pidFile = join(root, `${host}-${mode}.pid`) + const child = spawnProcess({ + program: host === 'electron' ? electron : process.execPath, + args: [entry, mode, String(holderMs), pidFile], + env, + stdio: ['ignore', 'pipe', 'pipe'] + }) + let output = '' + child.stdout?.on('data', (chunk) => (output += chunk)) + child.stderr?.on('data', (chunk) => (output += chunk)) + const [code] = await once(child, 'close', { signal: AbortSignal.timeout(holderMs + 30_000) }) + const pidText = readFileSync(pidFile, 'utf8') + const holderPid = Number(pidText) + const holderAliveAtClose = isAlive(holderPid) + if (holderAliveAtClose) { + process.kill(holderPid) + } + return { code, output, pidText, holderAliveAtClose } +} + +describeOnWindows('clearing std handle inheritance', () => { + // A regression pins the pipe for the 60 s holder; fail on the assertion, not the 5 s default. + it('keeps stdio inherit output and frees the pipe while a detached holder lives', async () => { + const run = await runChild('electron', 'clear', 60_000) + + expect(run.code, run.output).toBe(0) + expect(run.pidText).toMatch(/^\d+$/) + expect(run.output).toContain('inherited-output') + expect(run.holderAliveAtClose).toBe(true) + }, 90_000) + + // Cold Electron start on a CI runner plus the 3 s holder exceeds the 5 s default. + it('without it, Electron main lets the holder keep the pipe open until it exits', async () => { + const run = await runChild('electron', 'keep', 3_000) + + expect(run.code, run.output).toBe(0) + expect(run.output).toContain('inherited-output') + expect(run.holderAliveAtClose).toBe(false) + }, 60_000) + + // A regression pins the pipe for the 60 s holder; fail on the assertion, not the 5 s default. + it('plain Node already clears its own std handles, so it is no model for Orca', async () => { + const run = await runChild('node', 'keep', 60_000) + + expect(run.code, run.output).toBe(0) + expect(run.output).toContain('inherited-output') + expect(run.holderAliveAtClose).toBe(true) + }, 90_000) +}) diff --git a/src/shared/windows-security-addon.test-fixture.ts b/src/shared/windows-security-addon.test-fixture.ts index 44ad6329a6b..3f4ee1978e5 100644 --- a/src/shared/windows-security-addon.test-fixture.ts +++ b/src/shared/windows-security-addon.test-fixture.ts @@ -106,6 +106,7 @@ export function createFakeWindowsSecurity(): FakeWindowsSecurity { callsFor: (op, path) => fake.calls.filter((call) => call.op === op && (path === undefined || call.path === path)), currentUserSid: () => FAKE_USER_SID, + clearStdHandleInheritance: () => [], getDaclAces: (addonPath) => { const path = record('get', addonPath) return daclFromSddl(fake.dacls.get(path) ?? INHERITED_SDDL) diff --git a/src/shared/windows-security-addon.ts b/src/shared/windows-security-addon.ts index 055e731991c..4d41faedfb3 100644 --- a/src/shared/windows-security-addon.ts +++ b/src/shared/windows-security-addon.ts @@ -4,7 +4,12 @@ import { basename, isAbsolute, join, relative, toNamespacedPath } from 'node:pat import type * as WindowsSecurityModule from '@orca/windows-security' import { ORCAD_WINDOWS_SECURITY_FILENAME } from './orcad-artifacts' -export type { GrantAccessOptions, WindowsAce, WindowsDacl } from '@orca/windows-security' +export type { + GrantAccessOptions, + StdHandleInheritanceFailure, + WindowsAce, + WindowsDacl +} from '@orca/windows-security' export type WindowsSecurityAddon = typeof WindowsSecurityModule /** The addon (every path namespaced, async writes ordered per path) and the token's user SID. */ @@ -105,7 +110,8 @@ function prepare(raw: WindowsSecurityAddon): WindowsSecurity { grantAccess: (path, sid, mask, options) => raw.grantAccess(toNamespacedPath(path), sid, mask, options), grantAccessAsync: (path, sid, mask, options) => - inPathOrder(path, (namespaced) => raw.grantAccessAsync(namespaced, sid, mask, options)) + inPathOrder(path, (namespaced) => raw.grantAccessAsync(namespaced, sid, mask, options)), + clearStdHandleInheritance: () => raw.clearStdHandleInheritance() } } }