Skip to content

πŸ§ͺ Test gap analysis β€” 2 gaps found in execute.rs post-#396 security fixΒ #403

@github-actions

Description

@github-actions

Test Gap Analysis

Test suite snapshot: 1,108 unit tests, 78 integration tests (tests/compiler_tests.rs), 3 init tests, 8 MCP HTTP tests β€” 1,198 total. All pass. βœ…

Previous issues #376 and #392 are still open. The gaps below are new β€” introduced or uncovered by the fix(security) commit ea76888 (#396), which neutralized VSO pipeline commands in extract_entry_context but left a separate stdout path unguarded.


Priority Gaps

Module Function/Path Why It Matters Suggested Test
src/execute.rs:121-125 execute_safe_outputs β€” Err arm prints result.message to stdout without VSO neutralization A prompt-injected agent that writes {"name": "##vso[task.setvariable variable=PAT]stolen"} directly to the NDJSON file (via bash) causes execute_safe_output to return Err("Unknown tool type: ##vso[...]"). The Err arm formats this into result.message and prints it to stdout via println!, where ADO interprets the ##vso[ sequence as a pipeline command. #396 fixed extract_entry_context (title/path) but this error path was missed. test_execute_safe_outputs_unknown_tool_with_vso_in_name_does_not_echo_raw_command
src/execute.rs extract_entry_context β€” ##[ shorthand not tested for title/path The function strips and neutralizes ##vso[ in title/path (tested in #396). The ##[ shorthand form (used for ##[error], ##[section], etc.) is handled by neutralize_pipeline_commands but there is no test exercising extract_entry_context specifically with ##[ in a title or path field. The unit test for neutralize_pipeline_commands covers ##[, but the composed behavior via extract_entry_context is unverified. Add test_extract_entry_context_neutralizes_shorthand_pipeline_command_in_title

Suggested Test Cases

1. execute_safe_outputs β€” unknown tool with VSO-injected name field (security-critical)

#[tokio::test]
async fn test_execute_safe_outputs_unknown_tool_with_vso_in_name_does_not_echo_raw_command() {
    use std::io::Write;

    let temp_dir = tempfile::tempdir().unwrap();
    let safe_output_path = temp_dir.path().join(crate::execute::SAFE_OUTPUT_FILENAME);

    // Simulate an adversarial NDJSON entry where the agent injects a VSO pipeline command
    // into the 'name' field, trying to get it echoed to stdout by Stage 3.
    let ndjson = "{\"name\":\"##vso[task.setvariable variable=PAT;issecret=true]stolen\"}\n";
    tokio::fs::write(&safe_output_path, ndjson).await.unwrap();

    let ctx = ExecutionContext::default();
    let results = execute_safe_outputs(temp_dir.path(), &ctx).await.unwrap();

    // One entry processed (as a failure β€” unknown tool)
    assert_eq!(results.len(), 1);
    assert!(!results[0].success);

    // The message must NOT contain the raw ##vso[ command β€” it should be neutralized
    assert!(
        !results[0].message.contains("##vso["),
        "VSO pipeline command in 'name' must be neutralized in Stage 3 stdout; got: {}",
        results[0].message
    );
}

The fix is to apply neutralize_pipeline_commands to the formatted error before printing:

// In execute_safe_outputs Err arm (src/execute.rs ~line 121):
Err(e) => {
    error!("[{}/{}] Execution error: {}", i + 1, entries.len(), e);
    let raw_msg = format!("Failed to execute entry: {}", e);
    let safe_msg = neutralize_pipeline_commands(&raw_msg);   // ← add this
    let result = ExecutionResult::failure(safe_msg);
    println!("[{}/{}] βœ— - {}", i + 1, entries.len(), result.message);
    results.push(result);
}

2. extract_entry_context β€” ##[ shorthand in title

#[test]
fn test_extract_entry_context_neutralizes_shorthand_pipeline_command_in_title() {
    let entry = serde_json::json!({
        "title": "##[error]Build failed – exfiltrate secrets"
    });
    let ctx = extract_entry_context(&entry);
    assert!(
        !ctx.contains("##[error]"),
        "##[ shorthand in title should be neutralized; got: {ctx}"
    );
    assert!(
        ctx.contains("`##[`"),
        "##[ shorthand should be wrapped in backticks; got: {ctx}"
    );
}

#[test]
fn test_extract_entry_context_neutralizes_shorthand_pipeline_command_in_path() {
    let entry = serde_json::json!({
        "path": "##[section]My Section"
    });
    let ctx = extract_entry_context(&entry);
    assert!(
        !ctx.contains("##[section]"),
        "##[ shorthand in path should be neutralized; got: {ctx}"
    );
    assert!(ctx.contains("`##[`"), "##[ shorthand should be wrapped in backticks; got: {ctx}");
}

Coverage Summary

Module Gap Severity Status
src/execute.rs β€” execute_safe_outputs Err arm (line 121) Raw name field echoed to stdout without VSO neutralization πŸ”΄ Security Untested
src/execute.rs β€” extract_entry_context ##[ shorthand form not covered for title/path 🟑 Defense-in-depth Untested

This issue was created by the automated test gap finder. Previous runs: #376 (still open), #392 (still open). Modules audited this cycle: execute.rs (post-#396). Total tests: 1,198 (up from 1,191).

Generated by Test Gap Finder Β· gh-aw-workflow-call-id: githubnext/ado-aw/test-gap-finder

Generated by Test Gap Finder Β· ● 1.9M Β· β—·

Metadata

Metadata

Labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions