From 06fb47af24e233addd2a4afe0320ff58244cebe4 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 09:04:13 +0000 Subject: [PATCH] refactor(compile): reduce complexity of validate_github_issue_outputs_config Splits the 135-line, cognitive-complexity-25 function into six focused helpers: - validate_create_github_issue_approval_lanes - validate_github_issue_shared_policy - validate_github_issue_tool_specific_config - validate_github_issue_tools - validate_create_github_issue_injection - validate_set_github_issue_type_injection validate_github_issue_outputs_config now just orchestrates these calls. No behaviour change; all existing tests pass unmodified. Highest remaining complexity among the extracted helpers is 13 (down from 25), and the orchestrator itself is no longer flagged even at a cognitive-complexity-threshold of 10. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/compile/common.rs | 255 ++++++++++++++++++++++++------------------ 1 file changed, 149 insertions(+), 106 deletions(-) diff --git a/src/compile/common.rs b/src/compile/common.rs index b24239e47..de6f97076 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -2359,137 +2359,180 @@ fn require_same_approval_lane( } } -pub fn validate_github_issue_outputs_config(front_matter: &FrontMatter) -> Result<()> { - let github_tools = front_matter.github_issue_tool_names(); - if front_matter +/// Ensures every safe-output tool that accepts `create-github-issue`'s +/// temporary issue IDs sits in the same approval lane as its producer. +fn validate_create_github_issue_approval_lanes(front_matter: &FrontMatter) -> Result<()> { + if !front_matter .safe_outputs .contains_key("create-github-issue") { - for consumer in crate::compile::types::GITHUB_TEMPORARY_ID_CONSUMERS { - if front_matter.safe_outputs.contains_key(*consumer) { - require_same_approval_lane(front_matter, "create-github-issue", consumer)?; - } + return Ok(()); + } + for consumer in crate::compile::types::GITHUB_TEMPORARY_ID_CONSUMERS { + if front_matter.safe_outputs.contains_key(*consumer) { + require_same_approval_lane(front_matter, "create-github-issue", consumer)?; } } + Ok(()) +} - for tool in &github_tools { - let Some(config) = front_matter.github_issue_compiler_config(tool)? else { - continue; - }; - crate::safe_outputs::configured_github_repositories( - crate::safe_outputs::GithubRepositoryPolicy::new( - config.target_repo.as_deref(), - &config.allowed_repos, - ), +/// Validates the shared repository-policy and mutation-filter rules that +/// apply to every configured GitHub-issue safe-output tool. +fn validate_github_issue_shared_policy( + tool: &str, + config: &crate::compile::types::GithubIssueCompilerConfig, +) -> Result<()> { + crate::safe_outputs::configured_github_repositories( + crate::safe_outputs::GithubRepositoryPolicy::new( + config.target_repo.as_deref(), + &config.allowed_repos, + ), + ) + .map_err(|error| anyhow::anyhow!("safe-outputs.{tool} has invalid repository policy: {error}"))?; + if tool != "create-github-issue" { + crate::safe_outputs::validate_github_mutation_filter_config( + crate::safe_outputs::GithubMutationFilters { + required_labels: &config.required_labels, + required_title_prefix: config.required_title_prefix.as_deref(), + }, ) .map_err(|error| { - anyhow::anyhow!("safe-outputs.{tool} has invalid repository policy: {error}") + anyhow::anyhow!("safe-outputs.{tool} has invalid mutation filters: {error}") })?; - if tool != "create-github-issue" { - crate::safe_outputs::validate_github_mutation_filter_config( - crate::safe_outputs::GithubMutationFilters { - required_labels: &config.required_labels, - required_title_prefix: config.required_title_prefix.as_deref(), - }, - ) - .map_err(|error| { - anyhow::anyhow!("safe-outputs.{tool} has invalid mutation filters: {error}") - })?; - } - match tool.as_str() { - "comment-on-github-issue" => { - if let Some(config) = front_matter.comment_on_github_issue_config()? { - crate::safe_outputs::validate_comment_on_github_issue_config(&config)?; - } + } + Ok(()) +} + +/// Dispatches to the tool-specific validator for a single GitHub-issue +/// safe-output tool, if one is configured. +fn validate_github_issue_tool_specific_config(front_matter: &FrontMatter, tool: &str) -> Result<()> { + match tool { + "comment-on-github-issue" => { + if let Some(config) = front_matter.comment_on_github_issue_config()? { + crate::safe_outputs::validate_comment_on_github_issue_config(&config)?; } - "hide-github-issue-comment" => { - if let Some(config) = front_matter.hide_github_issue_comment_config()? { - crate::safe_outputs::validate_hide_github_issue_comment_config(&config)?; - } + } + "hide-github-issue-comment" => { + if let Some(config) = front_matter.hide_github_issue_comment_config()? { + crate::safe_outputs::validate_hide_github_issue_comment_config(&config)?; } - "add-github-issue-labels" => { - if let Some(config) = front_matter.add_github_issue_labels_config()? { - crate::safe_outputs::validate_add_github_issue_labels_config(&config)?; - } + } + "add-github-issue-labels" => { + if let Some(config) = front_matter.add_github_issue_labels_config()? { + crate::safe_outputs::validate_add_github_issue_labels_config(&config)?; } - "remove-github-issue-labels" => { - if let Some(config) = front_matter.remove_github_issue_labels_config()? { - crate::safe_outputs::validate_remove_github_issue_labels_config(&config)?; - } + } + "remove-github-issue-labels" => { + if let Some(config) = front_matter.remove_github_issue_labels_config()? { + crate::safe_outputs::validate_remove_github_issue_labels_config(&config)?; } - "close-github-issue" => { - if let Some(config) = front_matter.close_github_issue_config()? { - crate::safe_outputs::validate_close_github_issue_config(&config)?; - } + } + "close-github-issue" => { + if let Some(config) = front_matter.close_github_issue_config()? { + crate::safe_outputs::validate_close_github_issue_config(&config)?; } - "update-github-issue" => { - if let Some(config) = front_matter.update_github_issue_config()? { - crate::safe_outputs::validate_update_github_issue_config(&config)?; - } + } + "update-github-issue" => { + if let Some(config) = front_matter.update_github_issue_config()? { + crate::safe_outputs::validate_update_github_issue_config(&config)?; } - "set-github-issue-field" => { - if let Some(config) = front_matter.set_github_issue_field_config()? { - crate::safe_outputs::validate_set_github_issue_field_config(&config)?; - } + } + "set-github-issue-field" => { + if let Some(config) = front_matter.set_github_issue_field_config()? { + crate::safe_outputs::validate_set_github_issue_field_config(&config)?; } - "assign-github-issue-milestone" => { - if let Some(config) = front_matter.assign_github_issue_milestone_config()? { - crate::safe_outputs::validate_assign_github_issue_milestone_config(&config)?; - } + } + "assign-github-issue-milestone" => { + if let Some(config) = front_matter.assign_github_issue_milestone_config()? { + crate::safe_outputs::validate_assign_github_issue_milestone_config(&config)?; } - "assign-github-issue-to-user" => { - if let Some(config) = front_matter.assign_github_issue_to_user_config()? { - crate::safe_outputs::validate_assign_github_issue_to_user_config(&config)?; - } + } + "assign-github-issue-to-user" => { + if let Some(config) = front_matter.assign_github_issue_to_user_config()? { + crate::safe_outputs::validate_assign_github_issue_to_user_config(&config)?; } - "unassign-github-issue-from-user" => { - if let Some(config) = front_matter.unassign_github_issue_from_user_config()? { - crate::safe_outputs::validate_unassign_github_issue_from_user_config(&config)?; - } + } + "unassign-github-issue-from-user" => { + if let Some(config) = front_matter.unassign_github_issue_from_user_config()? { + crate::safe_outputs::validate_unassign_github_issue_from_user_config(&config)?; } - "link-github-sub-issue" => { - if let Some(config) = front_matter.link_github_sub_issue_config()? { - crate::safe_outputs::validate_link_github_sub_issue_config(&config)?; - } + } + "link-github-sub-issue" => { + if let Some(config) = front_matter.link_github_sub_issue_config()? { + crate::safe_outputs::validate_link_github_sub_issue_config(&config)?; } - _ => {} } + _ => {} } - if let Some(config) = front_matter.create_github_issue_config()? { - if let Some(prefix) = config.title_prefix.as_deref() { - crate::validate::reject_pipeline_injection( - prefix, - "safe-outputs.create-github-issue.title-prefix", - )?; - } - for label in &config.labels { - crate::validate::reject_pipeline_injection( - label, - "safe-outputs.create-github-issue.labels", - )?; - } - for label in &config.allowed_labels { - crate::validate::reject_pipeline_injection( - label, - "safe-outputs.create-github-issue.allowed-labels", - )?; - } - for assignee in &config.assignees { - crate::validate::reject_pipeline_injection( - assignee, - "safe-outputs.create-github-issue.assignees", - )?; - } + Ok(()) +} + +/// Validates the shared policy plus tool-specific config for every +/// configured GitHub-issue safe-output tool. +fn validate_github_issue_tools(front_matter: &FrontMatter, github_tools: &[String]) -> Result<()> { + for tool in github_tools { + let Some(config) = front_matter.github_issue_compiler_config(tool)? else { + continue; + }; + validate_github_issue_shared_policy(tool, &config)?; + validate_github_issue_tool_specific_config(front_matter, tool)?; } + Ok(()) +} - if let Some(config) = front_matter.set_github_issue_type_config()? { - for issue_type in &config.allowed { - crate::validate::reject_pipeline_injection( - issue_type, - "safe-outputs.set-github-issue-type.allowed", - )?; - } +/// Rejects pipeline-injection attempts in every user-controlled string field +/// of `safe-outputs.create-github-issue`. +fn validate_create_github_issue_injection(front_matter: &FrontMatter) -> Result<()> { + let Some(config) = front_matter.create_github_issue_config()? else { + return Ok(()); + }; + if let Some(prefix) = config.title_prefix.as_deref() { + crate::validate::reject_pipeline_injection( + prefix, + "safe-outputs.create-github-issue.title-prefix", + )?; + } + for label in &config.labels { + crate::validate::reject_pipeline_injection( + label, + "safe-outputs.create-github-issue.labels", + )?; } + for label in &config.allowed_labels { + crate::validate::reject_pipeline_injection( + label, + "safe-outputs.create-github-issue.allowed-labels", + )?; + } + for assignee in &config.assignees { + crate::validate::reject_pipeline_injection( + assignee, + "safe-outputs.create-github-issue.assignees", + )?; + } + Ok(()) +} + +/// Rejects pipeline-injection attempts in `safe-outputs.set-github-issue-type.allowed`. +fn validate_set_github_issue_type_injection(front_matter: &FrontMatter) -> Result<()> { + let Some(config) = front_matter.set_github_issue_type_config()? else { + return Ok(()); + }; + for issue_type in &config.allowed { + crate::validate::reject_pipeline_injection( + issue_type, + "safe-outputs.set-github-issue-type.allowed", + )?; + } + Ok(()) +} + +pub fn validate_github_issue_outputs_config(front_matter: &FrontMatter) -> Result<()> { + let github_tools = front_matter.github_issue_tool_names(); + validate_create_github_issue_approval_lanes(front_matter)?; + validate_github_issue_tools(front_matter, &github_tools)?; + validate_create_github_issue_injection(front_matter)?; + validate_set_github_issue_type_injection(front_matter)?; let _ = front_matter.github_app_permissions_for_tools(&github_tools)?; let _ = front_matter.github_safe_outputs_auth()?;