mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 08:37:49 +00:00
fix(windows): keep Orca's std handles out of every spawned child (#35678)
* fix(windows): keep Orca's std handles out of every spawned child libuv spawns with bInheritHandles=TRUE, so an inheritable std handle reached long-lived children such as the WSL holder and the daemon and held a launcher's pipe open after Orca exited. Clear HANDLE_FLAG_INHERIT on the std handles at the top of main startup; stdio 'inherit' still works because libuv hands the child its own inheritable duplicate. * test(windows): model Orca's Electron main, which keeps std handles inheritable Plain Node clears its std handles' inherit flag at startup, so a Node child never reproduced the leak. Electron main opts out of that, which is why Orca leaks and why the control arm now runs Electron; a plain-Node arm pins the difference. * test(windows): say why the std-handle tests need longer timeouts
This commit is contained in:
@@ -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') {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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') {
|
||||
|
||||
Vendored
+8
@@ -51,3 +51,11 @@ export declare function grantAccessAsync(
|
||||
accessMask: number,
|
||||
options: GrantAccessOptions
|
||||
): Promise<void>
|
||||
|
||||
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[]
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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<double>(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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
})
|
||||
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user