Skip to content

Commit 3fa2d15

Browse files
zkochanclaude
andcommitted
fix: address review feedback for proxyVarSubDir
- Validate proxyVarSubDir to reject dangerous values (leading slashes, .., shell metacharacters) in both posix and windows path extenders - Quote pathRef in Fish shell string match when using proxyVarSubDir - Add Fish and Nushell tests for proxyVarSubDir Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 70648b2 commit 3fa2d15

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

os/env/path-extender-posix/path-extender-posix.spec.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,35 @@ if not string match -q -- $PNPM_HOME $PATH
10081008
set -gx PATH "$PNPM_HOME" $PATH
10091009
end
10101010
# pnpm end
1011+
`)
1012+
})
1013+
it('should append to shell script with proxyVarSubDir', async () => {
1014+
fs.mkdirSync('.config/fish', { recursive: true })
1015+
fs.writeFileSync(configFile, '', 'utf8')
1016+
const report = await addDirToPosixEnvPath(pnpmHomeDir, {
1017+
proxyVarName: 'PNPM_HOME',
1018+
proxyVarSubDir: 'bin',
1019+
configSectionName: 'pnpm',
1020+
})
1021+
expect(report).toStrictEqual({
1022+
configFile: {
1023+
path: configFile,
1024+
changeType: 'appended',
1025+
},
1026+
oldSettings: ``,
1027+
newSettings: `set -gx PNPM_HOME "${pnpmHomeDir}"
1028+
if not string match -q -- "$PNPM_HOME/bin" $PATH
1029+
set -gx PATH "$PNPM_HOME/bin" $PATH
1030+
end`,
1031+
})
1032+
const configContent = fs.readFileSync(configFile, 'utf8')
1033+
expect(configContent).toEqual(`
1034+
# pnpm
1035+
set -gx PNPM_HOME "${pnpmHomeDir}"
1036+
if not string match -q -- "$PNPM_HOME/bin" $PATH
1037+
set -gx PATH "$PNPM_HOME/bin" $PATH
1038+
end
1039+
# pnpm end
10111040
`)
10121041
})
10131042
it('should append to empty shell script without using a proxy varialbe', async () => {
@@ -1208,6 +1237,31 @@ $env.PATH = ($env.PATH | split row (char esep) | prepend $env.PNPM_HOME )`,
12081237
$env.PNPM_HOME = "${pnpmHomeDir}"
12091238
$env.PATH = ($env.PATH | split row (char esep) | prepend $env.PNPM_HOME )
12101239
# pnpm end
1240+
`)
1241+
})
1242+
it('should append to shell script with proxyVarSubDir', async () => {
1243+
fs.mkdirSync('.config/nushell', { recursive: true })
1244+
fs.writeFileSync(configFile, '', 'utf8')
1245+
const report = await addDirToPosixEnvPath(pnpmHomeDir, {
1246+
proxyVarName: 'PNPM_HOME',
1247+
proxyVarSubDir: 'bin',
1248+
configSectionName: 'pnpm',
1249+
})
1250+
expect(report).toStrictEqual({
1251+
configFile: {
1252+
path: configFile,
1253+
changeType: 'appended',
1254+
},
1255+
oldSettings: ``,
1256+
newSettings: `$env.PNPM_HOME = "${pnpmHomeDir}"
1257+
$env.PATH = ($env.PATH | split row (char esep) | prepend ($env.PNPM_HOME | path join "bin") )`,
1258+
})
1259+
const configContent = fs.readFileSync(configFile, 'utf8')
1260+
expect(configContent).toEqual(`
1261+
# pnpm
1262+
$env.PNPM_HOME = "${pnpmHomeDir}"
1263+
$env.PATH = ($env.PATH | split row (char esep) | prepend ($env.PNPM_HOME | path join "bin") )
1264+
# pnpm end
12111265
`)
12121266
})
12131267
it('should append to empty shell script without using a proxy varialbe', async () => {

os/env/path-extender-posix/path-extender-posix.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,24 @@ export async function addDirToPosixEnvPath (
4343
dir: string,
4444
opts: AddDirToPosixEnvPathOpts
4545
): Promise<PathExtenderPosixReport> {
46+
if (opts.proxyVarSubDir) {
47+
validateSubDir(opts.proxyVarSubDir)
48+
}
4649
const currentShell = detectCurrentShell()
4750
return await updateShell(currentShell, dir, opts)
4851
}
4952

53+
function validateSubDir (subDir: string): void {
54+
if (
55+
subDir.startsWith('/') ||
56+
subDir.startsWith('\\') ||
57+
subDir.includes('..') ||
58+
/[;\n\r"'`$%|<>&]/.test(subDir)
59+
) {
60+
throw new PnpmError('INVALID_SUBDIR', `Invalid proxyVarSubDir: "${subDir}"`)
61+
}
62+
}
63+
5064
function detectCurrentShell () {
5165
if (process.env.ZSH_VERSION) return 'zsh'
5266
if (process.env.BASH_VERSION) return 'bash'
@@ -144,8 +158,9 @@ async function setupFishShell (dir: string, opts: AddDirToPosixEnvPathOpts): Pro
144158
const _createPathValue = createFishPathValue.bind(null, opts.position ?? 'start')
145159
if (opts.proxyVarName) {
146160
const pathRef = opts.proxyVarSubDir ? `$${opts.proxyVarName}/${opts.proxyVarSubDir}` : `$${opts.proxyVarName}`
161+
const matchPattern = opts.proxyVarSubDir ? `"${pathRef}"` : pathRef
147162
newSettings = `set -gx ${opts.proxyVarName} "${dir}"
148-
if not string match -q -- ${pathRef} $PATH
163+
if not string match -q -- ${matchPattern} $PATH
149164
set -gx PATH ${_createPathValue(pathRef)}
150165
end`
151166
} else {

os/env/path-extender-windows/path-extender-windows.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,21 @@ export async function addDirToWindowsEnvPath (dir: string, opts?: AddDirToWindow
6161

6262
export type EnvVariableChangeAction = 'skipped' | 'updated'
6363

64+
function validateSubDir (subDir: string): void {
65+
if (
66+
subDir.startsWith('/') ||
67+
subDir.startsWith('\\') ||
68+
subDir.includes('..') ||
69+
/[;%"'`<>&]/.test(subDir)
70+
) {
71+
throw new PnpmError('INVALID_SUBDIR', `Invalid proxyVarSubDir: "${subDir}"`)
72+
}
73+
}
74+
6475
async function _addDirToWindowsEnvPath (dir: string, opts: AddDirToWindowsEnvPathOpts = {}): Promise<PathExtenderWindowsReport> {
76+
if (opts.proxyVarSubDir) {
77+
validateSubDir(opts.proxyVarSubDir)
78+
}
6579
const addedDir = path.normalize(dir)
6680
const registryOutput = await getRegistryOutput()
6781
const changes: PathExtenderWindowsReport = []

0 commit comments

Comments
 (0)