From a71d5360a642dfc6c0676ff1fb3bf0d0611b71ae Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 09:04:51 +0000 Subject: [PATCH] refactor: reduce complexity of add_ado_proxy_findings in src/audit/findings.rs Split the 219-line add_ado_proxy_findings function (clippy::too_many_lines) into seven small, independently-testable helper functions, one per finding/recommendation rule: - add_ado_proxy_unhealthy_lifecycle_finding - add_ado_proxy_credential_unavailable_finding - add_ado_proxy_upstream_failed_finding - add_ado_proxy_out_of_scope_response_finding - add_ado_proxy_prompt_conflict_finding - add_ado_proxy_prohibited_request_finding - add_ado_proxy_malformed_record_finding Each helper uses an early return guard clause instead of a large if-block, matching the existing add_* rule functions elsewhere in this module. No behavior change: all 18 existing audit::findings tests continue to pass unmodified, and the full suite (3358 tests) passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/audit/findings.rs | 434 ++++++++++++++++++++++++------------------ 1 file changed, 247 insertions(+), 187 deletions(-) diff --git a/src/audit/findings.rs b/src/audit/findings.rs index bcbf7f61..a41615e1 100644 --- a/src/audit/findings.rs +++ b/src/audit/findings.rs @@ -37,129 +37,173 @@ fn add_ado_proxy_findings( return; }; - if proxy + add_ado_proxy_unhealthy_lifecycle_finding(proxy, findings, recommendations); + add_ado_proxy_credential_unavailable_finding(proxy, findings, recommendations); + add_ado_proxy_upstream_failed_finding(proxy, findings, recommendations); + add_ado_proxy_out_of_scope_response_finding(proxy, findings, recommendations); + add_ado_proxy_prompt_conflict_finding(proxy, findings, recommendations); + add_ado_proxy_prohibited_request_finding(proxy, findings, recommendations); + add_ado_proxy_malformed_record_finding(proxy, findings, recommendations); +} + +fn add_ado_proxy_unhealthy_lifecycle_finding( + proxy: &crate::audit::model::AdoProxyAnalysis, + findings: &mut Vec, + recommendations: &mut Vec, +) { + if !proxy .lifecycle .as_ref() .is_some_and(|lifecycle| !lifecycle.healthy_before_teardown) { - push_finding( - findings, - Finding { - category: String::from("ado_proxy"), - severity: Severity::High, - title: String::from("ado-proxy was not healthy before teardown"), - description: String::from( - "The proxy did not reach or retain its expected running/listening state before teardown.", - ), - impact: Some(String::from( - "Azure DevOps reads through wrapped az or the Azure DevOps MCP may have failed.", - )), - }, - ); - push_recommendation( - recommendations, - Recommendation { - priority: String::from("high"), - action: String::from("Inspect ado-proxy lifecycle diagnostics"), - reason: String::from( - "Container state and startup logs identify topology, CA, configuration, or lifecycle failures.", - ), - example: Some(String::from( - "Inspect agent_outputs_/logs/ado-proxy/container.log and container-state.txt", - )), - }, - ); + return; } + push_finding( + findings, + Finding { + category: String::from("ado_proxy"), + severity: Severity::High, + title: String::from("ado-proxy was not healthy before teardown"), + description: String::from( + "The proxy did not reach or retain its expected running/listening state before teardown.", + ), + impact: Some(String::from( + "Azure DevOps reads through wrapped az or the Azure DevOps MCP may have failed.", + )), + }, + ); + push_recommendation( + recommendations, + Recommendation { + priority: String::from("high"), + action: String::from("Inspect ado-proxy lifecycle diagnostics"), + reason: String::from( + "Container state and startup logs identify topology, CA, configuration, or lifecycle failures.", + ), + example: Some(String::from( + "Inspect agent_outputs_/logs/ado-proxy/container.log and container-state.txt", + )), + }, + ); +} + +fn add_ado_proxy_credential_unavailable_finding( + proxy: &crate::audit::model::AdoProxyAnalysis, + findings: &mut Vec, + recommendations: &mut Vec, +) { let credential_unavailable = proxy_reason_count(proxy, &["credential-unavailable"]); - if credential_unavailable > 0 { - push_finding( - findings, - Finding { - category: String::from("ado_proxy"), - severity: Severity::High, - title: String::from("ado-proxy credential was unavailable"), - description: format!( - "The proxy could not acquire its Azure DevOps read credential for {credential_unavailable} request(s)." - ), - impact: Some(String::from( - "Authorized Azure DevOps reads could not be forwarded upstream.", - )), - }, - ); - push_recommendation( - recommendations, - Recommendation { - priority: String::from("high"), - action: String::from("Inspect the permissions.read service connection"), - reason: String::from( - "The trusted proxy token source failed; the credential must not be moved into the agent.", - ), - example: None, - }, - ); + if credential_unavailable == 0 { + return; } + push_finding( + findings, + Finding { + category: String::from("ado_proxy"), + severity: Severity::High, + title: String::from("ado-proxy credential was unavailable"), + description: format!( + "The proxy could not acquire its Azure DevOps read credential for {credential_unavailable} request(s)." + ), + impact: Some(String::from( + "Authorized Azure DevOps reads could not be forwarded upstream.", + )), + }, + ); + push_recommendation( + recommendations, + Recommendation { + priority: String::from("high"), + action: String::from("Inspect the permissions.read service connection"), + reason: String::from( + "The trusted proxy token source failed; the credential must not be moved into the agent.", + ), + example: None, + }, + ); +} + +fn add_ado_proxy_upstream_failed_finding( + proxy: &crate::audit::model::AdoProxyAnalysis, + findings: &mut Vec, + recommendations: &mut Vec, +) { let upstream_failed = proxy_reason_count(proxy, &["upstream-failed"]); - if upstream_failed > 0 { - push_finding( - findings, - Finding { - category: String::from("ado_proxy"), - severity: Severity::High, - title: String::from("ado-proxy could not reach Azure DevOps upstream"), - description: format!( - "{upstream_failed} authorized request(s) failed while reaching the upstream service." - ), - impact: Some(String::from( - "The agent's Azure DevOps reads may be incomplete even though policy allowed them.", - )), - }, - ); - push_recommendation( - recommendations, - Recommendation { - priority: String::from("high"), - action: String::from("Inspect ado-proxy upstream connectivity"), - reason: String::from( - "AWF/Squid egress, CA trust, or Azure DevOps availability prevented an allowed request.", - ), - example: None, - }, - ); + if upstream_failed == 0 { + return; } + push_finding( + findings, + Finding { + category: String::from("ado_proxy"), + severity: Severity::High, + title: String::from("ado-proxy could not reach Azure DevOps upstream"), + description: format!( + "{upstream_failed} authorized request(s) failed while reaching the upstream service." + ), + impact: Some(String::from( + "The agent's Azure DevOps reads may be incomplete even though policy allowed them.", + )), + }, + ); + push_recommendation( + recommendations, + Recommendation { + priority: String::from("high"), + action: String::from("Inspect ado-proxy upstream connectivity"), + reason: String::from( + "AWF/Squid egress, CA trust, or Azure DevOps availability prevented an allowed request.", + ), + example: None, + }, + ); +} + +fn add_ado_proxy_out_of_scope_response_finding( + proxy: &crate::audit::model::AdoProxyAnalysis, + findings: &mut Vec, + recommendations: &mut Vec, +) { let out_of_scope_response = proxy_reason_count(proxy, &["out-of-scope-response"]); - if out_of_scope_response > 0 { - push_finding( - findings, - Finding { - category: String::from("security"), - severity: Severity::High, - title: String::from("ado-proxy blocked an over-broad upstream response"), - description: format!( - "Response filtering rejected {out_of_scope_response} response(s) containing resources outside the configured scope." - ), - impact: Some(String::from( - "The proxy prevented out-of-scope Azure DevOps data from reaching the agent.", - )), - }, - ); - push_recommendation( - recommendations, - Recommendation { - priority: String::from("high"), - action: String::from( - "Inspect the affected ado-proxy operation and response filter", - ), - reason: String::from( - "The response shape may have changed or the operation may require a tighter catalog filter; do not bypass response filtering.", - ), - example: None, - }, - ); + if out_of_scope_response == 0 { + return; } + push_finding( + findings, + Finding { + category: String::from("security"), + severity: Severity::High, + title: String::from("ado-proxy blocked an over-broad upstream response"), + description: format!( + "Response filtering rejected {out_of_scope_response} response(s) containing resources outside the configured scope." + ), + impact: Some(String::from( + "The proxy prevented out-of-scope Azure DevOps data from reaching the agent.", + )), + }, + ); + push_recommendation( + recommendations, + Recommendation { + priority: String::from("high"), + action: String::from("Inspect the affected ado-proxy operation and response filter"), + reason: String::from( + "The response shape may have changed or the operation may require a tighter catalog filter; do not bypass response filtering.", + ), + example: None, + }, + ); +} + +fn add_ado_proxy_prompt_conflict_finding( + proxy: &crate::audit::model::AdoProxyAnalysis, + findings: &mut Vec, + recommendations: &mut Vec, +) { let prompt_conflict_reasons = [ "capability-disabled", "out-of-scope", @@ -167,35 +211,41 @@ fn add_ado_proxy_findings( "query-not-allowed", ]; let prompt_conflicts = proxy_reason_count(proxy, &prompt_conflict_reasons); - if prompt_conflicts > 0 { - push_finding( - findings, - Finding { - category: String::from("configuration"), - severity: Severity::Medium, - title: String::from("Agent requests conflicted with permissions.read"), - description: format!( - "{prompt_conflicts} request(s) were denied by configured capability, scope, API-version, or query limits: {}.", - format_proxy_reasons(proxy, &prompt_conflict_reasons) - ), - impact: None, - }, - ); - push_recommendation( - recommendations, - Recommendation { - priority: String::from("medium"), - action: String::from( - "Align the agent prompt with effective Azure DevOps permissions", - ), - reason: String::from( - "The prompt requested data outside the declared front-matter contract. Deliberately review front matter only when broader access is legitimate.", - ), - example: None, - }, - ); + if prompt_conflicts == 0 { + return; } + push_finding( + findings, + Finding { + category: String::from("configuration"), + severity: Severity::Medium, + title: String::from("Agent requests conflicted with permissions.read"), + description: format!( + "{prompt_conflicts} request(s) were denied by configured capability, scope, API-version, or query limits: {}.", + format_proxy_reasons(proxy, &prompt_conflict_reasons) + ), + impact: None, + }, + ); + push_recommendation( + recommendations, + Recommendation { + priority: String::from("medium"), + action: String::from("Align the agent prompt with effective Azure DevOps permissions"), + reason: String::from( + "The prompt requested data outside the declared front-matter contract. Deliberately review front matter only when broader access is legitimate.", + ), + example: None, + }, + ); +} + +fn add_ado_proxy_prohibited_request_finding( + proxy: &crate::audit::model::AdoProxyAnalysis, + findings: &mut Vec, + recommendations: &mut Vec, +) { let prohibited_reasons = [ "method-not-read", "denied-route-family", @@ -204,61 +254,71 @@ fn add_ado_proxy_findings( "malformed-target", ]; let prohibited = proxy_reason_count(proxy, &prohibited_reasons); - if prohibited > 0 { - push_finding( - findings, - Finding { - category: String::from("security"), - severity: Severity::Medium, - title: String::from("ado-proxy blocked prohibited request shapes"), - description: format!( - "{prohibited} direct write, denied-family, unknown, or malformed request(s) were blocked: {}.", - format_proxy_reasons(proxy, &prohibited_reasons) - ), - impact: None, - }, - ); - push_recommendation( - recommendations, - Recommendation { - priority: String::from("medium"), - action: String::from("Remove unsupported Azure DevOps requests from the prompt"), - reason: String::from( - "Direct writes and uncatalogued APIs must not be enabled by widening the proxy policy.", - ), - example: None, - }, - ); + if prohibited == 0 { + return; } - if proxy.malformed_record_count > 0 { - push_finding( - findings, - Finding { - category: String::from("ado_proxy"), - severity: Severity::Medium, - title: String::from("ado-proxy decision log contained malformed records"), - description: format!( - "{} decision record(s) did not match the declared v1 schema.", - proxy.malformed_record_count - ), - impact: Some(String::from( - "The audit summary may omit affected proxy decisions.", - )), - }, - ); - push_recommendation( - recommendations, - Recommendation { - priority: String::from("medium"), - action: String::from("Check ado-proxy bundle/compiler schema compatibility"), - reason: String::from( - "The analyzer rejected records rather than guessing at an unknown shape.", - ), - example: None, - }, - ); + push_finding( + findings, + Finding { + category: String::from("security"), + severity: Severity::Medium, + title: String::from("ado-proxy blocked prohibited request shapes"), + description: format!( + "{prohibited} direct write, denied-family, unknown, or malformed request(s) were blocked: {}.", + format_proxy_reasons(proxy, &prohibited_reasons) + ), + impact: None, + }, + ); + push_recommendation( + recommendations, + Recommendation { + priority: String::from("medium"), + action: String::from("Remove unsupported Azure DevOps requests from the prompt"), + reason: String::from( + "Direct writes and uncatalogued APIs must not be enabled by widening the proxy policy.", + ), + example: None, + }, + ); +} + +fn add_ado_proxy_malformed_record_finding( + proxy: &crate::audit::model::AdoProxyAnalysis, + findings: &mut Vec, + recommendations: &mut Vec, +) { + if proxy.malformed_record_count == 0 { + return; } + + push_finding( + findings, + Finding { + category: String::from("ado_proxy"), + severity: Severity::Medium, + title: String::from("ado-proxy decision log contained malformed records"), + description: format!( + "{} decision record(s) did not match the declared v1 schema.", + proxy.malformed_record_count + ), + impact: Some(String::from( + "The audit summary may omit affected proxy decisions.", + )), + }, + ); + push_recommendation( + recommendations, + Recommendation { + priority: String::from("medium"), + action: String::from("Check ado-proxy bundle/compiler schema compatibility"), + reason: String::from( + "The analyzer rejected records rather than guessing at an unknown shape.", + ), + example: None, + }, + ); } fn proxy_reason_count(proxy: &crate::audit::model::AdoProxyAnalysis, reasons: &[&str]) -> u64 {