From 39d7823a7261f513551a3b844d2a76c519cde813 Mon Sep 17 00:00:00 2001 From: Bill Njoroge Date: Thu, 10 Sep 2026 22:42:42 -0400 Subject: [PATCH] feat(parser): align reusable workflow nesting depth to 10 and cap unique workflows at 50 --- crates/preloop-gha-parser/src/expand.rs | 62 ++++++- crates/preloop-gha-parser/src/lib.rs | 3 +- crates/preloop-gha-parser/src/lib_tests.rs | 178 +++++++++++++++++---- crates/preloop-gha-parser/src/models.rs | 14 +- 4 files changed, 225 insertions(+), 32 deletions(-) diff --git a/crates/preloop-gha-parser/src/expand.rs b/crates/preloop-gha-parser/src/expand.rs index f165293ea..a422cf119 100644 --- a/crates/preloop-gha-parser/src/expand.rs +++ b/crates/preloop-gha-parser/src/expand.rs @@ -15,6 +15,56 @@ use crate::{ MatrixValue, ParserError, ReusableCallMetadata, Step, Workflow, }; +/// Maximum number of workflow levels that may be connected (1 top-level caller + up to 9 nested reusable workflows). +pub const MAX_WORKFLOW_DEPTH: usize = 10; +/// Maximum nesting depth for called reusable workflows (up to 9 levels of reusable workflows below the top-level caller). +pub const MAX_REUSABLE_WORKFLOW_DEPTH: usize = MAX_WORKFLOW_DEPTH - 1; +/// Maximum unique reusable workflows that may be referenced in a single workflow run tree. +pub const MAX_UNIQUE_REUSABLE_WORKFLOWS: usize = 50; + +fn validate_reusable_workflow_tree( + workflow: &Workflow, + reusable_workflows: &BTreeMap, + depth: usize, + call_chain: &mut Vec, + unique_workflows: &mut std::collections::BTreeSet, +) -> Result<(), ParserError> { + for job in workflow.jobs.values() { + if let Some(uses) = &job.uses { + if depth >= MAX_REUSABLE_WORKFLOW_DEPTH { + return Err(ParserError::MaxNestingDepthExceeded); + } + let path = normalize_reusable_path(uses); + if call_chain.contains(&path) { + return Err(ParserError::MaxNestingDepthExceeded); + } + unique_workflows.insert(path.clone()); + if unique_workflows.len() > MAX_UNIQUE_REUSABLE_WORKFLOWS { + return Err(ParserError::MaxReusableWorkflowsExceeded { + count: unique_workflows.len(), + limit: MAX_UNIQUE_REUSABLE_WORKFLOWS, + }); + } + if let Some(yaml) = reusable_workflows + .get(uses) + .or_else(|| reusable_workflows.get(&path)) + { + let called = parse_workflow(yaml)?; + call_chain.push(path); + validate_reusable_workflow_tree( + &called, + reusable_workflows, + depth + 1, + call_chain, + unique_workflows, + )?; + call_chain.pop(); + } + } + } + Ok(()) +} + /// GitHub display name for one expanded job. /// /// When the job declares `name:`, expressions are resolved against the @@ -621,7 +671,7 @@ fn expand_jobs_with_reusables_internal( let global_env = workflow.env.clone().into_strings(); for (job_id, job) in &workflow.jobs { if let Some(uses) = &job.uses { - if depth >= 4 { + if depth >= MAX_REUSABLE_WORKFLOW_DEPTH { return Err(ParserError::MaxNestingDepthExceeded); } let path = normalize_reusable_path(uses); @@ -919,6 +969,16 @@ pub fn expand_jobs_with_reusables_and_shas_and_inputs_and_event( dispatch_inputs: Option<&BTreeMap>, event_name: Option<&str>, ) -> Result { + let mut unique_workflows = std::collections::BTreeSet::new(); + let mut call_chain = Vec::new(); + validate_reusable_workflow_tree( + workflow, + reusable_workflows, + 0, + &mut call_chain, + &mut unique_workflows, + )?; + let mut reusable_calls = BTreeMap::new(); let mut plans = expand_jobs_with_reusables_internal( workflow, diff --git a/crates/preloop-gha-parser/src/lib.rs b/crates/preloop-gha-parser/src/lib.rs index 750cef593..db94b0fc8 100644 --- a/crates/preloop-gha-parser/src/lib.rs +++ b/crates/preloop-gha-parser/src/lib.rs @@ -20,7 +20,8 @@ pub use expand::{ expand_jobs, expand_jobs_with_reusables, expand_jobs_with_reusables_and_shas, expand_jobs_with_reusables_and_shas_and_inputs, expand_jobs_with_reusables_and_shas_and_inputs_and_event, expand_reusable_call, - DEFAULT_TOKEN_PERMISSIONS, PERMISSION_SCOPES, + DEFAULT_TOKEN_PERMISSIONS, MAX_REUSABLE_WORKFLOW_DEPTH, MAX_UNIQUE_REUSABLE_WORKFLOWS, + MAX_WORKFLOW_DEPTH, PERMISSION_SCOPES, }; pub use models::*; pub use trigger::TriggerMismatch; diff --git a/crates/preloop-gha-parser/src/lib_tests.rs b/crates/preloop-gha-parser/src/lib_tests.rs index c16498b9c..955182e28 100644 --- a/crates/preloop-gha-parser/src/lib_tests.rs +++ b/crates/preloop-gha-parser/src/lib_tests.rs @@ -1463,7 +1463,7 @@ jobs: } #[test] -fn reusable_workflow_max_depth_exceeded() { +fn reusable_workflow_up_to_max_depth_succeeds() { let caller = parse_workflow( r#" on: push @@ -1475,39 +1475,24 @@ jobs: .unwrap(); let mut reusable = BTreeMap::new(); + for i in 1..9 { + reusable.insert( + format!(".github/workflows/level{i}.yml"), + format!("on: {{ workflow_call: {{}} }}\njobs:\n call{}:\n uses: ./.github/workflows/level{}.yml", i + 1, i + 1), + ); + } + // Level 9 is the 9th nested reusable workflow (10th level overall) — it has leaf steps. reusable.insert( - ".github/workflows/level1.yml".to_owned(), - "on: { workflow_call: {} }\njobs:\n call2:\n uses: ./.github/workflows/level2.yml" - .to_owned(), - ); - reusable.insert( - ".github/workflows/level2.yml".to_owned(), - "on: { workflow_call: {} }\njobs:\n call3:\n uses: ./.github/workflows/level3.yml" - .to_owned(), - ); - reusable.insert( - ".github/workflows/level3.yml".to_owned(), - "on: { workflow_call: {} }\njobs:\n call4:\n uses: ./.github/workflows/level4.yml" - .to_owned(), - ); - reusable.insert( - ".github/workflows/level4.yml".to_owned(), - "on: { workflow_call: {} }\njobs:\n call5:\n uses: ./.github/workflows/level5.yml" + ".github/workflows/level9.yml".to_owned(), + "on: { workflow_call: {} }\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: echo leaf" .to_owned(), ); - reusable.insert( - ".github/workflows/level5.yml".to_owned(), - "on: { workflow_call: {} }\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: echo leaf".to_owned(), - ); - // Deferred materialization: the root expansion emits a single caller node. - // Depth is enforced when each nested caller's subtree is expanded at - // runtime — the fifth nested call exceeds the limit of four. let root = expand_jobs_with_reusables(&caller, &reusable).unwrap(); assert_eq!(root.jobs.len(), 1); let mut node = root.jobs[0].clone(); - for level in 1..=3 { + for level in 1..=8 { let called = parse_workflow(&reusable[&format!(".github/workflows/level{level}.yml")]).unwrap(); let expanded = @@ -1516,8 +1501,145 @@ jobs: assert!(node.reusable_call.is_some()); } - let level4 = parse_workflow(&reusable[".github/workflows/level4.yml"]).unwrap(); - let res = crate::expand_reusable_call(&level4, &node, &reusable, &BTreeMap::new()); + // Expand level 9 leaf jobs + let level9 = parse_workflow(&reusable[".github/workflows/level9.yml"]).unwrap(); + let expanded = + crate::expand_reusable_call(&level9, &node, &reusable, &BTreeMap::new()).unwrap(); + assert_eq!(expanded.jobs.len(), 1); + assert!(expanded.jobs[0].reusable_call.is_none()); +} + +#[test] +fn reusable_workflow_max_depth_exceeded() { + let caller = parse_workflow( + r#" +on: push +jobs: + call1: + uses: ./.github/workflows/level1.yml +"#, + ) + .unwrap(); + + let mut reusable = BTreeMap::new(); + for i in 1..10 { + reusable.insert( + format!(".github/workflows/level{i}.yml"), + format!("on: {{ workflow_call: {{}} }}\njobs:\n call{}:\n uses: ./.github/workflows/level{}.yml", i + 1, i + 1), + ); + } + reusable.insert( + ".github/workflows/level10.yml".to_owned(), + "on: { workflow_call: {} }\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: echo leaf" + .to_owned(), + ); + + // 10 levels of nested reusable workflows means 11 workflow levels total, which exceeds the max depth of 10. + let root_res = expand_jobs_with_reusables(&caller, &reusable); + assert!(matches!( + root_res.unwrap_err(), + ParserError::MaxNestingDepthExceeded + )); +} + +#[test] +fn reusable_workflow_max_unique_succeeds() { + // Caller references 50 unique reusable workflows across 50 jobs. + let mut caller_yaml = "on: push\njobs:\n".to_owned(); + let mut reusable = BTreeMap::new(); + for i in 1..=50 { + caller_yaml.push_str(&format!( + " job{i}:\n uses: ./.github/workflows/sub{i}.yml\n" + )); + reusable.insert( + format!(".github/workflows/sub{i}.yml"), + "on: { workflow_call: {} }\njobs:\n leaf:\n runs-on: ubuntu-latest\n steps:\n - run: echo ok".to_owned(), + ); + } + + let caller = parse_workflow(&caller_yaml).unwrap(); + let root = expand_jobs_with_reusables(&caller, &reusable); + assert!(root.is_ok(), "50 unique reusable workflows must be allowed"); + assert_eq!(root.unwrap().jobs.len(), 50); +} + +#[test] +fn reusable_workflow_max_unique_exceeded() { + // Caller references 51 unique reusable workflows across 51 jobs. + let mut caller_yaml = "on: push\njobs:\n".to_owned(); + let mut reusable = BTreeMap::new(); + for i in 1..=51 { + caller_yaml.push_str(&format!( + " job{i}:\n uses: ./.github/workflows/sub{i}.yml\n" + )); + reusable.insert( + format!(".github/workflows/sub{i}.yml"), + "on: { workflow_call: {} }\njobs:\n leaf:\n runs-on: ubuntu-latest\n steps:\n - run: echo ok".to_owned(), + ); + } + + let caller = parse_workflow(&caller_yaml).unwrap(); + let res = expand_jobs_with_reusables(&caller, &reusable); + assert!(matches!( + res.unwrap_err(), + ParserError::MaxReusableWorkflowsExceeded { + count: 51, + limit: 50 + } + )); +} + +#[test] +fn reusable_workflow_duplicate_calls_do_not_count_towards_unique_limit() { + // Caller calls 2 unique reusable workflows across 60 jobs. + let mut caller_yaml = "on: push\njobs:\n".to_owned(); + let mut reusable = BTreeMap::new(); + reusable.insert( + ".github/workflows/subA.yml".to_owned(), + "on: { workflow_call: {} }\njobs:\n leaf:\n runs-on: ubuntu-latest\n steps:\n - run: echo A".to_owned(), + ); + reusable.insert( + ".github/workflows/subB.yml".to_owned(), + "on: { workflow_call: {} }\njobs:\n leaf:\n runs-on: ubuntu-latest\n steps:\n - run: echo B".to_owned(), + ); + for i in 1..=60 { + let target = if i % 2 == 0 { "subA" } else { "subB" }; + caller_yaml.push_str(&format!( + " job{i}:\n uses: ./.github/workflows/{target}.yml\n" + )); + } + + let caller = parse_workflow(&caller_yaml).unwrap(); + let root = expand_jobs_with_reusables(&caller, &reusable); + assert!(root.is_ok(), "60 calls to 2 unique workflows must succeed"); + assert_eq!(root.unwrap().jobs.len(), 60); +} + +#[test] +fn reusable_workflow_cycle_detected() { + let caller = parse_workflow( + r#" +on: push +jobs: + call1: + uses: ./.github/workflows/cycleA.yml +"#, + ) + .unwrap(); + + let mut reusable = BTreeMap::new(); + reusable.insert( + ".github/workflows/cycleA.yml".to_owned(), + "on: { workflow_call: {} }\njobs:\n callB:\n uses: ./.github/workflows/cycleB.yml" + .to_owned(), + ); + reusable.insert( + ".github/workflows/cycleB.yml".to_owned(), + "on: { workflow_call: {} }\njobs:\n callA:\n uses: ./.github/workflows/cycleA.yml" + .to_owned(), + ); + + let res = expand_jobs_with_reusables(&caller, &reusable); assert!(matches!( res.unwrap_err(), ParserError::MaxNestingDepthExceeded diff --git a/crates/preloop-gha-parser/src/models.rs b/crates/preloop-gha-parser/src/models.rs index b57e5c73c..a0bfaa009 100644 --- a/crates/preloop-gha-parser/src/models.rs +++ b/crates/preloop-gha-parser/src/models.rs @@ -79,9 +79,19 @@ pub enum ParserError { /// `on:` names an event GitHub does not recognize. #[error("invalid workflow trigger event `{0}`")] InvalidTriggerEvent(String), - /// Maximum nesting depth for reusable workflows exceeded. - #[error("maximum nested reusable workflows depth (4) exceeded")] + /// Maximum nesting depth for reusable workflows exceeded (maximum 10 connected workflow levels). + #[error("maximum nested reusable workflows depth (10) exceeded")] MaxNestingDepthExceeded, + /// Maximum unique reusable workflows limit exceeded in workflow tree. + #[error( + "maximum unique reusable workflows ({limit}) exceeded in workflow tree: found {count}" + )] + MaxReusableWorkflowsExceeded { + /// Number of unique reusable workflows found. + count: usize, + /// Maximum allowed unique reusable workflows. + limit: usize, + }, /// Called workflow does not declare `on: workflow_call` trigger. #[error("called workflow does not declare `on: workflow_call` trigger")] MissingWorkflowCallTrigger,