Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 50 additions & 1 deletion src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::collections::HashMap;
use std::path::Path;

use crate::ndjson::{self, SAFE_OUTPUT_FILENAME};
use crate::sanitize::neutralize_pipeline_commands;
use crate::safeoutputs::{
AddBuildTagResult, AddPrCommentResult, CreateBranchResult, CreateGitTagResult,
CreatePrResult, CreateWikiPageResult, CreateWorkItemResult, CommentOnWorkItemResult,
Expand Down Expand Up @@ -362,7 +363,7 @@ fn resolve_max(ctx: &ExecutionContext, tool_name: &str, default_max: u32) -> usi

/// Extract a human-readable context identifier from a safe-output entry for log messages.
/// Called before sanitization, so all string values are stripped of control characters
/// to prevent log injection.
/// and ADO pipeline commands are neutralized to prevent log injection via stdout.
fn extract_entry_context(entry: &Value) -> String {
if let Some(id) = entry.get("id").and_then(|v| v.as_u64()) {
return format!(" (work item #{})", id);
Expand All @@ -372,6 +373,7 @@ fn extract_entry_context(entry: &Value) -> String {
}
if let Some(title) = entry.get("title").and_then(|v| v.as_str()) {
let clean: String = title.chars().filter(|c| !c.is_control()).collect();
let clean = neutralize_pipeline_commands(&clean);
let truncated: &str = if clean.chars().count() > 40 {
&clean[..clean.char_indices().nth(40).map(|(i, _)| i).unwrap_or(clean.len())]
} else {
Expand All @@ -381,6 +383,7 @@ fn extract_entry_context(entry: &Value) -> String {
}
if let Some(path) = entry.get("path").and_then(|v| v.as_str()) {
let clean: String = path.chars().filter(|c| !c.is_control()).collect();
let clean = neutralize_pipeline_commands(&clean);
return format!(" (path: {})", clean);
}
String::new()
Expand Down Expand Up @@ -433,6 +436,52 @@ mod tests {
use std::collections::HashMap;
use std::path::PathBuf;

// ── extract_entry_context ─────────────────────────────────────────────────

#[test]
fn test_extract_entry_context_neutralizes_vso_in_title() {
let entry = serde_json::json!({
"title": "##vso[task.complete result=Failed]"
});
let ctx = extract_entry_context(&entry);
assert!(
!ctx.contains("##vso[task."),
"VSO command in title should be neutralized; got: {ctx}"
);
assert!(
ctx.contains("`##vso[`"),
"VSO command should be wrapped in backticks; got: {ctx}"
);
}

#[test]
fn test_extract_entry_context_neutralizes_vso_in_path() {
let entry = serde_json::json!({
"path": "##vso[task.setvariable variable=X]injected"
});
let ctx = extract_entry_context(&entry);
assert!(
!ctx.contains("##vso[task."),
"VSO command in path should be neutralized; got: {ctx}"
);
assert!(
ctx.contains("`##vso[`"),
"VSO command should be wrapped in backticks; got: {ctx}"
);
}

#[test]
fn test_extract_entry_context_preserves_normal_title() {
let entry = serde_json::json!({"title": "Fix login bug"});
assert_eq!(extract_entry_context(&entry), " (\"Fix login bug\")");
}

#[test]
fn test_extract_entry_context_prefers_id_over_title() {
let entry = serde_json::json!({"id": 42, "title": "should be ignored"});
assert_eq!(extract_entry_context(&entry), " (work item #42)");
}

#[tokio::test]
async fn test_execute_unknown_tool_fails() {
let entry = serde_json::json!({"name": "unknown_tool", "foo": "bar"});
Expand Down
41 changes: 33 additions & 8 deletions src/safeoutputs/create_wiki_page.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize};
use super::PATH_SEGMENT;
use super::resolve_wiki_branch;
use ado_aw_derive::SanitizeConfig;
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
use crate::sanitize::{SanitizeContent, neutralize_pipeline_commands, sanitize as sanitize_text};
use crate::tool_result;
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};

Expand Down Expand Up @@ -70,13 +70,17 @@ tool_result! {

impl SanitizeContent for CreateWikiPageResult {
fn sanitize_content_fields(&mut self) {
// Path is a structural identifier — sanitize lightly (remove control chars)
// but do not escape HTML or neutralize patterns that are valid in wiki paths.
self.path = self
.path
.chars()
.filter(|c| !c.is_control() || *c == '\t')
.collect();
// Path is a structural identifier — remove control characters and
// neutralize ADO pipeline commands to prevent VSO command injection
// via log output, without escaping HTML or corrupting valid path
// characters like slashes, spaces, or angle brackets.
self.path = neutralize_pipeline_commands(
&self
.path
.chars()
.filter(|c| !c.is_control() || *c == '\t')
.collect::<String>(),
);
self.content = sanitize_text(&self.content);
self.comment = self.comment.as_ref().map(|c| sanitize_text(c));
}
Expand Down Expand Up @@ -676,6 +680,27 @@ wiki-name: "MyProject.wiki"
assert_eq!(result.path, "/Folder/My Page");
}

#[test]
fn test_sanitize_neutralizes_vso_command_in_path() {
let params = CreateWikiPageParams {
path: "/##vso[task.setvariable variable=X]injected".to_string(),
content: "Some valid content here.".to_string(),
comment: None,
};
let mut result: CreateWikiPageResult = params.try_into().unwrap();
result.sanitize_content_fields();
assert!(
!result.path.contains("##vso[task."),
"VSO command in path should be neutralized; got: {}",
result.path
);
assert!(
result.path.contains("`##vso[`"),
"VSO command should be wrapped in backticks; got: {}",
result.path
);
}

// ── Executor (no-token failure) ───────────────────────────────────────────

#[tokio::test]
Expand Down
41 changes: 33 additions & 8 deletions src/safeoutputs/update_wiki_page.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize};
use super::PATH_SEGMENT;
use super::resolve_wiki_branch;
use ado_aw_derive::SanitizeConfig;
use crate::sanitize::{SanitizeContent, sanitize as sanitize_text};
use crate::sanitize::{SanitizeContent, neutralize_pipeline_commands, sanitize as sanitize_text};
use crate::tool_result;
use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate};

Expand Down Expand Up @@ -66,13 +66,17 @@ tool_result! {

impl SanitizeContent for UpdateWikiPageResult {
fn sanitize_content_fields(&mut self) {
// Path is a structural identifier — sanitize lightly (remove control chars)
// but do not escape HTML or neutralize patterns that are valid in wiki paths.
self.path = self
.path
.chars()
.filter(|c| !c.is_control() || *c == '\t')
.collect();
// Path is a structural identifier — remove control characters and
// neutralize ADO pipeline commands to prevent VSO command injection
// via log output, without escaping HTML or corrupting valid path
// characters like slashes, spaces, or angle brackets.
self.path = neutralize_pipeline_commands(
&self
.path
.chars()
.filter(|c| !c.is_control() || *c == '\t')
.collect::<String>(),
);
self.content = sanitize_text(&self.content);
self.comment = self.comment.as_ref().map(|c| sanitize_text(c));
}
Expand Down Expand Up @@ -648,6 +652,27 @@ wiki-name: "MyProject.wiki"
assert_eq!(result.path, "/Folder/My Page");
}

#[test]
fn test_sanitize_neutralizes_vso_command_in_path() {
let params = UpdateWikiPageParams {
path: "/##vso[task.setvariable variable=X]injected".to_string(),
content: "Some valid content here.".to_string(),
comment: None,
};
let mut result: UpdateWikiPageResult = params.try_into().unwrap();
result.sanitize_content_fields();
assert!(
!result.path.contains("##vso[task."),
"VSO command in path should be neutralized; got: {}",
result.path
);
assert!(
result.path.contains("`##vso[`"),
"VSO command should be wrapped in backticks; got: {}",
result.path
);
}

// ── Executor (no-token failure) ───────────────────────────────────────────

#[tokio::test]
Expand Down
2 changes: 1 addition & 1 deletion src/sanitize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ fn remove_control_characters(input: &str) -> String {
///
/// Also handles `##[` (the shorthand form used for `##[section]`, `##[error]`,
/// etc.) which ADO pipelines also interpret.
fn neutralize_pipeline_commands(input: &str) -> String {
pub(crate) fn neutralize_pipeline_commands(input: &str) -> String {
let mut result = String::with_capacity(input.len() + 32);
let mut rest = input;

Expand Down
Loading