Skip to content

Commit da83c03

Browse files
fix(security): neutralize pipeline commands in execute_safe_outputs Err arm (#405)
* Initial plan * fix(security): neutralize pipeline commands in execute_safe_outputs Err arm and add tests Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/7930418d-f26e-4a79-9b6f-88b2e80c0334 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
1 parent ea76888 commit da83c03

1 file changed

Lines changed: 68 additions & 1 deletion

File tree

src/execute.rs

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,9 @@ pub async fn execute_safe_outputs(
120120
}
121121
Err(e) => {
122122
error!("[{}/{}] Execution error: {}", i + 1, entries.len(), e);
123-
let result = ExecutionResult::failure(format!("Failed to execute entry: {}", e));
123+
let raw_msg = format!("Failed to execute entry: {}", e);
124+
let safe_msg = neutralize_pipeline_commands(&raw_msg);
125+
let result = ExecutionResult::failure(safe_msg);
124126
println!("[{}/{}] ✗ - {}", i + 1, entries.len(), result.message);
125127
results.push(result);
126128
}
@@ -962,6 +964,71 @@ mod tests {
962964
assert_eq!(extract_entry_context(&entry), " (path: /Page/Injected)");
963965
}
964966

967+
#[test]
968+
fn test_extract_entry_context_neutralizes_shorthand_pipeline_command_in_title() {
969+
let entry = serde_json::json!({
970+
"title": "##[error]Build failed – exfiltrate secrets"
971+
});
972+
let ctx = extract_entry_context(&entry);
973+
assert!(
974+
!ctx.contains("##[error]"),
975+
"##[ shorthand in title should be neutralized; got: {ctx}"
976+
);
977+
assert!(
978+
ctx.contains("`##[`"),
979+
"##[ shorthand should be wrapped in backticks; got: {ctx}"
980+
);
981+
}
982+
983+
#[test]
984+
fn test_extract_entry_context_neutralizes_shorthand_pipeline_command_in_path() {
985+
let entry = serde_json::json!({
986+
"path": "##[section]My Section"
987+
});
988+
let ctx = extract_entry_context(&entry);
989+
assert!(
990+
!ctx.contains("##[section]"),
991+
"##[ shorthand in path should be neutralized; got: {ctx}"
992+
);
993+
assert!(
994+
ctx.contains("`##[`"),
995+
"##[ shorthand should be wrapped in backticks; got: {ctx}"
996+
);
997+
}
998+
999+
#[tokio::test]
1000+
async fn test_execute_safe_outputs_unknown_tool_with_vso_in_name_does_not_echo_raw_command() {
1001+
let temp_dir = tempfile::tempdir().unwrap();
1002+
let safe_output_path = temp_dir.path().join(SAFE_OUTPUT_FILENAME);
1003+
1004+
// Simulate an adversarial NDJSON entry where the agent injects a VSO pipeline command
1005+
// into the 'name' field, trying to get it echoed to stdout by Stage 3.
1006+
let ndjson =
1007+
"{\"name\":\"##vso[task.setvariable variable=PAT;issecret=true]stolen\"}\n";
1008+
tokio::fs::write(&safe_output_path, ndjson).await.unwrap();
1009+
1010+
let ctx = ExecutionContext::default();
1011+
let results = execute_safe_outputs(temp_dir.path(), &ctx).await.unwrap();
1012+
1013+
// One entry processed (as a failure — unknown tool)
1014+
assert_eq!(results.len(), 1);
1015+
assert!(!results[0].success);
1016+
1017+
// The raw ##vso[task... pattern must not appear — neutralization breaks it at ##vso[
1018+
// so "##vso[task" cannot appear (it becomes "`##vso[`task").
1019+
assert!(
1020+
!results[0].message.contains("##vso[task"),
1021+
"Raw VSO pipeline command must not appear in Stage 3 output; got: {}",
1022+
results[0].message
1023+
);
1024+
// Confirm the neutralized (backtick-wrapped) form is present.
1025+
assert!(
1026+
results[0].message.contains("`##vso[`"),
1027+
"VSO command should be neutralized (wrapped in backticks); got: {}",
1028+
results[0].message
1029+
);
1030+
}
1031+
9651032
// --- resolve_max and DEFAULT_MAX unit tests ---
9661033

9671034
#[test]

0 commit comments

Comments
 (0)