fix(extensions): preserve command arguments through forms (#10527)
Co-authored-by: Lifei Zhou <lifei@squareup.com>
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Generated
-17
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user