fix: resolve path hijack (W-23905795)

package.json

@@ -15,6 +15,7 @@
     "@salesforce/sf-plugins-core": "^12.0.7",
     "@salesforce/ts-types": "^2.0.11",
     "change-case": "^5.4.2",
+    "cross-spawn": "^7.0.6",
     "ejs": "^3.1.10",
     "fast-glob": "^3.3.2",
     "got": "^13",
@@ -32,6 +33,7 @@
     "@salesforce/cli-plugins-testkit": "^5.3.20",
     "@salesforce/dev-scripts": "^10.2.11",
     "@salesforce/plugin-command-reference": "^3.1.5",
+    "@types/cross-spawn": "^6.0.6",
     "@types/ejs": "^3.1.5",
     "@types/js-yaml": "^4.0.5",
     "@types/lodash.defaultsdeep": "^4.6.9",

src/generator.ts

@@ -13,6 +13,7 @@ import { Ux } from '@salesforce/sf-plugins-core';
 import { Logger } from '@salesforce/core';
 import { colorize } from '@oclif/core/ux';
 import shelljs from 'shelljs';
+import spawn from 'cross-spawn';
 import replace from 'replace-in-file';
 import { fileExists } from './util.js';
 import { PackageJson } from './types.js';
@@ -88,17 +89,23 @@ export class Generator {
       return;
     }
 
-    const args = cmd.split(' ');
-    const bin = args[0];
+    // Tokenize into discrete argv entries so arguments are passed to the child
+    // process individually rather than as one shell-interpreted string. Callers
+    // double-quote arguments that may contain spaces (e.g. paths), so keep a
+    // quoted span as a single token and strip its surrounding quotes.
+    const [bin, ...args] = (cmd.match(/"[^"]*"|\S+/g) ?? []).map((token) => token.replace(/^"|"$/g, ''));
     const isBinPath = bin.includes('/') || bin.includes('\\');
-    const resolved = isBinPath ? bin : shelljs.which(bin);
+    const resolved = isBinPath ? bin : shelljs.which(bin)?.toString();
     if (!resolved) {
       throw new Error(`Could not find "${bin}" on PATH`);
     }
-    const resolvedCmd = [resolved.toString(), ...args.slice(1)].join(' ');
 
-    this.logger.debug(`Executing command: ${resolvedCmd}`);
-    shelljs.exec(resolvedCmd, { cwd: this.cwd });
+    this.logger.debug(`Executing command: ${[resolved, ...args].join(' ')}`);
+
+    // Pass argv as an array with no shell so a value like `C:\path\repo&calc.exe&\`
+    // is treated as literal data and cannot break out to hijack the shell.
+    // cross-spawn handles Windows .cmd/.bat shims (e.g. yarn.cmd) safely.
+    spawn.sync(resolved, args, { cwd: this.cwd, stdio: 'inherit' });
   }
 
   public async loadPjson(): Promise<PackageJson> {

test/commands/dev/generate/command3PP.nut.ts

@@ -62,6 +62,11 @@ describe('3PP', () => {
         env: {
           ...process.env,
           TESTKIT_EXECUTABLE_PATH: pluginExecutable,
+          // Disable wireit's GitHub Actions cache for the generated sub-project. In CI the
+          // outer job sets WIREIT_CACHE=github (plus cache credentials), which this child
+          // process would otherwise inherit and use to hit GitHub's cache service — a source
+          // of transient HTTP failures. Locally WIREIT_CACHE is unset (local disk cache).
+          WIREIT_CACHE: 'none',
         },
       });
       expect(result.code).to.equal(0);
@@ -73,7 +78,15 @@ describe('3PP', () => {
       const cmd = parts.pop();
       const unitTestFile = path.join(session.project.dir, 'test', 'commands', ...parts, `${cmd}.test.ts`);
       expect(await fileExists(unitTestFile)).to.be.true;
-      const result = shelljs.exec('yarn test:only', { cwd: session.project.dir });
+      const result = shelljs.exec('yarn test:only', {
+        cwd: session.project.dir,
+        env: {
+          ...process.env,
+          // See note above: avoid inheriting WIREIT_CACHE=github in CI, which makes the
+          // generated sub-project hit GitHub's flaky cache service.
+          WIREIT_CACHE: 'none',
+        },
+      });
       expect(result.code).to.equal(0);
       expect(result.stdout).include(name.replace(/:/g, ' '));
     });

test/commands/dev/generate/plugin.nut.ts

@@ -64,7 +64,11 @@ describe('dev generate plugin NUTs', () => {
     });
 
     expect(readFileSync(path.join(pluginDir, 'src', 'commands', 'hello', 'world.ts'), 'utf8')).to.include('Copyright');
-    expect(readFileSync(path.join(pluginDir, '.eslintrc.cjs'), 'utf8')).to.include('eslint-config-salesforce-license');
+    // The template's flat config re-exports eslint-config-salesforce-typescript, which now
+    // bundles the Apache license-header rule that used to come from eslint-config-salesforce-license.
+    expect(readFileSync(path.join(pluginDir, 'eslint.config.mjs'), 'utf8')).to.include(
+      'eslint-config-salesforce-typescript'
+    );
   });
 
   it('should generate a 3PP plugin', async () => {

test/generator.test.ts

@@ -5,21 +5,85 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
+import { normalize } from 'node:path';
 import { expect } from 'chai';
+import spawn from 'cross-spawn';
+import shelljs from 'shelljs';
 import { Generator } from '../src/generator.js';
 
+type SpawnCall = { bin: string; args: readonly string[]; options: { cwd?: string } };
+
+// `spawn.sync` is typed read-only, so mutate through a mutable view of the module.
+const spawnModule = spawn as { sync: typeof spawn.sync };
+
 describe('Generator.execute', () => {
+  let calls: SpawnCall[];
+  const originalSpawnSync = spawn.sync;
+  const originalWhich = shelljs.which;
+
+  beforeEach(() => {
+    calls = [];
+    // Intercept cross-spawn so no real process runs and we can assert on argv.
+    spawnModule.sync = ((bin: string, args: readonly string[], options: { cwd?: string }) => {
+      calls.push({ bin, args, options });
+      return {} as ReturnType<typeof spawn.sync>;
+    }) as typeof spawn.sync;
+  });
+
+  afterEach(() => {
+    spawnModule.sync = originalSpawnSync;
+    shelljs.which = originalWhich;
+  });
+
   it('should throw when binary is not found on PATH', () => {
     const generator = new Generator();
 
     expect(() => generator.execute('nonexistent-binary-xyz --help')).to.throw(
       'Could not find "nonexistent-binary-xyz" on PATH'
     );
+    expect(calls).to.have.length(0);
   });
 
   it('should not execute commands in dry-run mode', () => {
     const generator = new Generator({ dryRun: true });
 
     generator.execute('nonexistent-binary-xyz --help');
+
+    expect(calls).to.have.length(0);
+  });
+
+  it('should pass argv as an array without a shell so metacharacters cannot break out', () => {
+    const generator = new Generator();
+    generator.cwd = '/tmp/project';
+
+    // This is the hijack path from the original vulnerability. `&` must land in
+    // a single argv entry, never as a shell command separator. The bin is an
+    // absolute path, so PATH resolution is bypassed.
+    generator.execute('/usr/bin/git clone https://x.git "/repo/dir&calc.exe&"');
+
+    expect(calls).to.have.length(1);
+    expect(calls[0].bin).to.equal('/usr/bin/git');
+    expect(calls[0].args).to.deep.equal(['clone', 'https://x.git', '/repo/dir&calc.exe&']);
+    // The cwd setter normalizes paths, so compare against the platform-normalized form.
+    expect(calls[0].options).to.include({ cwd: normalize('/tmp/project') });
+  });
+
+  it('should keep a quoted argument containing spaces as a single token', () => {
+    const generator = new Generator();
+
+    generator.execute('/bin/yarn prettier --write "/My Docs/a.ts"');
+
+    expect(calls[0].bin).to.equal('/bin/yarn');
+    expect(calls[0].args).to.deep.equal(['prettier', '--write', '/My Docs/a.ts']);
+  });
+
+  it('should resolve a bare binary name via PATH', () => {
+    shelljs.which = (() => '/resolved/path/to/yarn') as unknown as typeof shelljs.which;
+    const generator = new Generator();
+
+    generator.execute('yarn install');
+
+    expect(calls[0].bin).to.equal('/resolved/path/to/yarn');
+    expect(calls[0].args).to.deep.equal(['install']);
   });
 });

yarn.lock

@@ -2633,6 +2633,13 @@
   dependencies:
     "@types/node" "*"
 
+"@types/cross-spawn@^6.0.6":
+  version "6.0.6"
+  resolved "https://registry.yarnpkg.com/@types/cross-spawn/-/cross-spawn-6.0.6.tgz#0163d0b79a6f85409e0decb8dcca17147f81fd22"
+  integrity sha512-fXRhhUkG4H3TQk5dBhQ7m/JDdSNHKwR2BBia62lhwEIq9xGiQKLxd6LymNhn47SjXhsUEPmxi+PKw2OkW4LLjA==
+  dependencies:
+    "@types/node" "*"
+
 "@types/ejs@^3.1.5":
   version "3.1.5"
   resolved "https://registry.yarnpkg.com/@types/ejs/-/ejs-3.1.5.tgz#49d738257cc73bafe45c13cb8ff240683b4d5117"
@@ -4060,7 +4067,7 @@ create-require@^1.1.0:
   resolved "https://registry.yarnpkg.com/create-require/-/create-require-1.1.1.tgz#c1d7e8f1e5f6cfc9ff65f9cd352d37348756c333"
   integrity sha512-dcKFX3jn0MpIaXjisoRvexIJVEKzaq7z2rZKxf+MSr9TkdmHmsU4m2lcLojrj/FHl8mk5VxMmYA+ftRkP/3oKQ==
 
-cross-spawn@^7.0.0, cross-spawn@^7.0.2, cross-spawn@^7.0.3:
+cross-spawn@^7.0.0, cross-spawn@^7.0.2, cross-spawn@^7.0.3, cross-spawn@^7.0.6:
   version "7.0.6"
   resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-7.0.6.tgz#8a58fe78f00dcd70c370451759dfbfaf03e8ee9f"
   integrity sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==