mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(terminal): avoid Windows PATH registry subprocesses (#13485)
This commit is contained in:
@@ -489,7 +489,7 @@ describe('CliInstaller', () => {
|
||||
const registryReader = new WindowsUserPathRegistryReader({
|
||||
platform: 'win32',
|
||||
registryLoader: async () => ({
|
||||
HK: { CU: 0x80000001 },
|
||||
HK: { CU: 0x80000001, LM: 0x80000002 },
|
||||
getRegistryKey: () => ({
|
||||
Path: { name: 'Path', type: 2, value: registryPath }
|
||||
})
|
||||
|
||||
@@ -7,7 +7,7 @@ import {
|
||||
|
||||
function registryModule(pathValue?: string, type = 2) {
|
||||
return {
|
||||
HK: { CU: 0x80000001 },
|
||||
HK: { CU: 0x80000001, LM: 0x80000002 },
|
||||
REG: { SZ: 1, EXPAND_SZ: 2 },
|
||||
getRegistryKey: vi.fn(() =>
|
||||
pathValue === undefined ? {} : { Path: { name: 'Path', type, value: pathValue } }
|
||||
|
||||
@@ -1,26 +1,17 @@
|
||||
import { createRequire } from 'node:module'
|
||||
import {
|
||||
loadWindowsNativeRegistry,
|
||||
WINDOWS_REG_EXPAND_SZ,
|
||||
WINDOWS_REG_SZ,
|
||||
type WindowsNativeRegistryModule
|
||||
} from '../windows-native-registry'
|
||||
|
||||
export type WindowsUserPathReadResult =
|
||||
| { state: 'success'; value: string | null; expandable: boolean }
|
||||
| { state: 'unknown'; detail: string }
|
||||
|
||||
type RegistryValue = {
|
||||
name?: unknown
|
||||
type?: unknown
|
||||
value?: unknown
|
||||
}
|
||||
|
||||
type WindowsRegistryModule = {
|
||||
HK: { CU: number }
|
||||
getRegistryKey: (
|
||||
root: number,
|
||||
path: string
|
||||
) => Record<string, RegistryValue | undefined> | null | undefined
|
||||
}
|
||||
|
||||
type WindowsUserPathRegistryReaderOptions = {
|
||||
platform?: NodeJS.Platform
|
||||
registryLoader?: () => Promise<WindowsRegistryModule>
|
||||
registryLoader?: () => Promise<WindowsNativeRegistryModule>
|
||||
now?: () => number
|
||||
cacheTtlMs?: number
|
||||
}
|
||||
@@ -28,19 +19,9 @@ type WindowsUserPathRegistryReaderOptions = {
|
||||
const DEFAULT_CACHE_TTL_MS = 1_000
|
||||
const USER_ENVIRONMENT_KEY = 'Environment'
|
||||
const USER_PATH_VALUE = 'Path'
|
||||
const REG_SZ = 1
|
||||
const REG_EXPAND_SZ = 2
|
||||
const requireFromMain = createRequire(__filename)
|
||||
|
||||
async function loadWindowsRegistryModule(): Promise<WindowsRegistryModule> {
|
||||
// Why: the optional dependency is not present in non-Windows installs, so
|
||||
// TypeScript and the main bundle must not resolve it eagerly.
|
||||
return requireFromMain('windows-native-registry') as WindowsRegistryModule
|
||||
}
|
||||
|
||||
export class WindowsUserPathRegistryReader {
|
||||
private readonly platform: NodeJS.Platform
|
||||
private readonly registryLoader: () => Promise<WindowsRegistryModule>
|
||||
private readonly registryLoader: () => Promise<WindowsNativeRegistryModule>
|
||||
private readonly now: () => number
|
||||
private readonly cacheTtlMs: number
|
||||
private cached: { readAt: number; result: WindowsUserPathReadResult } | null = null
|
||||
@@ -49,7 +30,7 @@ export class WindowsUserPathRegistryReader {
|
||||
|
||||
constructor(options: WindowsUserPathRegistryReaderOptions = {}) {
|
||||
this.platform = options.platform ?? process.platform
|
||||
this.registryLoader = options.registryLoader ?? loadWindowsRegistryModule
|
||||
this.registryLoader = options.registryLoader ?? (async () => loadWindowsNativeRegistry())
|
||||
this.now = options.now ?? Date.now
|
||||
this.cacheTtlMs = options.cacheTtlMs ?? DEFAULT_CACHE_TTL_MS
|
||||
}
|
||||
@@ -123,7 +104,7 @@ export class WindowsUserPathRegistryReader {
|
||||
return { state: 'success', value: null, expandable: false }
|
||||
}
|
||||
if (
|
||||
(pathEntry.type !== REG_SZ && pathEntry.type !== REG_EXPAND_SZ) ||
|
||||
(pathEntry.type !== WINDOWS_REG_SZ && pathEntry.type !== WINDOWS_REG_EXPAND_SZ) ||
|
||||
typeof pathEntry.value !== 'string'
|
||||
) {
|
||||
return {
|
||||
@@ -134,7 +115,7 @@ export class WindowsUserPathRegistryReader {
|
||||
return {
|
||||
state: 'success',
|
||||
value: pathEntry.value || null,
|
||||
expandable: pathEntry.type === REG_EXPAND_SZ
|
||||
expandable: pathEntry.type === WINDOWS_REG_EXPAND_SZ
|
||||
}
|
||||
} catch {
|
||||
return {
|
||||
|
||||
@@ -3,45 +3,11 @@ import { join } from 'node:path'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { registryQueryAsyncMock, registryQuerySyncMock } = vi.hoisted(() => ({
|
||||
registryQueryAsyncMock: vi.fn(),
|
||||
registryQuerySyncMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('node:child_process', async (importOriginal) => {
|
||||
const original = await importOriginal<Record<string, unknown>>()
|
||||
const originalExecFile = original.execFile as (
|
||||
command: string,
|
||||
commandArgs: string[],
|
||||
commandOptions: unknown,
|
||||
commandCallback: (error: Error | null, stdout: string, stderr: string) => void
|
||||
) => unknown
|
||||
const registryAwareExecFile = (
|
||||
file: string,
|
||||
args: string[],
|
||||
options: unknown,
|
||||
callback: (error: Error | null, stdout: string, stderr: string) => void
|
||||
): unknown => {
|
||||
if (file.toLowerCase().endsWith('\\reg.exe')) {
|
||||
return registryQueryAsyncMock(file, args, options, callback)
|
||||
}
|
||||
return originalExecFile(file, args, options, callback)
|
||||
}
|
||||
const customPromisify = Symbol.for('nodejs.util.promisify.custom')
|
||||
Object.defineProperty(registryAwareExecFile, customPromisify, {
|
||||
value: (originalExecFile as unknown as Record<symbol, unknown>)[customPromisify]
|
||||
})
|
||||
return {
|
||||
...original,
|
||||
execFile: registryAwareExecFile,
|
||||
execFileSync: registryQuerySyncMock
|
||||
}
|
||||
})
|
||||
|
||||
import {
|
||||
__resetPersistedWindowsPathCacheForTests,
|
||||
mergePersistedWindowsPathAsync
|
||||
} from '../pty/windows-environment-path'
|
||||
import { __setWindowsPathRegistryLoaderForTests } from '../pty/windows-path-registry-reader'
|
||||
import { execLocalPreflightCommand } from './preflight-command-exec'
|
||||
|
||||
describe.runIf(process.platform === 'win32')('Windows preflight Path refresh reproduction', () => {
|
||||
@@ -50,8 +16,7 @@ describe.runIf(process.platform === 'win32')('Windows preflight Path refresh rep
|
||||
|
||||
afterEach(() => {
|
||||
process.env.Path = originalPath
|
||||
registryQueryAsyncMock.mockReset()
|
||||
registryQuerySyncMock.mockReset()
|
||||
__setWindowsPathRegistryLoaderForTests()
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
for (const directory of fixtureDirs.splice(0)) {
|
||||
rmSync(directory, { recursive: true, force: true })
|
||||
@@ -68,22 +33,13 @@ describe.runIf(process.platform === 'win32')('Windows preflight Path refresh rep
|
||||
)
|
||||
|
||||
let persistedUserPath = ''
|
||||
registryQuerySyncMock.mockImplementation((_file, args: string[]) => {
|
||||
const value = String(args[1]).startsWith('HKCU') ? persistedUserPath : ''
|
||||
return ` Path REG_SZ ${value}\r\n`
|
||||
})
|
||||
registryQueryAsyncMock.mockImplementation(
|
||||
(
|
||||
_file: string,
|
||||
args: string[],
|
||||
_options: unknown,
|
||||
callback: (error: Error | null, stdout: string, stderr: string) => void
|
||||
) => {
|
||||
const value = String(args[1]).startsWith('HKCU') ? persistedUserPath : ''
|
||||
callback(null, ` Path REG_SZ ${value}\r\n`, '')
|
||||
return {} as never
|
||||
}
|
||||
)
|
||||
const getRegistryKey = vi.fn((root: number) => ({
|
||||
Path: { type: 1, value: root === 2 ? persistedUserPath : '' }
|
||||
}))
|
||||
__setWindowsPathRegistryLoaderForTests(() => ({
|
||||
HK: { LM: 1, CU: 2 },
|
||||
getRegistryKey
|
||||
}))
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
|
||||
await expect(execLocalPreflightCommand(command, ['/?'])).rejects.toMatchObject({
|
||||
@@ -97,7 +53,6 @@ describe.runIf(process.platform === 'win32')('Windows preflight Path refresh rep
|
||||
await expect(execLocalPreflightCommand(command, ['/?'])).resolves.toMatchObject({
|
||||
stdout: expect.any(String)
|
||||
})
|
||||
expect(registryQuerySyncMock).toHaveBeenCalledTimes(2)
|
||||
expect(registryQueryAsyncMock).toHaveBeenCalledTimes(2)
|
||||
expect(getRegistryKey).toHaveBeenCalledTimes(4)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
__resetPersistedWindowsPathCacheForTests,
|
||||
readPersistedWindowsPathSegments
|
||||
} from './windows-environment-path'
|
||||
import { __setWindowsPathRegistryLoaderForTests } from './windows-path-registry-reader'
|
||||
|
||||
const CREATE_PROCESS_DELAY_MS = 160
|
||||
const delayedExecFileSync = vi.hoisted(() =>
|
||||
vi.fn(() => {
|
||||
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, CREATE_PROCESS_DELAY_MS)
|
||||
return ' Path REG_EXPAND_SZ C:\\Delayed'
|
||||
})
|
||||
)
|
||||
|
||||
vi.mock('node:child_process', () => ({
|
||||
execFile: vi.fn(),
|
||||
execFileSync: delayedExecFileSync
|
||||
}))
|
||||
|
||||
describe('persisted Windows PATH process creation', () => {
|
||||
afterEach(() => {
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
__setWindowsPathRegistryLoaderForTests()
|
||||
delayedExecFileSync.mockClear()
|
||||
})
|
||||
|
||||
async function measureMainLoopGap<T>(run: () => T): Promise<{
|
||||
callMs: number
|
||||
maxGapMs: number
|
||||
result: T
|
||||
}> {
|
||||
let lastTick = performance.now()
|
||||
let maxGapMs = 0
|
||||
const timer = setInterval(() => {
|
||||
const now = performance.now()
|
||||
maxGapMs = Math.max(maxGapMs, now - lastTick - 5)
|
||||
lastTick = now
|
||||
}, 5)
|
||||
await new Promise((resolve) => setTimeout(resolve, 20))
|
||||
|
||||
const startedAt = performance.now()
|
||||
const result = run()
|
||||
const callMs = performance.now() - startedAt
|
||||
await new Promise((resolve) => setTimeout(resolve, 20))
|
||||
clearInterval(timer)
|
||||
return { callMs, maxGapMs, result }
|
||||
}
|
||||
|
||||
it('proves delayed process creation blocks while native registry reads create no process', async () => {
|
||||
const legacy = await measureMainLoopGap(() =>
|
||||
readPersistedWindowsPathSegments({
|
||||
platform: 'win32',
|
||||
env: { SystemRoot: 'C:\\Windows' },
|
||||
execFileSync: delayedExecFileSync as never
|
||||
})
|
||||
)
|
||||
|
||||
expect(legacy.result).toEqual(['C:\\Delayed', 'C:\\Delayed'])
|
||||
expect(delayedExecFileSync).toHaveBeenCalledTimes(2)
|
||||
expect(legacy.callMs).toBeGreaterThanOrEqual(CREATE_PROCESS_DELAY_MS * 2)
|
||||
expect(legacy.maxGapMs).toBeGreaterThanOrEqual(CREATE_PROCESS_DELAY_MS)
|
||||
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
delayedExecFileSync.mockClear()
|
||||
__setWindowsPathRegistryLoaderForTests(() => ({
|
||||
HK: { LM: 1, CU: 2 },
|
||||
getRegistryKey: (root) => ({
|
||||
Path: { type: 1, value: root === 1 ? 'C:\\Machine' : 'C:\\User' }
|
||||
})
|
||||
}))
|
||||
const native = readPersistedWindowsPathSegments({
|
||||
platform: 'win32',
|
||||
env: { SystemRoot: 'C:\\Windows' }
|
||||
})
|
||||
|
||||
expect(native).toEqual(['C:\\Machine', 'C:\\User'])
|
||||
expect(delayedExecFileSync).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { defaultExecFileMock, defaultExecFileSyncMock } = vi.hoisted(() => ({
|
||||
defaultExecFileMock: vi.fn(),
|
||||
@@ -18,8 +18,26 @@ import {
|
||||
readPersistedWindowsPathSegmentsAsync,
|
||||
resolvePathEnvKey
|
||||
} from './windows-environment-path'
|
||||
import { __setWindowsPathRegistryLoaderForTests } from './windows-path-registry-reader'
|
||||
|
||||
type ExecCallback = (error: Error | null, stdout: string, stderr: string) => void
|
||||
const registryGetKeyMock = vi.fn()
|
||||
|
||||
function registryPath(value: string): Record<string, unknown> {
|
||||
return { Path: { type: 1, value } }
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
registryGetKeyMock.mockReset()
|
||||
__setWindowsPathRegistryLoaderForTests(() => ({
|
||||
HK: { LM: 1, CU: 2 },
|
||||
getRegistryKey: registryGetKeyMock
|
||||
}))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
__setWindowsPathRegistryLoaderForTests()
|
||||
})
|
||||
|
||||
describe('readPersistedWindowsPathSegments', () => {
|
||||
it('reads machine and user Path values from the Windows registry', () => {
|
||||
@@ -67,18 +85,17 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
it('caches production registry reads briefly', () => {
|
||||
const originalPlatform = process.platform
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
defaultExecFileSyncMock
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\Machine\r\n')
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\User\r\n')
|
||||
registryGetKeyMock
|
||||
.mockReturnValueOnce(registryPath('C:\\Machine'))
|
||||
.mockReturnValueOnce(registryPath('C:\\User'))
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
|
||||
try {
|
||||
expect(readPersistedWindowsPathSegments()).toEqual(['C:\\Machine', 'C:\\User'])
|
||||
expect(readPersistedWindowsPathSegments()).toEqual(['C:\\Machine', 'C:\\User'])
|
||||
expect(defaultExecFileSyncMock).toHaveBeenCalledTimes(2)
|
||||
expect(registryGetKeyMock).toHaveBeenCalledTimes(2)
|
||||
} finally {
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
defaultExecFileSyncMock.mockReset()
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
|
||||
}
|
||||
})
|
||||
@@ -86,27 +103,26 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
it('force-refreshes the production registry cache', () => {
|
||||
const originalPlatform = process.platform
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
defaultExecFileSyncMock
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\Machine\r\n')
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\User\r\n')
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\Machine\r\n')
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\User;C:\\Program Files\\GitHub CLI\r\n')
|
||||
registryGetKeyMock
|
||||
.mockReturnValueOnce(registryPath('C:\\Machine'))
|
||||
.mockReturnValueOnce(registryPath('C:\\User'))
|
||||
.mockReturnValueOnce(registryPath('C:\\Machine'))
|
||||
.mockReturnValueOnce(registryPath('C:\\User;C:\\Program Files\\GitHub CLI'))
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
|
||||
try {
|
||||
expect(readPersistedWindowsPathSegments()).toEqual(['C:\\Machine', 'C:\\User'])
|
||||
expect(readPersistedWindowsPathSegments()).toEqual(['C:\\Machine', 'C:\\User'])
|
||||
expect(defaultExecFileSyncMock).toHaveBeenCalledTimes(2)
|
||||
expect(registryGetKeyMock).toHaveBeenCalledTimes(2)
|
||||
|
||||
expect(readPersistedWindowsPathSegments({ forceRefresh: true })).toEqual([
|
||||
'C:\\Machine',
|
||||
'C:\\User',
|
||||
'C:\\Program Files\\GitHub CLI'
|
||||
])
|
||||
expect(defaultExecFileSyncMock).toHaveBeenCalledTimes(4)
|
||||
expect(registryGetKeyMock).toHaveBeenCalledTimes(4)
|
||||
} finally {
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
defaultExecFileSyncMock.mockReset()
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
|
||||
}
|
||||
})
|
||||
@@ -114,9 +130,9 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
it('keeps the last good segments when a forced read hits a blocked registry', () => {
|
||||
const originalPlatform = process.platform
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
defaultExecFileSyncMock
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\Machine\r\n')
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\User\r\n')
|
||||
registryGetKeyMock
|
||||
.mockReturnValueOnce(registryPath('C:\\Machine'))
|
||||
.mockReturnValueOnce(registryPath('C:\\User'))
|
||||
.mockImplementation(() => {
|
||||
throw new Error('ERROR: Access is denied.')
|
||||
})
|
||||
@@ -131,7 +147,6 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
expect(readPersistedWindowsPathSegments()).toEqual(['C:\\Machine', 'C:\\User'])
|
||||
} finally {
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
defaultExecFileSyncMock.mockReset()
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
|
||||
}
|
||||
})
|
||||
@@ -139,10 +154,10 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
it('still clears the cache when the registry reports an empty Path', () => {
|
||||
const originalPlatform = process.platform
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
defaultExecFileSyncMock
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\Machine\r\n')
|
||||
.mockReturnValueOnce(' Path REG_SZ C:\\User\r\n')
|
||||
.mockReturnValue(' Path REG_SZ \r\n')
|
||||
registryGetKeyMock
|
||||
.mockReturnValueOnce(registryPath('C:\\Machine'))
|
||||
.mockReturnValueOnce(registryPath('C:\\User'))
|
||||
.mockReturnValue(registryPath(''))
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
|
||||
try {
|
||||
@@ -154,7 +169,6 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
expect(readPersistedWindowsPathSegments()).toEqual([])
|
||||
} finally {
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
defaultExecFileSyncMock.mockReset()
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
|
||||
}
|
||||
})
|
||||
@@ -162,13 +176,9 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
it('deduplicates concurrent forced asynchronous refreshes and merges each environment', async () => {
|
||||
const originalPlatform = process.platform
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const callbacks: ExecCallback[] = []
|
||||
defaultExecFileMock.mockImplementation(
|
||||
(_file: string, _args: string[], _options: unknown, callback: ExecCallback) => {
|
||||
callbacks.push(callback)
|
||||
return {} as never
|
||||
}
|
||||
)
|
||||
registryGetKeyMock
|
||||
.mockReturnValueOnce(registryPath('C:\\Machine'))
|
||||
.mockReturnValueOnce(registryPath('C:\\User'))
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
|
||||
try {
|
||||
@@ -177,41 +187,25 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
const first = mergePersistedWindowsPathAsync(firstEnv, { forceRefresh: true })
|
||||
const second = mergePersistedWindowsPathAsync(secondEnv, { forceRefresh: true })
|
||||
|
||||
expect(defaultExecFileMock).toHaveBeenCalledTimes(2)
|
||||
callbacks[0]?.(null, ' Path REG_SZ C:\\Machine\r\n', '')
|
||||
callbacks[1]?.(null, ' Path REG_SZ C:\\User\r\n', '')
|
||||
await Promise.all([first, second])
|
||||
expect(firstEnv.Path).toBe('C:\\First;C:\\Machine;C:\\User')
|
||||
expect(secondEnv.Path).toBe('C:\\Second;C:\\Machine;C:\\User')
|
||||
expect(registryGetKeyMock).toHaveBeenCalledTimes(2)
|
||||
} finally {
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
defaultExecFileMock.mockReset()
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps the last good cache when bounded asynchronous reads time out', async () => {
|
||||
it('keeps the last good cache when native registry reads fail', async () => {
|
||||
const originalPlatform = process.platform
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
defaultExecFileMock
|
||||
.mockImplementationOnce(
|
||||
(_file: string, _args: string[], _options: unknown, callback: ExecCallback) => {
|
||||
callback(null, ' Path REG_SZ C:\\Machine\r\n', '')
|
||||
return {} as never
|
||||
}
|
||||
)
|
||||
.mockImplementationOnce(
|
||||
(_file: string, _args: string[], _options: unknown, callback: ExecCallback) => {
|
||||
callback(null, ' Path REG_SZ C:\\User\r\n', '')
|
||||
return {} as never
|
||||
}
|
||||
)
|
||||
.mockImplementation(
|
||||
(_file: string, _args: string[], _options: unknown, callback: ExecCallback) => {
|
||||
callback(Object.assign(new Error('timed out'), { code: 'ETIMEDOUT' }), '', '')
|
||||
return {} as never
|
||||
}
|
||||
)
|
||||
registryGetKeyMock
|
||||
.mockReturnValueOnce(registryPath('C:\\Machine'))
|
||||
.mockReturnValueOnce(registryPath('C:\\User'))
|
||||
.mockImplementation(() => {
|
||||
throw new Error('registry unavailable')
|
||||
})
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
|
||||
try {
|
||||
@@ -223,7 +217,7 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
'C:\\Machine',
|
||||
'C:\\User'
|
||||
])
|
||||
expect(defaultExecFileMock).toHaveBeenCalledTimes(2)
|
||||
expect(registryGetKeyMock).toHaveBeenCalledTimes(2)
|
||||
await expect(readPersistedWindowsPathSegmentsAsync({ forceRefresh: true })).resolves.toEqual([
|
||||
'C:\\Machine',
|
||||
'C:\\User'
|
||||
@@ -232,11 +226,9 @@ describe('readPersistedWindowsPathSegments', () => {
|
||||
'C:\\Machine',
|
||||
'C:\\User'
|
||||
])
|
||||
expect(defaultExecFileMock).toHaveBeenCalledTimes(4)
|
||||
expect(defaultExecFileMock.mock.calls[2]?.[2]).toMatchObject({ timeout: 5_000 })
|
||||
expect(registryGetKeyMock).toHaveBeenCalledTimes(4)
|
||||
} finally {
|
||||
__resetPersistedWindowsPathCacheForTests()
|
||||
defaultExecFileMock.mockReset()
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
|
||||
}
|
||||
})
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
import { execFile, execFileSync } from 'node:child_process'
|
||||
import type { execFile, execFileSync } from 'node:child_process'
|
||||
import {
|
||||
expandWindowsEnvironmentVariables,
|
||||
expandWindowsPathEnvironmentVariables
|
||||
} from '../../shared/windows-environment-expansion'
|
||||
import { getRegExePath } from '../win32-utils'
|
||||
import { readWindowsPathRegistry } from './windows-path-registry-reader'
|
||||
|
||||
type ExecFile = typeof execFile
|
||||
type ExecFileSync = typeof execFileSync
|
||||
@@ -16,10 +17,7 @@ type ReadWindowsPathOptions = {
|
||||
platform?: NodeJS.Platform
|
||||
}
|
||||
|
||||
type RegistryPathRead = {
|
||||
failed: boolean
|
||||
segments: string[]
|
||||
}
|
||||
type RegistryPathRead = { failed: boolean; segments: string[] }
|
||||
|
||||
const WINDOWS_PATH_REGISTRY_KEYS = [
|
||||
['HKLM\\SYSTEM\\CurrentControlSet\\Control\\Session Manager\\Environment', 'Path'],
|
||||
@@ -71,6 +69,19 @@ function registryOutputSegments(
|
||||
: []
|
||||
}
|
||||
|
||||
function readNativeRegistryPaths(
|
||||
env: NodeJS.ProcessEnv,
|
||||
pathDelimiter: string
|
||||
): RegistryPathRead[] {
|
||||
return readWindowsPathRegistry().map((read) => ({
|
||||
failed: read.failed,
|
||||
segments:
|
||||
read.value === null
|
||||
? []
|
||||
: splitPathSegments(expandWindowsEnvironmentVariables(read.value, env), pathDelimiter)
|
||||
}))
|
||||
}
|
||||
|
||||
function keepLastGoodSegments(segments: string[], failedReads: number): string[] {
|
||||
if (failedReads === WINDOWS_PATH_REGISTRY_KEYS.length && persistedWindowsPathCache) {
|
||||
return [...persistedWindowsPathCache.segments]
|
||||
@@ -145,34 +156,39 @@ export function readPersistedWindowsPathSegments(options: ReadWindowsPathOptions
|
||||
return [...persistedWindowsPathCache.segments]
|
||||
}
|
||||
|
||||
const run = options.execFileSync ?? execFileSync
|
||||
const env = options.env ?? process.env
|
||||
const pathDelimiter = getPathDelimiter(platform)
|
||||
const segments: string[] = []
|
||||
let failedReads = 0
|
||||
|
||||
for (const [key, valueName] of WINDOWS_PATH_REGISTRY_KEYS) {
|
||||
try {
|
||||
const output = run(getRegExePath(env), ['query', key, '/v', valueName], {
|
||||
encoding: 'utf8',
|
||||
timeout: PERSISTED_WINDOWS_PATH_QUERY_TIMEOUT_MS,
|
||||
windowsHide: true
|
||||
const reads = options.execFileSync
|
||||
? WINDOWS_PATH_REGISTRY_KEYS.map(([key, valueName]) => {
|
||||
try {
|
||||
const output = options.execFileSync!(
|
||||
getRegExePath(env),
|
||||
['query', key, '/v', valueName],
|
||||
{
|
||||
encoding: 'utf8',
|
||||
timeout: PERSISTED_WINDOWS_PATH_QUERY_TIMEOUT_MS,
|
||||
windowsHide: true
|
||||
}
|
||||
)
|
||||
return {
|
||||
failed: false,
|
||||
segments: registryOutputSegments(output, valueName, env, pathDelimiter)
|
||||
}
|
||||
} catch {
|
||||
return { failed: true, segments: [] }
|
||||
}
|
||||
})
|
||||
segments.push(...registryOutputSegments(output, valueName, env, pathDelimiter))
|
||||
} catch {
|
||||
// Registry access can fail in stripped test containers or remote-like
|
||||
// Windows contexts. Existing PATH remains the fallback in those cases.
|
||||
failedReads += 1
|
||||
}
|
||||
}
|
||||
: readNativeRegistryPaths(env, pathDelimiter)
|
||||
const segments = reads.flatMap((read) => read.segments)
|
||||
const failedReads = reads.filter((read) => read.failed).length
|
||||
|
||||
if (!useProductionCache) {
|
||||
return segments
|
||||
}
|
||||
|
||||
// Why: local PTY spawn is a hot path on Windows, and each uncached read
|
||||
// runs two synchronous `reg.exe query` subprocesses. A short TTL keeps
|
||||
// terminal bursts cheap while still picking up newly installed CLIs soon.
|
||||
// Why: local PTY spawn is a hot path on Windows, and each uncached refresh performs two
|
||||
// synchronous native registry reads. A short TTL keeps terminal bursts cheap while still
|
||||
// picking up newly installed CLIs soon.
|
||||
return cachePersistedWindowsPathSegments(segments, failedReads)
|
||||
}
|
||||
|
||||
@@ -202,14 +218,22 @@ export async function readPersistedWindowsPathSegmentsAsync(
|
||||
return [...(await pendingPersistedWindowsPathRefresh)]
|
||||
}
|
||||
|
||||
const run = options.execFile ?? execFile
|
||||
const env = options.env ?? process.env
|
||||
const pathDelimiter = getPathDelimiter(platform)
|
||||
const executable = getRegExePath(env)
|
||||
const refresh = Promise.all(
|
||||
WINDOWS_PATH_REGISTRY_KEYS.map((registryValue) =>
|
||||
readRegistryPathAsync(run, executable, registryValue, env, pathDelimiter)
|
||||
)
|
||||
const refresh = (
|
||||
options.execFile
|
||||
? Promise.all(
|
||||
WINDOWS_PATH_REGISTRY_KEYS.map((registryValue) =>
|
||||
readRegistryPathAsync(
|
||||
options.execFile!,
|
||||
getRegExePath(env),
|
||||
registryValue,
|
||||
env,
|
||||
pathDelimiter
|
||||
)
|
||||
)
|
||||
)
|
||||
: Promise.resolve(readNativeRegistryPaths(env, pathDelimiter))
|
||||
).then((reads) => {
|
||||
const segments = reads.flatMap((read) => read.segments)
|
||||
if (!useProductionCache) {
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
__setWindowsPathRegistryLoaderForTests,
|
||||
readWindowsPathRegistry
|
||||
} from './windows-path-registry-reader'
|
||||
|
||||
describe('readWindowsPathRegistry', () => {
|
||||
afterEach(() => __setWindowsPathRegistryLoaderForTests())
|
||||
|
||||
it('reads machine and user PATH values without creating a process', () => {
|
||||
const getRegistryKey = vi
|
||||
.fn()
|
||||
.mockReturnValueOnce({ PATH: { type: 2, value: '%SystemRoot%\\System32' } })
|
||||
.mockReturnValueOnce({ Path: { type: 1, value: 'C:\\Users\\me\\bin' } })
|
||||
__setWindowsPathRegistryLoaderForTests(() => ({
|
||||
HK: { LM: 1, CU: 2 },
|
||||
getRegistryKey
|
||||
}))
|
||||
|
||||
expect(readWindowsPathRegistry()).toEqual([
|
||||
{ failed: false, value: '%SystemRoot%\\System32' },
|
||||
{ failed: false, value: 'C:\\Users\\me\\bin' }
|
||||
])
|
||||
expect(getRegistryKey).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
1,
|
||||
'SYSTEM\\CurrentControlSet\\Control\\Session Manager\\Environment'
|
||||
)
|
||||
expect(getRegistryKey).toHaveBeenNthCalledWith(2, 2, 'Environment')
|
||||
})
|
||||
|
||||
it('distinguishes an empty PATH from a failed registry read', () => {
|
||||
const getRegistryKey = vi
|
||||
.fn()
|
||||
.mockReturnValueOnce({ Path: { type: 1, value: '' } })
|
||||
.mockReturnValueOnce({ Path: { type: 4, value: 1 } })
|
||||
__setWindowsPathRegistryLoaderForTests(() => ({
|
||||
HK: { LM: 1, CU: 2 },
|
||||
getRegistryKey
|
||||
}))
|
||||
|
||||
expect(readWindowsPathRegistry()).toEqual([
|
||||
{ failed: false, value: '' },
|
||||
{ failed: true, value: null }
|
||||
])
|
||||
})
|
||||
|
||||
it('treats a missing PATH value as a failed query', () => {
|
||||
__setWindowsPathRegistryLoaderForTests(() => ({
|
||||
HK: { LM: 1, CU: 2 },
|
||||
getRegistryKey: vi.fn(() => ({}))
|
||||
}))
|
||||
|
||||
expect(readWindowsPathRegistry()).toEqual([
|
||||
{ failed: true, value: null },
|
||||
{ failed: true, value: null }
|
||||
])
|
||||
})
|
||||
|
||||
it('fails closed when the optional native module is unavailable', () => {
|
||||
__setWindowsPathRegistryLoaderForTests(() => {
|
||||
throw new Error('native module unavailable')
|
||||
})
|
||||
|
||||
expect(readWindowsPathRegistry()).toEqual([
|
||||
{ failed: true, value: null },
|
||||
{ failed: true, value: null }
|
||||
])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,67 @@
|
||||
import {
|
||||
loadWindowsNativeRegistry,
|
||||
WINDOWS_REG_EXPAND_SZ,
|
||||
WINDOWS_REG_SZ,
|
||||
type WindowsNativeRegistryModule
|
||||
} from '../windows-native-registry'
|
||||
|
||||
export type WindowsPathRegistryRead = {
|
||||
failed: boolean
|
||||
value: string | null
|
||||
}
|
||||
|
||||
const MACHINE_ENVIRONMENT_KEY = 'SYSTEM\\CurrentControlSet\\Control\\Session Manager\\Environment'
|
||||
const USER_ENVIRONMENT_KEY = 'Environment'
|
||||
const PATH_VALUE = 'Path'
|
||||
|
||||
let windowsRegistryLoader = loadWindowsNativeRegistry
|
||||
|
||||
function readRegistryPath(
|
||||
registry: WindowsNativeRegistryModule,
|
||||
root: number,
|
||||
key: string
|
||||
): WindowsPathRegistryRead {
|
||||
try {
|
||||
const values = registry.getRegistryKey(root, key)
|
||||
if (!values || typeof values !== 'object') {
|
||||
return { failed: true, value: null }
|
||||
}
|
||||
const entry = Object.entries(values).find(
|
||||
([name]) => name.toLowerCase() === PATH_VALUE.toLowerCase()
|
||||
)?.[1]
|
||||
if (!entry) {
|
||||
return { failed: true, value: null }
|
||||
}
|
||||
if (
|
||||
(entry.type !== WINDOWS_REG_SZ && entry.type !== WINDOWS_REG_EXPAND_SZ) ||
|
||||
typeof entry.value !== 'string'
|
||||
) {
|
||||
return { failed: true, value: null }
|
||||
}
|
||||
return { failed: false, value: entry.value }
|
||||
} catch {
|
||||
return { failed: true, value: null }
|
||||
}
|
||||
}
|
||||
|
||||
export function readWindowsPathRegistry(): WindowsPathRegistryRead[] {
|
||||
let registry: WindowsNativeRegistryModule
|
||||
try {
|
||||
registry = windowsRegistryLoader()
|
||||
} catch {
|
||||
return [
|
||||
{ failed: true, value: null },
|
||||
{ failed: true, value: null }
|
||||
]
|
||||
}
|
||||
return [
|
||||
readRegistryPath(registry, registry.HK.LM, MACHINE_ENVIRONMENT_KEY),
|
||||
readRegistryPath(registry, registry.HK.CU, USER_ENVIRONMENT_KEY)
|
||||
]
|
||||
}
|
||||
|
||||
export function __setWindowsPathRegistryLoaderForTests(
|
||||
loader?: () => WindowsNativeRegistryModule
|
||||
): void {
|
||||
windowsRegistryLoader = loader ?? loadWindowsNativeRegistry
|
||||
}
|
||||
@@ -0,0 +1,25 @@
|
||||
import { createRequire } from 'node:module'
|
||||
|
||||
export type WindowsNativeRegistryValue = {
|
||||
name?: unknown
|
||||
type?: unknown
|
||||
value?: unknown
|
||||
}
|
||||
|
||||
export type WindowsNativeRegistryModule = {
|
||||
HK: { CU: number; LM: number }
|
||||
getRegistryKey: (
|
||||
root: number,
|
||||
path: string
|
||||
) => Record<string, WindowsNativeRegistryValue | undefined> | null | undefined
|
||||
}
|
||||
|
||||
export const WINDOWS_REG_SZ = 1
|
||||
export const WINDOWS_REG_EXPAND_SZ = 2
|
||||
|
||||
const requireFromMain = createRequire(__filename)
|
||||
|
||||
export function loadWindowsNativeRegistry(): WindowsNativeRegistryModule {
|
||||
// Why: non-Windows installs omit this optional dependency, so never resolve it at module load.
|
||||
return requireFromMain('windows-native-registry') as WindowsNativeRegistryModule
|
||||
}
|
||||
Reference in New Issue
Block a user