diff --git a/src/tools.js b/src/tools.js index 941c2103..ab21d22c 100644 --- a/src/tools.js +++ b/src/tools.js @@ -44,6 +44,39 @@ export function getCustom(key, def = null, opts = {}) { return key in process.env ? process.env[key] : key in opts && typeof opts[key] === 'string' ? opts[key] : def } +/** + * Validates that an executable path does not use directory traversal or relative segments. + * @param {string} binPath - The executable path to validate. + * @returns {string} The validated path. + * @throws {Error} If the path contains '..' segments or is a relative path with separators. + */ +function validateExecutablePath(binPath) { + if (typeof binPath !== 'string' || binPath.length === 0) { + throw new Error('Executable path rejected: expected a non-empty string') + } + + if (binPath.startsWith('./') || binPath.startsWith('.\\')) { + throw new Error( + `Executable path rejected: relative paths starting with './' are not allowed: ${binPath}` + ) + } + + const segments = binPath.split(/[/\\]/) + if (segments.includes('..')) { + throw new Error( + `Executable path rejected: path contains directory traversal segment (..): ${binPath}` + ) + } + + if ((binPath.includes('/') || binPath.includes('\\')) && !path.isAbsolute(binPath)) { + throw new Error( + `Executable path rejected: relative paths are not allowed, use an absolute path or a bare command name: ${binPath}` + ) + } + + return binPath +} + /** * Utility function for looking up custom variable for a binary path. * Will look in the environment variables (1) or in opts (2) for a key with TRUSTIFY_DA_x_PATH, x is an @@ -55,7 +88,8 @@ export function getCustom(key, def = null, opts = {}) { * original name supplied */ export function getCustomPath(name, opts = {}) { - return getCustom(`TRUSTIFY_DA_${name.toUpperCase()}_PATH`, name, opts) + const resolvedPath = getCustom(`TRUSTIFY_DA_${name.toUpperCase()}_PATH`, name, opts) + return validateExecutablePath(resolvedPath) } /** diff --git a/test/tools.test.js b/test/tools.test.js index 89d2e8fb..437a02ca 100644 --- a/test/tools.test.js +++ b/test/tools.test.js @@ -2,7 +2,7 @@ import { expect } from 'chai' import esmock from 'esmock' import { afterEach } from 'mocha' -import { getCustom, getCustomPath} from "../src/tools.js" +import { getCustom, getCustomPath } from "../src/tools.js" /** @@ -65,6 +65,111 @@ suite('testing the various tools and utility functions', () => { }) + suite('test getCustomPath executable path validation', () => { + afterEach(() => delete process.env['TRUSTIFY_DA_DUMMY_PATH']) + + /** Verifies that bare command names pass validation (resolved via OS PATH lookup). */ + test('allows bare command names without path separators', () => { + const commands = ['mvn', 'npm', 'go', 'cargo', 'pip3'] + const saved = {} + for (const cmd of commands) { + const envKey = `TRUSTIFY_DA_${cmd.toUpperCase()}_PATH` + if (envKey in process.env) { + saved[envKey] = process.env[envKey] + delete process.env[envKey] + } + } + try { + for (const cmd of commands) { + expect(getCustomPath(cmd)).to.equal(cmd) + } + } finally { + for (const [key, val] of Object.entries(saved)) { + process.env[key] = val + } + } + }) + + /** Verifies that valid absolute paths are accepted. */ + test('allows valid absolute paths', () => { + process.env['TRUSTIFY_DA_DUMMY_PATH'] = '/usr/bin/mvn' + expect(getCustomPath('dummy')).to.equal('/usr/bin/mvn') + + process.env['TRUSTIFY_DA_DUMMY_PATH'] = '/usr/local/bin/npm' + expect(getCustomPath('dummy')).to.equal('/usr/local/bin/npm') + }) + + /** Reproducer: relative path with traversal segments must be rejected. */ + test('rejects relative paths containing ".." traversal segments', () => { + process.env['TRUSTIFY_DA_DUMMY_PATH'] = '../../etc/malicious' + expect(() => getCustomPath('dummy')).to.throw( + Error, 'path contains directory traversal segment (..)' + ) + }) + + /** Verifies that absolute paths with embedded traversal segments are rejected. */ + test('rejects absolute paths containing ".." traversal segments', () => { + process.env['TRUSTIFY_DA_DUMMY_PATH'] = '/usr/bin/../../../tmp/evil' + expect(() => getCustomPath('dummy')).to.throw( + Error, 'path contains directory traversal segment (..)' + ) + }) + + /** Verifies that paths starting with "./" (workspace-relative) are rejected. */ + test('rejects paths starting with "./"', () => { + process.env['TRUSTIFY_DA_DUMMY_PATH'] = './malicious.sh' + expect(() => getCustomPath('dummy')).to.throw( + Error, "relative paths starting with './' are not allowed" + ) + }) + + /** Verifies that traversal paths supplied via opts are also rejected. */ + test('rejects traversal paths provided via opts', () => { + const opts = { 'TRUSTIFY_DA_DUMMY_PATH': '../../tmp/evil' } + expect(() => getCustomPath('dummy', opts)).to.throw( + Error, 'path contains directory traversal segment (..)' + ) + }) + + /** Verifies that a bare '..' without path separators is rejected. */ + test('rejects bare ".." without path separators', () => { + process.env['TRUSTIFY_DA_DUMMY_PATH'] = '..' + expect(() => getCustomPath('dummy')).to.throw( + Error, 'path contains directory traversal segment (..)' + ) + }) + + /** Verifies that relative paths with separators (e.g. subdir/binary) are rejected. */ + test('rejects relative paths with separators', () => { + process.env['TRUSTIFY_DA_DUMMY_PATH'] = 'subdir/binary' + expect(() => getCustomPath('dummy')).to.throw( + Error, 'relative paths are not allowed, use an absolute path or a bare command name' + ) + }) + + /** Verifies that rejected paths include the offending path in the error message. */ + test('error message includes the rejected path', () => { + process.env['TRUSTIFY_DA_DUMMY_PATH'] = '../../sneaky/script' + expect(() => getCustomPath('dummy')).to.throw('../../sneaky/script') + }) + }) + + suite('test resolveBinary wrapper path regression', () => { + /** Verifies that resolveBinary with a wrapper path bypasses getCustomPath validation. */ + test('wrapper path from traverseForWrapper is not subject to path validation', async () => { + // Given: a mocked traverseForWrapper that returns a workspace-relative wrapper path + const tools = await esmock('../src/tools.js', {}, { + 'node:fs': { + accessSync: () => undefined + } + }) + + // When: resolveBinary finds a wrapper, it returns it directly without validation + const result = tools.resolveBinary('mvn', 'mvnw', '/workspace/project') + expect(result).to.equal('/workspace/project/mvnw') + }) + }) + suite('test the handleSpacesInPath utility function', () => { test('Windows Path with spaces', async () => {