diff --git a/src/mcp/tools/automationTools.ts b/src/mcp/tools/automationTools.ts index 3433334d..b631c760 100644 --- a/src/mcp/tools/automationTools.ts +++ b/src/mcp/tools/automationTools.ts @@ -16,187 +16,10 @@ import { log } from '../logging/logger.js'; import type { ServerConfig } from '../server.js'; import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js'; import { parseJUnitResults } from './antTools.js'; -import { sfSpawnHelper, SfNotFoundError } from './sfSpawn.js'; +import { runSfCommand } from './sfSpawn.js'; -// ── SF CLI discovery ────────────────────────────────────────────────────────── - -/** - * Returns candidate sf CLI paths in common npm/nvm/volta install locations. - * Used as a fallback when `sf` is not in PATH. - */ -export function getSfCommonPaths(): string[] { - const home = os.homedir(); - if (process.platform === 'win32') { - const appData = process.env['APPDATA'] ?? path.join(home, 'AppData', 'Roaming'); - return [ - path.join(appData, 'npm', 'sf.cmd'), - path.join('C:', 'Program Files', 'nodejs', 'sf.cmd'), - path.join('C:', 'Program Files (x86)', 'nodejs', 'sf.cmd'), - ]; - } - const candidates = [ - '/usr/local/bin/sf', - path.join(home, '.npm-global', 'bin', 'sf'), - path.join(home, '.local', 'bin', 'sf'), - path.join(home, '.volta', 'bin', 'sf'), - ]; - // nvm — scan the three most-recently installed Node versions - const nvmBinDir = path.join(process.env['NVM_DIR'] ?? path.join(home, '.nvm'), 'versions', 'node'); - if (fs.existsSync(nvmBinDir)) { - try { - for (const v of fs.readdirSync(nvmBinDir).sort().reverse().slice(0, 3)) { - candidates.push(path.join(nvmBinDir, v, 'bin', 'sf')); - } - } catch { - /* skip */ - } - } - return candidates; -} - -// ── Shared spawn helper ─────────────────────────────────────────────────────── - -const MAX_BUFFER = 50 * 1024 * 1024; // 50 MB — prevents ENOBUFS on verbose Provar runs - -interface SpawnResult { - stdout: string; - stderr: string; - exitCode: number; -} - -// Proactively resolve the sf executable path once on first use and cache it. -// This ensures sf is always found even when ENOENT is masked by other errors (e.g. ENOBUFS). -let cachedSfPath: string | null | undefined; // undefined = not yet probed - -/** - * Exposed for testing only — pre-seeds the cached sf executable path, bypassing the probe spawn. - * Pass `undefined` to reset the cache so the next call triggers a fresh probe. - */ -export function setSfPathCacheForTesting(value: string | null | undefined): void { - cachedSfPath = value; -} - -// Platform override used in tests so Windows-specific shell logic can be exercised on any OS. -let sfPlatformOverride: NodeJS.Platform | undefined; -/** Exposed for testing only — overrides process.platform for needsWindowsShell decisions. */ -export function setSfPlatformForTesting(platform: NodeJS.Platform | undefined): void { - sfPlatformOverride = platform; -} - -/** - * Returns true when spawning `executable` requires the Windows shell. - * On Windows, `.cmd` and `.bat` batch scripts cannot be executed directly by - * Node's spawnSync — they must be invoked through cmd.exe (i.e. shell: true). - * The bare name "sf" also needs this treatment on Windows because the file on - * disk is actually "sf.cmd" and Node won't auto-append the extension. - * - * The `platform` parameter defaults to `process.platform` and is exposed for - * unit testing so tests can verify both Windows and non-Windows behaviour - * without having to run on the corresponding OS. - */ -export function needsWindowsShell(executable: string, platform = process.platform): boolean { - if (platform !== 'win32') return false; - const lower = executable.toLowerCase(); - return lower.endsWith('.cmd') || lower.endsWith('.bat') || !path.extname(lower); -} - -function resolveSfExecutable(): string | null { - if (cachedSfPath !== undefined) return cachedSfPath; - const platform = sfPlatformOverride ?? process.platform; - - // Two-phase probe avoids false-positives on Windows with shell:true. - // When shell:true is used, cmd.exe spawns successfully even when `sf` is - // missing — it exits non-zero with "not recognised" in stderr but sets no - // probe.error. Trying shell:false first catches both cases correctly. - // - // First attempt: shell:false (works on Linux/macOS; gives ENOENT on Windows if - // sf.cmd is on PATH but requires the shell). - const probe = sfSpawnHelper.spawnSync('sf', ['--version'], { - encoding: 'utf-8', - shell: false, - maxBuffer: 1024 * 1024, - }); - if (!probe.error && probe.status === 0) { - cachedSfPath = 'sf'; - return cachedSfPath; - } - - // Windows fallback: retry with shell:true when the plain probe failed - // with ENOENT — meaning sf.cmd exists on PATH but can't run without the shell. - if (platform === 'win32' && (probe.error as NodeJS.ErrnoException | undefined)?.code === 'ENOENT') { - const probeShell = sfSpawnHelper.spawnSync('sf', ['--version'], { - encoding: 'utf-8', - shell: true, - maxBuffer: 1024 * 1024, - }); - if (!probeShell.error && probeShell.status === 0) { - cachedSfPath = 'sf'; - return cachedSfPath; - } - } - - // Fall back to common install locations - for (const candidate of getSfCommonPaths()) { - if (fs.existsSync(candidate)) { - cachedSfPath = candidate; - return cachedSfPath; - } - } - cachedSfPath = null; - return null; -} - -/** - * Reject shell metacharacters in an sf_path that will be executed via shell:true. - * On Windows, cmd.exe interprets & | ; < > ` ' " and newlines as shell syntax. - * A valid filesystem path should never contain these characters. - */ -function assertShellSafePath(sfPath: string): void { - if (/[&|;<>`'"\n\r]/.test(sfPath)) { - throw Object.assign( - new Error( - 'sf_path contains characters that are unsafe for shell execution on Windows ' + - '(& | ; < > ` \' " or line-breaks). Provide an absolute filesystem path to the sf executable.' - ), - { code: 'INVALID_SF_PATH' } - ); - } -} - -function runSfCommand(args: string[], sfPath?: string): SpawnResult { - // Use explicit path if provided; otherwise use cached probe result - const executable = sfPath ?? resolveSfExecutable(); - if (!executable) throw new SfNotFoundError(); - - const platform = sfPlatformOverride ?? process.platform; - const useShell = needsWindowsShell(executable, platform); - - // Guard against injection when shell:true is used with a user-supplied path. - // Common install locations returned by resolveSfExecutable() are safe by construction. - if (useShell && sfPath) { - assertShellSafePath(sfPath); - } - - const result = sfSpawnHelper.spawnSync(executable, args, { - encoding: 'utf-8', - shell: useShell, - maxBuffer: MAX_BUFFER, - }); - - if (result.error) { - const err = result.error as NodeJS.ErrnoException; - if (err.code === 'ENOENT') { - throw new SfNotFoundError(sfPath); - } - throw result.error; - } - - return { - stdout: result.stdout ?? '', - stderr: result.stderr ?? '', - exitCode: result.status ?? 1, - }; -} +// Re-export sf resolution helpers so existing test imports from automationTools continue to work +export { getSfCommonPaths, needsWindowsShell, setSfPathCacheForTesting, setSfPlatformForTesting } from './sfSpawn.js'; function handleSpawnError( err: unknown, diff --git a/src/mcp/tools/defectTools.ts b/src/mcp/tools/defectTools.ts index c408f01f..81ca6a70 100644 --- a/src/mcp/tools/defectTools.ts +++ b/src/mcp/tools/defectTools.ts @@ -45,8 +45,8 @@ interface FailureContext { // ── SF CLI helpers ───────────────────────────────────────────────────────────── -function runSfArgs(args: string[]): { stdout: string; stderr: string; exitCode: number } { - const { stdout, stderr, exitCode } = runSfCommand(args); +function runSfArgs(args: string[], sfPath?: string): { stdout: string; stderr: string; exitCode: number } { + const { stdout, stderr, exitCode } = runSfCommand(args, sfPath); return { stdout, stderr, exitCode }; } @@ -57,35 +57,22 @@ function formatSfCommandError(action: string, exitCode: number, stderr: string, : `${action} failed with exit code ${exitCode}`; } -function runQuery(soql: string, targetOrg: string): SfQueryResponse { - const { stdout, stderr, exitCode } = runSfArgs([ - 'data', - 'query', - '--query', - soql, - '--target-org', - targetOrg, - '--json', - ]); +function runQuery(soql: string, targetOrg: string, sfPath?: string): SfQueryResponse { + const { stdout, stderr, exitCode } = runSfArgs( + ['data', 'query', '--query', soql, '--target-org', targetOrg, '--json'], + sfPath + ); if (exitCode !== 0) { throw new Error(formatSfCommandError('Salesforce query', exitCode, stderr, stdout)); } return JSON.parse(stdout) as SfQueryResponse; } -function createRecord(sobject: string, values: string, targetOrg: string): string { - const { stdout, stderr, exitCode } = runSfArgs([ - 'data', - 'create', - 'record', - '--sobject', - sobject, - '--values', - values, - '--target-org', - targetOrg, - '--json', - ]); +function createRecord(sobject: string, values: string, targetOrg: string, sfPath?: string): string { + const { stdout, stderr, exitCode } = runSfArgs( + ['data', 'create', 'record', '--sobject', sobject, '--values', values, '--target-org', targetOrg, '--json'], + sfPath + ); if (exitCode !== 0) { throw new Error(formatSfCommandError(`Failed to create ${sobject}`, exitCode, stderr, stdout)); } @@ -115,12 +102,14 @@ export interface DefectCreateResult { export function createDefectsForRun( runId: string, targetOrg: string, - failedTestFilter?: string[] + failedTestFilter?: string[], + sfPath?: string ): { created: DefectCreateResult[]; skipped: number; message: string } { // Step 1: resolve job record ID from tracking ID const jobQuery = runQuery( `SELECT Id FROM provar__Test_Plan_Schedule_Job__c WHERE provar__Tracking_Id__c = '${soqlEscape(runId)}'`, - targetOrg + targetOrg, + sfPath ); if (jobQuery.result.totalSize === 0) { throw new Error(`No Test_Plan_Schedule_Job__c found with Tracking_Id__c = '${runId}'`); @@ -133,7 +122,8 @@ export function createDefectsForRun( FROM provar__Test_Cycle__c WHERE provar__Test_Plan_Schedule_Job__c = '${soqlEscape(jobId)}' LIMIT 1`, - targetOrg + targetOrg, + sfPath ); const cycle = cycleQuery.result.records[0] ?? {}; const browser = safeText(cycle['provar__Web_Browser__c'], 100); @@ -151,7 +141,8 @@ export function createDefectsForRun( FROM provar__Test_Execution__c WHERE provar__Test_Cycle__c = '${soqlEscape(cycleId)}' AND provar__Status__c = 'Failed'`, - targetOrg + targetOrg, + sfPath ); if (execQuery.result.totalSize === 0) { @@ -185,7 +176,8 @@ export function createDefectsForRun( AND provar__Result__c = 'Fail' ORDER BY provar__Sequence_No__c ASC LIMIT 1`, - targetOrg + targetOrg, + sfPath ); const step = stepQuery.result.records[0] ?? {}; @@ -227,7 +219,7 @@ export function createDefectsForRun( `provar__Description__c="${safeText(descLines, 2000)}" ` + 'provar__Status__c="Open"'; - const defectId = createRecord('provar__Defect__c', defectValues, targetOrg); + const defectId = createRecord('provar__Defect__c', defectValues, targetOrg, sfPath); // Step 5b: link TC → Defect const tcDefectValues = @@ -235,7 +227,7 @@ export function createDefectsForRun( `provar__Test_Case__c="${testCaseId}"` + (stepId ? ` provar__Test_Step__c="${stepId}"` : ''); - const tcDefectId = createRecord('provar__Test_Case_Defect__c', tcDefectValues, targetOrg); + const tcDefectId = createRecord('provar__Test_Case_Defect__c', tcDefectValues, targetOrg, sfPath); // Step 5c: link Execution → Defect (with step execution if available) const execDefectValues = @@ -243,7 +235,7 @@ export function createDefectsForRun( `provar__Test_Execution__c="${executionId}"` + (stepExecutionId ? ` provar__Test_Step_Execution__c="${stepExecutionId}"` : ''); - const execDefectId = createRecord('provar__Test_Execution_Defect__c', execDefectValues, targetOrg); + const execDefectId = createRecord('provar__Test_Execution_Defect__c', execDefectValues, targetOrg, sfPath); log('info', 'defect created', { defectId, tcDefectId, execDefectId, executionId }); @@ -281,14 +273,22 @@ export function registerQualityHubDefectCreate(server: McpServer): void { .describe( 'Optional filter — list of Test_Case__c record ID substrings to restrict defect creation to specific failures' ), + sf_path: z + .string() + .optional() + .describe( + 'Path to the sf CLI executable when not in PATH ' + + '(e.g. "C:\\\\Program Files\\\\sf\\\\bin\\\\sf.cmd" for the Windows standalone installer). ' + + 'Leave unset to use auto-discovery.' + ), }, }, - ({ run_id, target_org, failed_tests }) => { + ({ run_id, target_org, failed_tests, sf_path }) => { const requestId = makeRequestId(); log('info', 'provar_qualityhub_defect_create', { requestId, run_id, target_org }); try { - const result = createDefectsForRun(run_id, target_org, failed_tests); + const result = createDefectsForRun(run_id, target_org, failed_tests, sf_path); const response = { requestId, ...result }; return { content: [{ type: 'text' as const, text: JSON.stringify(response) }], diff --git a/src/mcp/tools/qualityHubTools.ts b/src/mcp/tools/qualityHubTools.ts index 74d037da..aa464af6 100644 --- a/src/mcp/tools/qualityHubTools.ts +++ b/src/mcp/tools/qualityHubTools.ts @@ -46,14 +46,25 @@ export function registerQualityHubConnect(server: McpServer): void { .optional() .default([]) .describe('Additional raw CLI flags to forward (e.g. ["--json"])'), + sf_path: z + .string() + .optional() + .describe( + 'Path to the sf CLI executable when not in PATH ' + + '(e.g. "C:\\\\Program Files\\\\sf\\\\bin\\\\sf.cmd" for the Windows standalone installer). ' + + 'Leave unset to use auto-discovery.' + ), }, }, - ({ target_org, flags }) => { + ({ target_org, flags, sf_path }) => { const requestId = makeRequestId(); log('info', 'provar_qualityhub_connect', { requestId, target_org }); try { - const result = runSfCommand(['provar', 'quality-hub', 'connect', '--target-org', target_org, ...flags]); + const result = runSfCommand( + ['provar', 'quality-hub', 'connect', '--target-org', target_org, ...flags], + sf_path + ); const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { @@ -87,9 +98,17 @@ export function registerQualityHubDisplay(server: McpServer): void { inputSchema: { target_org: z.string().optional().describe('SF org alias or username (uses default if omitted)'), flags: z.array(z.string()).optional().default([]).describe('Additional raw CLI flags to forward'), + sf_path: z + .string() + .optional() + .describe( + 'Path to the sf CLI executable when not in PATH ' + + '(e.g. "C:\\\\Program Files\\\\sf\\\\bin\\\\sf.cmd" for the Windows standalone installer). ' + + 'Leave unset to use auto-discovery.' + ), }, }, - ({ target_org, flags }) => { + ({ target_org, flags, sf_path }) => { const requestId = makeRequestId(); log('info', 'provar_qualityhub_display', { requestId, target_org }); @@ -97,7 +116,7 @@ export function registerQualityHubDisplay(server: McpServer): void { const args = ['provar', 'quality-hub', 'display', ...flags]; if (target_org) args.splice(3, 0, '--target-org', target_org); - const result = runSfCommand(args); + const result = runSfCommand(args, sf_path); const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { @@ -155,15 +174,26 @@ export function registerQualityHubTestRun(server: McpServer): void { .describe( 'Additional raw CLI flags (e.g. ["--plan-name", "SmokeTests"]). Avoid wildcards in --plan-name values — they skip QH plan-level reporting.' ), + sf_path: z + .string() + .optional() + .describe( + 'Path to the sf CLI executable when not in PATH ' + + '(e.g. "C:\\\\Program Files\\\\sf\\\\bin\\\\sf.cmd" for the Windows standalone installer). ' + + 'Leave unset to use auto-discovery.' + ), }, }, - ({ target_org, flags }) => { + ({ target_org, flags, sf_path }) => { const requestId = makeRequestId(); log('info', 'provar_qualityhub_testrun', { requestId, target_org }); try { const wildcardWarning = detectWildcardFlags(flags); - const result = runSfCommand(['provar', 'quality-hub', 'test', 'run', '--target-org', target_org, ...flags]); + const result = runSfCommand( + ['provar', 'quality-hub', 'test', 'run', '--target-org', target_org, ...flags], + sf_path + ); const response: Record = { requestId, exitCode: result.exitCode, @@ -207,25 +237,25 @@ export function registerQualityHubTestRunReport(server: McpServer): void { target_org: z.string().describe('SF org alias or username'), run_id: z.string().describe('Test run ID returned by provar_qualityhub_testrun'), flags: z.array(z.string()).optional().default([]).describe('Additional raw CLI flags'), + sf_path: z + .string() + .optional() + .describe( + 'Path to the sf CLI executable when not in PATH ' + + '(e.g. "C:\\\\Program Files\\\\sf\\\\bin\\\\sf.cmd" for the Windows standalone installer). ' + + 'Leave unset to use auto-discovery.' + ), }, }, - ({ target_org, run_id, flags }) => { + ({ target_org, run_id, flags, sf_path }) => { const requestId = makeRequestId(); log('info', 'provar_qualityhub_testrun_report', { requestId, target_org, run_id }); try { - const result = runSfCommand([ - 'provar', - 'quality-hub', - 'test', - 'run', - 'report', - '--target-org', - target_org, - '--run-id', - run_id, - ...flags, - ]); + const result = runSfCommand( + ['provar', 'quality-hub', 'test', 'run', 'report', '--target-org', target_org, '--run-id', run_id, ...flags], + sf_path + ); const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { @@ -279,25 +309,25 @@ export function registerQualityHubTestRunAbort(server: McpServer): void { target_org: z.string().describe('SF org alias or username'), run_id: z.string().describe('Test run ID to abort'), flags: z.array(z.string()).optional().default([]).describe('Additional raw CLI flags'), + sf_path: z + .string() + .optional() + .describe( + 'Path to the sf CLI executable when not in PATH ' + + '(e.g. "C:\\\\Program Files\\\\sf\\\\bin\\\\sf.cmd" for the Windows standalone installer). ' + + 'Leave unset to use auto-discovery.' + ), }, }, - ({ target_org, run_id, flags }) => { + ({ target_org, run_id, flags, sf_path }) => { const requestId = makeRequestId(); log('info', 'provar_qualityhub_testrun_abort', { requestId, target_org, run_id }); try { - const result = runSfCommand([ - 'provar', - 'quality-hub', - 'test', - 'run', - 'abort', - '--target-org', - target_org, - '--run-id', - run_id, - ...flags, - ]); + const result = runSfCommand( + ['provar', 'quality-hub', 'test', 'run', 'abort', '--target-org', target_org, '--run-id', run_id, ...flags], + sf_path + ); const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { @@ -336,22 +366,25 @@ export function registerQualityHubTestcaseRetrieve(server: McpServer): void { .optional() .default([]) .describe('Additional raw CLI flags (e.g. ["--user-story", "US-123"])'), + sf_path: z + .string() + .optional() + .describe( + 'Path to the sf CLI executable when not in PATH ' + + '(e.g. "C:\\\\Program Files\\\\sf\\\\bin\\\\sf.cmd" for the Windows standalone installer). ' + + 'Leave unset to use auto-discovery.' + ), }, }, - ({ target_org, flags }) => { + ({ target_org, flags, sf_path }) => { const requestId = makeRequestId(); log('info', 'provar_qualityhub_testcase_retrieve', { requestId, target_org }); try { - const result = runSfCommand([ - 'provar', - 'quality-hub', - 'testcase', - 'retrieve', - '--target-org', - target_org, - ...flags, - ]); + const result = runSfCommand( + ['provar', 'quality-hub', 'testcase', 'retrieve', '--target-org', target_org, ...flags], + sf_path + ); const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { diff --git a/src/mcp/tools/sfSpawn.ts b/src/mcp/tools/sfSpawn.ts index 5b7fd3c3..f103385b 100644 --- a/src/mcp/tools/sfSpawn.ts +++ b/src/mcp/tools/sfSpawn.ts @@ -5,6 +5,9 @@ * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import { spawnSync as _spawnSync } from 'node:child_process'; /** @@ -20,20 +23,18 @@ export const sfSpawnHelper = { export class SfNotFoundError extends Error { public readonly code = 'SF_NOT_FOUND'; public constructor(sfPath?: string) { - const where = sfPath - ? `at explicit path "${sfPath}"` - : 'in PATH or common npm/nvm/volta install locations'; + const where = sfPath ? `at explicit path "${sfPath}"` : 'in PATH or common npm/nvm/volta install locations'; super( `sf CLI not found ${where}. ` + - 'Install Salesforce CLI (npm install -g @salesforce/cli) and ensure the install directory is in your PATH, ' + - 'or pass sf_path pointing to the sf executable directly ' + - '(e.g. "~/.nvm/versions/node/v22.0.0/bin/sf").' + 'Install Salesforce CLI (npm install -g @salesforce/cli) and ensure the install directory is in your PATH, ' + + 'or pass sf_path pointing to the sf executable directly ' + + '(e.g. "~/.nvm/versions/node/v22.0.0/bin/sf").' ); this.name = 'SfNotFoundError'; } } -// ── Shared spawn helper ─────────────────────────────────────────────────────── +// ── Shared result type ──────────────────────────────────────────────────────── export interface SpawnResult { stdout: string; @@ -41,17 +42,175 @@ export interface SpawnResult { exitCode: number; } +const MAX_BUFFER = 50 * 1024 * 1024; // 50 MB — prevents ENOBUFS on verbose Provar runs + +// ── SF CLI discovery ────────────────────────────────────────────────────────── + +/** + * Returns candidate sf CLI paths in common install locations. + * Used as a fallback when `sf` is not in PATH. + */ +export function getSfCommonPaths(): string[] { + const home = os.homedir(); + if (process.platform === 'win32') { + const appData = process.env['APPDATA'] ?? path.join(home, 'AppData', 'Roaming'); + return [ + path.join(appData, 'npm', 'sf.cmd'), + path.join('C:', 'Program Files', 'nodejs', 'sf.cmd'), + path.join('C:', 'Program Files (x86)', 'nodejs', 'sf.cmd'), + // Windows standalone installer (https://developer.salesforce.com/tools/salesforcecli) + path.join('C:', 'Program Files', 'sf', 'bin', 'sf.cmd'), + path.join('C:', 'Program Files', 'sf', 'client', 'bin', 'sf.cmd'), + ]; + } + const candidates = [ + '/usr/local/bin/sf', + path.join(home, '.npm-global', 'bin', 'sf'), + path.join(home, '.local', 'bin', 'sf'), + path.join(home, '.volta', 'bin', 'sf'), + ]; + // nvm — scan the three most-recently installed Node versions + const nvmBinDir = path.join(process.env['NVM_DIR'] ?? path.join(home, '.nvm'), 'versions', 'node'); + if (fs.existsSync(nvmBinDir)) { + try { + for (const v of fs.readdirSync(nvmBinDir).sort().reverse().slice(0, 3)) { + candidates.push(path.join(nvmBinDir, v, 'bin', 'sf')); + } + } catch { + /* skip */ + } + } + return candidates; +} + +// Proactively resolve the sf executable path once on first use and cache it. +// This ensures sf is always found even when ENOENT is masked by other errors (e.g. ENOBUFS). +let cachedSfPath: string | null | undefined; // undefined = not yet probed + +/** + * Exposed for testing only — pre-seeds the cached sf executable path, bypassing the probe spawn. + * Pass `undefined` to reset the cache so the next call triggers a fresh probe. + */ +export function setSfPathCacheForTesting(value: string | null | undefined): void { + cachedSfPath = value; +} + +// Platform override used in tests so Windows-specific shell logic can be exercised on any OS. +let sfPlatformOverride: NodeJS.Platform | undefined; + +/** Exposed for testing only — overrides process.platform for needsWindowsShell decisions. */ +export function setSfPlatformForTesting(platform: NodeJS.Platform | undefined): void { + sfPlatformOverride = platform; +} + +/** + * Returns true when spawning `executable` requires the Windows shell. + * On Windows, `.cmd` and `.bat` batch scripts cannot be executed directly by + * Node's spawnSync — they must be invoked through cmd.exe (i.e. shell: true). + * The bare name "sf" also needs this treatment on Windows because the file on + * disk is actually "sf.cmd" and Node won't auto-append the extension. + */ +export function needsWindowsShell(executable: string, platform = process.platform): boolean { + if (platform !== 'win32') return false; + const lower = executable.toLowerCase(); + return lower.endsWith('.cmd') || lower.endsWith('.bat') || !path.extname(lower); +} + +function resolveSfExecutable(): string | null { + if (cachedSfPath !== undefined) return cachedSfPath; + const platform = sfPlatformOverride ?? process.platform; + + // Two-phase probe avoids false-positives on Windows with shell:true. + // When shell:true is used, cmd.exe spawns successfully even when `sf` is + // missing — it exits non-zero with "not recognised" in stderr but sets no + // probe.error. Trying shell:false first catches both cases correctly. + // + // First attempt: shell:false (works on Linux/macOS; gives ENOENT on Windows if + // sf.cmd is on PATH but requires the shell). + const probe = sfSpawnHelper.spawnSync('sf', ['--version'], { + encoding: 'utf-8', + shell: false, + maxBuffer: 1024 * 1024, + }); + if (!probe.error && probe.status === 0) { + cachedSfPath = 'sf'; + return cachedSfPath; + } + + // Windows fallback: retry with shell:true when the plain probe failed + // with ENOENT — meaning sf.cmd exists on PATH but can't run without the shell. + if (platform === 'win32' && (probe.error as NodeJS.ErrnoException | undefined)?.code === 'ENOENT') { + const probeShell = sfSpawnHelper.spawnSync('sf', ['--version'], { + encoding: 'utf-8', + shell: true, + maxBuffer: 1024 * 1024, + }); + if (!probeShell.error && probeShell.status === 0) { + cachedSfPath = 'sf'; + return cachedSfPath; + } + } + + // Fall back to common install locations + for (const candidate of getSfCommonPaths()) { + if (fs.existsSync(candidate)) { + cachedSfPath = candidate; + return cachedSfPath; + } + } + cachedSfPath = null; + return null; +} + +/** + * Reject shell metacharacters in an sf_path that will be executed via shell:true. + * On Windows, cmd.exe interprets & | ; < > ` ' " and newlines as shell syntax. + * A valid filesystem path should never contain these characters. + */ +function assertShellSafePath(sfPath: string): void { + if (/[&|;<>`'"\n\r]/.test(sfPath)) { + throw Object.assign( + new Error( + 'sf_path contains characters that are unsafe for shell execution on Windows ' + + '(& | ; < > ` \' " or line-breaks). Provide an absolute filesystem path to the sf executable.' + ), + { code: 'INVALID_SF_PATH' } + ); + } +} + /** * Run `sf ` synchronously and return stdout, stderr, and exit code. - * Throws SfNotFoundError if the `sf` binary is not in PATH. + * Throws SfNotFoundError if the `sf` binary cannot be found. + * Pass `sfPath` to override auto-discovery with an explicit executable path. */ -export function runSfCommand(args: string[]): SpawnResult { - const result = sfSpawnHelper.spawnSync('sf', args, { encoding: 'utf-8', shell: false }); +export function runSfCommand(args: string[], sfPath?: string): SpawnResult { + // Treat empty/whitespace sfPath as absent so auto-discovery runs instead of + // throwing SfNotFoundError with no useful path hint. + const trimmedSfPath = sfPath?.trim(); + const resolvedSfPath = trimmedSfPath !== '' ? trimmedSfPath : undefined; + const executable = resolvedSfPath ?? resolveSfExecutable(); + if (!executable) throw new SfNotFoundError(); + + const platform = sfPlatformOverride ?? process.platform; + const useShell = needsWindowsShell(executable, platform); + + // Guard against injection when shell:true is used with a user-supplied path. + // Common install locations returned by resolveSfExecutable() are safe by construction. + if (useShell && resolvedSfPath) { + assertShellSafePath(resolvedSfPath); + } + + const result = sfSpawnHelper.spawnSync(executable, args, { + encoding: 'utf-8', + shell: useShell, + maxBuffer: MAX_BUFFER, + }); if (result.error) { const err = result.error as NodeJS.ErrnoException; if (err.code === 'ENOENT') { - throw new SfNotFoundError(); + throw new SfNotFoundError(resolvedSfPath); } throw result.error; } diff --git a/test/unit/mcp/automationTools.test.ts b/test/unit/mcp/automationTools.test.ts index 381ec475..2a9c3c30 100644 --- a/test/unit/mcp/automationTools.test.ts +++ b/test/unit/mcp/automationTools.test.ts @@ -16,6 +16,7 @@ import { sfSpawnHelper } from '../../../src/mcp/tools/sfSpawn.js'; import { needsWindowsShell, setSfPlatformForTesting, + getSfCommonPaths, filterTestRunOutput, setSfResultsPathForTesting, } from '../../../src/mcp/tools/automationTools.js'; @@ -928,3 +929,31 @@ describe('filterTestRunOutput', () => { assert.ok(filtered.includes('INFO Done'), 'Real output should remain'); }); }); + +// ── getSfCommonPaths — B2a Windows standalone installer paths ───────────────── + +describe('getSfCommonPaths', () => { + it('includes Windows standalone installer paths on win32 (B2a fix)', () => { + const origPlatform = Object.getOwnPropertyDescriptor(process, 'platform'); + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + try { + const paths = getSfCommonPaths(); + const sfBinPath = paths.find( + (p) => p.includes('Program Files') && p.includes('sf') && p.includes('bin') && !p.includes('client') + ); + const sfClientPath = paths.find((p) => p.includes('Program Files') && p.includes('sf') && p.includes('client')); + assert.ok(sfBinPath, 'Expected C:\\Program Files\\sf\\bin\\sf.cmd in win32 paths'); + assert.ok(sfClientPath, 'Expected C:\\Program Files\\sf\\client\\bin\\sf.cmd in win32 paths'); + } finally { + if (origPlatform) Object.defineProperty(process, 'platform', origPlatform); + } + }); + + it('does not include Windows standalone paths on non-Windows platforms', () => { + if (process.platform !== 'win32') { + const paths = getSfCommonPaths(); + assert.ok(!paths.some((p) => p.includes('Program Files')), 'Windows paths should not appear on non-Windows'); + assert.ok(paths.includes('/usr/local/bin/sf'), 'Linux/macOS fallback path should be present'); + } + }); +}); diff --git a/test/unit/mcp/qualityHubTools.test.ts b/test/unit/mcp/qualityHubTools.test.ts index c7e8dc2a..11274d2f 100644 --- a/test/unit/mcp/qualityHubTools.test.ts +++ b/test/unit/mcp/qualityHubTools.test.ts @@ -7,9 +7,10 @@ /* eslint-disable camelcase */ import { strict as assert } from 'node:assert'; +import path from 'node:path'; import sinon from 'sinon'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; -import { sfSpawnHelper } from '../../../src/mcp/tools/sfSpawn.js'; +import { sfSpawnHelper, getSfCommonPaths, setSfPathCacheForTesting } from '../../../src/mcp/tools/sfSpawn.js'; // ── Minimal mock server ─────────────────────────────────────────────────────── @@ -74,6 +75,8 @@ describe('qualityHubTools', () => { beforeEach(async () => { server = new MockMcpServer(); spawnStub = sinon.stub(sfSpawnHelper, 'spawnSync'); + // Pre-seed the sf path cache to bypass the probe spawn — tests control spawnStub directly + setSfPathCacheForTesting('sf'); const { registerAllQualityHubTools } = await import('../../../src/mcp/tools/qualityHubTools.js'); registerAllQualityHubTools(server as unknown as McpServer); @@ -81,6 +84,7 @@ describe('qualityHubTools', () => { afterEach(() => { sinon.restore(); + setSfPathCacheForTesting(undefined); // reset cache so next test probes cleanly }); // ── provar_qualityhub_connect ─────────────────────────────────────────────── @@ -391,4 +395,73 @@ describe('qualityHubTools', () => { assert.equal(parseBody(result).error_code, 'SF_NOT_FOUND'); }); }); + + // ── sf_path threading ───────────────────────────────────────────────────────── + + describe('sf_path threading', () => { + it('provar_qualityhub_connect uses explicit sf_path as the executable', () => { + spawnStub.returns(makeSpawnResult('ok', '', 0)); + server.call('provar_qualityhub_connect', { + target_org: 'myorg', + flags: [], + sf_path: '/custom/bin/sf', + }); + const [cmd] = spawnStub.firstCall.args as [string, string[]]; + assert.equal(cmd, '/custom/bin/sf'); + }); + + it('provar_qualityhub_display uses explicit sf_path as the executable', () => { + spawnStub.returns(makeSpawnResult('ok', '', 0)); + server.call('provar_qualityhub_display', { flags: [], sf_path: '/custom/bin/sf' }); + const [cmd] = spawnStub.firstCall.args as [string, string[]]; + assert.equal(cmd, '/custom/bin/sf'); + }); + + it('returns SF_NOT_FOUND with path hint when explicit sf_path gives ENOENT', () => { + spawnStub.returns(makeEnoentResult()); + const result = server.call('provar_qualityhub_connect', { + target_org: 'myorg', + flags: [], + sf_path: '/nonexistent/sf', + }); + assert.ok(isError(result)); + const body = parseBody(result); + assert.equal(body.error_code, 'SF_NOT_FOUND'); + assert.ok((body.message as string).includes('/nonexistent/sf'), 'message should include the bad path'); + }); + + it('returns SF_NOT_FOUND when no sf found and cache is null', () => { + setSfPathCacheForTesting(null); // simulate: probe already ran, nothing found + const result = server.call('provar_qualityhub_connect', { target_org: 'myorg', flags: [] }); + assert.ok(isError(result)); + assert.equal(parseBody(result).error_code, 'SF_NOT_FOUND'); + }); + }); + + // ── getSfCommonPaths — B2a Windows standalone installer paths ───────────────── + + describe('getSfCommonPaths', () => { + it('includes Windows standalone installer paths on win32', () => { + const origPlatform = Object.getOwnPropertyDescriptor(process, 'platform'); + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + try { + const paths = getSfCommonPaths(); + const expectedBin = path.join('C:', 'Program Files', 'sf', 'bin', 'sf.cmd'); + const expectedClientBin = path.join('C:', 'Program Files', 'sf', 'client', 'bin', 'sf.cmd'); + assert.ok(paths.includes(expectedBin), `Expected ${expectedBin} in common paths`); + assert.ok(paths.includes(expectedClientBin), `Expected ${expectedClientBin} in common paths`); + } finally { + if (origPlatform) Object.defineProperty(process, 'platform', origPlatform); + } + }); + + it('returns non-empty list on non-Windows platforms', () => { + // On the current test platform (Linux/macOS), paths should include /usr/local/bin/sf + if (process.platform !== 'win32') { + const paths = getSfCommonPaths(); + assert.ok(paths.length > 0); + assert.ok(paths.includes('/usr/local/bin/sf')); + } + }); + }); }); diff --git a/test/unit/mcp/sfSpawn.test.ts b/test/unit/mcp/sfSpawn.test.ts new file mode 100644 index 00000000..60317464 --- /dev/null +++ b/test/unit/mcp/sfSpawn.test.ts @@ -0,0 +1,357 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +import { strict as assert } from 'node:assert'; +import os from 'node:os'; +import path from 'node:path'; +import sinon from 'sinon'; +import { + sfSpawnHelper, + SfNotFoundError, + getSfCommonPaths, + needsWindowsShell, + runSfCommand, + setSfPathCacheForTesting, + setSfPlatformForTesting, + soqlEscape, +} from '../../../src/mcp/tools/sfSpawn.js'; + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +function makeSpawnOk( + stdout = '', + stderr = '' +): { stdout: string; stderr: string; status: number; error: undefined; pid: number; output: never[]; signal: null } { + return { stdout, stderr, status: 0, error: undefined, pid: 1, output: [], signal: null }; +} + +function makeSpawnFail( + exitCode = 1, + stdout = '', + stderr = '' +): { stdout: string; stderr: string; status: number; error: undefined; pid: number; output: never[]; signal: null } { + return { stdout, stderr, status: exitCode, error: undefined, pid: 1, output: [], signal: null }; +} + +function makeEnoent(): { + stdout: string; + stderr: string; + status: null; + error: Error & { code: string }; + pid: undefined; + output: never[]; + signal: null; +} { + return { + stdout: '', + stderr: '', + status: null, + error: Object.assign(new Error('spawn sf ENOENT'), { code: 'ENOENT' }), + pid: undefined, + output: [], + signal: null, + }; +} + +// ── SfNotFoundError ─────────────────────────────────────────────────────────── + +describe('SfNotFoundError', () => { + it('has code SF_NOT_FOUND', () => { + const err = new SfNotFoundError(); + assert.equal(err.code, 'SF_NOT_FOUND'); + assert.equal(err.name, 'SfNotFoundError'); + }); + + it('generic message mentions PATH and npm install hint', () => { + const err = new SfNotFoundError(); + assert.ok(err.message.includes('npm install -g @salesforce/cli')); + assert.ok(err.message.includes('PATH')); + }); + + it('path-specific message names the explicit path', () => { + const err = new SfNotFoundError('/custom/sf'); + assert.ok(err.message.includes('/custom/sf')); + assert.ok(err.message.includes('at explicit path')); + }); +}); + +// ── soqlEscape ──────────────────────────────────────────────────────────────── + +describe('soqlEscape', () => { + it('leaves strings without quotes unchanged', () => { + assert.equal(soqlEscape('hello world'), 'hello world'); + }); + + it('escapes single quotes', () => { + assert.equal(soqlEscape("O'Brien"), "O\\'Brien"); + }); + + it('escapes multiple quotes', () => { + assert.equal(soqlEscape("it's a 'test'"), "it\\'s a \\'test\\'"); + }); + + it('handles empty string', () => { + assert.equal(soqlEscape(''), ''); + }); +}); + +// ── getSfCommonPaths ────────────────────────────────────────────────────────── + +describe('getSfCommonPaths', () => { + it('returns an array of strings', () => { + const paths = getSfCommonPaths(); + assert.ok(Array.isArray(paths)); + assert.ok(paths.length > 0); + for (const p of paths) assert.equal(typeof p, 'string'); + }); + + it('includes Windows standalone installer paths on win32', () => { + const originalDescriptor = Object.getOwnPropertyDescriptor(process, 'platform'); + try { + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + const paths = getSfCommonPaths(); + assert.ok( + paths.some((p) => p.includes(path.join('Program Files', 'sf', 'bin'))), + 'should include C:\\Program Files\\sf\\bin\\sf.cmd' + ); + assert.ok( + paths.some((p) => p.includes(path.join('Program Files', 'sf', 'client', 'bin'))), + 'should include C:\\Program Files\\sf\\client\\bin\\sf.cmd' + ); + } finally { + if (originalDescriptor) Object.defineProperty(process, 'platform', originalDescriptor); + } + }); + + it('all Windows paths end with .cmd', () => { + const originalDescriptor = Object.getOwnPropertyDescriptor(process, 'platform'); + try { + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + const paths = getSfCommonPaths(); + for (const p of paths) assert.ok(p.endsWith('.cmd'), `expected .cmd: ${p}`); + } finally { + if (originalDescriptor) Object.defineProperty(process, 'platform', originalDescriptor); + } + }); + + it('includes home-relative paths on non-Windows', () => { + const originalDescriptor = Object.getOwnPropertyDescriptor(process, 'platform'); + try { + Object.defineProperty(process, 'platform', { value: 'linux', configurable: true }); + const paths = getSfCommonPaths(); + assert.ok(paths.includes('/usr/local/bin/sf')); + assert.ok(paths.some((p) => p.includes(os.homedir()))); + } finally { + if (originalDescriptor) Object.defineProperty(process, 'platform', originalDescriptor); + } + }); +}); + +// ── needsWindowsShell ───────────────────────────────────────────────────────── + +describe('needsWindowsShell', () => { + it('returns false on non-Windows regardless of executable', () => { + assert.equal(needsWindowsShell('sf', 'linux'), false); + assert.equal(needsWindowsShell('sf.cmd', 'darwin'), false); + assert.equal(needsWindowsShell('/usr/bin/sf', 'linux'), false); + }); + + it('returns true for .cmd on win32', () => { + assert.equal(needsWindowsShell('sf.cmd', 'win32'), true); + assert.equal(needsWindowsShell('C:\\npm\\sf.cmd', 'win32'), true); + }); + + it('returns true for .bat on win32', () => { + assert.equal(needsWindowsShell('sf.bat', 'win32'), true); + }); + + it('returns true for bare name (no extension) on win32', () => { + assert.equal(needsWindowsShell('sf', 'win32'), true); + }); + + it('returns false for .exe on win32', () => { + assert.equal(needsWindowsShell('sf.exe', 'win32'), false); + }); + + it('is case-insensitive for extension check', () => { + assert.equal(needsWindowsShell('SF.CMD', 'win32'), true); + assert.equal(needsWindowsShell('SF.BAT', 'win32'), true); + }); +}); + +// ── runSfCommand ────────────────────────────────────────────────────────────── + +describe('runSfCommand', () => { + let spawnStub: sinon.SinonStub; + + beforeEach(() => { + spawnStub = sinon.stub(sfSpawnHelper, 'spawnSync'); + setSfPathCacheForTesting('sf'); + setSfPlatformForTesting('linux'); + }); + + afterEach(() => { + sinon.restore(); + setSfPathCacheForTesting(undefined); + setSfPlatformForTesting(undefined); + }); + + it('returns stdout, stderr, and exitCode on success', () => { + spawnStub.returns(makeSpawnOk('output', 'warn')); + const result = runSfCommand(['data', 'query']); + assert.equal(result.stdout, 'output'); + assert.equal(result.stderr, 'warn'); + assert.equal(result.exitCode, 0); + }); + + it('passes args array to spawnSync', () => { + spawnStub.returns(makeSpawnOk()); + runSfCommand(['provar', 'quality-hub', 'connect', '--target-org', 'myorg']); + const [cmd, args] = spawnStub.firstCall.args as [string, string[]]; + assert.equal(cmd, 'sf'); + assert.deepEqual(args, ['provar', 'quality-hub', 'connect', '--target-org', 'myorg']); + }); + + it('uses explicit sfPath instead of cached path', () => { + spawnStub.returns(makeSpawnOk()); + runSfCommand(['--version'], '/custom/path/sf'); + const [cmd] = spawnStub.firstCall.args as [string, string[]]; + assert.equal(cmd, '/custom/path/sf'); + }); + + it('falls through to auto-discovery when sfPath is empty string', () => { + // Empty string is not a valid path — should behave as if sfPath was absent + spawnStub.returns(makeSpawnOk('ok')); + runSfCommand(['--version'], ''); + const [cmd] = spawnStub.firstCall.args as [string, string[]]; + assert.equal(cmd, 'sf'); // uses the cached path, not the empty string + }); + + it('falls through to auto-discovery when sfPath is whitespace only', () => { + spawnStub.returns(makeSpawnOk('ok')); + runSfCommand(['--version'], ' '); + const [cmd] = spawnStub.firstCall.args as [string, string[]]; + assert.equal(cmd, 'sf'); + }); + + it('throws SfNotFoundError when cache is null and no sfPath given', () => { + setSfPathCacheForTesting(null); + assert.throws( + () => runSfCommand(['--version']), + (err) => { + assert.ok(err instanceof SfNotFoundError); + assert.equal(err.code, 'SF_NOT_FOUND'); + return true; + } + ); + }); + + it('throws SfNotFoundError with path hint when explicit sfPath gives ENOENT', () => { + spawnStub.returns(makeEnoent()); + assert.throws( + () => runSfCommand(['--version'], '/bad/path/sf'), + (err) => { + assert.ok(err instanceof SfNotFoundError); + assert.ok(err.message.includes('/bad/path/sf')); + return true; + } + ); + }); + + it('rethrows non-ENOENT errors', () => { + const genericErr = new Error('unexpected failure'); + spawnStub.returns({ + stdout: '', + stderr: '', + status: null, + error: genericErr, + pid: undefined, + output: [], + signal: null, + }); + assert.throws(() => runSfCommand(['--version']), genericErr); + }); + + it('returns exitCode 1 when status is null and no error', () => { + spawnStub.returns({ stdout: '', stderr: '', status: null, error: undefined, pid: 1, output: [], signal: null }); + const result = runSfCommand(['--version']); + assert.equal(result.exitCode, 1); + }); + + it('returns non-zero exitCode from failed command', () => { + spawnStub.returns(makeSpawnFail(2, '', 'error text')); + const result = runSfCommand(['bad', 'command']); + assert.equal(result.exitCode, 2); + assert.equal(result.stderr, 'error text'); + }); + + describe('Windows shell path injection guard', () => { + beforeEach(() => { + setSfPlatformForTesting('win32'); + }); + + it('rejects sfPath containing & on win32', () => { + // Path ends with .cmd (triggers shell) but has & in directory name + assert.throws(() => runSfCommand(['--version'], 'C:\\sf & malicious\\sf.cmd'), /unsafe for shell execution/); + }); + + it('rejects sfPath containing | on win32', () => { + assert.throws(() => runSfCommand(['--version'], 'C:\\sf|evil\\sf.cmd'), /unsafe for shell execution/); + }); + + it('accepts clean .cmd path on win32', () => { + spawnStub.returns(makeSpawnOk('ok')); + // Should not throw — clean path + const result = runSfCommand(['--version'], 'C:\\Program Files\\sf\\bin\\sf.cmd'); + assert.equal(result.exitCode, 0); + }); + }); + + describe('probe-based resolution', () => { + beforeEach(() => { + setSfPathCacheForTesting(undefined); // force a fresh probe + setSfPlatformForTesting('linux'); + }); + + it('caches "sf" when shell:false probe succeeds', () => { + spawnStub.returns(makeSpawnOk('sf/2.0.0')); + runSfCommand(['--version']); + const [, args, opts] = spawnStub.firstCall.args as [string, string[], { shell: boolean }]; + assert.deepEqual(args, ['--version']); // probe call + assert.equal(opts.shell, false); + }); + + it('returns SF_NOT_FOUND when probe fails and no common path exists', () => { + // Both probe attempts fail with ENOENT, no common paths match + spawnStub.returns(makeEnoent()); + assert.throws( + () => runSfCommand(['--version']), + (err) => { + assert.ok(err instanceof SfNotFoundError); + return true; + } + ); + }); + + it('Windows two-phase probe: retries with shell:true on ENOENT', () => { + setSfPlatformForTesting('win32'); + // First call (shell:false) → ENOENT + spawnStub.onFirstCall().returns(makeEnoent()); + // Second call (shell:true) → success + spawnStub.onSecondCall().returns(makeSpawnOk('sf/2.0.0')); + // Third call → the actual command + spawnStub.onThirdCall().returns(makeSpawnOk('result')); + + const result = runSfCommand(['--version']); + assert.equal(result.stdout, 'result'); + + // Verify second probe used shell:true + const [, , opts] = spawnStub.secondCall.args as [string, string[], { shell: boolean }]; + assert.equal(opts.shell, true); + }); + }); +});