Skip to content

Commit 2575246

Browse files
fix: block template marker delimiters in front matter identity fields (#315)
* Initial plan * fix: reject template marker delimiters in front matter identity fields Block `{{` in name, description, and trigger fields to prevent second-order template injection where a value like `{{ agent_content }}` would be expanded by a later replacement pass, allowing arbitrary YAML injection into the generated pipeline. Closes #302 Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/91f10480-fd27-4ac7-9105-cbea31200461 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> * fix: correct brace escaping in template marker error message Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/91f10480-fd27-4ac7-9105-cbea31200461 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> * fix: add pipeline command check and include offending value in error messages Address rust-review feedback: - Add missing contains_pipeline_command() check to reject_pipeline_injection() to match the docstring's claim of ##vso[/##[ protection - Include Found: '{}' in template marker error for debuggability - Add test cases for pipeline command rejection Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/6b4b1bb0-b925-4b27-a00c-d0f4e576b263 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
1 parent e3e0072 commit 2575246

2 files changed

Lines changed: 61 additions & 4 deletions

File tree

src/compile/common.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,9 @@ pub fn generate_parameters(parameters: &[PipelineParameter]) -> Result<String> {
137137
/// Validate front matter `name` and `description` fields.
138138
///
139139
/// These values are substituted directly into the pipeline YAML template and must not
140-
/// contain ADO expressions (`${{`, `$(`, `$[`) which could disclose secrets or manipulate
141-
/// pipeline logic. Newlines are also rejected to prevent YAML structure injection.
140+
/// contain ADO expressions (`${{`, `$(`, `$[`), the compiler's own template marker
141+
/// delimiter (`{{`), or newlines — any of which could disclose secrets or manipulate
142+
/// pipeline logic via second-order injection.
142143
pub fn validate_front_matter_identity(front_matter: &FrontMatter) -> Result<()> {
143144
for (field, value) in [("name", &front_matter.name), ("description", &front_matter.description)] {
144145
validate::reject_pipeline_injection(value, field)?;
@@ -3146,6 +3147,24 @@ mod tests {
31463147
assert!(result.unwrap_err().to_string().contains("single line"));
31473148
}
31483149

3150+
#[test]
3151+
fn test_validate_front_matter_identity_rejects_template_marker_in_name() {
3152+
let mut fm = minimal_front_matter();
3153+
fm.name = "{{ agent_content }}".to_string();
3154+
let result = validate_front_matter_identity(&fm);
3155+
assert!(result.is_err());
3156+
assert!(result.unwrap_err().to_string().contains("template marker"));
3157+
}
3158+
3159+
#[test]
3160+
fn test_validate_front_matter_identity_rejects_template_marker_in_description() {
3161+
let mut fm = minimal_front_matter();
3162+
fm.description = "{{ copilot_params }}".to_string();
3163+
let result = validate_front_matter_identity(&fm);
3164+
assert!(result.is_err());
3165+
assert!(result.unwrap_err().to_string().contains("template marker"));
3166+
}
3167+
31493168
#[test]
31503169
fn test_validate_front_matter_identity_rejects_newline_in_trigger_pipeline_name() {
31513170
let mut fm = minimal_front_matter();

src/validate.rs

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,15 @@ pub fn contains_newline(s: &str) -> bool {
9999
s.contains('\n') || s.contains('\r')
100100
}
101101

102+
/// Returns true if the string contains the compiler's template marker
103+
/// delimiter (`{{`). Values substituted into the pipeline template must
104+
/// not contain this sequence — otherwise a second-order substitution can
105+
/// inject arbitrary content (e.g., `{{ agent_content }}` in the `name`
106+
/// field would be expanded by a later replacement pass).
107+
pub fn contains_template_marker(s: &str) -> bool {
108+
s.contains("{{")
109+
}
110+
102111
/// Reject ADO template expressions (`${{`), macro expressions (`$(`), and runtime
103112
/// expressions (`$[`) in a string value. Parameter definitions should only contain
104113
/// literal values — expressions could enable information disclosure or logic manipulation
@@ -135,8 +144,8 @@ pub fn reject_ado_expressions_in_value(
135144
}
136145

137146
/// Reject values that could cause pipeline injection: ADO expressions,
138-
/// pipeline commands (`##vso[`, `##[`), and newlines. A combined check
139-
/// for fields embedded into YAML templates.
147+
/// pipeline commands (`##vso[`, `##[`), template markers (`{{`), and
148+
/// newlines. A combined check for fields embedded into YAML templates.
140149
pub fn reject_pipeline_injection(value: &str, field_name: &str) -> Result<()> {
141150
if contains_ado_expression(value) {
142151
anyhow::bail!(
@@ -146,6 +155,22 @@ pub fn reject_pipeline_injection(value: &str, field_name: &str) -> Result<()> {
146155
value,
147156
);
148157
}
158+
if contains_pipeline_command(value) {
159+
anyhow::bail!(
160+
"Front matter '{}' contains an ADO pipeline command ('##vso[' or '##[') which is not allowed. \
161+
Pipeline commands could manipulate pipeline behavior. Found: '{}'",
162+
field_name,
163+
value,
164+
);
165+
}
166+
if contains_template_marker(value) {
167+
anyhow::bail!(
168+
"Front matter '{}' contains a template marker delimiter '{{{{' which is not allowed. \
169+
Template markers could cause second-order injection into the generated pipeline. Found: '{}'",
170+
field_name,
171+
value,
172+
);
173+
}
149174
if contains_newline(value) {
150175
anyhow::bail!(
151176
"Front matter '{}' must be a single line (no newlines). \
@@ -481,6 +506,15 @@ mod tests {
481506
assert!(!contains_newline("single line"));
482507
}
483508

509+
#[test]
510+
fn test_contains_template_marker() {
511+
assert!(contains_template_marker("{{ agent_content }}"));
512+
assert!(contains_template_marker("prefix {{ something }} suffix"));
513+
assert!(contains_template_marker("{{no_spaces}}"));
514+
assert!(!contains_template_marker("normal text"));
515+
assert!(!contains_template_marker("just a single { brace"));
516+
}
517+
484518
#[test]
485519
fn test_reject_ado_expressions() {
486520
assert!(reject_ado_expressions("normal value", "param", "field").is_ok());
@@ -494,6 +528,10 @@ mod tests {
494528
assert!(reject_pipeline_injection("normal value", "field").is_ok());
495529
assert!(reject_pipeline_injection("$(SYSTEM_ACCESSTOKEN)", "field").is_err());
496530
assert!(reject_pipeline_injection("value\ninjected", "field").is_err());
531+
assert!(reject_pipeline_injection("{{ agent_content }}", "field").is_err());
532+
assert!(reject_pipeline_injection("{{ copilot_params }}", "field").is_err());
533+
assert!(reject_pipeline_injection("##vso[task.setvariable]x", "field").is_err());
534+
assert!(reject_pipeline_injection("##[section]foo", "field").is_err());
497535
}
498536

499537
// ── DNS domain validators ───────────────────────────────────────────

0 commit comments

Comments
 (0)