Skip to content

fix(safeoutputs): block VSO command injection via repository alias across Stage 3 PR-safe-output executors#482

Merged
jamesadevine merged 4 commits intomainfrom
copilot/fix-vso-command-injection-another-one
May 9, 2026
Merged

fix(safeoutputs): block VSO command injection via repository alias across Stage 3 PR-safe-output executors#482
jamesadevine merged 4 commits intomainfrom
copilot/fix-vso-command-injection-another-one

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 9, 2026

Summary

A red-team finding showed that repository aliases could carry ##vso[...] payloads into Stage 3 failure/log messages, allowing Azure DevOps command execution from executor stdout. This PR hardens both Stage 1 validation and Stage 3 sanitization for repository selectors across the affected and related PR-safe-output tools.

  • Stage 3 sanitization hardening

    • Applied sanitize_config(...) to repository in:
      • add_pr_comment
      • reply_to_pr_review_comment
      • create_branch
      • create_git_tag
      • create_pr (broadened pass)
  • Stage 1 input rejection

    • Added reject_pipeline_injection(..., "repository") in Validate::validate() for:
      • add_pr_comment
      • reply_to_pr_review_comment
      • create_branch
      • create_git_tag
      • create_pr (broadened pass)
      • submit_pr_review (broadened pass)
      • resolve_pr_thread (broadened pass)
      • update_pr (broadened pass)
  • Coverage additions

    • Added focused unit tests in newly touched modules to verify:
      • repository pipeline-command payloads are rejected during validation
      • repository values are neutralized during Stage 3 sanitization where applicable
if let Some(repository) = &self.repository {
    reject_pipeline_injection(repository, "repository")?;
}

self.repository = self.repository.as_deref().map(sanitize_config);

Test plan

  • Ran focused unit tests for all touched safe-output modules, including:
    • add_pr_comment
    • reply_to_pr_comment
    • create_branch
    • create_git_tag
    • create_pr
    • submit_pr_review
    • resolve_pr_thread
    • update_pr
  • Ran full repository validation:
    • cargo build
    • cargo test
    • cargo clippy --all-targets --all-features

Copilot AI and others added 2 commits May 9, 2026 07:09
…peline commands

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/349cfa4f-c108-45df-989a-99e9468287d2

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix VSO command injection via unsanitized repository alias fix(safeoutputs): block VSO command injection via repository alias in Stage 3 executors May 9, 2026
Copilot AI requested a review from jamesadevine May 9, 2026 07:14
Copy link
Copy Markdown
Collaborator

@jamesadevine jamesadevine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/rust-review

@jamesadevine jamesadevine marked this pull request as ready for review May 9, 2026 07:36
@jamesadevine
Copy link
Copy Markdown
Collaborator

/rust-review

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

Rust PR Reviewer completed successfully!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

🔍 Rust PR Review

Summary: Fix is correct and well-structured, but incomplete — four more tools with repository fields are missing the same reject_pipeline_injection guard added here.

Findings

🐛 Bugs / Logic Issues

Incomplete fix — sibling tools still vulnerable at Stage 1

The PR patches four tools (add_pr_comment, reply_to_pr_comment, create_branch, create_git_tag) but four more tools expose an identical repository: Option<String> field with the same Stage 3 failure-message echo surface:

File sanitize_config in Stage 3? reject_pipeline_injection in Stage 1?
src/safeoutputs/resolve_pr_thread.rs ❌ missing
src/safeoutputs/submit_pr_review.rs ❌ missing
src/safeoutputs/update_pr.rs ❌ missing
src/safeoutputs/create_pr.rs ❌ missing ❌ missing

create_pr.rs is the most significant gap: its sanitize_content_fields doesn't call sanitize_config on self.repository at all (only title, description, and agent_labels are sanitized), so a ##vso[...] payload in repository passes through both stages entirely unhandled.

Example of the missing guard (same pattern as the PR):

// In Validate for ResolvePrThreadParams / SubmitPrReviewParams / UpdatePrParams
if let Some(repository) = &self.repository {
    reject_pipeline_injection(repository, "repository")?;
}

// In CreatePrParams validate (repository is Option<String>):
if let Some(repository) = &self.repository {
    reject_pipeline_injection(repository, "repository")?;
}

// In SanitizeContent for CreatePrResult:
self.repository = self.repository.as_deref().map(sanitize_config);

✅ What Looks Good

  • The two-layer defense (Stage 1 reject_pipeline_injection + Stage 3 sanitize_config) is the right model and correctly implemented for the four patched tools.
  • Replacing the bare filter(|c| !c.is_control()) with sanitize_config in add_pr_comment and create_git_tag is a positive hygiene improvement — sanitize_config covers VSO command neutralization, not just control character stripping.
  • Tests correctly validate both layers independently (validation rejection via params.try_into(), sanitization neutralization via direct struct construction). The assertion against `##vso[` precisely matches the expected sanitize_config output format.
  • Error propagation uses ? and anyhow throughout — consistent with project conventions.

Generated by Rust PR Reviewer for issue #482 · ● 584.3K ·

@jamesadevine
Copy link
Copy Markdown
Collaborator

@copilot Apply the approach more broadly as suggested.

…fe-output tools

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/b7312f9d-b619-4ed9-8c59-6d3a96a7c7d8

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 9, 2026

@copilot Apply the approach more broadly as suggested.

Applied the broader hardening pass in 2c8a6e7: added reject_pipeline_injection repository validation to the remaining PR-related safe-output tools (submit_pr_review, update_pr, and the broader pass includes create_pr/resolve_pr_thread), plus repository sanitization coverage where needed and focused tests.

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:

  • spsprodeus21.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.09htbv/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.0u7509smcuwo5os4ju77q7jeo.0sbksg1.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.0akb96/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.0zzv2mhl9y0uspvvkk1en9z6h.0sbksg1.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.0cnohc/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.123w8riw2xk2gb4akpd78tyy0.0sbksg1.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.0j9gwi/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.1ipnyscq7xzqct01izgeqc668.0sbksg1.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.0kg5ly/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.1jflsllpbaz4ftramkubaw55h.0sbksg1.rcgu.o ir1gbquroop6wnzt53cddm.0ore2kd.rcgu.o 0abv8afwjggt1a8w0bf5rl.0ore2kd.rcgu.o wgj25py8baros3o1db8zml.0ore2kd.rcgu.o 5bv2lkcooukxozii8fsel7.0ore2kd.rcgu.o rd9ienqxxlyub6qkl412u5.0ore2kd.rcgu.o prmown4krihroqnb005g0q.0ore2kd.rcgu.o m154ngj1frih3f70ogqvha.0ore2kd.rcgu.o q19goibqf191irb3tyysvl.0ore2kd.rcgu.o u5hqo361ey62ur7f0myy3b.0ore2kd.rcgu.o qqbd3cersmtofqse5qhmhp.0ore2kd.rcgu.o ztk95xrasgyhqmum1btr35.0ore2kd.rcgu.o vleynuk2eeivkgq01srag5.0ore2kd.rcgu.o nzak5ij2p42gq44or3qp7m.0ore2kd.rcgu.o k68dzxh5tdc81a2fhjffqn.0ore2kd.rcgu.o (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb --cfg feature=&#34;default&#34; --cfg targ�� target/debug/dep--error-format=json ihzhw6anxkluncgq1ej.1gq4sej.rcgu.o l8fq8uxu7sxlcj3uzz0.1gq4sej.rcgu.o lo42qr131vmmjx863r4.1gq4sej.rcgu.o fu2kzxwsgg2hxzecru4.1gq4sej.rcgu.o 6qib85zurymf8oiht59.1gq4sej.rcgu.o raugkjjflbqa3j7ums0.1gq4sej.rcgu.o favt5d2vuuw2klw0gna.1gq4sej.rcgu.o wt1c3n5jo2ym7wi9j4w.1gq4sej.rcgu.o dw2p9o4xd46w4jl8z7r.1gq4sej.rcgu.o 80d6akzm4hj0vtkirae.1gq4sej.rcgu.o i4hlrkroyeytgk5f1ob.1gq4sej.rcgu.o k0nl4yhvcdkhqydgtkn.1gq4sej.rcgu.o ccopntrxq7tkt1sleh2.1gq4sej.rcgu.o whh27ujzmz85qzhc7v6.1gq4sej.rcgu.o (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb src/probe/proc_macro_span_file.rs --target x86_64-REDACTED-linux-gnu ug/d�� NbEvu/raw-dylibs 6_64-REDACTED-linux-gnu/lib lib/rustlib/x86_64-REDACTED-linux-gnu/bin/rust-lld ug/deps/async_trrustc a53ca7aa2edea-cgu.0.rcgu.o a53ca7aa2edea-cgu.1.rcgu.o k0g.rcgu.o f6dc�� ba3e0fd.rlib ug/deps/libsyn-1x86_64-REDACTED-linux-gnu rking/target/deb--crate-type rking/target/deb/home/REDACTED/.rustup/toolchains/stable-x86_64-REDACTED-linux-gnu/bin/rustc rking/target/deb--cfg=procmacro2_build_probe rking/target/deb--edition=2021 cgu.00.rcgu.o (dns block)
  • spsprodweu4.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.09htbv/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.0u7509smcuwo5os4ju77q7jeo.0sbksg1.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.0akb96/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.0zzv2mhl9y0uspvvkk1en9z6h.0sbksg1.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.0cnohc/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.123w8riw2xk2gb4akpd78tyy0.0sbksg1.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.0j9gwi/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.1ipnyscq7xzqct01izgeqc668.0sbksg1.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-3d482cacf903b1b1.0kg5ly/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/compiler_tests-6af687e078bb5aea.1jflsllpbaz4ftramkubaw55h.0sbksg1.rcgu.o ir1gbquroop6wnzt53cddm.0ore2kd.rcgu.o 0abv8afwjggt1a8w0bf5rl.0ore2kd.rcgu.o wgj25py8baros3o1db8zml.0ore2kd.rcgu.o 5bv2lkcooukxozii8fsel7.0ore2kd.rcgu.o rd9ienqxxlyub6qkl412u5.0ore2kd.rcgu.o prmown4krihroqnb005g0q.0ore2kd.rcgu.o m154ngj1frih3f70ogqvha.0ore2kd.rcgu.o q19goibqf191irb3tyysvl.0ore2kd.rcgu.o u5hqo361ey62ur7f0myy3b.0ore2kd.rcgu.o qqbd3cersmtofqse5qhmhp.0ore2kd.rcgu.o ztk95xrasgyhqmum1btr35.0ore2kd.rcgu.o vleynuk2eeivkgq01srag5.0ore2kd.rcgu.o nzak5ij2p42gq44or3qp7m.0ore2kd.rcgu.o k68dzxh5tdc81a2fhjffqn.0ore2kd.rcgu.o (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb --cfg feature=&#34;default&#34; --cfg targ�� target/debug/dep--error-format=json ihzhw6anxkluncgq1ej.1gq4sej.rcgu.o l8fq8uxu7sxlcj3uzz0.1gq4sej.rcgu.o lo42qr131vmmjx863r4.1gq4sej.rcgu.o fu2kzxwsgg2hxzecru4.1gq4sej.rcgu.o 6qib85zurymf8oiht59.1gq4sej.rcgu.o raugkjjflbqa3j7ums0.1gq4sej.rcgu.o favt5d2vuuw2klw0gna.1gq4sej.rcgu.o wt1c3n5jo2ym7wi9j4w.1gq4sej.rcgu.o dw2p9o4xd46w4jl8z7r.1gq4sej.rcgu.o 80d6akzm4hj0vtkirae.1gq4sej.rcgu.o i4hlrkroyeytgk5f1ob.1gq4sej.rcgu.o k0nl4yhvcdkhqydgtkn.1gq4sej.rcgu.o ccopntrxq7tkt1sleh2.1gq4sej.rcgu.o whh27ujzmz85qzhc7v6.1gq4sej.rcgu.o (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-9143a4cd0b5c71cb src/probe/proc_macro_span_file.rs --target x86_64-REDACTED-linux-gnu ug/d�� NbEvu/raw-dylibs 6_64-REDACTED-linux-gnu/lib lib/rustlib/x86_64-REDACTED-linux-gnu/bin/rust-lld ug/deps/async_trrustc a53ca7aa2edea-cgu.0.rcgu.o a53ca7aa2edea-cgu.1.rcgu.o k0g.rcgu.o f6dc�� ba3e0fd.rlib ug/deps/libsyn-1x86_64-REDACTED-linux-gnu rking/target/deb--crate-type rking/target/deb/home/REDACTED/.rustup/toolchains/stable-x86_64-REDACTED-linux-gnu/bin/rustc rking/target/deb--cfg=procmacro2_build_probe rking/target/deb--edition=2021 cgu.00.rcgu.o (dns block)

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

Copilot AI changed the title fix(safeoutputs): block VSO command injection via repository alias in Stage 3 executors fix(safeoutputs): block VSO command injection via repository alias across Stage 3 PR-safe-output executors May 9, 2026
Copilot AI requested a review from jamesadevine May 9, 2026 08:09
@jamesadevine jamesadevine merged commit 6f4a6dd into main May 9, 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: VSO command injection via unsanitized repository alias in Stage 3 safe output executors

2 participants