fix: improve trigger.pipeline validation with expression checks and better errors

[jamesadevine]

Closes #189, closes #188.

Changes

• ADO expression rejection (chore: add ADO expression check to trigger.pipeline fields or document why it's omitted #188): Added ${{ }}, $(), and $[] checks to triggers.pipeline.name, triggers.pipeline.project, and each triggers.pipeline.branches entry. These now match the existing checks on name and description fields, providing defense-in-depth.

• Better error messages (fix: include offending branch name in trigger.pipeline validation error message #189): Branch validation errors now include the offending value using {:?} formatting, making it easy to identify which branch entry triggered the failure when multiple branches are configured.

Testing

Added 3 new tests covering expression rejection for trigger pipeline name, project, and branch fields. All 727 unit tests pass.

[github-actions]

🔍 Rust PR Review

Summary: Looks good — correct, well-tested, and security logic is sound. A few minor consistency nits worth cleaning up.

Findings

⚠️ Suggestions

• src/compile/common.rs:191 — The single-element for (field, value) in [("triggers.pipeline.name", pipeline.name.as_str())] loop is unnecessary overhead. Since project is Option<String> and can't share the same iteration pattern, this loop will always contain exactly one element. Either inline the checks directly, or add a comment explaining why the loop pattern was chosen (e.g., in anticipation of adding more String fields). Not wrong, just oddly structured.

• src/compile/common.rs:217-220 — The project newline error message was not updated to include the offending value, while the expression errors for the same field (project) do print Found: '{}'. Inconsistent:

  // expression error — includes value ✅
  "...Use literal values only. Found: '{}'", project
  // newline error — missing value ❌
  "...'triggers.pipeline.project' must be a single line..."

  Consider adding Found: '{}' to match the newline errors for name/description and the expression error for project.

• src/compile/common.rs:226 vs 195 — Branch errors use {:?} (Debug/double-quoted) while name/project errors use '{}' (Display/single-quoted). Minor inconsistency — either format is fine, but a single style across all trigger field errors would be cleaner.

✅ What Looks Good

• Validation fires at line 54 in standalone.rs, well before generate_pipeline_resources at line 79 — the sanitization gate is correctly placed.
• The single-quote escaping in generate_pipeline_resources (replace('\'', "''")) is correct YAML escaping and ADO expression validation now prevents any $()/${{}}/$[] from reaching that codepath.
• New tests cover all three fields (name, project, branches) with realistic injection payloads ($(System.AccessToken), $[variables['token']]). Good coverage.
• No unsafe unwrap() added; consistent use of anyhow::bail!.

Generated by Rust PR Reviewer for issue #196 · ● 1.1M · ◷