Skip to content

Commit d124401

Browse files
jdaltonjdalton
committed
fix: cross-platform correctness in home/git/bin/process-lock
- env/home.ts: fall back to $USERPROFILE when $HOME is unset. HOME is typically unset on Windows; previously getHome() returned undefined there, breaking downstream pnpm/yarn path resolution. - git.ts: split git output on /\r?\n/ so Windows (CRLF) is handled. Wrap realpathSync in try/catch that falls back to the input path for EACCES/EPERM/EIO (container/overlay FS) while still re-throwing ENOENT/ENOTDIR so missing-path errors remain visible. - bin.ts: findRealNpm/Pnpm/Yarn now branch on WIN32 and guard every env-derived path against undefined. Previously path.join(getHome() as string, ...) would throw TypeError when HOME was unset. - process-lock.ts: replace Math.max(lastIndexOf('/'), '\\') with path.dirname() so parent-dir resolution works for mixed or absent separators. Hoist the fs destructure out of module scope (was subverting the lazy-require pattern). Use Date.now()/1000 for utimesSync to avoid allocating a Date on every touch tick. Covered by a new home.test.mts regression for the USERPROFILE fallback; existing git/process-lock/bin suites already exercise the changed code paths.
1 parent 24c18c8 commit d124401

5 files changed

Lines changed: 145 additions & 65 deletions

File tree

‎src/bin.ts‎

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -207,16 +207,28 @@ export function findRealNpm(): string {
207207
const fs = getFs()
208208
const path = getPath()
209209

210-
// Try to find npm in the same directory as the node executable.
210+
// Try to find npm alongside the node executable. On Windows this is
211+
// npm.cmd; on POSIX it's the bare npm shim.
211212
const nodeDir = path.dirname(process.execPath)
212-
const npmInNodeDir = path.join(nodeDir, 'npm')
213-
214-
if (fs.existsSync(npmInNodeDir)) {
215-
return npmInNodeDir
213+
const nodeDirCandidates = WIN32
214+
? [path.join(nodeDir, 'npm.cmd'), path.join(nodeDir, 'npm')]
215+
: [path.join(nodeDir, 'npm')]
216+
for (const candidate of nodeDirCandidates) {
217+
if (fs.existsSync(candidate)) {
218+
return candidate
219+
}
216220
}
217221

218-
// Try common npm locations.
219-
const commonPaths = ['/usr/local/bin/npm', '/usr/bin/npm']
222+
// Try common npm locations per platform.
223+
const appdata = getAppdata()
224+
const commonPaths = WIN32
225+
? [
226+
appdata ? path.join(appdata, 'npm', 'npm.cmd') : '',
227+
appdata ? path.join(appdata, 'npm', 'npm') : '',
228+
'C:\\Program Files\\nodejs\\npm.cmd',
229+
'C:\\Program Files\\nodejs\\npm',
230+
].filter(Boolean)
231+
: ['/usr/local/bin/npm', '/usr/bin/npm']
220232
const result = findRealBin('npm', commonPaths)
221233

222234
// If we found a valid path, return it.
@@ -246,27 +258,31 @@ export function findRealNpm(): string {
246258
*/
247259
export function findRealPnpm(): string {
248260
const path = getPath()
261+
const home = getHome()
262+
const appdata = getAppdata()
263+
const localappdata = getLocalappdata()
264+
const xdgDataHome = getXdgDataHome()
249265

250-
// Try common pnpm locations.
266+
// Try common pnpm locations. Guard each env-derived path with its
267+
// existence — getHome()/getAppdata()/etc. can all return undefined.
251268
const commonPaths = WIN32
252269
? [
253-
// Windows common paths.
254-
path.join(getAppdata() as string, 'npm', 'pnpm.cmd'),
255-
path.join(getAppdata() as string, 'npm', 'pnpm'),
256-
path.join(getLocalappdata() as string, 'pnpm', 'pnpm.cmd'),
257-
path.join(getLocalappdata() as string, 'pnpm', 'pnpm'),
270+
appdata ? path.join(appdata, 'npm', 'pnpm.cmd') : '',
271+
appdata ? path.join(appdata, 'npm', 'pnpm') : '',
272+
localappdata ? path.join(localappdata, 'pnpm', 'pnpm.cmd') : '',
273+
localappdata ? path.join(localappdata, 'pnpm', 'pnpm') : '',
258274
'C:\\Program Files\\nodejs\\pnpm.cmd',
259275
'C:\\Program Files\\nodejs\\pnpm',
260276
].filter(Boolean)
261277
: [
262-
// Unix common paths.
263278
'/usr/local/bin/pnpm',
264279
'/usr/bin/pnpm',
265-
path.join(
266-
(getXdgDataHome() as string) || `${getHome() as string}/.local/share`,
267-
'pnpm/pnpm',
268-
),
269-
path.join(getHome() as string, '.pnpm/pnpm'),
280+
xdgDataHome
281+
? path.join(xdgDataHome, 'pnpm/pnpm')
282+
: home
283+
? path.join(home, '.local/share/pnpm/pnpm')
284+
: '',
285+
home ? path.join(home, '.pnpm/pnpm') : '',
270286
].filter(Boolean)
271287

272288
return findRealBin('pnpm', commonPaths) ?? ''
@@ -283,17 +299,28 @@ export function findRealPnpm(): string {
283299
*/
284300
export function findRealYarn(): string {
285301
const path = getPath()
302+
const home = getHome()
303+
const appdata = getAppdata()
286304

287-
// Try common yarn locations.
288-
const commonPaths = [
289-
'/usr/local/bin/yarn',
290-
'/usr/bin/yarn',
291-
path.join(getHome() as string, '.yarn/bin/yarn'),
292-
path.join(
293-
getHome() as string,
294-
'.config/yarn/global/node_modules/.bin/yarn',
295-
),
296-
].filter(Boolean)
305+
// Try common yarn locations per platform. Guard env-derived paths with
306+
// existence checks — getHome()/getAppdata() can return undefined.
307+
const commonPaths = WIN32
308+
? [
309+
appdata ? path.join(appdata, 'npm', 'yarn.cmd') : '',
310+
appdata ? path.join(appdata, 'npm', 'yarn') : '',
311+
home ? path.join(home, '.yarn/bin/yarn.cmd') : '',
312+
home ? path.join(home, '.yarn/bin/yarn') : '',
313+
'C:\\Program Files\\nodejs\\yarn.cmd',
314+
'C:\\Program Files\\nodejs\\yarn',
315+
].filter(Boolean)
316+
: [
317+
'/usr/local/bin/yarn',
318+
'/usr/bin/yarn',
319+
home ? path.join(home, '.yarn/bin/yarn') : '',
320+
home
321+
? path.join(home, '.config/yarn/global/node_modules/.bin/yarn')
322+
: '',
323+
].filter(Boolean)
297324

298325
return findRealBin('yarn', commonPaths) ?? ''
299326
}

‎src/env/home.ts‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,34 @@
11
/**
2-
* HOME environment variable getter.
3-
* Points to the user's home directory.
2+
* @fileoverview HOME environment variable getter with Windows fallback.
3+
* Returns the user's home directory. On Windows, HOME is typically unset —
4+
* fall back to USERPROFILE before giving up, matching the resolution order
5+
* used by npm, git, and Node's os.homedir().
46
*/
57

68
import { getEnvValue } from './rewire'
79

810
/**
9-
* Returns the value of the HOME environment variable.
11+
* Returns the user's home directory path.
1012
*
11-
* @returns The user's home directory path, or `undefined` if not set
13+
* Resolution order:
14+
* 1. `$HOME` (POSIX, and sometimes set on Windows by shells like Git Bash)
15+
* 2. `$USERPROFILE` (Windows default, e.g. `C:\Users\alice`)
16+
*
17+
* Returns `undefined` only when neither is set, which on modern systems is
18+
* exceedingly rare outside of sandboxed or minimal-env test harnesses.
19+
*
20+
* @returns The user's home directory path, or `undefined` if not resolvable
1221
*
1322
* @example
1423
* ```typescript
1524
* import { getHome } from '@socketsecurity/lib/env/home'
1625
*
1726
* const home = getHome()
18-
* // e.g. '/tmp/user' or undefined
27+
* // POSIX: '/Users/alice'
28+
* // Windows: 'C:\\Users\\alice'
1929
* ```
2030
*/
2131
/*@__NO_SIDE_EFFECTS__*/
2232
export function getHome(): string | undefined {
23-
return getEnvValue('HOME')
33+
return getEnvValue('HOME') ?? getEnvValue('USERPROFILE')
2434
}

‎src/git.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,12 @@ const gitRootCache = new Map<string, string>()
185185
/**
186186
* Get the real path with caching to avoid repeated filesystem calls.
187187
* Validates cache with existsSync() which is cheaper than realpathSync().
188+
*
189+
* ENOENT/ENOTDIR are re-thrown because the caller explicitly passed a path
190+
* they expect to exist — swallowing these would turn "file not found" into
191+
* a silent no-op. Other errors (EACCES, EPERM, EIO) fall back to the input
192+
* path since they can happen on container/overlay filesystems where the
193+
* path exists but realpath resolution is restricted.
188194
* @private
189195
*/
190196
function getCachedRealpath(pathname: string): string {
@@ -197,7 +203,16 @@ function getCachedRealpath(pathname: string): string {
197203
// Cached path no longer exists, remove stale entry.
198204
realpathCache.delete(pathname)
199205
}
200-
const resolved = fs.realpathSync(pathname)
206+
let resolved: string
207+
try {
208+
resolved = fs.realpathSync(pathname)
209+
} catch (error) {
210+
const code = (error as NodeJS.ErrnoException).code
211+
if (code === 'ENOENT' || code === 'ENOTDIR') {
212+
throw error
213+
}
214+
resolved = pathname
215+
}
201216
realpathCache.set(pathname, resolved)
202217
return resolved
203218
}
@@ -241,7 +256,7 @@ function getGitPath(): string {
241256
* ```
242257
*/
243258
function getCwd(): string {
244-
return getFs().realpathSync(process.cwd())
259+
return getCachedRealpath(process.cwd())
245260
}
246261

247262
/**
@@ -259,7 +274,7 @@ function getCwd(): string {
259274
* @returns Object containing spawn arguments for all, unstaged, and staged operations.
260275
*/
261276
function getGitDiffSpawnArgs(cwd?: string | undefined): GitDiffSpawnArgs {
262-
const resolvedCwd = cwd ? getFs().realpathSync(cwd) : getCwd()
277+
const resolvedCwd = cwd ? getCachedRealpath(cwd) : getCwd()
263278
return {
264279
all: [
265280
getGitPath(),
@@ -495,9 +510,10 @@ function parseGitDiffStdout(
495510
cwdOption === defaultRoot ? defaultRoot : getCachedRealpath(cwdOption)
496511
const rootPath = defaultRoot
497512
// Split into lines without trimming to preserve leading spaces in porcelain format.
513+
// Handle both LF (POSIX) and CRLF (Windows) — git on Windows emits CRLF by default.
498514
let rawFiles = stdout
499515
? stripAnsi(stdout)
500-
.split('\n')
516+
.split(/\r?\n/)
501517
.map(line => line.trimEnd())
502518
.filter(line => line)
503519
: []

‎src/process-lock.ts‎

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@
4040
* - Automatic cleanup on process exit
4141
*/
4242

43+
import { safeDeleteSync } from './fs'
44+
import { getDefaultLogger } from './logger'
45+
import { pRetry } from './promises'
46+
import { onExit } from './signal-exit'
47+
4348
let _fs: typeof import('node:fs') | undefined
4449
/**
4550
* Lazily load the fs module to avoid Webpack errors.
@@ -53,13 +58,18 @@ function getFs() {
5358
return _fs as typeof import('node:fs')
5459
}
5560

56-
import { safeDeleteSync } from './fs'
57-
import { getDefaultLogger } from './logger'
58-
import { pRetry } from './promises'
59-
import { onExit } from './signal-exit'
60-
61-
const fs = getFs()
62-
const { existsSync, mkdirSync, statSync, utimesSync } = fs
61+
let _path: typeof import('node:path') | undefined
62+
/**
63+
* Lazily load the path module to avoid Webpack errors.
64+
* @private
65+
*/
66+
/*@__NO_SIDE_EFFECTS__*/
67+
function getPath() {
68+
if (_path === undefined) {
69+
_path = /*@__PURE__*/ require('node:path')
70+
}
71+
return _path as typeof import('node:path')
72+
}
6373

6474
const logger = getDefaultLogger()
6575

@@ -128,9 +138,10 @@ class ProcessLockManager {
128138
this.touchTimers.clear()
129139

130140
// Clean up all active locks.
141+
const fs = getFs()
131142
for (const lockPath of this.activeLocks) {
132143
try {
133-
if (existsSync(lockPath)) {
144+
if (fs.existsSync(lockPath)) {
134145
safeDeleteSync(lockPath, { recursive: true })
135146
}
136147
} catch {
@@ -150,9 +161,12 @@ class ProcessLockManager {
150161
*/
151162
private touchLock(lockPath: string): void {
152163
try {
153-
if (existsSync(lockPath)) {
154-
const now = new Date()
155-
utimesSync(lockPath, now, now)
164+
const fs = getFs()
165+
if (fs.existsSync(lockPath)) {
166+
// utimesSync accepts numeric timestamps (seconds). Pass Date.now() / 1000
167+
// to avoid the Date allocation on every touch tick.
168+
const now = Date.now() / 1000
169+
fs.utimesSync(lockPath, now, now)
156170
}
157171
} catch (error) {
158172
logger.warn(
@@ -209,7 +223,7 @@ class ProcessLockManager {
209223
try {
210224
// Use single statSync call instead of existsSync + statSync.
211225
// throwIfNoEntry: false returns undefined if path doesn't exist.
212-
const stats = statSync(lockPath, { throwIfNoEntry: false })
226+
const stats = getFs().statSync(lockPath, { throwIfNoEntry: false })
213227
if (!stats) {
214228
return false
215229
}
@@ -275,23 +289,24 @@ class ProcessLockManager {
275289
}
276290

277291
// Check if lock already exists before creating.
278-
if (existsSync(lockPath)) {
292+
const fs = getFs()
293+
if (fs.existsSync(lockPath)) {
279294
throw new Error(`Lock already exists: ${lockPath}`)
280295
}
281296

282-
// Ensure parent directory exists.
283-
const lastSlash = Math.max(
284-
lockPath.lastIndexOf('/'),
285-
lockPath.lastIndexOf('\\'),
286-
)
287-
if (lastSlash > 0) {
288-
mkdirSync(lockPath.slice(0, lastSlash), { recursive: true })
297+
// Ensure parent directory exists. Use path.dirname() so that both
298+
// POSIX and Windows separators (and mixed-separator inputs) are
299+
// handled correctly — the previous Math.max(lastIndexOf('/'), '\\')
300+
// approach failed on relative paths and mixed-separator inputs.
301+
const parent = getPath().dirname(lockPath)
302+
if (parent && parent !== '.' && parent !== lockPath) {
303+
fs.mkdirSync(parent, { recursive: true })
289304
}
290305

291306
// Atomic lock acquisition via mkdir (without recursive).
292307
// Without recursive, mkdirSync throws EEXIST if another process
293308
// created the directory between the existsSync check and here.
294-
mkdirSync(lockPath)
309+
fs.mkdirSync(lockPath)
295310

296311
// Track lock for cleanup.
297312
this.activeLocks.add(lockPath)
@@ -398,7 +413,7 @@ class ProcessLockManager {
398413
this.stopTouchTimer(lockPath)
399414

400415
try {
401-
if (existsSync(lockPath)) {
416+
if (getFs().existsSync(lockPath)) {
402417
safeDeleteSync(lockPath, { recursive: true })
403418
}
404419
this.activeLocks.delete(lockPath)

‎test/unit/env/home.test.mts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
/**
22
* @fileoverview Unit tests for HOME environment variable getter.
33
*
4-
* Tests getHome() which retrieves the user's home directory path via HOME env var.
5-
* Returns home path string or undefined if not set. Unix/Linux standard.
6-
* On Windows, use getUserprofile() instead (USERPROFILE env var).
7-
* Uses rewire for isolated testing. Critical for resolving user-specific paths.
4+
* Tests getHome(), which resolves the user's home directory. It reads $HOME
5+
* first (POSIX + most shells), then falls back to $USERPROFILE for Windows
6+
* environments where $HOME is typically unset. Uses rewire for isolated
7+
* testing. Critical for resolving user-specific paths cross-platform.
88
*/
99

1010
import { getHome } from '@socketsecurity/lib/env/home'
@@ -150,5 +150,17 @@ describe('env/home', () => {
150150
setEnv('HOME', '/home/user/snap/app/common')
151151
expect(getHome()).toBe('/home/user/snap/app/common')
152152
})
153+
154+
it('should fall back to USERPROFILE when HOME is unset (Windows)', () => {
155+
setEnv('HOME', undefined)
156+
setEnv('USERPROFILE', 'C:\\Users\\alice')
157+
expect(getHome()).toBe('C:\\Users\\alice')
158+
})
159+
160+
it('should prefer HOME over USERPROFILE when both are set', () => {
161+
setEnv('HOME', '/Users/alice')
162+
setEnv('USERPROFILE', 'C:\\Users\\alice')
163+
expect(getHome()).toBe('/Users/alice')
164+
})
153165
})
154166
})

0 commit comments

Comments
 (0)