Skip to content

Commit b6c0106

Browse files
NEW @W-21600165@ - Add pattern_not_regex support to reduce false positives in custom rules (#439)
1 parent caf9ab6 commit b6c0106

8 files changed

Lines changed: 181 additions & 1 deletion

File tree

packages/code-analyzer-regex-engine/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@salesforce/code-analyzer-regex-engine",
33
"description": "Plugin package that adds 'regex' as an engine into Salesforce Code Analyzer",
4-
"version": "0.33.1-SNAPSHOT",
4+
"version": "0.34.0-SNAPSHOT",
55
"author": "The Salesforce Code Analyzer Team",
66
"license": "BSD-3-Clause",
77
"homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",

packages/code-analyzer-regex-engine/src/config.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ export type RegexRule = {
3131
// The regular expression that triggers a violation when matched against the contents of a file.
3232
regex: string;
3333

34+
// [Optional] The negative pattern - matches that also match this pattern will be excluded from violations.
35+
// This allows you to exclude false positives by specifying patterns that should NOT be flagged.
36+
// Example: regex: /(To|From):\s*\$\([^)]+\)/ with regex_ignore: /\$\(validatedMessageId\)/
37+
regex_ignore?: string;
38+
3439
// The extensions of the files that you would like to test the regular expression against.
3540
// If not defined, or equal to null, then all text-based files of any file extension will be tested.
3641
file_extensions?: string[];
@@ -74,6 +79,10 @@ export function validateAndNormalizeConfig(valueExtractor: ConfigValueExtractor)
7479
const description: string = ruleExtractor.extractRequiredString('description');
7580
const rawRegexString: string = ruleExtractor.extractRequiredString('regex');
7681
const regexString: string = validateRegexString(rawRegexString, ruleExtractor.getFieldPath('regex'));
82+
const rawPatternNotRegex: string | undefined = ruleExtractor.extractString('regex_ignore');
83+
const patternNotRegexString: string | undefined = rawPatternNotRegex
84+
? validateRegexString(rawPatternNotRegex, ruleExtractor.getFieldPath('regex_ignore'))
85+
: undefined;
7786
const rawFileExtensions: string[] | undefined = ruleExtractor.extractArray('file_extensions',
7887
(element, fieldPath) => ValueValidator.validateString(element, fieldPath, FILE_EXT_PATTERN));
7988

@@ -85,6 +94,7 @@ export function validateAndNormalizeConfig(valueExtractor: ConfigValueExtractor)
8594
severity: ruleExtractor.extractSeverityLevel('severity', DEFAULT_SEVERITY_LEVEL)!,
8695
tags: ruleExtractor.extractArray('tags', ValueValidator.validateString, [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CUSTOM])!,
8796
...(rawFileExtensions ? { file_extensions: normalizeFileExtensions(rawFileExtensions) } : {}),
97+
...(patternNotRegexString ? { regex_ignore: patternNotRegexString } : {}),
8898
}
8999
}
90100
return {

packages/code-analyzer-regex-engine/src/engine.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,11 @@ export class RegexEngine extends Engine {
139139
const contextuallyDerivedEol: string = contextuallyDeriveEolString(fileContents);
140140
const newlineIndexes: number[] = getNewlineIndices(fileContents, contextuallyDerivedEol);
141141

142+
// Get negative pattern if defined
143+
const patternNotRegex = this.regexRules[ruleName].regex_ignore
144+
? convertToRegex(this.regexRules[ruleName].regex_ignore!)
145+
: undefined;
146+
142147
for (const match of fileContents.matchAll(regex)) {
143148
let startIndex: number = match.index;
144149
let matchLength: number = match[0].length;
@@ -150,6 +155,18 @@ export class RegexEngine extends Engine {
150155
matchLength = match.groups.target.length;
151156
}
152157

158+
// Skip this match if it also matches the negative pattern
159+
if (patternNotRegex) {
160+
const matchedText = fileContents.substring(startIndex, startIndex + matchLength);
161+
if (patternNotRegex.test(matchedText)) {
162+
// Reset regex state for next iteration
163+
patternNotRegex.lastIndex = 0;
164+
continue;
165+
}
166+
// Reset regex state for next iteration
167+
patternNotRegex.lastIndex = 0;
168+
}
169+
153170
const startLine: number = getLineNumber(startIndex, newlineIndexes);
154171
const startColumn: number = getColumnNumber(startIndex, newlineIndexes, contextuallyDerivedEol);
155172
const endLine: number = getLineNumber(startIndex + matchLength, newlineIndexes);

packages/code-analyzer-regex-engine/src/messages.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
1111
` {rule_name} is the name you would like to give to your custom rule\n` +
1212
` {rule_property_name} is the name of one of the rule properties. You may specify the following rule properties:\n` +
1313
` 'regex' - The regular expression that triggers a violation when matched against the contents of a file.\n` +
14+
` 'regex_ignore' - [Optional] The negative pattern - matches that also match this pattern will be excluded.\n` +
15+
` This allows you to exclude false positives by specifying patterns that should NOT be flagged.\n` +
16+
` Example: To match email headers with user input but exclude 'validatedMessageId':\n` +
17+
` regex: /(To|From):\\s*\\$\\([^)]+\\)/gi\n` +
18+
` regex_ignore: /\\$\\(\\s*validatedMessageId\\s*\\)/gi\n` +
1419
` 'file_extensions' - The extensions of the files that you would like to test the regular expression against.\n` +
1520
` 'description' - A description of the rule's purpose\n` +
1621
` 'violation_message' - [Optional] The message emitted when a rule violation occurs.\n` +
@@ -32,6 +37,13 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
3237
` violation_message: "A comment with a TODO statement was found. Please remove TODO statements from your apex code."\n` +
3338
` severity: "Info"\n` +
3439
` tags: ["TechDebt"]\n` +
40+
` "DataWeaveEmailHeaderInjection":\n` +
41+
` regex: /(To|From|Subject):\\s*\\$\\([^)]+\\)/gi\n` +
42+
` regex_ignore: /\\$\\(\\s*validatedMessageId\\s*\\)/gi\n` +
43+
` file_extensions: [".dwl"]\n` +
44+
` description: "Detects user input in email headers, excluding validated IDs."\n` +
45+
` severity: "Critical"\n` +
46+
` tags: ["Security"]\n` +
3547
`-------------------------------------------`,
3648

3749
UnsupportedEngineName:

packages/code-analyzer-regex-engine/test/engine.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,89 @@ describe('Tests for runRules', () => {
10311031
});
10321032
});
10331033

1034+
describe('Tests for regex_ignore', () => {
1035+
it('regex_ignore should exclude matches that match the negative pattern', async () => {
1036+
const customRulesWithNegativePattern: RegexRules = {
1037+
EmailHeaderInjection: {
1038+
regex: '/(To|From|Subject|In-Reply-To|References):\\s*\\$\\([^)]+\\)/gi',
1039+
regex_ignore: '/\\$\\(\\s*validatedMessageId\\s*\\)/gi',
1040+
description: "Detects user input in email headers, excluding validatedMessageId",
1041+
file_extensions: [".dwl"],
1042+
violation_message: "User input detected in email header",
1043+
severity: SeverityLevel.Critical,
1044+
tags: ["Security"]
1045+
}
1046+
};
1047+
1048+
const testEngine = new RegexEngine(customRulesWithNegativePattern, RULE_RESOURCE_URLS);
1049+
const runOptions: RunOptions = createRunOptions(
1050+
new Workspace('id', [path.resolve(__dirname, "test-data", "patternNotRegex")]));
1051+
const runResults: EngineRunResults = await testEngine.runRules(["EmailHeaderInjection"], runOptions);
1052+
1053+
// emailHeaders_WithValidatedId.dwl has $(validatedMessageId) - should be excluded
1054+
// emailHeaders_WithUnsanitizedPayload.dwl has $(payload.x) - should be violations
1055+
const validatedIdViolations = runResults.violations.filter(v =>
1056+
v.codeLocations[0].file.includes('emailHeaders_WithValidatedId.dwl'));
1057+
const unsanitizedViolations = runResults.violations.filter(v =>
1058+
v.codeLocations[0].file.includes('emailHeaders_WithUnsanitizedPayload.dwl'));
1059+
1060+
expect(validatedIdViolations).toHaveLength(0); // Should be excluded by regex_ignore
1061+
expect(unsanitizedViolations.length).toBeGreaterThan(0); // Should have violations
1062+
});
1063+
1064+
it('Rule without regex_ignore should behave normally', async () => {
1065+
const customRulesWithoutNegativePattern: RegexRules = {
1066+
EmailHeaderInjection: {
1067+
regex: '/(To|From|Subject|In-Reply-To|References):\\s*\\$\\([^)]+\\)/gi',
1068+
description: "Detects user input in email headers",
1069+
file_extensions: [".dwl"],
1070+
violation_message: "User input detected in email header",
1071+
severity: SeverityLevel.Critical,
1072+
tags: ["Security"]
1073+
}
1074+
};
1075+
1076+
const testEngine = new RegexEngine(customRulesWithoutNegativePattern, RULE_RESOURCE_URLS);
1077+
const runOptions: RunOptions = createRunOptions(
1078+
new Workspace('id', [path.resolve(__dirname, "test-data", "patternNotRegex")]));
1079+
const runResults: EngineRunResults = await testEngine.runRules(["EmailHeaderInjection"], runOptions);
1080+
1081+
// Without regex_ignore, both files should have violations
1082+
const validatedIdViolations = runResults.violations.filter(v =>
1083+
v.codeLocations[0].file.includes('emailHeaders_WithValidatedId.dwl'));
1084+
const unsanitizedViolations = runResults.violations.filter(v =>
1085+
v.codeLocations[0].file.includes('emailHeaders_WithUnsanitizedPayload.dwl'));
1086+
1087+
expect(validatedIdViolations.length).toBeGreaterThan(0); // Should have violations
1088+
expect(unsanitizedViolations.length).toBeGreaterThan(0); // Should have violations
1089+
});
1090+
1091+
it('regex_ignore with multiple exclusion patterns', async () => {
1092+
const customRulesWithMultipleExclusions: RegexRules = {
1093+
EmailHeaderInjection: {
1094+
regex: '/(To|From|Subject):\\s*\\$\\([^)]+\\)/gi',
1095+
regex_ignore: '/\\$\\((validatedMessageId|sanitizeHeader|getSafeEmailHeader)\\s*[^)]*\\)/gi',
1096+
description: "Detects user input in email headers, excluding safe functions",
1097+
file_extensions: [".dwl"],
1098+
violation_message: "User input detected in email header",
1099+
severity: SeverityLevel.Critical,
1100+
tags: ["Security"]
1101+
}
1102+
};
1103+
1104+
const testEngine = new RegexEngine(customRulesWithMultipleExclusions, RULE_RESOURCE_URLS);
1105+
const runOptions: RunOptions = createRunOptions(
1106+
new Workspace('id', [path.resolve(__dirname, "test-data", "patternNotRegex")]));
1107+
const runResults: EngineRunResults = await testEngine.runRules(["EmailHeaderInjection"], runOptions);
1108+
1109+
// validatedMessageId should still be excluded
1110+
const validatedIdViolations = runResults.violations.filter(v =>
1111+
v.codeLocations[0].file.includes('emailHeaders_WithValidatedId.dwl'));
1112+
1113+
expect(validatedIdViolations).toHaveLength(0);
1114+
});
1115+
});
1116+
10341117
describe('Tests for getEngineVersion', () => {
10351118
it('Outputs something resembling a Semantic Version', async () => {
10361119
const version: string = await engine.getEngineVersion();

packages/code-analyzer-regex-engine/test/plugin.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,4 +498,38 @@ describe('RegexEnginePlugin Custom Config Tests', () => {
498498
await expect(plugin.createEngineConfig("regex", valueExtractor)).rejects.toThrow(
499499
getMessageFromCatalog(SHARED_MESSAGE_CATALOG,'ConfigValueMustBeOfType', 'engines.regex.custom_rules.NoTodos.tags', 'array', 'string'));
500500
});
501+
502+
it("If user creates a rule with regex_ignore, it should be included in the config", async () => {
503+
const rawConfig = {
504+
custom_rules: {
505+
"EmailHeaderInjection": {
506+
regex: String.raw`/(To|From|Subject):\s*\$\([^)]+\)/gi`,
507+
regex_ignore: String.raw`/\$\(\s*validatedMessageId\s*\)/gi`,
508+
description: "Detects user input in email headers",
509+
file_extensions: [".dwl"]
510+
}
511+
}
512+
};
513+
const valueExtractor: ConfigValueExtractor = new ConfigValueExtractor(rawConfig, 'engines.regex');
514+
const resolvedConfig: ConfigObject = await plugin.createEngineConfig("regex", valueExtractor);
515+
const pluginEngine: RegexEngine = await plugin.createEngine("regex", resolvedConfig) as RegexEngine;
516+
517+
expect(pluginEngine._getRegexRules()["EmailHeaderInjection"].regex_ignore).toBeDefined();
518+
expect(pluginEngine._getRegexRules()["EmailHeaderInjection"].regex_ignore).toContain('validatedMessageId');
519+
});
520+
521+
it("If user creates a rule with invalid regex_ignore, ensure correct error is emitted", async () => {
522+
const rawConfig = {
523+
custom_rules: {
524+
"BadRule": {
525+
...SAMPLE_RAW_CUSTOM_RULE_DEFINITION,
526+
regex_ignore: "/bad[pattern/gi"
527+
}
528+
}
529+
};
530+
const valueExtractor: ConfigValueExtractor = new ConfigValueExtractor(rawConfig, 'engines.regex');
531+
await expect(plugin.createEngineConfig("regex", valueExtractor)).rejects.toThrow(
532+
getMessage('InvalidConfigurationValueWithReason', 'engines.regex.custom_rules.BadRule.regex_ignore',
533+
getMessage('InvalidRegexDueToError', '/bad[pattern/gi', "Invalid regular expression: /bad[pattern/gi: Unterminated character class")));
534+
});
501535
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
%dw 2.0
2+
output application/java
3+
---
4+
{
5+
headers: {
6+
To: $(payload.recipientEmail),
7+
Subject: $(payload.subject),
8+
From: $(payload.fromAddress)
9+
},
10+
body: payload.message
11+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
%dw 2.0
2+
output application/java
3+
---
4+
{
5+
headers: {
6+
To: "customer@example.com",
7+
Subject: "Property Update",
8+
"In-Reply-To": $(validatedMessageId),
9+
References: $(validatedMessageId),
10+
From: "noreply@dreamhouse.com"
11+
},
12+
body: payload.message
13+
}

0 commit comments

Comments
 (0)