Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion src/main/runtime/harness-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,37 @@ export function buildHarnessArguments(
]
}

/**
* The captured PATH, looked up the way Windows actually stores it.
*
* `resolveShellEnvironment()` returns a plain object built by `parseEnvOutput`,
* keyed by whatever case the environment block reported — and Windows does not
* normalise that case, it follows the registry value name. A machine whose
* PATH value name is stored lowercase yields the key `path`, which an
* exact-case read misses entirely, launching the Harness with an empty PATH
* (issue #232). `process.env` never has this problem because Node makes it
* case-insensitive on win32 — but spreading it into a plain object keeps
* only the stored casing, so every copy needs this lookup too.
*
* POSIX keeps the exact read: there `path` and `PATH` are genuinely
* different variables.
*/
export function resolveEnvironmentPath(
environment: NodeJS.ProcessEnv,
platform: NodeJS.Platform = process.platform
): string {
if (platform !== 'win32') return environment.PATH ?? ''
// Exact-case reads first; the scan is the last resort for other casings.
// A real Windows block stores a single casing, so the order between them
// is never observable outside synthetic inputs.
const direct = environment.Path ?? environment.PATH
if (direct !== undefined) return direct
for (const [name, value] of Object.entries(environment)) {
if (/^path$/iu.test(name) && value !== undefined) return value
}
return ''
}

export function buildHarnessSpawnOptions(
launchDirectory: string,
dshHome: string,
Expand Down Expand Up @@ -233,7 +264,7 @@ export function buildHarnessSpawnOptions(
// the dedicated lock-recovery runner instead (see pnpm-runner.mjs).
npm_config_side_effects_cache: 'false',
PNPM_CONFIG_SIDE_EFFECTS_CACHE: 'false',
[pathKey]: environment[pathKey] ?? environment.PATH ?? ''
[pathKey]: resolveEnvironmentPath(environment, platform)
},
stdio: ['pipe', 'pipe', 'pipe'],
windowsHide: true,
Expand Down
10 changes: 5 additions & 5 deletions src/main/runtime/profile-plugin-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { spawn } from 'node:child_process'
import { existsSync } from 'node:fs'
import { chmod, mkdir, readdir, stat, writeFile } from 'node:fs/promises'
import { delimiter, dirname, join } from 'node:path'
import { resolveEnvironmentPath } from './harness-runtime'

const PROFILE = 'web'
const OPERATION_TIMEOUT_MS = 15 * 60 * 1000
Expand Down Expand Up @@ -146,11 +147,10 @@ export function buildProfilePluginCommandEnvironment(
const result = { ...environment }
delete result.ELECTRON_RUN_AS_NODE

const currentPath =
(process.platform === 'win32' ? result.Path : result.PATH) ??
result.PATH ??
result.Path ??
''
// The spread above keeps only the casing the OS block actually stores —
// even for `process.env`, whose case-insensitivity does not survive a
// copy — so the PATH read must be case-insensitive itself (issue #232).
const currentPath = resolveEnvironmentPath(result)
const parts = currentPath.split(delimiter).filter(Boolean)
const additions = [shimDirectory, dirname(nodeExecutablePath)].filter(
(directory) => !parts.includes(directory)
Expand Down
22 changes: 22 additions & 0 deletions test/profile-plugin-command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { join } from 'node:path'
import { afterEach, describe, expect, it } from 'vitest'
import {
buildPnpmShimCommand,
buildProfilePluginCommandEnvironment,
buildProfilePluginRemoveArguments,
diagnosticLine,
removeProfilePluginWithDsh
Expand Down Expand Up @@ -142,3 +143,24 @@ describe('profile pnpm shim and failure reporting', () => {
expect(diagnosticLine(' ')).toBeUndefined()
})
})

describe('buildProfilePluginCommandEnvironment', () => {
it('keeps the user PATH when the environment block stores it lowercase', () => {
// Spreading `process.env` keeps only the casing the OS block stores, so
// on a machine whose registry PATH value name is lowercase the previous
// exact-case read produced an empty base PATH — plugin commands then ran
// with just the shim and bundled-node directories (issue #232).
const userPath = 'C:\\Windows\\System32;C:\\Users\\tester\\bin'
const result = buildProfilePluginCommandEnvironment(
{ path: userPath },
'C:\\shim',
'C:\\bundled\\node.exe'
)
if (process.platform === 'win32') {
expect(result.PATH).toContain(userPath)
} else {
// POSIX: `path` is a different variable and must stay out of PATH.
expect(result.PATH).not.toContain(userPath)
}
})
})
26 changes: 23 additions & 3 deletions test/runtime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
extractSlotConflictName,
formatExitCode,
isHarnessStartupProbeHealthy,
resolveEnvironmentPath,
resolveShellEnvironment,
updateReadyStability
} from '../src/main/runtime/harness-runtime'
Expand Down Expand Up @@ -133,6 +134,25 @@ describe('Harness launch contract', () => {
}
})

it('finds the Windows PATH when the environment block stores it lowercase', () => {
// Windows environment variable names are case-insensitive and the captured
// block is not normalised, so a machine whose registry PATH value name is
// lowercase hands `resolveShellEnvironment()` the key `path`. An exact-case
// read misses it and the Harness launches with no PATH at all — every
// PATH-resolved tool call fails with ENOENT (issue #232).
const userPath = 'C:\\Windows\\System32;C:\\Users\\tester\\bin'
const options = buildHarnessSpawnOptions('C:\\launch-root', 'C:\\harness', 'win32', {
path: userPath
})
// Every key spelling PATH must carry the value: whichever of them survives
// Node's win32 case-dedupe, the child receives the user's PATH.
const pathEntries = Object.entries(options.env ?? {}).filter(([name]) =>
/^path$/iu.test(name)
)
expect(pathEntries.length).toBeGreaterThan(0)
for (const [, value] of pathEntries) expect(value).toBe(userPath)
})

it('passes the internal-loader flag directly to bundled Node.js', () => {
expect(
buildNodeArguments(
Expand Down Expand Up @@ -247,8 +267,8 @@ describe('shell environment resolution', () => {
() => {
const env = resolveShellEnvironment()
expect(env).toBeDefined()
// Windows may preserve the conventional mixed-case key.
expect(env.PATH ?? env.Path).toBeTruthy()
// Windows preserves whatever casing the environment block stores.
expect(resolveEnvironmentPath(env)).toBeTruthy()
},
20_000
)
Expand All @@ -262,7 +282,7 @@ describe('shell environment resolution', () => {

it('produces a PATH that includes platform-standard system directories', () => {
const env = resolveShellEnvironment()
const path = env.PATH ?? env.Path ?? ''
const path = resolveEnvironmentPath(env)
if (process.platform === 'win32') {
expect(path).toMatch(/[A-Za-z]:\\/)
} else {
Expand Down
18 changes: 17 additions & 1 deletion test/shell-environment-encoding.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { describe, expect, it } from 'vitest'
import { withoutUndecodableValues } from '../src/main/runtime/harness-runtime'
import {
resolveEnvironmentPath,
withoutUndecodableValues
} from '../src/main/runtime/harness-runtime'

/**
* Windows PowerShell writes stdout in the console codepage. Decoding that as
Expand Down Expand Up @@ -42,4 +45,17 @@ describe('captured shell environment', () => {
it('preserves an empty value', () => {
expect(withoutUndecodableValues({ EMPTY: '' }, {})).toEqual({ EMPTY: '' })
})

// Windows does not normalise environment variable name casing either — the
// captured block follows the registry value name, so PATH can arrive as
// `path` or any other spelling and an exact-case read misses it (issue #232).
it('reads PATH case-insensitively only on Windows', () => {
expect(resolveEnvironmentPath({ path: 'C:\\lower' }, 'win32')).toBe('C:\\lower')
expect(resolveEnvironmentPath({ PaTh: 'C:\\mixed' }, 'win32')).toBe('C:\\mixed')
// POSIX: `path` is a different variable, not a spelling of PATH.
expect(resolveEnvironmentPath({ path: '/ignored' }, 'linux')).toBe('')
expect(resolveEnvironmentPath({ PATH: '/usr/bin', path: '/ignored' }, 'linux')).toBe(
'/usr/bin'
)
})
})