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

Author: Copilot

src/safeoutputs/add_pr_comment.rs

@@ -7,9 +7,10 @@ use serde::{Deserialize, Serialize};
 
 use super::PATH_SEGMENT;
 use ado_aw_derive::SanitizeConfig;
-use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
+use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
 use crate::tool_result;
 use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
+use crate::validate::reject_pipeline_injection;
 use anyhow::{Context, ensure};
 
 /// Parameters for adding a comment thread on a pull request
@@ -89,6 +90,7 @@ impl Validate for AddPrCommentParams {
         if let Some(fp) = &self.file_path {
             validate_file_path(fp)?;
         }
+        reject_pipeline_injection(&self.repository, "repository")?;
         Ok(())
     }
 }
@@ -112,8 +114,8 @@ tool_result! {
 impl SanitizeContent for AddPrCommentResult {
     fn sanitize_content_fields(&mut self) {
         self.content = sanitize_text(&self.content);
-        // Strip control characters from structural fields for defense-in-depth
-        self.repository = self.repository.chars().filter(|c| !c.is_control()).collect();
+        self.repository = sanitize_config(&self.repository);
+        // Strip control characters from remaining structural fields for defense-in-depth
         self.status = self.status.chars().filter(|c| !c.is_control()).collect();
         self.file_path = self.file_path.as_ref().map(|fp| {
             fp.chars().filter(|c| !c.is_control()).collect()
@@ -463,6 +465,21 @@ mod tests {
         assert!(result.is_err());
     }
 
+    #[test]
+    fn test_validation_rejects_repository_pipeline_command() {
+        let params = AddPrCommentParams {
+            pull_request_id: 42,
+            content: "This is a valid comment body text.".to_string(),
+            repository: "##vso[task.setvariable variable=x]y".to_string(),
+            file_path: None,
+            start_line: None,
+            line: None,
+            status: "active".to_string(),
+        };
+        let result: Result<AddPrCommentResult, _> = params.try_into();
+        assert!(result.is_err());
+    }
+
     #[test]
     fn test_validation_rejects_line_without_file_path() {
         let params = AddPrCommentParams {
@@ -632,4 +649,33 @@ allowed-statuses:
             "uppercase 'Active' should match config value 'active'"
         );
     }
+
+    #[test]
+    fn test_sanitize_content_neutralizes_repository_pipeline_command() {
+        let params = AddPrCommentParams {
+            pull_request_id: 42,
+            content: "This is a valid comment body text.".to_string(),
+            repository: "##vso[task.setvariable variable=x]y".to_string(),
+            file_path: None,
+            start_line: None,
+            line: None,
+            status: "active".to_string(),
+        };
+        let mut result = AddPrCommentResult {
+            name: "add-pr-comment".to_string(),
+            pull_request_id: params.pull_request_id,
+            content: params.content,
+            repository: params.repository,
+            file_path: params.file_path,
+            start_line: params.start_line,
+            line: params.line,
+            status: params.status,
+        };
+        result.sanitize_content_fields();
+        assert!(
+            result.repository.contains("`##vso[`"),
+            "repository pipeline command should be neutralized with backticks: {}",
+            result.repository
+        );
+    }
 }

src/safeoutputs/create_branch.rs

@@ -10,6 +10,7 @@ use super::{PATH_SEGMENT, validate_git_ref_name};
 use crate::sanitize::{SanitizeContent, sanitize_config};
 use crate::tool_result;
 use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
+use crate::validate::reject_pipeline_injection;
 use anyhow::{Context, ensure};
 
 /// Parameters for creating a branch
@@ -71,6 +72,9 @@ impl Validate for CreateBranchParams {
             );
             validate_git_ref_name(branch, "source_branch")?;
         }
+        if let Some(repository) = &self.repository {
+            reject_pipeline_injection(repository, "repository")?;
+        }
 
         Ok(())
     }
@@ -92,6 +96,7 @@ tool_result! {
 impl SanitizeContent for CreateBranchResult {
     fn sanitize_content_fields(&mut self) {
         self.branch_name = sanitize_config(&self.branch_name);
+        self.repository = self.repository.as_deref().map(sanitize_config);
     }
 }
 
@@ -459,6 +464,18 @@ mod tests {
         assert!(result.is_err());
     }
 
+    #[test]
+    fn test_validation_rejects_repository_pipeline_command() {
+        let params = CreateBranchParams {
+            branch_name: "feature/new-work".to_string(),
+            source_branch: Some("main".to_string()),
+            source_commit: None,
+            repository: Some("##vso[task.setvariable variable=x]y".to_string()),
+        };
+        let result: Result<CreateBranchResult, _> = params.try_into();
+        assert!(result.is_err());
+    }
+
     #[test]
     fn test_validation_rejects_branch_starting_with_dash() {
         let params = CreateBranchParams {
@@ -546,4 +563,22 @@ allowed-source-branches:
         assert_eq!(config.allowed_repositories, vec!["self", "other-repo"]);
         assert_eq!(config.allowed_source_branches, vec!["main", "develop"]);
     }
+
+    #[test]
+    fn test_sanitize_content_neutralizes_repository_pipeline_command() {
+        let mut result = CreateBranchResult {
+            name: "create-branch".to_string(),
+            branch_name: "feature/new-work".to_string(),
+            source_branch: Some("main".to_string()),
+            source_commit: None,
+            repository: Some("##vso[task.setvariable variable=x]y".to_string()),
+        };
+        result.sanitize_content_fields();
+        let repository = result.repository.as_deref().unwrap_or("");
+        assert!(
+            repository.contains("`##vso[`"),
+            "repository pipeline command should be neutralized with backticks: {}",
+            repository
+        );
+    }
 }

src/safeoutputs/create_git_tag.rs

@@ -7,9 +7,10 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
 use super::{PATH_SEGMENT, validate_git_ref_name};
-use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
+use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
 use crate::tool_result;
 use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
+use crate::validate::reject_pipeline_injection;
 use anyhow::{Context, ensure};
 
 /// Parameters for creating a git tag (agent-provided)
@@ -75,6 +76,9 @@ impl Validate for CreateGitTagParams {
                 "message must be at least 5 characters"
             );
         }
+        if let Some(repository) = &self.repository {
+            reject_pipeline_injection(repository, "repository")?;
+        }
 
         Ok(())
     }
@@ -106,9 +110,7 @@ impl SanitizeContent for CreateGitTagResult {
         self.commit = self.commit.as_ref().map(|c| {
             c.chars().filter(|ch| !ch.is_control()).collect()
         });
-        self.repository = self.repository.as_ref().map(|r| {
-            r.chars().filter(|ch| !ch.is_control()).collect()
-        });
+        self.repository = self.repository.as_deref().map(sanitize_config);
     }
 }
 
@@ -477,6 +479,18 @@ mod tests {
         assert!(result.is_err());
     }
 
+    #[test]
+    fn test_validation_rejects_repository_pipeline_command() {
+        let params = CreateGitTagParams {
+            tag_name: "v1.0.0".to_string(),
+            commit: None,
+            message: Some("Valid message".to_string()),
+            repository: Some("##vso[task.setvariable variable=x]y".to_string()),
+        };
+        let result: Result<CreateGitTagResult, _> = params.try_into();
+        assert!(result.is_err());
+    }
+
     #[test]
     fn test_result_serializes_correctly() {
         let params = CreateGitTagParams {
@@ -517,4 +531,22 @@ message-prefix: "[release] "
         assert_eq!(config.allowed_repositories, vec!["self", "my-lib"]);
         assert_eq!(config.message_prefix.as_deref(), Some("[release] "));
     }
+
+    #[test]
+    fn test_sanitize_content_neutralizes_repository_pipeline_command() {
+        let mut result = CreateGitTagResult {
+            name: "create-git-tag".to_string(),
+            tag_name: "v1.0.0".to_string(),
+            commit: None,
+            message: Some("Valid message".to_string()),
+            repository: Some("##vso[task.setvariable variable=x]y".to_string()),
+        };
+        result.sanitize_content_fields();
+        let repository = result.repository.as_deref().unwrap_or("");
+        assert!(
+            repository.contains("`##vso[`"),
+            "repository pipeline command should be neutralized with backticks: {}",
+            repository
+        );
+    }
 }

src/safeoutputs/create_pr.rs

@@ -9,6 +9,7 @@ use ado_aw_derive::SanitizeConfig;
 use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, ToolResult, Validate};
 use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
 use crate::tool_result;
+use crate::validate::reject_pipeline_injection;
 use anyhow::{Context, ensure};
 
 /// Maximum allowed patch file size (5 MB)
@@ -254,6 +255,9 @@ impl Validate for CreatePrParams {
             self.description.len() >= 10,
             "PR description must be at least 10 characters"
         );
+        if let Some(repository) = &self.repository {
+            reject_pipeline_injection(repository, "repository")?;
+        }
         Ok(())
     }
 }
@@ -320,6 +324,7 @@ impl SanitizeContent for CreatePrResult {
     fn sanitize_content_fields(&mut self) {
         self.title = sanitize_text(&self.title);
         self.description = sanitize_text(&self.description);
+        self.repository = sanitize_config(&self.repository);
         for label in &mut self.agent_labels {
             *label = sanitize_config(label);
         }
@@ -2092,6 +2097,37 @@ mod tests {
         assert!(params.validate().is_err());
     }
 
+    #[test]
+    fn test_validate_params_rejects_repository_pipeline_command() {
+        let params = CreatePrParams {
+            title: "Fix bug in parser".to_string(),
+            description: "This PR fixes a critical bug in the parser module.".to_string(),
+            repository: Some("##vso[task.setvariable variable=x]y".to_string()),
+            labels: vec![],
+        };
+        assert!(params.validate().is_err());
+    }
+
+    #[test]
+    fn test_sanitize_content_neutralizes_repository_pipeline_command() {
+        let mut result = CreatePrResult::new(
+            "Fix bug in parser".to_string(),
+            "This PR fixes a critical bug in the parser module.".to_string(),
+            "agent/fix-parser".to_string(),
+            "/tmp/test.patch".to_string(),
+            "##vso[task.setvariable variable=x]y".to_string(),
+            vec![],
+            None,
+            "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef".to_string(),
+        );
+        result.sanitize_content_fields();
+        assert!(
+            result.repository.contains("`##vso[`"),
+            "repository pipeline command should be neutralized with backticks: {}",
+            result.repository
+        );
+    }
+
     #[test]
     fn test_config_default_target_branch() {
         let config = CreatePrConfig::default();

src/safeoutputs/reply_to_pr_comment.rs

@@ -7,9 +7,10 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
 use super::PATH_SEGMENT;
-use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
+use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
 use crate::tool_result;
 use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
+use crate::validate::reject_pipeline_injection;
 use anyhow::{Context, ensure};
 
 /// Parameters for replying to an existing review comment thread on a pull request
@@ -42,6 +43,9 @@ impl Validate for ReplyToPrCommentParams {
             self.content.len() >= 10,
             "content must be at least 10 characters"
         );
+        if let Some(repository) = &self.repository {
+            reject_pipeline_injection(repository, "repository")?;
+        }
         Ok(())
     }
 }
@@ -62,6 +66,7 @@ tool_result! {
 impl SanitizeContent for ReplyToPrCommentResult {
     fn sanitize_content_fields(&mut self) {
         self.content = sanitize_text(&self.content);
+        self.repository = self.repository.as_deref().map(sanitize_config);
     }
 }
 
@@ -318,6 +323,18 @@ mod tests {
         assert!(result.is_err());
     }
 
+    #[test]
+    fn test_validation_rejects_repository_pipeline_command() {
+        let params = ReplyToPrCommentParams {
+            pull_request_id: 42,
+            thread_id: 7,
+            content: "This is a valid reply body text.".to_string(),
+            repository: Some("##vso[task.setvariable variable=x]y".to_string()),
+        };
+        let result: Result<ReplyToPrCommentResult, _> = params.try_into();
+        assert!(result.is_err());
+    }
+
     #[test]
     fn test_result_serializes_correctly() {
         let params = ReplyToPrCommentParams {
@@ -353,4 +370,22 @@ allowed-repositories:
         assert_eq!(config.comment_prefix, Some("[Agent] ".to_string()));
         assert_eq!(config.allowed_repositories, vec!["self", "other-repo"]);
     }
+
+    #[test]
+    fn test_sanitize_content_neutralizes_repository_pipeline_command() {
+        let mut result = ReplyToPrCommentResult {
+            name: "reply-to-pr-review-comment".to_string(),
+            pull_request_id: 42,
+            thread_id: 7,
+            content: "This is a valid reply body text.".to_string(),
+            repository: Some("##vso[task.setvariable variable=x]y".to_string()),
+        };
+        result.sanitize_content_fields();
+        let repository = result.repository.as_deref().unwrap_or("");
+        assert!(
+            repository.contains("`##vso[`"),
+            "repository pipeline command should be neutralized with backticks: {}",
+            repository
+        );
+    }
 }