From 29af64a74428cfbb0acee001564b764b6ccbbcf5 Mon Sep 17 00:00:00 2001 From: Jasper Date: Tue, 21 Jul 2026 07:15:03 -0500 Subject: [PATCH] fix(extensions): preserve command arguments through forms (#10527) Co-authored-by: Lifei Zhou --- ui/desktop/package.json | 2 - .../settings/extensions/utils.test.ts | 56 +++++++- .../components/settings/extensions/utils.ts | 128 +++++++++++++++--- ui/pnpm-lock.yaml | 17 --- 4 files changed, 166 insertions(+), 37 deletions(-) diff --git a/ui/desktop/package.json b/ui/desktop/package.json index a119e6666..6afe87e9d 100644 --- a/ui/desktop/package.json +++ b/ui/desktop/package.json @@ -95,7 +95,6 @@ "remark-breaks": "^4.0.0", "remark-gfm": "^4.0.1", "remark-math": "^6.0.0", - "shell-quote": "^1.8.3", "split-type": "^0.3.4", "swr": "^2.4.0", "tailwind-merge": "^3.5.0", @@ -137,7 +136,6 @@ "@types/react": "^19.2.14", "@types/react-dom": "^19.2.3", "@types/react-syntax-highlighter": "^15.5.13", - "@types/shell-quote": "^1.7.5", "@types/turndown": "^5.0.6", "@types/yauzl": "^2.10.3", "@typescript-eslint/eslint-plugin": "^8.56.1", diff --git a/ui/desktop/src/components/settings/extensions/utils.test.ts b/ui/desktop/src/components/settings/extensions/utils.test.ts index 8150fa4f3..b364d019c 100644 --- a/ui/desktop/src/components/settings/extensions/utils.test.ts +++ b/ui/desktop/src/components/settings/extensions/utils.test.ts @@ -7,6 +7,7 @@ import { extractCommand, extractExtensionName, splitCmdAndArgs, + combineCmdAndArgs, DEFAULT_EXTENSION_TIMEOUT, } from './utils'; import type { FixedExtensionEntry } from '../../ConfigContext'; @@ -211,7 +212,7 @@ describe('Extension Utils', () => { const formData = extensionToFormData(extension); expect(formData.cmd).toBe( - '"/Applications/IntelliJ IDEA.app/Contents/jbr/Contents/Home/bin/java" -classpath "/path/with spaces/lib.jar" Main' + "'/Applications/IntelliJ IDEA.app/Contents/jbr/Contents/Home/bin/java' -classpath '/path/with spaces/lib.jar' Main" ); }); @@ -263,6 +264,32 @@ describe('Extension Utils', () => { expect(cmd).toBe('node'); expect(args).toEqual(['/My "Project"/bin/run']); }); + + it.each([ + ['-"c"', 'touch /tmp/pwned'], + ['--flag=value;next', '|', '>', '*'], + ['', 'both\'and"quotes', '$HOME', '`command`'], + ['line one\nline two', 'trailing\\', 'café'], + ])('faithfully roundtrips metacharacter-bearing argv %#', (...args) => { + const combined = combineCmdAndArgs('npx', args); + expect(splitCmdAndArgs(combined)).toEqual({ cmd: 'npx', args }); + }); + + it('does not synthesize a blocked npx flag through the extension form', () => { + const extension: FixedExtensionEntry = { + type: 'stdio', + name: 'quoted-flag', + description: 'quoted flag regression', + cmd: 'npx', + args: ['-"c"', 'touch /tmp/pwned'], + enabled: true, + }; + + expect(createExtensionConfig(extensionToFormData(extension))).toMatchObject({ + cmd: 'npx', + args: ['-"c"', 'touch /tmp/pwned'], + }); + }); }); describe('createExtensionConfig', () => { @@ -464,12 +491,39 @@ describe('Extension Utils', () => { }); }); + it('treats apostrophes as literal Windows command-line characters', () => { + expect(splitCmdAndArgs("node C:\\Users\\O'Connor\\ext.js --owner=O'Connor")).toEqual({ + cmd: 'node', + args: ["C:\\Users\\O'Connor\\ext.js", "--owner=O'Connor"], + }); + }); + it('handles a quoted Windows path containing spaces', () => { expect(splitCmdAndArgs('"C:\\Program Files\\app\\ext.js"')).toEqual({ cmd: 'C:\\Program Files\\app\\ext.js', args: [], }); }); + + it('preserves a quoted UNC path', () => { + expect(splitCmdAndArgs(String.raw`node "\\server\share with space\extension.js"`)).toEqual({ + cmd: 'node', + args: [String.raw`\\server\share with space\extension.js`], + }); + }); + + it('preserves a quoted path ending in a backslash', () => { + expect(splitCmdAndArgs(String.raw`node "C:\dir with space\\"`)).toEqual({ + cmd: 'node', + args: ['C:\\dir with space\\'], + }); + }); + + it('roundtrips quoted Windows paths and metacharacters', () => { + const cmd = 'C:\\Program Files\\nodejs\\npx.cmd'; + const args = ["C:\\Users\\O'Connor\\extension.js", '-"c"', 'a&b', 'quoted "value"\\']; + expect(splitCmdAndArgs(combineCmdAndArgs(cmd, args))).toEqual({ cmd, args }); + }); }); }); diff --git a/ui/desktop/src/components/settings/extensions/utils.ts b/ui/desktop/src/components/settings/extensions/utils.ts index a4eb77ee4..34d8ddcbe 100644 --- a/ui/desktop/src/components/settings/extensions/utils.ts +++ b/ui/desktop/src/components/settings/extensions/utils.ts @@ -1,6 +1,5 @@ import type { FixedExtensionEntry } from '../../ConfigContext'; import type { ExtensionConfig } from '../../../types/extensions'; -import { parse as parseShellQuote } from 'shell-quote'; // Default extension timeout in seconds // TODO: keep in sync with rust better @@ -110,7 +109,10 @@ export function extensionToFormData(extension: FixedExtensionEntry): ExtensionFo extension.type === 'platform' ? 'stdio' : extension.type, - cmd: extension.type === 'stdio' ? combineCmdAndArgs(extension.cmd, extension.args ?? []) : undefined, + cmd: + extension.type === 'stdio' + ? combineCmdAndArgs(extension.cmd, extension.args ?? []) + : undefined, endpoint: extension.type === 'streamable_http' || extension.type === 'sse' ? (extension.uri ?? undefined) @@ -203,14 +205,7 @@ export function splitCmdAndArgs(str: string): { cmd: string; args: string[] } { return { cmd: '', args: [] }; } - // shell-quote treats `\` as a POSIX escape character, so a Windows path like - // `C:\Users\name\ext.js` would lose its backslashes and become `C:Usersnameext.js`. - // Doubling backslashes on Windows lets them survive parsing (shell-quote unescapes - // `\\` back to `\`), while still honoring quotes for paths containing spaces. - const toParse = isWindowsPlatform() ? trimmed.replace(/\\/g, '\\\\') : trimmed; - - const parsed = parseShellQuote(toParse); - const words = parsed.filter((item): item is string => typeof item === 'string').map(String); + const words = parseCommandLine(trimmed, isWindowsPlatform()); const cmd = words[0] || ''; const args = words.slice(1); @@ -221,14 +216,113 @@ export function splitCmdAndArgs(str: string): { cmd: string; args: string[] } { }; } +function parseCommandLine(value: string, windows: boolean): string[] { + const words: string[] = []; + let word = ''; + let wordStarted = false; + let quote: "'" | '"' | undefined; + + for (let index = 0; index < value.length; index += 1) { + const character = value[index]; + + if (windows && quote !== "'" && character === '\\') { + let runEnd = index; + while (value[runEnd] === '\\') { + runEnd += 1; + } + + const backslashCount = runEnd - index; + if (value[runEnd] === '"') { + word += '\\'.repeat(Math.floor(backslashCount / 2)); + if (backslashCount % 2 === 0) { + quote = quote === '"' ? undefined : '"'; + } else { + word += '"'; + } + index = runEnd; + } else { + word += '\\'.repeat(backslashCount); + index = runEnd - 1; + } + wordStarted = true; + } else if (quote) { + if (character === quote) { + quote = undefined; + } else if (quote === '"' && character === '\\' && index + 1 < value.length) { + const next = value[index + 1]; + if (next === '"' || next === '\\' || next === '$') { + word += next; + index += 1; + } else { + word += character; + } + } else { + word += character; + } + wordStarted = true; + } else if (/\s/.test(character)) { + if (wordStarted) { + words.push(word); + word = ''; + wordStarted = false; + } + } else if (character === '"' || (!windows && character === "'")) { + quote = character; + wordStarted = true; + } else if (character === '\\' && !windows && index + 1 < value.length) { + word += value[index + 1]; + wordStarted = true; + index += 1; + } else { + word += character; + wordStarted = true; + } + } + + if (wordStarted) { + words.push(word); + } + + return words; +} + export function combineCmdAndArgs(cmd: string, args: string[]): string { - return [cmd, ...args] - .map((a) => { - if (!a.includes(' ')) return a; - if (a.includes('"')) return `'${a}'`; - return `"${a}"`; - }) - .join(' '); + const windows = isWindowsPlatform(); + return [cmd, ...args].map((value) => quoteCommandPart(value, windows)).join(' '); +} + +function quoteCommandPart(value: string, windows: boolean): string { + if (windows) { + return quoteWindowsCommandPart(value); + } + + if (/^[A-Za-z0-9_@%+=:,./-]+$/.test(value)) { + return value; + } + return `'${value.replace(/'/g, `'"'"'`)}'`; +} + +function quoteWindowsCommandPart(value: string): string { + if (value.length > 0 && !/[\s"]/u.test(value)) { + return value; + } + + let quoted = '"'; + let backslashCount = 0; + + for (const character of value) { + if (character === '\\') { + backslashCount += 1; + } else if (character === '"') { + quoted += '\\'.repeat(backslashCount * 2 + 1) + character; + backslashCount = 0; + } else { + quoted += '\\'.repeat(backslashCount) + character; + backslashCount = 0; + } + } + + return quoted + '\\'.repeat(backslashCount * 2) + '"'; } export function extractCommand(link: string): string { diff --git a/ui/pnpm-lock.yaml b/ui/pnpm-lock.yaml index d3288bcd5..4cbcafdbf 100644 --- a/ui/pnpm-lock.yaml +++ b/ui/pnpm-lock.yaml @@ -156,9 +156,6 @@ importers: remark-math: specifier: ^6.0.0 version: 6.0.0 - shell-quote: - specifier: ^1.8.3 - version: 1.8.3 split-type: specifier: ^0.3.4 version: 0.3.4 @@ -277,9 +274,6 @@ importers: '@types/react-syntax-highlighter': specifier: ^15.5.13 version: 15.5.13 - '@types/shell-quote': - specifier: ^1.7.5 - version: 1.7.5 '@types/turndown': specifier: ^5.0.6 version: 5.0.6 @@ -3041,9 +3035,6 @@ packages: '@types/serve-static@2.2.0': resolution: {integrity: sha512-8mam4H1NHLtu7nmtalF7eyBH14QyOASmcxHhSfEoRyr0nP/YdoesEtU+uSRvMe96TW/HPTtkoKqQLl53N7UXMQ==} - '@types/shell-quote@1.7.5': - resolution: {integrity: sha512-+UE8GAGRPbJVQDdxi16dgadcBfQ+KG2vgZhV1+3A1XmHbmwcdwhCUwIdy+d3pAGrbvgRoVSjeI9vOWyq376Yzw==} - '@types/turndown@5.0.6': resolution: {integrity: sha512-ru00MoyeeouE5BX4gRL+6m/BsDfbRayOskWqUvh7CLGW+UXxHQItqALa38kKnOiZPqJrtzJUgAC2+F0rL1S4Pg==} @@ -6419,10 +6410,6 @@ packages: resolution: {integrity: sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==} engines: {node: '>=8'} - shell-quote@1.8.3: - resolution: {integrity: sha512-ObmnIF4hXNg1BqhnHmgbDETF8dLPCggZWBjkQfhZpbszZnYur5DUljTcCHii5LC3J5E0yeO/1LIMyH+UvHQgyw==} - engines: {node: '>= 0.4'} - side-channel-list@1.0.0: resolution: {integrity: sha512-FCLHtRD/gnpCiCHEiJLOwdmFP+wzCmDEkc9y7NsYxeF4u7Btsn1ZuwgwJGxImImHicJArLP4R0yX4c2KCrMrTA==} engines: {node: '>= 0.4'} @@ -10126,8 +10113,6 @@ snapshots: '@types/http-errors': 2.0.5 '@types/node': 25.5.0 - '@types/shell-quote@1.7.5': {} - '@types/turndown@5.0.6': {} '@types/unist@2.0.11': {} @@ -14262,8 +14247,6 @@ snapshots: shebang-regex@3.0.0: {} - shell-quote@1.8.3: {} - side-channel-list@1.0.0: dependencies: es-errors: 1.3.0