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
11 changes: 10 additions & 1 deletion src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ fn log_and_print_entry_result(i: usize, total: usize, tool_name: &str, result: &
warn!("[{}/{}] {} failed: {}", i + 1, total, tool_name, result.message);
}
let symbol = if result.is_warning() { "⚠" } else if result.success { "✓" } else { "✗" };
println!("[{}/{}] {} - {} - {}", i + 1, total, tool_name, symbol, result.message);
let safe_msg = neutralize_pipeline_commands(&result.message);
println!("[{}/{}] {} - {} - {}", i + 1, total, tool_name, symbol, safe_msg);
}

/// Parse a JSON entry as `T` and run it through `execute_sanitized`.
Expand Down Expand Up @@ -486,6 +487,14 @@ mod tests {
assert_eq!(extract_entry_context(&entry), " (work item #42)");
}

#[test]
fn test_stdout_print_neutralizes_result_message_pipeline_commands() {
let message = "Uploaded '##vso[task.setvariable variable=X]y.txt'";
let safe = neutralize_pipeline_commands(message);
assert!(!safe.contains("##vso[task"));
assert!(safe.contains("`##vso[`"));
}

#[tokio::test]
async fn test_execute_unknown_tool_fails() {
let entry = serde_json::json!({"name": "unknown_tool", "foo": "bar"});
Expand Down
22 changes: 22 additions & 0 deletions src/safeoutputs/upload_build_attachment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ impl Validate for UploadBuildAttachmentParams {
!self.file_path.contains(':'),
"file_path must not contain ':'"
);
ensure!(
!self.file_path.contains("##vso[") && !self.file_path.contains("##["),
"file_path must not contain Azure DevOps pipeline command sequences"
);
for component in self.file_path.split(['/', '\\']) {
ensure!(
is_safe_path_segment(component),
Expand Down Expand Up @@ -729,6 +733,24 @@ mod tests {
.is_err());
}

#[test]
fn test_validation_rejects_pipeline_command_sequences_in_file_path() {
assert!(
make_params(
Some(1),
"agent-report",
"##vso[task.setvariable variable=EXPLOIT]value.txt"
)
.validate()
.is_err()
);
assert!(
make_params(Some(1), "agent-report", "##[error]value.txt")
.validate()
.is_err()
);
}

#[test]
fn test_result_serializes_correctly_with_build_id() {
let result = UploadBuildAttachmentResult::new(
Expand Down
22 changes: 22 additions & 0 deletions src/safeoutputs/upload_pipeline_artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ impl Validate for UploadPipelineArtifactParams {
!self.file_path.contains(':'),
"file_path must not contain ':'"
);
ensure!(
!self.file_path.contains("##vso[") && !self.file_path.contains("##["),
"file_path must not contain Azure DevOps pipeline command sequences"
);
for component in self.file_path.split(['/', '\\']) {
ensure!(
is_safe_path_segment(component),
Expand Down Expand Up @@ -776,6 +780,24 @@ mod tests {
.is_err());
}

#[test]
fn test_validation_rejects_pipeline_command_sequences_in_file_path() {
assert!(
make_params(
None,
"report",
"##vso[task.setvariable variable=EXPLOIT]value.txt"
)
.validate()
.is_err()
);
assert!(
make_params(None, "report", "##[error]value.txt")
.validate()
.is_err()
);
}

#[test]
fn test_dry_run_summary() {
let result = UploadPipelineArtifactResult::new(
Expand Down
30 changes: 26 additions & 4 deletions src/safeoutputs/upload_workitem_attachment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ impl Validate for UploadWorkitemAttachmentParams {
!contains_newline(&self.file_path),
"file_path must not contain newlines or carriage returns"
);
ensure!(
!self.file_path.contains("##vso[") && !self.file_path.contains("##["),
"file_path must not contain Azure DevOps pipeline command sequences"
);
ensure!(
!self
.file_path
Expand Down Expand Up @@ -222,10 +226,10 @@ impl Executor for UploadWorkitemAttachmentResult {
// acceptable because the injection risk from binary attachments is negligible.
if let Ok(text) = std::str::from_utf8(&file_bytes) {
if text.contains("##vso[") {
return Ok(ExecutionResult::failure(format!(
"File '{}' contains '##vso[' command injection sequence",
self.file_path
)));
return Ok(ExecutionResult::failure(
"Uploaded file contains '##vso[' command injection sequence — upload rejected"
.to_string(),
));
}
}

Expand Down Expand Up @@ -521,6 +525,24 @@ mod tests {
assert!(result.is_err());
}

#[test]
fn test_validation_rejects_pipeline_command_sequences_in_file_path() {
let vso = UploadWorkitemAttachmentParams {
work_item_id: 42,
file_path: "##vso[task.setvariable variable=EXPLOIT]value.txt".to_string(),
comment: None,
};
let shorthand = UploadWorkitemAttachmentParams {
work_item_id: 42,
file_path: "##[error]value.txt".to_string(),
comment: None,
};
let vso_result: Result<UploadWorkitemAttachmentResult, _> = vso.try_into();
let shorthand_result: Result<UploadWorkitemAttachmentResult, _> = shorthand.try_into();
assert!(vso_result.is_err());
assert!(shorthand_result.is_err());
}

#[test]
fn test_result_serializes_correctly() {
let params = UploadWorkitemAttachmentParams {
Expand Down