diff --git a/src/providers/python_controller.js b/src/providers/python_controller.js index 6bfcb90f..13798049 100644 --- a/src/providers/python_controller.js +++ b/src/providers/python_controller.js @@ -101,25 +101,28 @@ export default class Python_controller { prepareEnvironment() { if(!this.realEnvironment) { - this.pythonEnvDir = path.join(path.sep, "tmp", "trustify_da_env_js") + const isWindows = os.platform() === 'win32' + // Windows hosts keep the venv in the OS-configured temp directory so it is + // writable without elevation and independent of the current drive. POSIX + // hosts keep the existing fixed /tmp location. + // Match path semantics to the detected platform so Windows paths stay + // drive-aware even when the Windows branch runs on another host (tests). + const venvPath = isWindows ? path.win32 : path.posix + this.pythonEnvDir = isWindows + ? venvPath.join(os.tmpdir(), "trustify_da_env_js") + : venvPath.join("/", "tmp", "trustify_da_env_js") try { invokeCommand(this.pathToPythonBin, ['-m', 'venv', this.pythonEnvDir]) } catch (error) { throw new Error('Failed creating virtual python environment', {cause: error}) } - const isWindows = os.platform() === 'win32' // venv always installs the unversioned `python` and `pip` names regardless of // how the source interpreter is named: `Scripts\python.exe`/`Scripts\pip.exe` // on Windows and `bin/python`/`bin/pip` on POSIX. A `python3` source must not // be mapped to the nonexistent `Scripts\python3.exe` (TC-7485). const envBinDir = isWindows ? "Scripts" : "bin" - this.pathToPipBin = path.join(path.sep, this.pythonEnvDir, envBinDir, this.#decideIfWindowsOrLinuxPath("pip")) - this.pathToPythonBin = path.join(path.sep, this.pythonEnvDir, envBinDir, this.#decideIfWindowsOrLinuxPath("python")) - if(isWindows) { - let driveLetter = path.parse(process.cwd()).root - this.pathToPythonBin = `${driveLetter}${this.pathToPythonBin.substring(1)}` - this.pathToPipBin = `${driveLetter}${this.pathToPipBin.substring(1)}` - } + this.pathToPipBin = venvPath.join(this.pythonEnvDir, envBinDir, this.#decideIfWindowsOrLinuxPath("pip")) + this.pathToPythonBin = venvPath.join(this.pythonEnvDir, envBinDir, this.#decideIfWindowsOrLinuxPath("python")) // upgrade pip version to latest try { invokeCommand(this.pathToPythonBin, ['-m', 'pip', 'install', '--upgrade', 'pip']) diff --git a/src/tools.js b/src/tools.js index 4909f9ba..f7368015 100644 --- a/src/tools.js +++ b/src/tools.js @@ -1,6 +1,7 @@ import { execFileSync } from "child_process"; import fs from 'node:fs' import path from 'node:path' +import { fileURLToPath } from 'node:url' import { EOL } from "os"; import { HttpsProxyAgent } from "https-proxy-agent"; @@ -236,14 +237,23 @@ export function resolveBinary(globalBinary, localWrapper, startDir, opts = {}) { * @returns {string} */ export function invokeCommand(bin, args, opts={}) { - // .bat and .cmd files can't be executed in windows with execFileSync without {shell: true}, so we - // special case them here to keep the amount of escaping we need to do to a minimum. - // https://nodejs.org/docs/latest-v20.x/api/child_process.html#spawning-bat-and-cmd-files-on-windows - // https://github.com/nodejs/node/issues/52681#issuecomment-2076426887 if (process.platform === 'win32') { - opts = {...opts, shell: true} - args = args.map(arg => handleSpacesInPath(arg)) - bin = handleSpacesInPath(bin) + bin = resolveWindowsCommand(bin, opts.cwd) + // Native executables take literal argv values; never pre-quote them or enable a shell. + opts = { ...opts, shell: false, windowsVerbatimArguments: false } + if (/\.(?:bat|cmd)$/i.test(bin)) { + // Batch files require cmd.exe. Escape its command string ourselves instead of + // passing an unescaped argument array with shell:true (Node does not escape it). + // npm .bin shims reparse arguments, requiring a second layer of caret escaping. + const isShim = /node_modules[\\/]\.bin[\\/][^\\/]+\.cmd$/i.test(bin) + const command = [ + escapeWindowsShellToken(path.win32.normalize(bin)), + ...args.map(arg => escapeWindowsShellToken(arg, true, isShim)) + ].join(' ') + bin = process.env.ComSpec || 'cmd.exe' + args = ['/d', '/v:off', '/s', '/c', `"${command}"`] + opts = { ...opts, windowsVerbatimArguments: true } + } } opts = { @@ -264,6 +274,55 @@ export function invokeCommand(bin, args, opts={}) { return execFileSync(bin, args, {...{stdio: 'pipe', encoding: 'utf-8'}, ...opts}) } +/** Resolves Windows commands before classifying PATH batch wrappers and local npm shims. */ +function resolveWindowsCommand(bin, cwd) { + const hasExtension = /\.(?:exe|com|bat|cmd)$/i.test(bin) + if (/\.(?:exe|com)$/i.test(bin) || (hasExtension && path.win32.isAbsolute(bin))) { + return bin + } + cwd = cwd instanceof URL ? fileURLToPath(cwd) : cwd || process.cwd() + const directories = /[\\/]/.test(bin) ? [''] : ['', ...(process.env.PATH || '').split(';')] + const extensions = hasExtension + ? [''] + : (process.env.PATHEXT || '.COM;.EXE;.BAT;.CMD').split(';').filter(Boolean) + for (const directory of directories) { + for (const extension of extensions) { + const candidate = path.win32.resolve(cwd, directory.replace(/^"|"$/g, ''), bin + extension) + if (fs.statSync(candidate, { throwIfNoEntry: false })?.isFile()) { + return candidate + } + } + } + return bin +} + +/** Quotes Windows argv values and escapes cmd syntax, including percent and bang expansion. */ +function escapeWindowsShellToken(value, argument = false, doubleEscape = false) { + if (/[\r\n]/.test(value)) { + throw new Error('Windows batch command inputs must not contain line breaks') + } + if (argument) { + // Windows argv quoting: double backslashes before quotes and the closing quote. + // A single pass avoids regex backtracking on long runs of backslashes. + let quoted = '"' + let backslashes = 0 + for (const character of value) { + if (character === '\\') { + backslashes++ + continue + } + quoted += '\\'.repeat(character === '"' ? backslashes * 2 + 1 : backslashes) + character + backslashes = 0 + } + value = quoted + '\\'.repeat(backslashes * 2) + '"' + } + // cmd metacharacters must be escaped even inside quotes: quoting alone permits expansion. + // This is the cmd token-escaping strategy also used by cross-spawn. + const metacharacters = /([()\][%!^"`<>&|;, *?])/g + value = value.replace(metacharacters, '^$1') + return doubleEscape ? value.replace(metacharacters, '^$1') : value +} + export const TRUSTIFY_DA_TOKEN_HEADER = "trust-da-token"; export const TRUSTIFY_DA_TELEMETRY_ID_HEADER = "telemetry-anonymous-id"; diff --git a/test/providers/python_controller.test.js b/test/providers/python_controller.test.js index 77529c40..8cd9b6f2 100644 --- a/test/providers/python_controller.test.js +++ b/test/providers/python_controller.test.js @@ -15,15 +15,16 @@ import { const REQUIREMENTS_MANIFEST = 'test/providers/tst_manifests/pip/pip_requirements_txt_no_ignore/requirements.txt' /** - * Loads Python_controller with os.platform() and command invocation stubbed, then - * constructs a virtual-environment controller so the venv setup invocations can be - * inspected. Stubbing the parser keeps the test independent of the tree-sitter wasm - * artifacts. + * Loads Python_controller with os.platform(), os.tmpdir() and command invocation + * stubbed, then constructs a virtual-environment controller so the venv setup + * invocations can be inspected. Stubbing the parser keeps the test independent of + * the tree-sitter wasm artifacts. * @param {string} platform - platform reported by os.platform() * @param {string} sourcePython - configured source python binary + * @param {string} tempDir - directory reported by os.tmpdir() * @return {Promise<{controller: Python_controller, invocations: Array<{bin: string, args: string[]}>}>} */ -async function constructVirtualEnvController(platform, sourcePython) { +async function constructVirtualEnvController(platform, sourcePython, tempDir = os.tmpdir()) { const invocations = [] /** Records a command invocation instead of executing it. */ const recordInvocation = (bin, args) => { @@ -31,7 +32,7 @@ async function constructVirtualEnvController(platform, sourcePython) { return '' } const { default: MockedPythonController } = await esmock('../../src/providers/python_controller.js', { - os: { default: { platform: () => platform }, EOL: '\n' }, + os: { default: { platform: () => platform, tmpdir: () => tempDir }, EOL: '\n' }, '../../src/tools.js': { environmentVariableIsPopulated: () => false, getCustom: () => undefined, @@ -118,12 +119,15 @@ suite('Python_controller BEST_EFFORTS validation', function() { }) suite('Python_controller virtual environment executables', function() { + const windowsTempDir = 'C:\\Users\\da-test\\AppData\\Local\\Temp' + const windowsVenvDir = path.win32.join(windowsTempDir, 'trustify_da_env_js') + // Windows venv executable names must not depend on how the source interpreter is named (TC-7485). for (const sourcePython of ['python3', 'python']) { /** Verifies that a Windows venv created from `sourcePython` upgrades pip via Scripts/python.exe. */ test(`upgrades pip via Scripts/python.exe on Windows for a ${sourcePython} source`, async function() { - // Given a Windows host and a configured source interpreter named sourcePython - const { controller, invocations } = await constructVirtualEnvController('win32', sourcePython) + // Given a Windows host with a configured temp directory and a source interpreter named sourcePython + const { controller, invocations } = await constructVirtualEnvController('win32', sourcePython, windowsTempDir) // When the venv is prepared while the controller is constructed const venvInvocation = invocations[0] @@ -132,27 +136,18 @@ suite('Python_controller virtual environment executables', function() { // Then the venv is created from the configured source interpreter expect(invocations).to.have.lengthOf(2) expect(venvInvocation.bin).to.equal(sourcePython) - expect(venvInvocation.args).to.deep.equal(['-m', 'venv', path.join(path.sep, 'tmp', 'trustify_da_env_js')]) - - // Then the executable paths are anchored to the venv directory that was created. - // Resolving the recorded target makes this match the controller's drive-prefixed - // Windows paths on a native host as well as under the mocked win32 platform. - const expectedVenvDir = path.resolve(venvInvocation.args[2]) - expect(path.resolve(controller.pythonEnvDir)).to.equal(expectedVenvDir) - const expectedPythonBin = path.join(expectedVenvDir, 'Scripts', 'python.exe') - const expectedPipBin = path.join(expectedVenvDir, 'Scripts', 'pip.exe') // Then the pip upgrade runs through that venv's Scripts/python.exe exactly + const expectedPythonBin = path.win32.join(windowsVenvDir, 'Scripts', 'python.exe') expect(pipUpgradeInvocation.args).to.deep.equal(['-m', 'pip', 'install', '--upgrade', 'pip']) expect(pipUpgradeInvocation.bin).to.equal(expectedPythonBin) expect(controller.pathToPythonBin).to.equal(expectedPythonBin) // Then the venv pip path is exactly that venv's Scripts/pip.exe - expect(controller.pathToPipBin).to.equal(expectedPipBin) + expect(controller.pathToPipBin).to.equal(path.win32.join(windowsVenvDir, 'Scripts', 'pip.exe')) // Then no invocation targets the nonexistent Scripts/python3.exe expect(path.basename(pipUpgradeInvocation.bin)).to.not.equal('python3.exe') - expect(invocations.map(invocation => path.basename(invocation.bin))).to.not.include('python3.exe') expect(invocations.map(invocation => path.basename(invocation.bin))).to.not.include('pip3.exe') }) } @@ -162,8 +157,8 @@ suite('Python_controller virtual environment executables', function() { for (const sourcePython of ['python3', 'python']) { /** Verifies that POSIX venvs expose bin/python and bin/pip regardless of the source interpreter name. */ test(`uses bin/python and bin/pip on non-Windows hosts for a ${sourcePython} source`, async function() { - // Given a POSIX host and a configured source interpreter named sourcePython - const { controller, invocations } = await constructVirtualEnvController('linux', sourcePython) + // Given a POSIX host whose configured temp directory is not /tmp + const { controller, invocations } = await constructVirtualEnvController('linux', sourcePython, '/var/tmp') // When the venv is prepared while the controller is constructed const venvInvocation = invocations[0] @@ -172,18 +167,45 @@ suite('Python_controller virtual environment executables', function() { // Then the venv is created from the configured source interpreter expect(invocations).to.have.lengthOf(2) expect(venvInvocation.bin).to.equal(sourcePython) - expect(venvInvocation.args[2]).to.equal(path.join(path.sep, 'tmp', 'trustify_da_env_js')) - // Then the pip upgrade runs through the venv bin/python exactly + // Then the venv still uses the fixed /tmp location with bin/python and bin/pip + expect(venvInvocation.args[2]).to.equal('/tmp/trustify_da_env_js') expect(pipUpgradeInvocation.args).to.deep.equal(['-m', 'pip', 'install', '--upgrade', 'pip']) - expect(pipUpgradeInvocation.bin).to.equal(path.join(venvInvocation.args[2], 'bin', 'python')) - expect(controller.pathToPythonBin).to.equal(path.join(venvInvocation.args[2], 'bin', 'python')) - - // Then the venv pip path is bin/pip - expect(controller.pathToPipBin).to.equal(path.join(venvInvocation.args[2], 'bin', 'pip')) + expect(pipUpgradeInvocation.bin).to.equal('/tmp/trustify_da_env_js/bin/python') + expect(controller.pathToPythonBin).to.equal('/tmp/trustify_da_env_js/bin/python') + expect(controller.pathToPipBin).to.equal('/tmp/trustify_da_env_js/bin/pip') }) } }) + +suite('Python_controller virtual environment placement', function() { + /** + * Verifies that Windows venvs are created and upgraded under the OS-configured temp + * directory instead of a root-relative \tmp path on the current drive. + */ + test('creates and upgrades the Windows venv under the configured temp directory', async function() { + // Given Windows configured D:\Temp on the D: drive + const tempDir = 'D:\\Temp' + const { controller, invocations } = await constructVirtualEnvController('win32', 'python', tempDir) + + // When the virtual environment is prepared on controller construction + const venvInvocation = invocations[0] + const pipUpgradeInvocation = invocations[1] + const expectedVenvDir = path.win32.join(tempDir, 'trustify_da_env_js') + + // Then the venv is created in the configured temp directory, on its drive + expect(venvInvocation.args).to.deep.equal(['-m', 'venv', expectedVenvDir]) + expect(controller.pythonEnvDir).to.equal(expectedVenvDir) + expect(path.win32.parse(controller.pythonEnvDir).root).to.equal('D:\\') + + // Then the old current-drive-root \tmp location is not used + expect(controller.pythonEnvDir).to.not.equal('C:\\tmp\\trustify_da_env_js') + + // Then the pip upgrade runs from the same venv's Scripts directory + expect(pipUpgradeInvocation.bin).to.equal(path.win32.join(expectedVenvDir, 'Scripts', 'python.exe')) + expect(controller.pathToPipBin).to.equal(path.win32.join(expectedVenvDir, 'Scripts', 'pip.exe')) + }) +}) suite('Python_controller requirements file decoding', () => { const requirementsText = 'blinker==1.9.0\r\ncertifi==2026.10.23\r\n' let tempDir diff --git a/test/tools.test.js b/test/tools.test.js index 8d4fd376..b63551b0 100644 --- a/test/tools.test.js +++ b/test/tools.test.js @@ -1,6 +1,7 @@ import { expect } from 'chai' import esmock from 'esmock' import { afterEach } from 'mocha' +import { createSandbox } from 'sinon' import { getCustom, getCustomPath } from "../src/tools.js" @@ -19,6 +20,35 @@ async function mockToolsPartial(operatingSystem) { ) } +/** Runs the real command boundary with a simulated host and intercepted subprocess/filesystem. */ +async function interceptCommand(platform, run, files = []) { + const invocations = [] + const tools = await esmock('../src/tools.js', { + child_process: { + execFileSync: (bin, args, opts) => { + invocations.push({ bin, args, opts }) + return 'command output' + } + }, + 'node:fs': { + statSync: file => files.includes(file) ? { isFile: () => true } : undefined + } + }) + const sandbox = createSandbox() + sandbox.stub(process, 'platform').value(platform) + sandbox.stub(process, 'env').value({ + ...process.env, + PATH: 'C:\\Tools', + PATHEXT: '.COM;.EXE;.BAT;.CMD', + ComSpec: 'C:\\Windows\\System32\\cmd.exe' + }) + try { + await run(tools, invocations) + } finally { + sandbox.restore() + } +} + suite('testing the various tools and utility functions', () => { suite('test the getCustom utility function', () => { afterEach(() => delete process.env['DUMMY_KEY']) @@ -190,5 +220,131 @@ suite('testing the various tools and utility functions', () => { }) + suite('invokeCommand subprocess boundary', () => { + for (const tempDir of ['D:\\Temp', 'D:\\Temp Folder', 'D:\\Temp&Untrusted']) { + /** Verifies venv targets and interpreters reach native execution as literal, complete paths. */ + test(`keeps native Windows paths literal under ${tempDir}`, async () => { + await interceptCommand('win32', (tools, invocations) => { + // Given a target and interpreter on a different drive from the working directory + const target = `${tempDir}\\trustify_da_env_js` + const interpreter = `${target}\\Scripts\\python.exe` + const opts = { cwd: 'C:\\workspace', timeout: 5000 } + // When creating the venv and upgrading pip + tools.invokeCommand('python.exe', ['-m', 'venv', target], opts) + const result = tools.invokeCommand(interpreter, ['-m', 'pip', 'install', '--upgrade', 'pip'], opts) + // Then neither call exposes the paths to a shell or pre-quotes the native arguments + expect(invocations.map(({ bin, args, opts }) => ({ bin, args, shell: opts.shell }))) + .to.deep.equal([ + { bin: 'python.exe', args: ['-m', 'venv', target], shell: false }, + { bin: interpreter, args: ['-m', 'pip', 'install', '--upgrade', 'pip'], shell: false } + ]) + expect(invocations[1].opts).to.include({ cwd: 'C:\\workspace', timeout: 5000, maxBuffer: 10 * 1024 * 1024 }) + expect(opts).to.deep.equal({ cwd: 'C:\\workspace', timeout: 5000 }) + expect(result).to.equal('command output') + }) + }) + } + + for (const extension of ['exe', 'com']) { + /** Verifies caller options cannot accidentally restore shell interpretation of native paths. */ + test(`disables shell and verbatim argument options for Windows .${extension}`, async () => { + await interceptCommand('win32', (tools, invocations) => { + // Given shell and verbatim options that would bypass native argument protection + const bin = `D:\\Tools & More\\python.${extension}` + // When invoking a native executable + tools.invokeCommand(bin, ['%TEMP%', '!value!', 'a"b', 'a&b'], { shell: true, windowsVerbatimArguments: true }) + // Then Node retains responsibility for quoting individual native arguments + expect(invocations[0]).to.include({ bin }) + expect(invocations[0].args).to.deep.equal(['%TEMP%', '!value!', 'a"b', 'a&b']) + expect(invocations[0].opts).to.include({ shell: false, windowsVerbatimArguments: false }) + }) + }) + } + + for (const extension of ['bat', 'CMD']) { + /** Verifies batch wrappers receive cmd-escaped command and argument tokens, including expansion syntax. */ + test(`escapes Windows .${extension} wrapper inputs`, async () => { + await interceptCommand('win32', (tools, invocations) => { + // Given a configured wrapper and data containing cmd metacharacters + const bin = `D:\\Tools & More\\wrapper.${extension}` + // When invoking the wrapper (never executed by this test) + tools.invokeCommand(bin, ['D:\\Temp&Untrusted', '%TEMP%', '!value!', 'a"b', 'tail\\', '']) + // Then cmd autorun/delayed expansion are disabled and all tokens are escaped + expect(invocations[0].bin).to.equal('C:\\Windows\\System32\\cmd.exe') + expect(invocations[0].args).to.deep.equal([ + '/d', '/v:off', '/s', '/c', + `"D:\\Tools^ ^&^ More\\wrapper.${extension} ^"D:\\Temp^&Untrusted^" ^"^%TEMP^%^" ^"^!value^!^" ^"a\\^"b^" ^"tail\\\\^" ^"^""` + ]) + expect(invocations[0].opts).to.include({ shell: false, windowsVerbatimArguments: true }) + }) + }) + } + + for (const [name, file, expectedBin, expectedArgs] of [ + ['python', 'C:\\Tools\\python.EXE', 'C:\\Tools\\python.EXE', ['--version']], + ['npm', 'C:\\Tools\\npm.CMD', 'C:\\Windows\\System32\\cmd.exe', ['/d', '/v:off', '/s', '/c', '"C:\\Tools\\npm.CMD ^"--version^""']], + ['D:\\project\\node_modules\\.bin\\npm', 'D:\\project\\node_modules\\.bin\\npm.CMD', 'C:\\Windows\\System32\\cmd.exe', ['/d', '/v:off', '/s', '/c', '"D:\\project\\node_modules\\.bin\\npm.CMD ^^^"--version^^^""']] + ]) { + /** Verifies extensionless native commands and batch shims retain Windows command lookup support. */ + test(`resolves Windows command ${name}`, async () => { + await interceptCommand('win32', (tools, invocations) => { + // When resolving an extensionless command through PATH/PATHEXT + tools.invokeCommand(name, ['--version'], { cwd: 'C:\\workspace' }) + // Then the selected file determines native versus escaped batch invocation + expect(invocations[0].bin).to.equal(expectedBin) + expect(invocations[0].args).to.deep.equal(expectedArgs) + expect(invocations[0].opts.shell).to.equal(false) + }, [file]) + }) + } + + /** Verifies explicit .cmd names get the same shim reparse protection as extensionless names. */ + test('resolves PATH shims before escaping explicit .cmd commands', async () => { + const shim = 'D:\\project\\node_modules\\.bin\\npm.cmd' + await interceptCommand('win32', (tools, invocations) => { + // Given both names resolve to the same shim, with an exact-extension decoy present + process.env.PATH = 'D:\\project\\node_modules\\.bin' + process.env.PATHEXT = '.exe;.cmd' + const args = ['D:\\Temp&Untrusted', 'a"&b', '%TEMP%', '!value!'] + // When invoking both the extensionless and explicit-extension names + tools.invokeCommand('npm', args) + tools.invokeCommand('npm.cmd', args) + // Then the shim arguments survive both parsing passes, without selecting npm.cmd.exe + const expectedArgs = ['/d', '/v:off', '/s', '/c', + '"D:\\project\\node_modules\\.bin\\npm.cmd ^^^"D:\\Temp^^^&Untrusted^^^" ^^^"a\\^^^"^^^&b^^^" ^^^"^^^%TEMP^^^%^^^" ^^^"^^^!value^^^!^^^""'] + expect(invocations.map(({ bin, args }) => ({ bin, args }))).to.deep.equal([ + { bin: 'C:\\Windows\\System32\\cmd.exe', args: expectedArgs }, + { bin: 'C:\\Windows\\System32\\cmd.exe', args: expectedArgs } + ]) + }, [shim, `${shim}.exe`]) + }) + + /** Verifies batch arguments cannot introduce command lines that caret escaping does not protect. */ + test('rejects line breaks before invoking a Windows wrapper', async () => { + await interceptCommand('win32', (tools, invocations) => { + for (const value of ['safe\ncommand', 'safe\rcommand']) { + expect(() => tools.invokeCommand('mvn.cmd', [value])).to.throw('line breaks') + } + expect(invocations).to.deep.equal([]) + }) + }) + + /** Verifies POSIX command inputs and explicit caller options pass through unchanged. */ + test('preserves non-Windows invocation behavior', async () => { + await interceptCommand('linux', (tools, invocations) => { + // Given POSIX paths and literal arguments + const bin = '/tools with spaces/python' + const args = ['-m', 'venv', '/tmp/a&b', '$HOME'] + // When invoking the non-Windows command + tools.invokeCommand(bin, args, { cwd: '/workspace', shell: false }) + // Then no Windows resolution, quoting or shell flags are introduced + expect(invocations[0].bin).to.equal(bin) + expect(invocations[0].args).to.deep.equal(args) + expect(invocations[0].opts).to.include({ cwd: '/workspace', shell: false }) + expect(invocations[0].opts).not.to.have.property('windowsVerbatimArguments') + }) + }) + }) + })