From 69aaa9b3d94d5f4fa78d00f7d39a441b8e9f873c Mon Sep 17 00:00:00 2001 From: James Devine Date: Thu, 7 May 2026 11:39:59 +0100 Subject: [PATCH] fix(safeoutputs): use sanitize_config for identifier fields instead of sanitize_text sanitize_text() wraps @-signs in backticks and escapes HTML, which corrupts identifier fields like tags, branch names, labels, and repo names. Switch all identifier fields across 10 safe-output files from sanitize_text() to sanitize_config(), which only strips control characters, neutralizes pipeline commands, and enforces size limits. Rich-text fields (title, description, body, content, comment, reason) remain on sanitize_text(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/safeoutputs/add_build_tag.rs | 4 ++-- src/safeoutputs/create_branch.rs | 4 ++-- src/safeoutputs/create_pr.rs | 4 ++-- src/safeoutputs/create_work_item.rs | 4 ++-- src/safeoutputs/link_work_items.rs | 4 ++-- src/safeoutputs/queue_build.rs | 6 +++--- src/safeoutputs/resolve_pr_thread.rs | 6 +++--- src/safeoutputs/submit_pr_review.rs | 6 +++--- src/safeoutputs/update_pr.rs | 12 ++++++------ src/safeoutputs/update_work_item.rs | 18 +++++++++--------- 10 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/safeoutputs/add_build_tag.rs b/src/safeoutputs/add_build_tag.rs index 65e35bfa..db17e06d 100644 --- a/src/safeoutputs/add_build_tag.rs +++ b/src/safeoutputs/add_build_tag.rs @@ -7,7 +7,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use super::PATH_SEGMENT; -use crate::sanitize::{SanitizeContent, sanitize as sanitize_text}; +use crate::sanitize::{SanitizeContent, sanitize_config}; use crate::tool_result; use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; @@ -58,7 +58,7 @@ tool_result! { impl SanitizeContent for AddBuildTagResult { fn sanitize_content_fields(&mut self) { - self.tag = sanitize_text(&self.tag); + self.tag = sanitize_config(&self.tag); } } diff --git a/src/safeoutputs/create_branch.rs b/src/safeoutputs/create_branch.rs index aea7756f..82f67c4e 100644 --- a/src/safeoutputs/create_branch.rs +++ b/src/safeoutputs/create_branch.rs @@ -7,7 +7,7 @@ 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_config}; use crate::tool_result; use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; @@ -91,7 +91,7 @@ tool_result! { impl SanitizeContent for CreateBranchResult { fn sanitize_content_fields(&mut self) { - self.branch_name = sanitize_text(&self.branch_name); + self.branch_name = sanitize_config(&self.branch_name); } } diff --git a/src/safeoutputs/create_pr.rs b/src/safeoutputs/create_pr.rs index 4b26613d..1909b4b3 100644 --- a/src/safeoutputs/create_pr.rs +++ b/src/safeoutputs/create_pr.rs @@ -7,7 +7,7 @@ use tokio::process::Command; use ado_aw_derive::SanitizeConfig; use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, ToolResult, Validate}; -use crate::sanitize::{SanitizeContent, sanitize as sanitize_text}; +use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config}; use crate::tool_result; use anyhow::{Context, ensure}; @@ -321,7 +321,7 @@ impl SanitizeContent for CreatePrResult { self.title = sanitize_text(&self.title); self.description = sanitize_text(&self.description); for label in &mut self.agent_labels { - *label = sanitize_text(label); + *label = sanitize_config(label); } } } diff --git a/src/safeoutputs/create_work_item.rs b/src/safeoutputs/create_work_item.rs index 86a2effb..5e0db165 100644 --- a/src/safeoutputs/create_work_item.rs +++ b/src/safeoutputs/create_work_item.rs @@ -9,7 +9,7 @@ use super::PATH_SEGMENT; use crate::tool_result; use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use ado_aw_derive::SanitizeConfig; -use crate::sanitize::{SanitizeContent, sanitize as sanitize_text}; +use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config}; use anyhow::{Context, ensure}; /// Parameters for creating a work item @@ -63,7 +63,7 @@ impl SanitizeContent for CreateWorkItemResult { self.title = sanitize_text(&self.title); self.description = sanitize_text(&self.description); for tag in &mut self.tags { - *tag = sanitize_text(tag); + *tag = sanitize_config(tag); } } } diff --git a/src/safeoutputs/link_work_items.rs b/src/safeoutputs/link_work_items.rs index d49dfefb..42cff8f3 100644 --- a/src/safeoutputs/link_work_items.rs +++ b/src/safeoutputs/link_work_items.rs @@ -7,7 +7,7 @@ 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::safeoutputs::comment_on_work_item::CommentTarget; @@ -94,7 +94,7 @@ tool_result! { impl SanitizeContent for LinkWorkItemsResult { fn sanitize_content_fields(&mut self) { - self.link_type = sanitize_text(&self.link_type); + self.link_type = sanitize_config(&self.link_type); self.comment = self.comment.as_deref().map(sanitize_text); } } diff --git a/src/safeoutputs/queue_build.rs b/src/safeoutputs/queue_build.rs index 3c7abb11..98522297 100644 --- a/src/safeoutputs/queue_build.rs +++ b/src/safeoutputs/queue_build.rs @@ -7,7 +7,7 @@ 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 anyhow::{Context, ensure}; @@ -71,13 +71,13 @@ impl SanitizeContent for QueueBuildResult { self.reason = Some(sanitize_text(reason)); } if let Some(branch) = &self.branch { - self.branch = Some(sanitize_text(branch)); + self.branch = Some(sanitize_config(branch)); } if let Some(params) = &self.parameters { self.parameters = Some( params .iter() - .map(|(k, v)| (sanitize_text(k), sanitize_text(v))) + .map(|(k, v)| (sanitize_config(k), sanitize_config(v))) .collect(), ); } diff --git a/src/safeoutputs/resolve_pr_thread.rs b/src/safeoutputs/resolve_pr_thread.rs index 3cd68c1c..10e291ed 100644 --- a/src/safeoutputs/resolve_pr_thread.rs +++ b/src/safeoutputs/resolve_pr_thread.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::resolve_repo_name; use super::PATH_SEGMENT; -use crate::sanitize::{SanitizeContent, sanitize as sanitize_text}; +use crate::sanitize::{SanitizeContent, sanitize_config}; use crate::tool_result; use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use anyhow::{Context, ensure}; @@ -89,9 +89,9 @@ tool_result! { impl SanitizeContent for ResolvePrThreadResult { fn sanitize_content_fields(&mut self) { - self.status = sanitize_text(&self.status); + self.status = sanitize_config(&self.status); if let Some(ref repo) = self.repository { - self.repository = Some(sanitize_text(repo)); + self.repository = Some(sanitize_config(repo)); } } } diff --git a/src/safeoutputs/submit_pr_review.rs b/src/safeoutputs/submit_pr_review.rs index b184eeb1..12f44615 100644 --- a/src/safeoutputs/submit_pr_review.rs +++ b/src/safeoutputs/submit_pr_review.rs @@ -7,7 +7,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use super::{PATH_SEGMENT, resolve_repo_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 anyhow::{Context, ensure}; @@ -93,9 +93,9 @@ tool_result! { impl SanitizeContent for SubmitPrReviewResult { fn sanitize_content_fields(&mut self) { - self.event = sanitize_text(&self.event); + self.event = sanitize_config(&self.event); self.body = self.body.as_deref().map(sanitize_text); - self.repository = self.repository.as_deref().map(sanitize_text); + self.repository = self.repository.as_deref().map(sanitize_config); } } diff --git a/src/safeoutputs/update_pr.rs b/src/safeoutputs/update_pr.rs index e22e6086..7b7e01c2 100644 --- a/src/safeoutputs/update_pr.rs +++ b/src/safeoutputs/update_pr.rs @@ -7,7 +7,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use super::{PATH_SEGMENT, resolve_repo_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 anyhow::{Context, ensure}; @@ -150,17 +150,17 @@ tool_result! { impl SanitizeContent for UpdatePrResult { fn sanitize_content_fields(&mut self) { - self.repository = self.repository.as_deref().map(sanitize_text); - self.operation = sanitize_text(&self.operation); + self.repository = self.repository.as_deref().map(sanitize_config); + self.operation = sanitize_config(&self.operation); self.reviewers = self .reviewers .as_ref() - .map(|rs| rs.iter().map(|r| sanitize_text(r)).collect()); + .map(|rs| rs.iter().map(|r| sanitize_config(r)).collect()); self.labels = self .labels .as_ref() - .map(|ls| ls.iter().map(|l| sanitize_text(l)).collect()); - self.vote = self.vote.as_deref().map(sanitize_text); + .map(|ls| ls.iter().map(|l| sanitize_config(l)).collect()); + self.vote = self.vote.as_deref().map(sanitize_config); self.description = self.description.as_deref().map(sanitize_text); } } diff --git a/src/safeoutputs/update_work_item.rs b/src/safeoutputs/update_work_item.rs index 504f100f..f0a6a349 100644 --- a/src/safeoutputs/update_work_item.rs +++ b/src/safeoutputs/update_work_item.rs @@ -9,7 +9,7 @@ use super::PATH_SEGMENT; use crate::tool_result; use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; use ado_aw_derive::SanitizeConfig; -use crate::sanitize::{SanitizeContent, sanitize as sanitize_text}; +use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config}; use anyhow::{Context, ensure}; /// Parameters for updating a work item @@ -92,14 +92,14 @@ impl SanitizeContent for UpdateWorkItemResult { fn sanitize_content_fields(&mut self) { self.title = self.title.as_deref().map(sanitize_text); self.body = self.body.as_deref().map(sanitize_text); - self.state = self.state.as_deref().map(sanitize_text); - self.area_path = self.area_path.as_deref().map(sanitize_text); - self.iteration_path = self.iteration_path.as_deref().map(sanitize_text); - self.assignee = self.assignee.as_deref().map(sanitize_text); + self.state = self.state.as_deref().map(sanitize_config); + self.area_path = self.area_path.as_deref().map(sanitize_config); + self.iteration_path = self.iteration_path.as_deref().map(sanitize_config); + self.assignee = self.assignee.as_deref().map(sanitize_config); self.tags = self .tags .as_ref() - .map(|ts| ts.iter().map(|t| sanitize_text(t)).collect()); + .map(|ts| ts.iter().map(|t| sanitize_config(t)).collect()); } } @@ -954,11 +954,11 @@ target: 42 let mut result: UpdateWorkItemResult = params.try_into().unwrap(); result.sanitize_content_fields(); - // @mentions should be neutralized + // @mentions should be neutralized in rich-text fields assert!(result.title.as_deref().unwrap().contains("`@user`")); - // tags should be sanitized + // tags are identifiers — @mentions should NOT be backtick-wrapped let tags = result.tags.as_ref().unwrap(); - assert!(tags[1].contains("`@two`")); + assert_eq!(tags[1], "tag @two"); } // -------------------------------------------------------------------------