diff --git a/packages/mcp-provider-devops/src/getPipelineMP.ts b/packages/mcp-provider-devops/src/getPipelineMP.ts index 20b40983..5b4e7b77 100644 --- a/packages/mcp-provider-devops/src/getPipelineMP.ts +++ b/packages/mcp-provider-devops/src/getPipelineMP.ts @@ -1,4 +1,5 @@ import type { Connection } from "@salesforce/core"; +import { validateSalesforceId } from "./shared/soqlUtils.js"; export interface DevopsPipelineRecordMP { Id: string; @@ -13,10 +14,13 @@ export interface DevopsPipelineRecordMP { */ export async function getPipelineMP(connection: Connection, projectId: string): Promise { try { + // Validate projectId to prevent SOQL injection + const validatedProjectId = validateSalesforceId(projectId, 'projectId'); + const query = ` SELECT Id, Name, sf_devops__Activated__c, sf_devops__Project__c FROM sf_devops__Pipeline__c - WHERE sf_devops__Project__c = '${projectId}' + WHERE sf_devops__Project__c = '${validatedProjectId}' AND sf_devops__Activated__c = true LIMIT 1 `; diff --git a/packages/mcp-provider-devops/src/getPipelineStagesMP.ts b/packages/mcp-provider-devops/src/getPipelineStagesMP.ts index 8b32f239..64e75e9e 100644 --- a/packages/mcp-provider-devops/src/getPipelineStagesMP.ts +++ b/packages/mcp-provider-devops/src/getPipelineStagesMP.ts @@ -1,4 +1,5 @@ import type { Connection } from "@salesforce/core"; +import { validateSalesforceId } from "./shared/soqlUtils.js"; export interface PipelineStageRecordMP { Id: string; @@ -17,6 +18,9 @@ export interface PipelineStageRecordMP { */ export async function fetchPipelineStagesMP(connection: Connection, pipelineId: string): Promise { try { + // Validate pipelineId to prevent SOQL injection + const validatedPipelineId = validateSalesforceId(pipelineId, 'pipelineId'); + const query = ` SELECT Id, @@ -28,7 +32,7 @@ export async function fetchPipelineStagesMP(connection: Connection, pipelineId: sf_devops__Next_Stage__c, sf_devops__Next_Stage__r.Name FROM sf_devops__Pipeline_Stage__c - WHERE sf_devops__Pipeline__c = '${pipelineId}' + WHERE sf_devops__Pipeline__c = '${validatedPipelineId}' `; const result: any = await connection.query(query); const records: any[] = result?.records || []; diff --git a/packages/mcp-provider-devops/src/getWorkItems.ts b/packages/mcp-provider-devops/src/getWorkItems.ts index 8585f65c..00bcc553 100644 --- a/packages/mcp-provider-devops/src/getWorkItems.ts +++ b/packages/mcp-provider-devops/src/getWorkItems.ts @@ -1,5 +1,6 @@ import { type Connection } from "@salesforce/core"; import { computeFirstStageId, fetchPipelineStages, getBranchNameFromStage, getPipelineIdForProject, findStageById, resolveTargetStageId } from "./shared/pipelineUtils.js"; +import { validateSalesforceId, validateWorkItemName } from "./shared/soqlUtils.js"; import type { WorkItem } from "./types/WorkItem.js"; type ProjectStagesContext = { pipelineId: string; stages: any[]; firstStageId: string | undefined }; @@ -203,6 +204,9 @@ function mapRawItemToWorkItem(item: any, ctx: ProjectStagesContext | null, provi export async function fetchWorkItems(connection: Connection, projectId: string): Promise { try { + // Validate projectId to prevent SOQL injection + const validatedProjectId = validateSalesforceId(projectId, 'projectId'); + const query = ` SELECT Id, @@ -220,7 +224,7 @@ export async function fetchWorkItems(connection: Connection, projectId: string): DevopsPipelineStageId, DevopsProjectId FROM WorkItem - WHERE DevopsProjectId = '${projectId}' + WHERE DevopsProjectId = '${validatedProjectId}' `; @@ -246,6 +250,9 @@ export async function fetchWorkItems(connection: Connection, projectId: string): */ export async function fetchWorkItemByName(connection: Connection, workItemName: string): Promise { try { + // Validate workItemName to prevent SOQL injection + const validatedWorkItemName = validateWorkItemName(workItemName); + const query = ` SELECT Id, @@ -263,7 +270,7 @@ export async function fetchWorkItemByName(connection: Connection, workItemName: DevopsPipelineStageId, DevopsProjectId FROM WorkItem - WHERE Name = '${workItemName}' + WHERE Name = '${validatedWorkItemName}' LIMIT 1 `; diff --git a/packages/mcp-provider-devops/src/getWorkItemsMP.ts b/packages/mcp-provider-devops/src/getWorkItemsMP.ts index 058c482c..f7573452 100644 --- a/packages/mcp-provider-devops/src/getWorkItemsMP.ts +++ b/packages/mcp-provider-devops/src/getWorkItemsMP.ts @@ -2,6 +2,7 @@ import type { Connection } from "@salesforce/core"; import type { WorkItem } from "./types/WorkItem.js"; import { getPipelineMP } from "./getPipelineMP.js"; import { fetchPipelineStagesMP } from "./getPipelineStagesMP.js"; +import { validateSalesforceId, validateWorkItemName } from "./shared/soqlUtils.js"; function inferRepoTypeFromUrl(repoUrl: string): string { @@ -56,6 +57,9 @@ export async function fetchWorkItemByNameMP(connection: Connection, workItemName } async function queryWorkItemByName(connection: any, workItemName: string): Promise { + // Validate workItemName to prevent SOQL injection + const validatedWorkItemName = validateWorkItemName(workItemName); + const query = ` SELECT Id, Name, @@ -64,10 +68,10 @@ async function queryWorkItemByName(connection: any, workItemName: string): Promi sf_devops__State__c, sf_devops__Concluded__c, sf_devops__Assigned_To__c, sf_devops__Assigned_To__r.Name, - sf_devops__Branch__c, sf_devops__Branch__r.Name, + sf_devops__Branch__c, sf_devops__Branch__r.Name, sf_devops__Branch__r.sf_devops__Repository__r.sf_devops__Url__c, sf_devops__Project__c FROM sf_devops__Work_Item__c - WHERE Name = '${workItemName}' + WHERE Name = '${validatedWorkItemName}' LIMIT 1 `; const result: any = await connection.query(query); @@ -118,10 +122,14 @@ function orderStagesFromFirst(idToStage: Map, firstStageId?: string async function getCompletedStageIds(connection: any, workItemId: string): Promise> { const completed = new Set(); + + // Validate workItemId to prevent SOQL injection + const validatedWorkItemId = validateSalesforceId(workItemId, 'workItemId'); + const q = ` SELECT Id, sf_devops__Pipeline_Stage__c, sf_devops__Deployment_Result__r.sf_devops__Completion_Date__c FROM sf_devops__Work_Item_Promote__c - WHERE sf_devops__Work_Item__c = '${workItemId}' + WHERE sf_devops__Work_Item__c = '${validatedWorkItemId}' AND sf_devops__Deployment_Result__r.sf_devops__Completion_Date__c != NULL AND sf_devops__Deployment_Result__r.sf_devops__Deployment_Id__c != NULL `; diff --git a/packages/mcp-provider-devops/src/shared/soqlUtils.ts b/packages/mcp-provider-devops/src/shared/soqlUtils.ts new file mode 100644 index 00000000..3b6c49ed --- /dev/null +++ b/packages/mcp-provider-devops/src/shared/soqlUtils.ts @@ -0,0 +1,114 @@ +/** + * Utility functions for safe SOQL query construction + */ + +/** + * Validates that a string is a valid Salesforce ID (15 or 18 characters). + * Returns true if valid, false otherwise. + */ +export function isValidSalesforceId(id: string): boolean { + if (typeof id !== 'string') { + return false; + } + // Salesforce IDs are either 15 or 18 characters, alphanumeric + const trimmed = id.trim(); + const idPattern = /^[a-zA-Z0-9]{15}$|^[a-zA-Z0-9]{18}$/; + return idPattern.test(trimmed); +} + +/** + * Escapes single quotes in a string for use in SOQL queries. + * Throws an error if the value contains SQL injection attempts. + */ +export function escapeSoqlString(value: string): string { + if (typeof value !== 'string') { + throw new Error('SOQL escape requires string input'); + } + + // Escape single quotes by doubling them (SOQL standard) + return value.replace(/'/g, "\\'"); +} + +/** + * Validates and returns a Salesforce ID, or throws an error. + * Use this for ID fields to prevent injection. + */ +export function validateSalesforceId(id: string, fieldName: string = 'ID'): string { + if (!isValidSalesforceId(id)) { + throw new Error(`Invalid Salesforce ID format for ${fieldName}: ${id}`); + } + return id.trim(); +} + +/** + * Validates a work item name format (WI-XXXXXXX). + * Returns true if valid, false otherwise. + */ +export function isValidWorkItemName(name: string): boolean { + if (typeof name !== 'string') { + return false; + } + // Work item names follow the pattern: WI- followed by digits + const workItemPattern = /^WI-\d+$/; + return workItemPattern.test(name.trim()); +} + +/** + * Checks if a string contains potential SQL injection patterns. + * Returns true if injection patterns are detected. + */ +export function containsSqlInjectionPatterns(value: string): boolean { + if (typeof value !== 'string') { + return true; + } + + const trimmed = value.trim(); + + // Check for common SQL injection patterns + const injectionPatterns = [ + /--/, // SQL comment + /;/, // Statement terminator + /\bOR\b/i, // OR keyword + /\bAND\b/i, // AND keyword + /\bUNION\b/i, // UNION keyword + /\bSELECT\b/i, // SELECT keyword + /\bDROP\b/i, // DROP keyword + /\bINSERT\b/i, // INSERT keyword + /\bUPDATE\b/i, // UPDATE keyword + /\bDELETE\b/i, // DELETE keyword + /\bEXEC\b/i, // EXEC keyword + ]; + + // Check for injection patterns (excluding quotes which we'll escape) + const withoutQuotes = trimmed.replace(/'/g, ''); + return injectionPatterns.some(pattern => pattern.test(withoutQuotes)) || + (trimmed.includes("'") && injectionPatterns.some(pattern => pattern.test(trimmed))); +} + +/** + * Validates and escapes a work item name for SOQL. + * Accepts WI-XXXXXXX format or any alphanumeric string without injection patterns. + * Throws an error if the format is invalid or contains injection attempts. + */ +export function validateWorkItemName(name: string): string { + const trimmed = name.trim(); + + // Check for SQL injection patterns first + if (containsSqlInjectionPatterns(trimmed)) { + throw new Error(`Invalid work item name: potential SQL injection detected in "${name}"`); + } + + // If it matches the standard WI-XXXXXXX format, it's valid + if (isValidWorkItemName(trimmed)) { + return escapeSoqlString(trimmed); + } + + // Also allow alphanumeric names with spaces, hyphens, and underscores (for managed package work items) + // but ensure no special characters that could be used for injection + const safeNamePattern = /^[a-zA-Z0-9\s_'-]+$/; + if (safeNamePattern.test(trimmed)) { + return escapeSoqlString(trimmed); + } + + throw new Error(`Invalid work item name format: ${name}`); +} diff --git a/packages/mcp-provider-devops/test/shared/soqlUtils.test.ts b/packages/mcp-provider-devops/test/shared/soqlUtils.test.ts new file mode 100644 index 00000000..23b5394f --- /dev/null +++ b/packages/mcp-provider-devops/test/shared/soqlUtils.test.ts @@ -0,0 +1,188 @@ +import { expect } from 'chai'; +import { + isValidSalesforceId, + escapeSoqlString, + validateSalesforceId, + isValidWorkItemName, + validateWorkItemName, + containsSqlInjectionPatterns +} from '../../src/shared/soqlUtils.js'; + +describe('soqlUtils', () => { + describe('isValidSalesforceId', () => { + it('should accept valid 15-character Salesforce IDs', () => { + expect(isValidSalesforceId('001xx000003DGb2')).to.be.true; + expect(isValidSalesforceId('a07EE00002aJTv0')).to.be.true; + }); + + it('should accept valid 18-character Salesforce IDs', () => { + expect(isValidSalesforceId('001xx000003DGb2AAG')).to.be.true; + expect(isValidSalesforceId('a07EE00002aJTv0YAG')).to.be.true; + }); + + it('should reject IDs with invalid length', () => { + expect(isValidSalesforceId('123')).to.be.false; + expect(isValidSalesforceId('001xx000003DGb2AAGEXTRA')).to.be.false; + }); + + it('should reject IDs with SQL injection attempts', () => { + expect(isValidSalesforceId("001' OR 1=1--")).to.be.false; + expect(isValidSalesforceId("'; DROP TABLE--")).to.be.false; + }); + + it('should reject non-string values', () => { + expect(isValidSalesforceId(null as any)).to.be.false; + expect(isValidSalesforceId(undefined as any)).to.be.false; + expect(isValidSalesforceId(123 as any)).to.be.false; + }); + }); + + describe('escapeSoqlString', () => { + it('should escape single quotes', () => { + expect(escapeSoqlString("O'Brien")).to.equal("O\\'Brien"); + expect(escapeSoqlString("It's working")).to.equal("It\\'s working"); + }); + + it('should handle strings without quotes', () => { + expect(escapeSoqlString("normal text")).to.equal("normal text"); + }); + + it('should handle empty strings', () => { + expect(escapeSoqlString("")).to.equal(""); + }); + + it('should throw on non-string values', () => { + expect(() => escapeSoqlString(null as any)).to.throw(); + expect(() => escapeSoqlString(123 as any)).to.throw(); + }); + }); + + describe('validateSalesforceId', () => { + it('should return trimmed valid IDs', () => { + expect(validateSalesforceId('001xx000003DGb2')).to.equal('001xx000003DGb2'); + expect(validateSalesforceId(' a07EE00002aJTv0 ')).to.equal('a07EE00002aJTv0'); + }); + + it('should throw on invalid IDs', () => { + expect(() => validateSalesforceId('invalid')).to.throw(/Invalid Salesforce ID/); + expect(() => validateSalesforceId("001' OR 1=1--")).to.throw(/Invalid Salesforce ID/); + }); + + it('should include field name in error message', () => { + try { + validateSalesforceId('invalid', 'projectId'); + expect.fail('Should have thrown'); + } catch (error: any) { + expect(error.message).to.include('projectId'); + } + }); + }); + + describe('isValidWorkItemName', () => { + it('should accept valid work item names', () => { + expect(isValidWorkItemName('WI-12345')).to.be.true; + expect(isValidWorkItemName('WI-0001')).to.be.true; + expect(isValidWorkItemName('WI-9999999')).to.be.true; + }); + + it('should reject invalid formats', () => { + expect(isValidWorkItemName('W-12345')).to.be.false; + expect(isValidWorkItemName('WI12345')).to.be.false; + expect(isValidWorkItemName('WI-')).to.be.false; + expect(isValidWorkItemName('WI-ABC')).to.be.false; + }); + + it('should reject SQL injection attempts', () => { + expect(isValidWorkItemName("WI-0001' OR Name != 'x")).to.be.false; + expect(isValidWorkItemName("'; DROP TABLE--")).to.be.false; + }); + + it('should reject non-string values', () => { + expect(isValidWorkItemName(null as any)).to.be.false; + expect(isValidWorkItemName(undefined as any)).to.be.false; + }); + }); + + describe('validateWorkItemName', () => { + it('should return escaped valid work item names', () => { + expect(validateWorkItemName('WI-12345')).to.equal('WI-12345'); + expect(validateWorkItemName(' WI-0001 ')).to.equal('WI-0001'); + }); + + it('should allow alphanumeric work item names for managed packages', () => { + expect(validateWorkItemName('Test Work Item')).to.equal('Test Work Item'); + expect(validateWorkItemName('My_Work_Item-123')).to.equal('My_Work_Item-123'); + expect(validateWorkItemName('Feature 2024')).to.equal('Feature 2024'); + }); + + it('should escape single quotes in work item names', () => { + expect(validateWorkItemName("O'Brien Task")).to.equal("O\\'Brien Task"); + }); + + it('should throw on SQL injection attempts', () => { + expect(() => validateWorkItemName("WI-0001' OR 1=1--")).to.throw(/SQL injection/); + expect(() => validateWorkItemName("Test'; DROP TABLE--")).to.throw(/SQL injection/); + expect(() => validateWorkItemName("WI-001 UNION SELECT")).to.throw(/SQL injection/); + expect(() => validateWorkItemName("Task OR Name != 'x'")).to.throw(/SQL injection/); + }); + + it('should throw on special characters that could be dangerous', () => { + expect(() => validateWorkItemName('Task