From fcc9b6b9b0c70ff14b4fc136f45289ad72b1c8ad Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 5 Aug 2026 09:43:33 +0000 Subject: [PATCH] refactor: reduce complexity of populate_custom_safe_output_jobs in custom_jobs.rs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/audit/analyzers/custom_jobs.rs | 293 +++++++++++++++++------------ 1 file changed, 174 insertions(+), 119 deletions(-) diff --git a/src/audit/analyzers/custom_jobs.rs b/src/audit/analyzers/custom_jobs.rs index 1edcb3ec..3564373a 100644 --- a/src/audit/analyzers/custom_jobs.rs +++ b/src/audit/analyzers/custom_jobs.rs @@ -200,126 +200,16 @@ pub async fn populate_custom_safe_output_jobs( let mut reports = Vec::with_capacity(catalog.entries.len()); for (tool, entry) in catalog.entries { - let graph_job_id = graph_job_id_for_tool(audit, &tool); - let graph_display_name = graph_job_id - .as_deref() - .and_then(|job_id| unique_graph_display_name(audit, job_id)); - let metadata_job_id = entry - .metadata - .as_ref() - .and_then(|metadata| normalize_optional_string(metadata.job_id.clone())); - if let (Some(graph_job_id), Some(metadata_job_id)) = - (graph_job_id.as_deref(), metadata_job_id.as_deref()) - && graph_job_id != metadata_job_id - { - findings.push(Finding { - category: String::from("safe_outputs"), - severity: Severity::High, - title: format!("Custom job metadata identity mismatch for {tool}"), - description: format!( - "The typed pipeline graph assigns custom tool '{tool}' to ADO job \ - '{graph_job_id}', but the aw_info marker claims '{metadata_job_id}'." - ), - impact: Some(String::from( - "Untrusted runtime metadata does not match the compiler-derived job identity.", - )), - }); - } - let expected_job_id = graph_job_id.clone().or(metadata_job_id); - - let metadata_approval_path = entry - .metadata - .as_ref() - .and_then(|metadata| normalize_optional_string(metadata.approval_path.clone())); - let graph_approval_path = graph_job_id - .as_deref() - .and_then(|job_id| approval_path_from_graph(audit, job_id)); - if graph_job_id.is_some() - && metadata_approval_path.is_some() - && metadata_approval_path != graph_approval_path - { - findings.push(Finding { - category: String::from("safe_outputs"), - severity: Severity::High, - title: format!("Custom job metadata approval mismatch for {tool}"), - description: format!( - "The typed pipeline graph assigns custom tool '{tool}' to approval path \ - '{}', but the aw_info marker claims '{}'.", - graph_approval_path.as_deref().unwrap_or("automatic"), - metadata_approval_path.as_deref().unwrap_or("automatic") - ), - impact: Some(String::from( - "Untrusted runtime metadata does not match the compiler-derived approval path.", - )), - }); - } - let graph_is_authoritative = graph_job_id.is_some(); - let approval_path = if graph_is_authoritative { - graph_approval_path - } else { - metadata_approval_path - }; - if let (Some(component), Some(config_digest)) = ( - entry.component.as_ref(), - entry.config_schema_digest.as_ref(), - ) && !component.schema_digest.is_empty() - && component.schema_digest != *config_digest - { - findings.push(Finding { - category: String::from("safe_outputs"), - severity: Severity::High, - title: format!("Custom component provenance mismatch for {tool}"), - description: format!( - "The aw_info marker declares schema_digest={} for custom tool '{tool}', but compiler-generated custom-tools.json hashes to {}.", - component.schema_digest, config_digest - ), - impact: Some(String::from( - "The proposal schema does not match the compile-time provenance recorded for the custom component.", - )), - }); - } - - let matches = matching_timeline_jobs( - &audit.jobs, - &tool, - expected_job_id.as_deref(), - graph_display_name.as_deref(), - ); - let selected = matches.last().copied(); - + let correlation = correlate_tool_with_graph(audit, &tool, &entry, &mut findings); let previous_entry = previous.iter().find(|candidate| candidate.tool == tool); - let ado_job = selected - .map(custom_ado_job_from_timeline) - .or_else(|| previous_entry.and_then(|entry| entry.ado_job.clone())); - let expected_job_id = expected_job_id - .or_else(|| previous_entry.and_then(|entry| entry.expected_job_id.clone())); - let staged_requested = entry - .metadata - .as_ref() - .and_then(|metadata| metadata.staged_requested) - .or_else(|| previous_entry.and_then(|entry| entry.staged_requested)); - let component_provenance = entry - .component - .or_else(|| previous_entry.and_then(|entry| entry.component_provenance.clone())); - let proposal_time_acknowledgement = entry.proposal_time_acknowledgement.or_else(|| { - previous_entry.and_then(|entry| entry.proposal_time_acknowledgement.clone()) - }); - - reports.push(CustomSafeOutputJobAudit { - tool: tool.clone(), - proposed_count: proposal_counts.get(&tool).copied().unwrap_or(0), - expected_job_id, - component_provenance, - approval_path: if graph_is_authoritative { - approval_path - } else { - approval_path - .or_else(|| previous_entry.and_then(|entry| entry.approval_path.clone())) - }, - staged_requested, - proposal_time_acknowledgement, - ado_job, - }); + reports.push(build_report_for_tool( + audit, + &tool, + entry, + correlation, + &proposal_counts, + previous_entry, + )); } add_report_findings(audit, &reports, &mut findings); @@ -333,6 +223,171 @@ pub async fn populate_custom_safe_output_jobs( Ok(()) } +/// Result of correlating a catalog entry with the typed pipeline graph: +/// the ADO job identity, its display name, and the effective approval path +/// (graph-derived, falling back to metadata when the graph has no match). +struct ToolGraphCorrelation { + expected_job_id: Option, + graph_display_name: Option, + approval_path: Option, + graph_is_authoritative: bool, +} + +/// Cross-check a catalog entry's `aw_info` metadata against the typed +/// pipeline graph, pushing High-severity findings on any mismatch, and +/// return the resolved job/approval identity to use for report building. +fn correlate_tool_with_graph( + audit: &AuditData, + tool: &str, + entry: &CatalogEntry, + findings: &mut Vec, +) -> ToolGraphCorrelation { + let graph_job_id = graph_job_id_for_tool(audit, tool); + let graph_display_name = graph_job_id + .as_deref() + .and_then(|job_id| unique_graph_display_name(audit, job_id)); + let metadata_job_id = entry + .metadata + .as_ref() + .and_then(|metadata| normalize_optional_string(metadata.job_id.clone())); + if let (Some(graph_job_id), Some(metadata_job_id)) = + (graph_job_id.as_deref(), metadata_job_id.as_deref()) + && graph_job_id != metadata_job_id + { + findings.push(Finding { + category: String::from("safe_outputs"), + severity: Severity::High, + title: format!("Custom job metadata identity mismatch for {tool}"), + description: format!( + "The typed pipeline graph assigns custom tool '{tool}' to ADO job \ + '{graph_job_id}', but the aw_info marker claims '{metadata_job_id}'." + ), + impact: Some(String::from( + "Untrusted runtime metadata does not match the compiler-derived job identity.", + )), + }); + } + let expected_job_id = graph_job_id.clone().or(metadata_job_id); + + let metadata_approval_path = entry + .metadata + .as_ref() + .and_then(|metadata| normalize_optional_string(metadata.approval_path.clone())); + let graph_approval_path = graph_job_id + .as_deref() + .and_then(|job_id| approval_path_from_graph(audit, job_id)); + if graph_job_id.is_some() + && metadata_approval_path.is_some() + && metadata_approval_path != graph_approval_path + { + findings.push(Finding { + category: String::from("safe_outputs"), + severity: Severity::High, + title: format!("Custom job metadata approval mismatch for {tool}"), + description: format!( + "The typed pipeline graph assigns custom tool '{tool}' to approval path \ + '{}', but the aw_info marker claims '{}'.", + graph_approval_path.as_deref().unwrap_or("automatic"), + metadata_approval_path.as_deref().unwrap_or("automatic") + ), + impact: Some(String::from( + "Untrusted runtime metadata does not match the compiler-derived approval path.", + )), + }); + } + let graph_is_authoritative = graph_job_id.is_some(); + let approval_path = if graph_is_authoritative { + graph_approval_path + } else { + metadata_approval_path + }; + if let (Some(component), Some(config_digest)) = + (entry.component.as_ref(), entry.config_schema_digest.as_ref()) + && !component.schema_digest.is_empty() + && component.schema_digest != *config_digest + { + findings.push(Finding { + category: String::from("safe_outputs"), + severity: Severity::High, + title: format!("Custom component provenance mismatch for {tool}"), + description: format!( + "The aw_info marker declares schema_digest={} for custom tool '{tool}', but compiler-generated custom-tools.json hashes to {}.", + component.schema_digest, config_digest + ), + impact: Some(String::from( + "The proposal schema does not match the compile-time provenance recorded for the custom component.", + )), + }); + } + + ToolGraphCorrelation { + expected_job_id, + graph_display_name, + approval_path, + graph_is_authoritative, + } +} + +/// Build the final `CustomSafeOutputJobAudit` report for a tool, merging +/// the freshly-correlated graph/metadata identity with the matching ADO +/// timeline job and falling back to the previous run's report for any +/// field the current artifacts don't supply. +fn build_report_for_tool( + audit: &AuditData, + tool: &str, + entry: CatalogEntry, + correlation: ToolGraphCorrelation, + proposal_counts: &BTreeMap, + previous_entry: Option<&CustomSafeOutputJobAudit>, +) -> CustomSafeOutputJobAudit { + let ToolGraphCorrelation { + expected_job_id, + graph_display_name, + approval_path, + graph_is_authoritative, + } = correlation; + + let matches = matching_timeline_jobs( + &audit.jobs, + tool, + expected_job_id.as_deref(), + graph_display_name.as_deref(), + ); + let selected = matches.last().copied(); + + let ado_job = selected + .map(custom_ado_job_from_timeline) + .or_else(|| previous_entry.and_then(|entry| entry.ado_job.clone())); + let expected_job_id = + expected_job_id.or_else(|| previous_entry.and_then(|entry| entry.expected_job_id.clone())); + let staged_requested = entry + .metadata + .as_ref() + .and_then(|metadata| metadata.staged_requested) + .or_else(|| previous_entry.and_then(|entry| entry.staged_requested)); + let component_provenance = entry + .component + .or_else(|| previous_entry.and_then(|entry| entry.component_provenance.clone())); + let proposal_time_acknowledgement = entry + .proposal_time_acknowledgement + .or_else(|| previous_entry.and_then(|entry| entry.proposal_time_acknowledgement.clone())); + + CustomSafeOutputJobAudit { + tool: tool.to_string(), + proposed_count: proposal_counts.get(tool).copied().unwrap_or(0), + expected_job_id, + component_provenance, + approval_path: if graph_is_authoritative { + approval_path + } else { + approval_path.or_else(|| previous_entry.and_then(|entry| entry.approval_path.clone())) + }, + staged_requested, + proposal_time_acknowledgement, + ado_job, + } +} + fn discover_custom_proposal_tools( audit: &AuditData, proposal_counts: &BTreeMap,