Skip to content

Commit

Permalink
Merge pull request #2699 from microsoft/octogonz/rush-issue2695
Browse files Browse the repository at this point in the history
[rush] Fix an issue where "rushx" CLI parameters were not escaped properly
  • Loading branch information
octogonz authored May 14, 2021
2 parents 60d5daf + b466161 commit b3aa0f3
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 3 deletions.
13 changes: 11 additions & 2 deletions apps/rush-lib/src/cli/RushXCommandLine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,21 @@ export class RushXCommandLine {
}

const remainingArgs: string[] = args.slice(1);

let commandWithArgs: string = scriptBody;
let commandWithArgsForDisplay: string = scriptBody;
if (remainingArgs.length > 0) {
commandWithArgs += ' ' + remainingArgs.join(' ');
// This approach is based on what NPM 7 now does:
// https://github.com/npm/run-script/blob/47a4d539fb07220e7215cc0e482683b76407ef9b/lib/run-script-pkg.js#L34
const escapedRemainingArgs: string[] = remainingArgs.map((x) => Utilities.escapeShellParameter(x));

commandWithArgs += ' ' + escapedRemainingArgs.join(' ');

// Display it nicely without the extra quotes
commandWithArgsForDisplay += ' ' + remainingArgs.join(' ');
}

console.log('Executing: ' + JSON.stringify(commandWithArgs) + os.EOL);
console.log('Executing: ' + JSON.stringify(commandWithArgsForDisplay) + os.EOL);

const packageFolder: string = path.dirname(packageJsonFilePath);

Expand Down
7 changes: 6 additions & 1 deletion apps/rush-lib/src/utilities/Utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,9 @@ export class Utilities {
* Example: 'hello there' --> '"hello there"'
*/
public static escapeShellParameter(parameter: string): string {
return `"${parameter}"`;
// This approach is based on what NPM 7 now does:
// https://github.com/npm/run-script/blob/47a4d539fb07220e7215cc0e482683b76407ef9b/lib/run-script-pkg.js#L34
return JSON.stringify(parameter);
}

/**
Expand Down Expand Up @@ -854,6 +856,9 @@ export class Utilities {
if (result.error && (result.error as any).errno === 'ENOENT') {
// This is a workaround for GitHub issue #25330
// https://github.com/nodejs/node-v0.x-archive/issues/25330
//
// TODO: The fully worked out solution for this problem is now provided by the "Executable" API
// from @rushstack/node-core-library
result = child_process.spawnSync(command + '.cmd', args, options);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Fix an issue where \"rushx\" CLI arguments were not escaped properly (GitHub #2695)",
"type": "none"
}
],
"packageName": "@microsoft/rush",
"email": "[email protected]"
}

0 comments on commit b3aa0f3

Please sign in to comment.