feat: Validation Rule Analyzer — surface formula constraints as inline YAML comments (v2.10.0)

[jdschleicher]

Summary

• Introduces ValidationRuleAnalyzerService to parse Salesforce validation rule XML and build a field → constraint map
• Wires constraint data into RecipeService so generated YAML fields get ### Driven by ValidationRule "..." inline comments
• Unifies comment-appending logic via a single appendValidationRuleComment helper across all field type cases
• Wraps generateRecipeFromConfigurationDetail in vscode.window.withProgress with three phased progress messages
• Extends createRecipeFilesInSubdirectory to accept progress/token/totalFileCount for future per-file reporting
• Activates ExtensionCommandService.test.ts (7 new tests) previously held as placeholder

Issue

Closes #40

Test plan

• npm run jest-test — 396 tests passing, zero regressions
• npm run compile — zero TypeScript errors
• npm run lint — zero ESLint errors
• ValidationRuleAnalyzerService.test.ts covers formula parsing, field reference extraction, constraint map building
• RecipeService.test.ts covers validation rule comment appended across all field type return paths
• ExtensionCommandService.test.ts covers withProgress options, phase order, and param pass-through to createRecipeFilesInSubdirectory

🤖 Generated with Claude Code

[jdschleicher]

Triple Code Review — PR #41: Validation Rule Analyzer (v2.10.0)

---

Reviewer Verdict Summary

Reviewer	Role	Verdict
Reviewer 1	Senior TypeScript / Architecture Engineer	REQUEST CHANGES
Reviewer 2	Senior Performance Engineer	COMMENT
Reviewer 3	Senior Security Engineer	REQUEST CHANGES

---

CRITICAL Findings (Block Merge)

[CRITICAL — Security] Regex pattern injection into new Function() execution

File: src/treecipe/src/RecipeFakerService.ts/FakerJSRecipeFakerService/FakerJSRecipeFakerService.ts line 525
Pipeline: XML regex pattern → YAML recipe → FakerJSRecipeProcessor.getFakerJSExpressionEvaluation() → new Function('faker', 'dateUtils', \return (${preparedCode})`)`

The REGEX_PATTERN regex captures everything between quotes as ([^"]+) and embeds it directly into a JavaScript regex literal in the generated YAML recipe:

return `${this.openingRecipeSyntax} faker.helpers.fromRegExp(/${regexConstraint.value}/) ${this.closingRecipeSyntax}`;

A validation rule XML file in the user's Salesforce project (or a malicious shared project) can contain:

<errorConditionFormula>NOT(REGEX(Phone, "^test/) + (()=>{require('child_process').execSync('malicious-cmd')})() + /"))</errorConditionFormula>

This generates:

${{ faker.helpers.fromRegExp(/^test/) + (()=>{require('child_process').execSync('malicious-cmd')})() + //) }}

When runFakerByRecipe is executed, FakerJSRecipeProcessor feeds this string into new Function(...) and executes it as live JavaScript in the VS Code extension host process. This is arbitrary code execution triggered by a file in the user's workspace.

Required fix: Sanitize the extracted regex pattern before embedding. At minimum, reject patterns containing /) or escape forward slashes. Better: validate the extracted string is a valid regex using new RegExp(value) in a try/catch before embedding, and escape any / characters as \/.

---

[CRITICAL — Architecture] date/datetime case newly split from OOTB fallback — existing OOTB date overrides silently bypassed

Files: RecipeService.ts lines 114–118, FakerJSRecipeFakerService.ts (prior OOTB map)

Before this PR, date and datetime fields fell through the switch to getFakeValueIfExpectedSalesforceFieldType(), which applied OOTB object-specific overrides (e.g., CloseDate, ActivityDate, Birthdate each had custom date ranges in the OOTB map). The new explicit case 'date': case 'datetime': intercepts ALL date/datetime fields before the OOTB check, permanently bypassing those overrides. Object-specific date fields like CloseDate will now silently get a generic 2023-01-01 → today range instead of the previously tuned −30 days to +90 days business-relevant range.

Required fix: Either check the OOTB map first before calling buildDateRecipeValue, or apply the fallback ordering so OOTB overrides take precedence.

---

WARNING Findings (Should Fix Before Merge)

[WARNING — Architecture] YAML block scalar + inline comment = malformed YAML

File: RecipeService.ts appendValidationRuleComment()

buildNumericRecipeValueWithPrecisionAndScale and buildDateRecipeValue return YAML block scalar strings beginning with | followed by a newline and indented content:

|
            ${{ faker.number.int({min: 100, max: 99999}) }}

appendValidationRuleComment appends the comment with ${value} ${comment}, producing:

|
            ${{ faker.number.int({min: 100, max: 99999}) }} ### Driven by ValidationRule "Rule_Min"

This is valid as a comment within a YAML block scalar body (YAML ignores #-prefixed tokens inside literal block strings), but means the comment is inside the string value, not a true YAML comment, and will appear literally in the recipe file output. This is intentional per the PR description, but note the comment is embedded inside the block scalar text, not on the YAML key line. Consider documenting this behavior clearly, or moving the comment to the preceding line as a true YAML comment (# Driven by... on its own line before the field key).

[WARNING — Architecture/Parity] Snowfakery buildTextRecipeValueWithLength missing lengthMin handling

File: src/treecipe/src/RecipeFakerService.ts/SnowfakeryRecipeFakerService/SnowfakeryRecipeFakerService.ts line 413

FakerJS implements lengthMin in buildTextRecipeValueWithLength via .padEnd(effectiveMin, 'x'). Snowfakery's equivalent only handles lengthMax and silently ignores lengthMin constraints, producing shorter-than-required strings. This violates the dual-backend parity requirement stated in CLAUDE.md.

Required fix: Add lengthMin handling to Snowfakery's buildTextRecipeValueWithLength (e.g., fake.lexify('?' * effectiveMin) + fake.text(max_nb_chars=${effectiveMax - effectiveMin})).

[WARNING — Logic] toExpr defaults to new Date() in all unconstrained paths

File: FakerJSRecipeFakerService.ts buildDateRecipeValue()

Both the relMaxConstraint branch and the final else default both produce new Date() (today), making the range [2023-01-01, today] for all unconstrained date fields. While this matches the prior OOTB behavior for generic date fields, it means future-dated fields (e.g., contract end dates) will never generate future dates from this new code path. Consider whether the default toExpr should be something like new Date(new Date().setFullYear(new Date().getFullYear() + 1)) for non-constrained fields.

[WARNING — Performance] Sequential per-file validation rule I/O within object processing loop

File: DirectoryProcessor.ts readValidationRuleConstraintsForObject()

Each object directory triggers a separate vscode.workspace.fs.readDirectory() + N×readFile() calls for validation rules, sequenced inside the existing per-object for loop (processAllObjectsAndRelationships). For a project with 50 objects and 10 validation rules each, this adds 550 sequential async I/O calls to the critical path. Consider using Promise.all() for the inner per-file reads within each object, similar to how processFieldsDirectory could batch reads.

[WARNING — Performance] _progress, _token, _totalFileCount parameters accepted but unused

File: DirectoryProcessor.ts createRecipeFilesInSubdirectory() lines 270–273

The three new parameters (_progress, _token, _totalFileCount) are declared but never consumed inside the method body (underscore prefix confirms intentional non-use). The token cancellation signal is also not checked anywhere in the withProgress callback's async work, meaning the user's "Cancel" button has no effect. This is scaffolding debt — either wire cancellation now or note it as a tracked follow-up in a // TODO comment.

[WARNING — Type Safety] any typed intermediate in parseValidationRuleXml

File: ValidationRuleAnalyzerService.ts line 79

let parsed: any;
xml2js.parseString(xmlContent, (error, result) => { parsed = result; });

result is typed any and so is parsed. All downstream accesses use optional chaining (rule.active?.[0]) which silently returns undefined on shape mismatch. Define a typed interface for the raw xml2js output (e.g., interface RawValidationRuleXml { ValidationRule?: { active?: string[]; ... } }) to get compile-time safety and avoid silent misparse bugs.

[WARNING — Security] errorMessage and ruleName embedded in YAML comments without sanitization

Files: ValidationRuleAnalyzerService.ts buildUnknownRuleYamlComment() and buildDrivenByValidationRuleComment()

Rule names and error messages from XML are embedded directly into YAML comment strings. A rule name or message containing a newline character could break the YAML line structure and inject content into the recipe at parse time. Sanitize by stripping or replacing newlines and other control characters before embedding in comment strings.

---

Positive Observations

• The getConstraintsFromValidationRuleXml correctly short-circuits on inactive rules — this is the right behavior and avoids phantom constraints.
• The groupConstraintsByField fallback to full availablePicklistChoices when all values are excluded (rather than producing an empty list) is a safe defensive choice.
• The extractFirstFieldReferenceFromFormula heuristic for unknown formulas is a reasonable best-effort and the UNKNOWN_FIELD sentinel makes the unknown case visible in the YAML output.
• Removing the leftover console.log statements from FakerJSRecipeProcessor is a clean housekeeping change.
• Test coverage is substantial: 439-line test file for ValidationRuleAnalyzerService, 7 new tests for ExtensionCommandService, and comment-propagation tests in RecipeService. The mock XML fixtures are realistic.
• The withProgress UX upgrade in generateRecipeFromConfigurationDetail is a genuine improvement for user feedback during long recipe generation runs.

---

Overall Verdict: REQUEST CHANGES

Two critical issues must be resolved before merge:

1. Regex pattern → new Function injection is an arbitrary code execution risk that activates during normal runFakerByRecipe usage on any Salesforce project containing validation rules with regex formulas.
2. Date/datetime OOTB override bypass silently regresses field-specific date behavior for known Salesforce objects (Opportunity CloseDate, Event ActivityDate, etc.).

The Snowfakery lengthMin parity gap and the unused cancellation token should also be addressed in this PR since they are part of the same feature surface.

🤖 Generated with Claude Code