From b86ad486d19a0bd9ddca8cdfef6a0b04d2949f6b Mon Sep 17 00:00:00 2001 From: Adva Oren Date: Tue, 4 Aug 2026 14:51:22 +0300 Subject: [PATCH 1/6] fix(tools): add executable path validation to prevent directory traversal Adds validateExecutablePath() to reject user-configured executable paths containing '..' traversal segments or './' relative prefixes before they reach execFileSync(). Bare command names and absolute paths are allowed. Wrapper paths from traverseForWrapper() are unaffected. Implements TC-5485 Assisted-by: Claude Code --- src/tools.js | 30 +++++++++++++++++- test/tools.test.js | 78 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 106 insertions(+), 2 deletions(-) diff --git a/src/tools.js b/src/tools.js index 941c2103..9a2070f3 100644 --- a/src/tools.js +++ b/src/tools.js @@ -44,6 +44,33 @@ 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 starts with './'. + */ +function validateExecutablePath(binPath) { + if (!binPath.includes('/') && !binPath.includes('\\')) { + return binPath + } + + 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}` + ) + } + + 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 +82,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..c6440bc9 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,82 @@ 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'] + for (const cmd of commands) { + expect(getCustomPath(cmd)).to.equal(cmd) + } + }) + + /** 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') + delete process.env['TRUSTIFY_DA_DUMMY_PATH'] + + 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 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 () => { From db61fcfc5737d138f3b2483aed15558bc02f605a Mon Sep 17 00:00:00 2001 From: Adva Oren Date: Tue, 4 Aug 2026 15:17:51 +0300 Subject: [PATCH 2/6] fix(test): save and restore env vars in bare command name test The CI runner has TRUSTIFY_DA_PIP3_PATH set, causing getCustomPath('pip3') to return the env var value instead of the bare name. Save and restore any matching env vars during the test to isolate from the CI environment. Implements TC-5485 Assisted-by: Claude Code --- test/tools.test.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/tools.test.js b/test/tools.test.js index c6440bc9..c65c2e69 100644 --- a/test/tools.test.js +++ b/test/tools.test.js @@ -71,8 +71,22 @@ suite('testing the various tools and utility functions', () => { /** 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) { - expect(getCustomPath(cmd)).to.equal(cmd) + 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 + } } }) From 105360048435d83e3d18b5ca6ddebbe33c04dc77 Mon Sep 17 00:00:00 2001 From: Adva Oren Date: Tue, 11 Aug 2026 13:59:54 +0300 Subject: [PATCH 3/6] fix(tools): close bare `..` bypass in validateExecutablePath Remove the early-return optimization that short-circuited validation for inputs without path separators. `..` contains no `/` or `\` and was incorrectly treated as a safe bare command name, skipping the segments.includes('..') check entirely. Without the early return, '..'.split(/[/\\]/) correctly produces ['..'] which the existing segment check catches. Implements TC-5619 Assisted-by: Claude Code --- src/tools.js | 4 ---- test/tools.test.js | 8 ++++++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/tools.js b/src/tools.js index 9a2070f3..745ca3dd 100644 --- a/src/tools.js +++ b/src/tools.js @@ -51,10 +51,6 @@ export function getCustom(key, def = null, opts = {}) { * @throws {Error} If the path contains '..' segments or starts with './'. */ function validateExecutablePath(binPath) { - if (!binPath.includes('/') && !binPath.includes('\\')) { - return binPath - } - if (binPath.startsWith('./') || binPath.startsWith('.\\')) { throw new Error( `Executable path rejected: relative paths starting with './' are not allowed: ${binPath}` diff --git a/test/tools.test.js b/test/tools.test.js index c65c2e69..394783b7 100644 --- a/test/tools.test.js +++ b/test/tools.test.js @@ -132,6 +132,14 @@ suite('testing the various tools and utility functions', () => { ) }) + /** 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 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' From bce64481a742297ea0ea32b1c87c066d31782750 Mon Sep 17 00:00:00 2001 From: Adva Oren Date: Tue, 11 Aug 2026 14:37:43 +0300 Subject: [PATCH 4/6] fix(tools): reject relative paths with separators in executable validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tighten validateExecutablePath to reject any path that contains a separator but is not absolute (e.g. subdir/binary, bin/mvn). Custom executable paths must be either a bare command name resolved via PATH or an explicit absolute path — relative paths with directory components could resolve to workspace-internal files. Implements TC-5619 Assisted-by: Claude Code --- src/tools.js | 8 +++++++- test/tools.test.js | 8 ++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/tools.js b/src/tools.js index 745ca3dd..0a8a178f 100644 --- a/src/tools.js +++ b/src/tools.js @@ -48,7 +48,7 @@ export function getCustom(key, def = null, opts = {}) { * 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 starts with './'. + * @throws {Error} If the path contains '..' segments or is a relative path with separators. */ function validateExecutablePath(binPath) { if (binPath.startsWith('./') || binPath.startsWith('.\\')) { @@ -64,6 +64,12 @@ function validateExecutablePath(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 } diff --git a/test/tools.test.js b/test/tools.test.js index 394783b7..dd0ad5f1 100644 --- a/test/tools.test.js +++ b/test/tools.test.js @@ -140,6 +140,14 @@ suite('testing the various tools and utility functions', () => { ) }) + /** 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' From e5b43f0c4ea94f12948b8271e413fd75ee1768ea Mon Sep 17 00:00:00 2001 From: Adva Oren Date: Tue, 11 Aug 2026 14:39:50 +0300 Subject: [PATCH 5/6] fix(test): remove redundant env var delete between assertions The next line immediately overwrites the env var, and afterEach handles cleanup. Addresses reviewer nit. Implements TC-5619 Assisted-by: Claude Code --- test/tools.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/tools.test.js b/test/tools.test.js index dd0ad5f1..437a02ca 100644 --- a/test/tools.test.js +++ b/test/tools.test.js @@ -94,7 +94,6 @@ suite('testing the various tools and utility functions', () => { test('allows valid absolute paths', () => { process.env['TRUSTIFY_DA_DUMMY_PATH'] = '/usr/bin/mvn' expect(getCustomPath('dummy')).to.equal('/usr/bin/mvn') - delete process.env['TRUSTIFY_DA_DUMMY_PATH'] process.env['TRUSTIFY_DA_DUMMY_PATH'] = '/usr/local/bin/npm' expect(getCustomPath('dummy')).to.equal('/usr/local/bin/npm') From 286d1c37d2eac62e9edb4fa095343191c594912a Mon Sep 17 00:00:00 2001 From: Adva Oren Date: Tue, 11 Aug 2026 14:44:53 +0300 Subject: [PATCH 6/6] fix(tools): add type guard for non-string inputs in path validation Add a typeof check at the top of validateExecutablePath to throw a descriptive error instead of a TypeError if null, undefined, or a non-string value is passed. Not reachable from current callers but hardens the function against future call-site changes. Implements TC-5619 Assisted-by: Claude Code --- src/tools.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tools.js b/src/tools.js index 0a8a178f..ab21d22c 100644 --- a/src/tools.js +++ b/src/tools.js @@ -51,6 +51,10 @@ export function getCustom(key, def = null, opts = {}) { * @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}`