Skip to content

Commit ea43b11

Browse files
jamesadevineCopilot
jamesadevine
and
Copilot
authored
fix(safeoutputs): use sanitize_config for identifier fields instead of sanitize_text (#433)
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>
1 parent 64e9709 commit ea43b11

10 files changed

Lines changed: 34 additions & 34 deletions

‎src/safeoutputs/add_build_tag.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use schemars::JsonSchema;
77
use serde::{Deserialize, Serialize};
88

99
use super::PATH_SEGMENT;
10-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
10+
use crate::sanitize::{SanitizeContent, sanitize_config};
1111
use crate::tool_result;
1212
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
1313
use anyhow::{Context, ensure};
@@ -58,7 +58,7 @@ tool_result! {
5858

5959
impl SanitizeContent for AddBuildTagResult {
6060
fn sanitize_content_fields(&mut self) {
61-
self.tag = sanitize_text(&self.tag);
61+
self.tag = sanitize_config(&self.tag);
6262
}
6363
}
6464

‎src/safeoutputs/create_branch.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use schemars::JsonSchema;
77
use serde::{Deserialize, Serialize};
88

99
use super::{PATH_SEGMENT, validate_git_ref_name};
10-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
10+
use crate::sanitize::{SanitizeContent, sanitize_config};
1111
use crate::tool_result;
1212
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
1313
use anyhow::{Context, ensure};
@@ -91,7 +91,7 @@ tool_result! {
9191

9292
impl SanitizeContent for CreateBranchResult {
9393
fn sanitize_content_fields(&mut self) {
94-
self.branch_name = sanitize_text(&self.branch_name);
94+
self.branch_name = sanitize_config(&self.branch_name);
9595
}
9696
}
9797

‎src/safeoutputs/create_pr.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use tokio::process::Command;
77

88
use ado_aw_derive::SanitizeConfig;
99
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, ToolResult, Validate};
10-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
10+
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
1111
use crate::tool_result;
1212
use anyhow::{Context, ensure};
1313

@@ -321,7 +321,7 @@ impl SanitizeContent for CreatePrResult {
321321
self.title = sanitize_text(&self.title);
322322
self.description = sanitize_text(&self.description);
323323
for label in &mut self.agent_labels {
324-
*label = sanitize_text(label);
324+
*label = sanitize_config(label);
325325
}
326326
}
327327
}

‎src/safeoutputs/create_work_item.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use super::PATH_SEGMENT;
99
use crate::tool_result;
1010
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
1111
use ado_aw_derive::SanitizeConfig;
12-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
12+
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
1313
use anyhow::{Context, ensure};
1414

1515
/// Parameters for creating a work item
@@ -63,7 +63,7 @@ impl SanitizeContent for CreateWorkItemResult {
6363
self.title = sanitize_text(&self.title);
6464
self.description = sanitize_text(&self.description);
6565
for tag in &mut self.tags {
66-
*tag = sanitize_text(tag);
66+
*tag = sanitize_config(tag);
6767
}
6868
}
6969
}

‎src/safeoutputs/link_work_items.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};
77

88
use super::PATH_SEGMENT;
99
use ado_aw_derive::SanitizeConfig;
10-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
10+
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
1111
use crate::tool_result;
1212
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
1313
use crate::safeoutputs::comment_on_work_item::CommentTarget;
@@ -94,7 +94,7 @@ tool_result! {
9494

9595
impl SanitizeContent for LinkWorkItemsResult {
9696
fn sanitize_content_fields(&mut self) {
97-
self.link_type = sanitize_text(&self.link_type);
97+
self.link_type = sanitize_config(&self.link_type);
9898
self.comment = self.comment.as_deref().map(sanitize_text);
9999
}
100100
}

‎src/safeoutputs/queue_build.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};
77

88
use super::PATH_SEGMENT;
99
use ado_aw_derive::SanitizeConfig;
10-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
10+
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
1111
use crate::tool_result;
1212
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
1313
use anyhow::{Context, ensure};
@@ -71,13 +71,13 @@ impl SanitizeContent for QueueBuildResult {
7171
self.reason = Some(sanitize_text(reason));
7272
}
7373
if let Some(branch) = &self.branch {
74-
self.branch = Some(sanitize_text(branch));
74+
self.branch = Some(sanitize_config(branch));
7575
}
7676
if let Some(params) = &self.parameters {
7777
self.parameters = Some(
7878
params
7979
.iter()
80-
.map(|(k, v)| (sanitize_text(k), sanitize_text(v)))
80+
.map(|(k, v)| (sanitize_config(k), sanitize_config(v)))
8181
.collect(),
8282
);
8383
}

‎src/safeoutputs/resolve_pr_thread.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize};
88

99
use super::resolve_repo_name;
1010
use super::PATH_SEGMENT;
11-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
11+
use crate::sanitize::{SanitizeContent, sanitize_config};
1212
use crate::tool_result;
1313
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
1414
use anyhow::{Context, ensure};
@@ -89,9 +89,9 @@ tool_result! {
8989

9090
impl SanitizeContent for ResolvePrThreadResult {
9191
fn sanitize_content_fields(&mut self) {
92-
self.status = sanitize_text(&self.status);
92+
self.status = sanitize_config(&self.status);
9393
if let Some(ref repo) = self.repository {
94-
self.repository = Some(sanitize_text(repo));
94+
self.repository = Some(sanitize_config(repo));
9595
}
9696
}
9797
}

‎src/safeoutputs/submit_pr_review.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use schemars::JsonSchema;
77
use serde::{Deserialize, Serialize};
88

99
use super::{PATH_SEGMENT, resolve_repo_name};
10-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
10+
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
1111
use crate::tool_result;
1212
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
1313
use anyhow::{Context, ensure};
@@ -93,9 +93,9 @@ tool_result! {
9393

9494
impl SanitizeContent for SubmitPrReviewResult {
9595
fn sanitize_content_fields(&mut self) {
96-
self.event = sanitize_text(&self.event);
96+
self.event = sanitize_config(&self.event);
9797
self.body = self.body.as_deref().map(sanitize_text);
98-
self.repository = self.repository.as_deref().map(sanitize_text);
98+
self.repository = self.repository.as_deref().map(sanitize_config);
9999
}
100100
}
101101

‎src/safeoutputs/update_pr.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use schemars::JsonSchema;
77
use serde::{Deserialize, Serialize};
88

99
use super::{PATH_SEGMENT, resolve_repo_name};
10-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
10+
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
1111
use crate::tool_result;
1212
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
1313
use anyhow::{Context, ensure};
@@ -150,17 +150,17 @@ tool_result! {
150150

151151
impl SanitizeContent for UpdatePrResult {
152152
fn sanitize_content_fields(&mut self) {
153-
self.repository = self.repository.as_deref().map(sanitize_text);
154-
self.operation = sanitize_text(&self.operation);
153+
self.repository = self.repository.as_deref().map(sanitize_config);
154+
self.operation = sanitize_config(&self.operation);
155155
self.reviewers = self
156156
.reviewers
157157
.as_ref()
158-
.map(|rs| rs.iter().map(|r| sanitize_text(r)).collect());
158+
.map(|rs| rs.iter().map(|r| sanitize_config(r)).collect());
159159
self.labels = self
160160
.labels
161161
.as_ref()
162-
.map(|ls| ls.iter().map(|l| sanitize_text(l)).collect());
163-
self.vote = self.vote.as_deref().map(sanitize_text);
162+
.map(|ls| ls.iter().map(|l| sanitize_config(l)).collect());
163+
self.vote = self.vote.as_deref().map(sanitize_config);
164164
self.description = self.description.as_deref().map(sanitize_text);
165165
}
166166
}

‎src/safeoutputs/update_work_item.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use super::PATH_SEGMENT;
99
use crate::tool_result;
1010
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};
1111
use ado_aw_derive::SanitizeConfig;
12-
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
12+
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text, sanitize_config};
1313
use anyhow::{Context, ensure};
1414

1515
/// Parameters for updating a work item
@@ -92,14 +92,14 @@ impl SanitizeContent for UpdateWorkItemResult {
9292
fn sanitize_content_fields(&mut self) {
9393
self.title = self.title.as_deref().map(sanitize_text);
9494
self.body = self.body.as_deref().map(sanitize_text);
95-
self.state = self.state.as_deref().map(sanitize_text);
96-
self.area_path = self.area_path.as_deref().map(sanitize_text);
97-
self.iteration_path = self.iteration_path.as_deref().map(sanitize_text);
98-
self.assignee = self.assignee.as_deref().map(sanitize_text);
95+
self.state = self.state.as_deref().map(sanitize_config);
96+
self.area_path = self.area_path.as_deref().map(sanitize_config);
97+
self.iteration_path = self.iteration_path.as_deref().map(sanitize_config);
98+
self.assignee = self.assignee.as_deref().map(sanitize_config);
9999
self.tags = self
100100
.tags
101101
.as_ref()
102-
.map(|ts| ts.iter().map(|t| sanitize_text(t)).collect());
102+
.map(|ts| ts.iter().map(|t| sanitize_config(t)).collect());
103103
}
104104
}
105105

@@ -954,11 +954,11 @@ target: 42
954954
let mut result: UpdateWorkItemResult = params.try_into().unwrap();
955955
result.sanitize_content_fields();
956956

957-
// @mentions should be neutralized
957+
// @mentions should be neutralized in rich-text fields
958958
assert!(result.title.as_deref().unwrap().contains("`@user`"));
959-
// tags should be sanitized
959+
// tags are identifiers — @mentions should NOT be backtick-wrapped
960960
let tags = result.tags.as_ref().unwrap();
961-
assert!(tags[1].contains("`@two`"));
961+
assert_eq!(tags[1], "tag @two");
962962
}
963963

964964
// -------------------------------------------------------------------------

0 commit comments

Comments
 (0)