Skip to content

fix: block template marker delimiters in front matter identity fields#315

Merged
jamesadevine merged 4 commits intomainfrom
copilot/fix-second-order-template-injection
Apr 24, 2026
Merged

fix: block template marker delimiters in front matter identity fields#315
jamesadevine merged 4 commits intomainfrom
copilot/fix-second-order-template-injection

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 24, 2026

Second-order template injection via name: "{{ agent_content }}". The sequential replacement fold in compile_shared processes {{ agent_name }} before {{ agent_content }}, so a name containing a template marker becomes a live marker that gets expanded on the next pass — injecting the entire markdown body (attacker-controlled YAML) into the pipeline as an unquoted top-level scalar. Injected jobs: blocks run outside AWF sandbox with unrestricted network and token access.

Changes

  • src/validate.rs: Added contains_template_marker() detecting {{ in values. Integrated into reject_pipeline_injection() so all fields embedded into YAML templates (name, description, trigger fields) are covered. Also added the missing contains_pipeline_command() check to reject_pipeline_injection() so the function body now matches its docstring's claim of ##vso[/##[ protection. All error messages now include the offending value (Found: '{}') for debuggability.
  • src/compile/common.rs: Updated validate_front_matter_identity doc comment. Added tests for template marker rejection in both name and description.

Copilot AI and others added 2 commits April 24, 2026 10:12
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>
Copilot AI changed the title [WIP] Fix second-order template injection via name field fix: block template marker delimiters in front matter identity fields Apr 24, 2026
Copilot AI requested a review from jamesadevine April 24, 2026 10:18
@jamesadevine
Copy link
Copy Markdown
Collaborator

/rust-review

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 2026

Rust PR Reviewer completed successfully!

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: Fix is correct and well-targeted — good to merge with one minor note.

Findings

🐛 Bugs / Logic Issues

  • src/validate.rs:147-148 — docstring claims ##vso[ / ##[ protection that isn't implemented

    The doc comment for reject_pipeline_injection was updated to say it rejects "pipeline commands (##vso[, ##[)", but the function body never calls contains_pipeline_command. This was a pre-existing gap, but the updated docstring makes the discrepancy more prominent. Either the check should be added, or the docstring should be corrected to not claim that protection.

    // Docstring says this is covered, but this call is missing:
    if contains_pipeline_command(value) {
        anyhow::bail!("Front matter '{}' contains a pipeline command ...", field_name);
    }

⚠️ Suggestions

  • src/validate.rs:158-163 — template marker error omits the offending value

    The ADO expression error includes Found: '{}' for debuggability, but the new template marker error does not. When a user gets this error from automated tooling (e.g., a CI compile step), it helps to know what value triggered it.

    // Consistent with the ADO expression message above it:
    "Front matter '{}' contains a template marker delimiter '\{\{\{\{' which is not allowed. \
     Template markers could cause second-order injection into the generated pipeline. Found: '{}'",
    field_name,
    value,  // <-- add this

✅ What Looks Good

  • The vulnerability is real and the fix is correctly targeted. The fold in compile_shared is sequential — \{\{ agent_name }} is replaced at step 2 and \{\{ agent_content }} at step 13, so a name containing \{\{ agent_content }} really would get expanded on the later pass. The \{\{ check short-circuits this cleanly.
  • contains_ado_expression / contains_template_marker ordering is correct. Since $\{\{ contains \{\{, the ADO check fires first for ADO expressions; the template marker check only catches bare \{\{ — no double-rejection or missed case.
  • Tests are thoroughcontains_template_marker unit tests cover with/without spaces and both single and double braces; reject_pipeline_injection integration cases and the two new validate_front_matter_identity tests all exercise the right paths.
  • Scope is appropriatetriggers.pipeline.name/project/branches already went through reject_pipeline_injection, so they inherit the new protection automatically without any extra changes needed.

Generated by Rust PR Reviewer for issue #315 · ● 987.6K ·

@jamesadevine
Copy link
Copy Markdown
Collaborator

@copilot Implement feedback bugs and suggestions!

…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>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 24, 2026

@copilot Implement feedback bugs and suggestions!

Both items addressed in 5add8a3:

  1. Bug fix: Added the missing contains_pipeline_command() check to reject_pipeline_injection() so the function body now matches its docstring — ##vso[ and ##[ are rejected.
  2. Suggestion: Template marker error now includes Found: '{}' with the offending value, consistent with the ADO expression error above it.

Also added test cases for pipeline command rejection in test_reject_pipeline_injection.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dev.azure.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-6cf427a7041718b7 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-6cf427a7041718b7 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-1719f97d5e45e437.1055o8a2vb7q8e48jeeilmr1y.10gzb9l.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-1719f97d5e45e437.11n7zdmwuhn57008tex8tsp3r.10gzb9l.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-1719f97d5e45e437.15kji6znvqd3x991dnpwqn7ya.10gzb9l.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-1719f97d5e45e437.187rd0ayf2q2llc7o4kvnpi6r.10gzb9l.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-1719f97d5e45e437.1e88zc8hdxev6ttq0lkfpdgf7.10gzb9l.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-1719f97d5e45e437.1qge80p9w13oh0fd84mu9zgeb.10gzb9l.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustchbfEC7/rmeta.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.3/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-1719f97d5e45e437.22odgfbovno51kcxcg2igtda7.10gzb9l.rcgu.o -Wl,--as-needed -Wl,-Bstatic /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/libsyn-1500b8e8e9aaac8c.rlib b lib/rustlib/x86_/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/thiserror_impl-a69babd28d2d3b2cc .rlib lib/rustlib/x86_64-REDACTED-linux-gnu/bin/self-contained/cc s/tracing_attribcc s/tracing_attrib-Wl,--version-script=/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustchbfE/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-1719f97d5e45e437.02fvb9nh78bmtwuc43t06a8zo.10gzb9l.rcgu.o s/tracing_attrib-Wl,--no-undefined-version s/tracing_attrib-m64 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from jamesadevine April 24, 2026 10:31
@jamesadevine jamesadevine merged commit 2575246 into main Apr 24, 2026
13 checks passed
@jamesadevine jamesadevine deleted the copilot/fix-second-order-template-injection branch April 24, 2026 10:41
This was referenced Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🔴 Red Team Audit — High: Second-order template injection via name field injects arbitrary jobs into pipeline YAML

2 participants