diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 17aafe5..6ec0feb 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -8,6 +8,10 @@ updates: cargo: patterns: - "*" + # serde-saphyr patches only its latest release, and its releases can change how YAML parses, + # so its bumps get their own pull request instead of waiting on unrelated breakage. + exclude-patterns: + - "serde-saphyr" cooldown: default-days: 7 - package-ecosystem: "github-actions" diff --git a/Cargo.lock b/Cargo.lock index eac3781..3fe878a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,6 +11,17 @@ dependencies = [ "memchr", ] +[[package]] +name = "annotate-snippets" +version = "0.12.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c86cd1c51b95d71dde52bca69ed225008f6ff4c8cc825b08042aa1ef823e1980" +dependencies = [ + "anstyle", + "memchr", + "unicode-width 0.2.2", +] + [[package]] name = "anstream" version = "0.6.18" @@ -66,6 +77,12 @@ version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" +[[package]] +name = "arraydeque" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d902e3d592a523def97af8f317b08ce16b7ab854c1985a0c671e6f15cebc236" + [[package]] name = "arrayvec" version = "0.7.6" @@ -189,8 +206,8 @@ dependencies = [ "regex", "rusty-hook", "serde", + "serde-saphyr", "serde_json", - "serde_yaml", "similar", "tempfile", "tracing", @@ -203,6 +220,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" +[[package]] +name = "core_detect" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f8f80099a98041a3d1622845c271458a2d73e688351bf3cb999266764b81d48" + [[package]] name = "crossbeam-channel" version = "0.5.17" @@ -256,6 +279,29 @@ version = "1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7fcaabb2fef8c910e7f4c7ce9f67a1283a1715879a7c230ca9d6d1ae31f16d91" +[[package]] +name = "encoding_rs" +version = "0.8.42" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e985e0451871ad22fb8d2b6b076e2028a502a0d3950998c2c5c0a4f9b5d9679" +dependencies = [ + "cfg-if", + "core_detect", + "multiversion_no_op", + "rustversion", + "scopeguard", + "simdutf8", +] + +[[package]] +name = "encoding_rs_io" +version = "0.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fba3fe847045ecff794b9c138293a80db914678c453ad63fbf0c6a9eb6e00b22" +dependencies = [ + "encoding_rs", +] + [[package]] name = "enum_dispatch" version = "0.3.13" @@ -275,15 +321,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2d328fc287c61314c4a61af7cfdcbd7e678e39778488c7cb13ec133ce0f4059" dependencies = [ "fsio", - "indexmap 1.9.3", + "indexmap", ] -[[package]] -name = "equivalent" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" - [[package]] name = "errno" version = "0.3.13" @@ -340,7 +380,7 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14dbbfd5c71d70241ecf9e6f13737f7b5ce823821063188d7e46c41d371eebd5" dependencies = [ - "unicode-width", + "unicode-width 0.1.10", ] [[package]] @@ -375,16 +415,20 @@ dependencies = [ ] [[package]] -name = "hashbrown" -version = "0.12.3" +name = "granit-parser" +version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +checksum = "da55feca5d1625466bae068f9fd4b87892caeae70c3fe671ddb0f50d41aebf37" +dependencies = [ + "arraydeque", + "smallvec", +] [[package]] name = "hashbrown" -version = "0.15.1" +version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3a9bfc1af68b1726ea47d3d5109de126281def866b33970e10fbab11b5dafab3" +checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" [[package]] name = "heck" @@ -415,17 +459,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" dependencies = [ "autocfg", - "hashbrown 0.12.3", -] - -[[package]] -name = "indexmap" -version = "2.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "707907fe3c25f5424cce2cb7e1cbcafee6bdbe735ca90ef77c29e84591e5b9da" -dependencies = [ - "equivalent", - "hashbrown 0.15.1", + "hashbrown", ] [[package]] @@ -519,6 +553,12 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "multiversion_no_op" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "743fb55ba31b18fb1ecef6bdc9aa2743314978ac084044301a7eee33fb99a20d" + [[package]] name = "nias" version = "0.5.0" @@ -720,12 +760,6 @@ dependencies = [ "toml", ] -[[package]] -name = "ryu" -version = "1.0.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f91339c0467de62360649f8d3e185ca8de4224ff281f66000de5eb2a77a79041" - [[package]] name = "same-file" version = "1.0.6" @@ -735,6 +769,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + [[package]] name = "semver" version = "1.0.17" @@ -751,6 +791,20 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-saphyr" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a253edfdea34032da21af84e16fcc94f4c7a6ba237f9f3bc2dbf43cea8535c44" +dependencies = [ + "annotate-snippets", + "encoding_rs_io", + "granit-parser", + "num-traits", + "serde_core", + "smallvec", +] + [[package]] name = "serde_core" version = "1.0.229" @@ -784,19 +838,6 @@ dependencies = [ "zmij", ] -[[package]] -name = "serde_yaml" -version = "0.9.34+deprecated" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" -dependencies = [ - "indexmap 2.6.0", - "itoa", - "ryu", - "serde", - "unsafe-libyaml", -] - [[package]] name = "sharded-slab" version = "0.1.4" @@ -806,6 +847,12 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "simdutf8" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3a9fe34e3e7a50316060351f37187a3f546bce95496156754b601a5fa71b76e" + [[package]] name = "similar" version = "3.2.0" @@ -972,10 +1019,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0edd1e5b14653f783770bce4a4dabb4a5108a5370a5f5d8cfe8710c361f6c8b" [[package]] -name = "unsafe-libyaml" -version = "0.2.11" +name = "unicode-width" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" +checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254" [[package]] name = "utf8parse" diff --git a/Cargo.toml b/Cargo.toml index 1f4937b..6bcf46c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,9 @@ rayon = "1.10.0" regex = "1.11.1" serde = { version = "1.0.219", features = ["derive"] } serde_json = "1.0.143" -serde_yaml = "0.9.34" +# Deserialize only: `default-features = false` leaves out the serializer and YAML comment +# capture, which we do not use. +serde-saphyr = { version = "2.0.0", default-features = false, features = ["deserialize"] } similar = "3.2.0" tempfile = "3.21.0" tracing = "0.1.41" diff --git a/README.md b/README.md index ebbafcf..f97c13c 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,7 @@ codeowners gv --no-cache - `cache_directory` (default: `'tmp/cache/codeowners'`) - `ignore_dirs` (default includes: `.git`, `node_modules`, `tmp`, etc.) - `executable_name` (default: `'codeowners'`): Customize the command name shown in validation error messages. Useful when using `codeowners-rs` via wrappers like the [code_ownership](https://github.com/rubyatscale/code_ownership) Ruby gem. +- `yaml_max_nodes` (default: `20000`): The most YAML nodes (mappings, sequences and scalars) allowed in one team file or `package.yml`. A file over the limit fails with `budget breached: Nodes`. Raise it if a large team file hits the limit; real files have at most a few thousand nodes. The limits on a file's total string bytes, parser events and anchor copies grow with it. It does not apply to `config/code_ownership.yml` itself. Example configuration with custom executable name: diff --git a/src/common_test.rs b/src/common_test.rs index 57a8bd8..fdb0ffa 100644 --- a/src/common_test.rs +++ b/src/common_test.rs @@ -1,10 +1,6 @@ #[cfg(test)] pub mod tests { - use std::{ - error::Error, - fs::{self, File}, - path::PathBuf, - }; + use std::{error::Error, fs, path::PathBuf}; use indoc::indoc; @@ -111,8 +107,7 @@ pub mod tests { fs::write(test_config.temp_dir_path.join(&project_file.relative_path), &project_file.content)?; } - let config_file = File::open(test_config.temp_dir_path.join(test_config.relative_code_ownership_config_yml_path))?; - let config: Config = serde_yaml::from_reader(config_file)?; + let config = Config::load_from_path(&test_config.temp_dir_path.join(test_config.relative_code_ownership_config_yml_path))?; let codeowners_file_path = &test_config.temp_dir_path.join(".github/CODEOWNERS"); let cache: Cache = NoopCache::default().into(); diff --git a/src/config.rs b/src/config.rs index 620185f..3d7002a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,36 +1,48 @@ use serde::Deserialize; -use std::{fs::File, path::Path}; +use std::path::Path; #[derive(Deserialize, Debug, Clone)] pub struct Config { + #[serde(deserialize_with = "crate::yaml::strings_or_nulls")] pub owned_globs: Vec, - #[serde(default = "ruby_package_paths")] + #[serde(default = "ruby_package_paths", deserialize_with = "crate::yaml::strings_or_nulls")] pub ruby_package_paths: Vec, - #[serde(alias = "js_package_paths", default = "javascript_package_paths")] + #[serde( + alias = "js_package_paths", + default = "javascript_package_paths", + deserialize_with = "crate::yaml::strings_or_nulls" + )] pub javascript_package_paths: Vec, - #[serde(default = "team_file_glob")] + #[serde(default = "team_file_glob", deserialize_with = "crate::yaml::strings_or_nulls")] pub team_file_glob: Vec, - #[serde(default = "unowned_globs")] + #[serde(default = "unowned_globs", deserialize_with = "crate::yaml::strings_or_nulls")] pub unowned_globs: Vec, - #[serde(alias = "unbuilt_gems_path", default = "vendored_gems_path")] + #[serde( + alias = "unbuilt_gems_path", + default = "vendored_gems_path", + deserialize_with = "crate::yaml::string_or_null" + )] pub vendored_gems_path: String, - #[serde(default = "default_cache_directory")] + #[serde(default = "default_cache_directory", deserialize_with = "crate::yaml::string_or_null")] pub cache_directory: String, - #[serde(default = "default_ignore_dirs")] + #[serde(default = "default_ignore_dirs", deserialize_with = "crate::yaml::strings_or_nulls")] pub ignore_dirs: Vec, - #[serde(default = "default_executable_name")] + #[serde(default = "default_executable_name", deserialize_with = "crate::yaml::string_or_null")] pub executable_name: String, - #[serde(default = "default_codeowners_path")] + #[serde(default = "default_codeowners_path", deserialize_with = "crate::yaml::string_or_null")] pub codeowners_path: String, + + #[serde(default = "default_yaml_max_nodes")] + pub yaml_max_nodes: usize, } #[allow(dead_code)] @@ -91,19 +103,22 @@ fn default_codeowners_path() -> String { ".github".to_string() } +fn default_yaml_max_nodes() -> usize { + crate::yaml::DEFAULT_MAX_NODES +} + impl Config { pub fn load_from_path(path: &Path) -> std::result::Result { - let file = File::open(path).map_err(|e| format!("Can't open config file: {} ({})", path.to_string_lossy(), e))?; - serde_yaml::from_reader(file).map_err(|e| format!("Can't parse config file: {} ({})", path.to_string_lossy(), e)) + let bytes = std::fs::read(path).map_err(|e| format!("Can't open config file: {} ({})", path.to_string_lossy(), e))?; + // The config can't raise its own limit, and at a few hundred nodes it never needs to. + crate::yaml::from_slice(&bytes, crate::yaml::DEFAULT_MAX_NODES) + .map_err(|e| format!("Can't parse config file: {} ({})", path.to_string_lossy(), e)) } } #[cfg(test)] mod tests { - use std::{ - error::Error, - fs::{self, File}, - }; + use std::{error::Error, fs}; use indoc::indoc; use tempfile::tempdir; @@ -120,8 +135,7 @@ mod tests { - \"{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx,json,yml}\" "}; fs::write(&config_path, config_str)?; - let config_file = File::open(&config_path)?; - let config: Config = serde_yaml::from_reader(config_file)?; + let config = Config::load_from_path(&config_path)?; assert_eq!( config.owned_globs, vec!["{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx,json,yml}"] @@ -135,6 +149,23 @@ mod tests { ); assert_eq!(config.vendored_gems_path, "vendored/"); assert_eq!(config.executable_name, "codeowners generate"); + assert_eq!(config.yaml_max_nodes, 20_000); + Ok(()) + } + + #[test] + fn test_parse_config_with_custom_yaml_max_nodes() -> Result<(), Box> { + let temp_dir = tempdir()?; + let config_path = temp_dir.path().join("config.yml"); + let config_str = indoc! {" + --- + owned_globs: + - \"**/*.rb\" + yaml_max_nodes: 200000 + "}; + fs::write(&config_path, config_str)?; + let config = Config::load_from_path(&config_path)?; + assert_eq!(config.yaml_max_nodes, 200_000); Ok(()) } @@ -149,8 +180,7 @@ mod tests { executable_name: my-custom-codeowners "}; fs::write(&config_path, config_str)?; - let config_file = File::open(&config_path)?; - let config: Config = serde_yaml::from_reader(config_file)?; + let config = Config::load_from_path(&config_path)?; assert_eq!(config.executable_name, "my-custom-codeowners"); Ok(()) } @@ -165,8 +195,7 @@ mod tests { - \"**/*.rb\" "}; fs::write(&config_path, config_str)?; - let config_file = File::open(&config_path)?; - let config: Config = serde_yaml::from_reader(config_file)?; + let config = Config::load_from_path(&config_path)?; assert_eq!(config.executable_name, "codeowners generate"); Ok(()) } @@ -181,8 +210,7 @@ mod tests { - \"**/*.rb\" "}; fs::write(&config_path, config_str)?; - let config_file = File::open(&config_path)?; - let config: Config = serde_yaml::from_reader(config_file)?; + let config = Config::load_from_path(&config_path)?; assert_eq!(config.codeowners_path, ".github"); Ok(()) } @@ -198,9 +226,67 @@ mod tests { codeowners_path: docs "}; fs::write(&config_path, config_str)?; - let config_file = File::open(&config_path)?; - let config: Config = serde_yaml::from_reader(config_file)?; + let config = Config::load_from_path(&config_path)?; assert_eq!(config.codeowners_path, "docs"); Ok(()) } + + #[test] + fn test_parse_config_with_empty_values() -> Result<(), Box> { + let temp_dir = tempdir()?; + let config_path = temp_dir.path().join("config.yml"); + let config_str = indoc! {" + --- + owned_globs: + - \"**/*.rb\" + - + ruby_package_paths: + - + javascript_package_paths: + - + team_file_glob: + - + unowned_globs: + - + ignore_dirs: + - + vendored_gems_path: + cache_directory: + executable_name: + codeowners_path: + "}; + fs::write(&config_path, config_str)?; + let config = Config::load_from_path(&config_path)?; + assert_eq!(config.owned_globs, vec!["**/*.rb", ""]); + for globs in [ + &config.ruby_package_paths, + &config.javascript_package_paths, + &config.team_file_glob, + &config.unowned_globs, + &config.ignore_dirs, + ] { + assert_eq!(globs, &[""]); + } + for value in [ + &config.vendored_gems_path, + &config.cache_directory, + &config.executable_name, + &config.codeowners_path, + ] { + assert_eq!(value, ""); + } + Ok(()) + } + + #[test] + fn test_load_from_path_reports_parse_error_on_one_line() -> Result<(), Box> { + let temp_dir = tempdir()?; + let config_path = temp_dir.path().join("config.yml"); + fs::write(&config_path, "owned_globs: {app: true}\n")?; + let error = Config::load_from_path(&config_path).unwrap_err(); + assert!(error.starts_with("Can't parse config file: "), "{error}"); + assert!(error.contains("line 1"), "{error}"); + assert!(!error.contains('\n'), "{error}"); + Ok(()) + } } diff --git a/src/lib.rs b/src/lib.rs index 13d53a4..eb35bd2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,3 +9,4 @@ pub mod project_builder; pub mod project_file_builder; pub mod runner; pub(crate) mod tracked_files; +pub(crate) mod yaml; diff --git a/src/ownership/codeowners_file_parser.rs b/src/ownership/codeowners_file_parser.rs index fd25691..1dc538a 100644 --- a/src/ownership/codeowners_file_parser.rs +++ b/src/ownership/codeowners_file_parser.rs @@ -14,6 +14,7 @@ pub struct Parser { pub project_root: PathBuf, pub codeowners_file_path: PathBuf, pub team_file_globs: Vec, + pub yaml_max_nodes: usize, } impl Parser { @@ -40,7 +41,7 @@ impl Parser { let codeowners_entries = parse_codeowners_entries(self.codeowners_file_path.to_string_lossy().into_owned()); - let teams_by_name = teams_by_github_team_name(self.absolute_team_files_globs()); + let teams_by_name = teams_by_github_team_name(self.absolute_team_files_globs(), self.yaml_max_nodes); let result: HashMap> = file_inputs .par_iter() @@ -65,13 +66,13 @@ impl Parser { } #[memoize] -fn teams_by_github_team_name(team_file_glob: Vec) -> HashMap { +fn teams_by_github_team_name(team_file_glob: Vec, yaml_max_nodes: usize) -> HashMap { let mut teams = HashMap::new(); for glob in team_file_glob { match glob::glob(&glob) { Ok(paths) => { for path in paths.filter_map(Result::ok) { - let team = match Team::from_team_file_path(path.clone()) { + let team = match Team::from_team_file_path(path.clone(), yaml_max_nodes) { Ok(team) => team, Err(e) => { eprintln!("Error parsing team file: {e:?}, path: {}", path.display()); diff --git a/src/ownership/codeowners_query.rs b/src/ownership/codeowners_query.rs index 562bb48..3486e25 100644 --- a/src/ownership/codeowners_query.rs +++ b/src/ownership/codeowners_query.rs @@ -8,6 +8,7 @@ pub(crate) fn teams_for_files_from_codeowners( project_root: &Path, codeowners_file_path: &Path, team_file_globs: &[String], + yaml_max_nodes: usize, file_paths: &[String], ) -> Result>, String> { let relative_file_paths: Vec = file_paths @@ -26,6 +27,7 @@ pub(crate) fn teams_for_files_from_codeowners( codeowners_file_path: codeowners_file_path.to_path_buf(), project_root: project_root.to_path_buf(), team_file_globs: team_file_globs.to_vec(), + yaml_max_nodes, }; parser.teams_from_files_paths(&relative_file_paths).map_err(|e| e.to_string()) diff --git a/src/ownership/file_owner_resolver.rs b/src/ownership/file_owner_resolver.rs index cc7f9fa..42da95b 100644 --- a/src/ownership/file_owner_resolver.rs +++ b/src/ownership/file_owner_resolver.rs @@ -20,7 +20,7 @@ pub fn find_file_owners(project_root: &Path, config: &Config, file_path: &Path) }; let relative_file_path = crate::path_utils::relative_to_buf(project_root, &absolute_file_path); - let loaded = loaded_teams(teams_cache_root(project_root), config.team_file_glob.clone())?; + let loaded = loaded_teams(teams_cache_root(project_root), config.team_file_glob.clone(), config.yaml_max_nodes)?; let teams = &loaded.teams; let teams_by_name = &loaded.teams_by_name; @@ -105,9 +105,9 @@ struct LoadedTeams { teams_by_name: HashMap, } -type TeamCacheKey = (PathBuf, Vec); +type TeamCacheKey = (PathBuf, Vec, usize); -// Parsing every team file dominates a lookup, so load them once per project root and glob list and share +// Parsing every team file dominates a lookup, so load them once per project root, glob list and node limit and share // the result across threads for the life of the process. static TEAM_CACHE: LazyLock> = LazyLock::new(Default::default); @@ -122,8 +122,12 @@ fn team_cache() -> MutexGuard<'static, TeamCache> { TEAM_CACHE.lock().unwrap_or_else(PoisonError::into_inner) } -fn loaded_teams(project_root: PathBuf, team_file_globs: Vec) -> std::result::Result, String> { - let key = (project_root, team_file_globs); +fn loaded_teams( + project_root: PathBuf, + team_file_globs: Vec, + yaml_max_nodes: usize, +) -> std::result::Result, String> { + let key = (project_root, team_file_globs, yaml_max_nodes); let generation = { let cache = team_cache(); if let Some(loaded) = cache.loaded.get(&key) { @@ -132,7 +136,7 @@ fn loaded_teams(project_root: PathBuf, team_file_globs: Vec) -> std::res cache.generation }; - let load = load_teams(&key.0, &key.1)?; + let load = load_teams(&key.0, &key.1, key.2)?; let teams_by_name = build_teams_by_name_map(&load.teams); let loaded = Arc::new(LoadedTeams { teams: load.teams, @@ -178,7 +182,7 @@ struct TeamLoad { skipped_team_file: bool, } -fn load_teams(project_root: &Path, team_file_globs: &[String]) -> std::result::Result { +fn load_teams(project_root: &Path, team_file_globs: &[String], yaml_max_nodes: usize) -> std::result::Result { let mut teams: Vec = Vec::new(); let mut skipped_team_file = false; for glob_str in team_file_globs { @@ -193,7 +197,7 @@ fn load_teams(project_root: &Path, team_file_globs: &[String]) -> std::result::R continue; } }; - match Team::from_team_file_path(path.clone()) { + match Team::from_team_file_path(path.clone(), yaml_max_nodes) { Ok(team) => teams.push(team), Err(e) => { eprintln!("Error parsing team file: {e:?}, path: {}", path.display()); @@ -266,7 +270,7 @@ fn nearest_package_owner( if glob_list_matches(rel_str, &config.ruby_package_paths) { let pkg_yml = current.join("package.yml"); if pkg_yml.exists() { - match crate::project_builder::ruby_package_owner(&pkg_yml) { + match crate::project_builder::ruby_package_owner(&pkg_yml, config.yaml_max_nodes) { Ok(owner) => { if let Some(team) = owner.and_then(|owner| teams_by_name.get(&owner)) { let package_path = parent_rel.join("package.yml"); @@ -377,6 +381,7 @@ mod tests { ignore_dirs: vec![], executable_name: "codeowners".to_string(), codeowners_path: ".github".to_string(), + yaml_max_nodes: crate::yaml::DEFAULT_MAX_NODES, } } @@ -678,7 +683,11 @@ mod tests { #[test] fn test_clear_team_cache_during_a_load_is_not_undone_by_its_result() { let _guard = TEAM_CACHE_TEST_LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); - let key: TeamCacheKey = (PathBuf::from("/clear-during-load"), vec!["config/teams/**/*.yml".to_string()]); + let key: TeamCacheKey = ( + PathBuf::from("/clear-during-load"), + vec!["config/teams/**/*.yml".to_string()], + crate::yaml::DEFAULT_MAX_NODES, + ); let loaded = || { Arc::new(LoadedTeams { teams: Vec::new(), @@ -826,4 +835,32 @@ mod tests { assert_eq!(owner_of(&config_b), Some("Billing".to_string())); assert_eq!(owner_of(&config_a), Some("Payroll".to_string())); } + + #[test] + fn test_team_cache_is_keyed_on_yaml_max_nodes() { + let td = tempdir().unwrap(); + let root = td.path(); + fs::create_dir_all(root.join("config/teams")).unwrap(); + fs::write( + root.join("config/teams/payroll.yml"), + "name: Payroll\ngithub:\n team: '@PayrollTeam'\nowned_globs:\n - app/**/*\n", + ) + .unwrap(); + let config_with_max_nodes = |yaml_max_nodes: usize| crate::config::Config { + yaml_max_nodes, + ..build_config_for_temp("frontend/**/*", "packs/**/*", "vendored") + }; + let owner_of = |config: &crate::config::Config| { + find_file_owners(root, config, Path::new("app/a.rb")) + .unwrap() + .first() + .map(|owner| owner.team.name.clone()) + }; + + assert_eq!( + owner_of(&config_with_max_nodes(crate::yaml::DEFAULT_MAX_NODES)), + Some("Payroll".to_string()) + ); + assert_eq!(owner_of(&config_with_max_nodes(3)), None, "a team file over the limit is skipped"); + } } diff --git a/src/project.rs b/src/project.rs index 12b09a6..3deb58e 100644 --- a/src/project.rs +++ b/src/project.rs @@ -2,7 +2,6 @@ use core::fmt; use std::{ collections::HashMap, fmt::Display, - fs::File, path::{Path, PathBuf}, }; @@ -44,9 +43,9 @@ pub struct Team { } impl Team { - pub fn from_team_file_path(absolute_path: PathBuf) -> Result> { - let file = File::open(&absolute_path).change_context(Error::Io)?; - let deserializer: deserializers::Team = serde_yaml::from_reader(file).change_context(Error::SerdeYaml)?; + pub fn from_team_file_path(absolute_path: PathBuf, yaml_max_nodes: usize) -> Result> { + let bytes = std::fs::read(&absolute_path).change_context(Error::Io)?; + let deserializer: deserializers::Team = crate::yaml::from_project_slice(&bytes, yaml_max_nodes).change_context(Error::SerdeYaml)?; Ok(Self { path: absolute_path.to_owned(), name: deserializer.name, @@ -117,6 +116,7 @@ pub mod deserializers { #[derive(Deserialize)] pub struct Github { + #[serde(deserialize_with = "crate::yaml::string_or_null")] pub team: String, #[serde(default = "bool_false")] pub do_not_add_to_codeowners_file: bool, @@ -124,20 +124,25 @@ pub mod deserializers { #[derive(Deserialize)] pub struct Ruby { - #[serde(default = "empty_string_vec")] + #[serde(default = "empty_string_vec", deserialize_with = "crate::yaml::strings_or_nulls")] pub owned_gems: Vec, } #[derive(Deserialize)] pub struct Team { + #[serde(deserialize_with = "crate::yaml::string_or_null")] pub name: String, pub github: Github, pub ruby: Option, - #[serde(default = "empty_string_vec")] + #[serde(default = "empty_string_vec", deserialize_with = "crate::yaml::strings_or_nulls")] pub owned_globs: Vec, - #[serde(alias = "unowned_globs", default = "empty_string_vec")] + #[serde( + alias = "unowned_globs", + default = "empty_string_vec", + deserialize_with = "crate::yaml::strings_or_nulls" + )] pub subtracted_globs: Vec, } @@ -200,6 +205,8 @@ impl Project { #[cfg(test)] mod tests { + use indoc::indoc; + use super::*; #[test] @@ -229,4 +236,72 @@ mod tests { assert_eq!(map.get("a").unwrap().name, vg1.name); assert_eq!(map.get("b").unwrap().name, vg2.name); } + + fn team_from_yaml(yaml: &str) -> Result> { + let temp_file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(temp_file.path(), yaml).unwrap(); + Team::from_team_file_path(temp_file.path().to_path_buf(), crate::yaml::DEFAULT_MAX_NODES) + } + + #[test] + fn test_team_from_team_file_path_accepts_empty_values() { + let yaml = indoc! {" + name: + github: + team: + do_not_add_to_codeowners_file: true + owned_globs: + - app/payroll/** + - + subtracted_globs: + - + ruby: + owned_gems: + - + "}; + + let team = team_from_yaml(yaml).unwrap(); + assert_eq!(team.name, ""); + assert_eq!(team.github_team, ""); + assert!(team.avoid_ownership); + assert_eq!(team.owned_globs, vec!["app/payroll/**".to_string(), "".to_string()]); + assert_eq!(team.subtracted_globs, vec!["".to_string()]); + assert_eq!(team.owned_gems, vec!["".to_string()]); + } + + #[test] + fn test_team_from_team_file_path_ignores_duplicate_unknown_keys() { + let yaml = indoc! {" + name: Payroll + github: + team: '@acme/payroll' + members: + - a + members: + - b + slack_channel: '#a' + slack_channel: '#b' + "}; + + let team = team_from_yaml(yaml).unwrap(); + assert_eq!(team.github_team, "@acme/payroll"); + } + + #[test] + fn test_team_from_team_file_path_reads_only_true_and_false_as_booleans() { + let yaml = "name: Payroll\ngithub:\n team: '@acme/payroll'\n do_not_add_to_codeowners_file: True\n"; + assert!(team_from_yaml(yaml).unwrap().avoid_ownership); + + let yaml = "name: Payroll\ngithub:\n team: '@acme/payroll'\n do_not_add_to_codeowners_file: yes\n"; + assert!(team_from_yaml(yaml).is_err()); + } + + #[test] + fn test_team_from_team_file_path_reports_missing_field_once() { + let error = team_from_yaml("name: Payroll\n").unwrap_err(); + + let message = format!("{error:?}"); + assert!(message.contains("missing field `github`"), "{message}"); + assert_eq!(message.matches("missing field").count(), 1, "{message}"); + } } diff --git a/src/project_builder.rs b/src/project_builder.rs index e0d06ee..f7b892e 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -212,6 +212,7 @@ impl<'a> ProjectBuilder<'a> { Vec, ); + let yaml_max_nodes = self.config.yaml_max_nodes; let (project_files, packages, vendored_gems, directory_codeowners, teams): Accumulator = entry_types .into_par_iter() .try_fold( @@ -242,7 +243,7 @@ impl<'a> ProjectBuilder<'a> { } } EntryType::RubyPackage(absolute_path, relative_path) => { - match ruby_package_owner(&absolute_path) + match ruby_package_owner(&absolute_path, yaml_max_nodes) .attach_with(|| format!("Failed to read ruby package: {}", absolute_path.display())) { Ok(Some(owner)) => { @@ -282,7 +283,7 @@ impl<'a> ProjectBuilder<'a> { }); } EntryType::TeamFile(absolute_path, _relative_path) => { - let team = Team::from_team_file_path(absolute_path.clone()) + let team = Team::from_team_file_path(absolute_path.clone(), yaml_max_nodes) .change_context(Error::Io) .attach_with(|| format!("Failed to read team file: {}", absolute_path.display()))?; team_files.push(team); @@ -336,9 +337,10 @@ fn matches_globs(path: &Path, globs: &[String]) -> bool { } } -pub(crate) fn ruby_package_owner(path: &Path) -> Result, Report> { - let file = File::open(path).change_context(Error::Io)?; - let deserializer: deserializers::RubyPackage = serde_yaml::from_reader(file).change_context(Error::SerdeYaml)?; +pub(crate) fn ruby_package_owner(path: &Path, yaml_max_nodes: usize) -> Result, Report> { + let bytes = std::fs::read(path).change_context(Error::Io)?; + let deserializer: deserializers::RubyPackage = + crate::yaml::from_project_slice(&bytes, yaml_max_nodes).change_context(Error::SerdeYaml)?; let top_level_owner = deserializer.owner; let metadata_owner = deserializer.metadata.and_then(|metadata| metadata.owner); @@ -364,6 +366,8 @@ fn javascript_package_owner(path: &Path) -> Result, Report #[cfg(test)] mod tests { + use indoc::indoc; + use super::*; const OWNED_GLOB: &str = "{app,components,config,frontend,lib,packs,spec,danger,script}/**/*.{rb,arb,erb,rake,js,jsx,ts,tsx}"; @@ -384,7 +388,7 @@ mod tests { let temp_file = tempfile::NamedTempFile::new().unwrap(); std::fs::write(temp_file.path(), yaml).unwrap(); - let owner = ruby_package_owner(temp_file.path()).unwrap(); + let owner = ruby_package_owner(temp_file.path(), crate::yaml::DEFAULT_MAX_NODES).unwrap(); assert_eq!(owner, Some("TeamA".to_string())); } @@ -394,7 +398,7 @@ mod tests { let temp_file = tempfile::NamedTempFile::new().unwrap(); std::fs::write(temp_file.path(), yaml).unwrap(); - let owner = ruby_package_owner(temp_file.path()).unwrap(); + let owner = ruby_package_owner(temp_file.path(), crate::yaml::DEFAULT_MAX_NODES).unwrap(); assert_eq!(owner, Some("TeamB".to_string())); } @@ -404,7 +408,7 @@ mod tests { let temp_file = tempfile::NamedTempFile::new().unwrap(); std::fs::write(temp_file.path(), yaml).unwrap(); - let result = ruby_package_owner(temp_file.path()); + let result = ruby_package_owner(temp_file.path(), crate::yaml::DEFAULT_MAX_NODES); assert!(result.is_err()); } @@ -414,7 +418,7 @@ mod tests { let temp_file = tempfile::NamedTempFile::new().unwrap(); std::fs::write(temp_file.path(), yaml).unwrap(); - let owner = ruby_package_owner(temp_file.path()).unwrap(); + let owner = ruby_package_owner(temp_file.path(), crate::yaml::DEFAULT_MAX_NODES).unwrap(); assert_eq!(owner, Some("TeamA".to_string())); } @@ -424,7 +428,46 @@ mod tests { let temp_file = tempfile::NamedTempFile::new().unwrap(); std::fs::write(temp_file.path(), yaml).unwrap(); - let owner = ruby_package_owner(temp_file.path()).unwrap(); + let owner = ruby_package_owner(temp_file.path(), crate::yaml::DEFAULT_MAX_NODES).unwrap(); assert_eq!(owner, None); } + + #[test] + fn test_ruby_package_owner_empty_file_has_no_owner() { + for yaml in ["", "# no keys yet\n", "---\n"] { + let temp_file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(temp_file.path(), yaml).unwrap(); + + let owner = ruby_package_owner(temp_file.path(), crate::yaml::DEFAULT_MAX_NODES).expect(yaml); + assert_eq!(owner, None, "{yaml:?}"); + } + } + + #[test] + fn test_ruby_package_owner_ignores_duplicate_unknown_keys() { + let yaml = indoc! {" + enforce_dependencies: true + enforce_privacy: true + enforce_privacy: false + metadata: + slack: '#a' + slack: '#b' + owner: TeamA + "}; + let temp_file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(temp_file.path(), yaml).unwrap(); + + let owner = ruby_package_owner(temp_file.path(), crate::yaml::DEFAULT_MAX_NODES).unwrap(); + assert_eq!(owner, Some("TeamA".to_string())); + } + + #[test] + fn test_ruby_package_owner_duplicate_owner_last_wins() { + let yaml = "owner: TeamA\nowner: TeamB\n"; + let temp_file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(temp_file.path(), yaml).unwrap(); + + let owner = ruby_package_owner(temp_file.path(), crate::yaml::DEFAULT_MAX_NODES).unwrap(); + assert_eq!(owner, Some("TeamB".to_string())); + } } diff --git a/src/runner/api.rs b/src/runner/api.rs index bf4e9c9..8de27bf 100644 --- a/src/runner/api.rs +++ b/src/runner/api.rs @@ -69,6 +69,7 @@ pub fn teams_for_files_from_codeowners( &run_config.project_root, &codeowners_file_path, &config.team_file_glob, + config.yaml_max_nodes, file_paths, ) .map_err(Error::Io)?; diff --git a/src/yaml.rs b/src/yaml.rs new file mode 100644 index 0000000..a177628 --- /dev/null +++ b/src/yaml.rs @@ -0,0 +1,203 @@ +//! Options keep serde_yaml 0.9's semantics where serde-saphyr allows it: these files are written for Ruby's Psych, +//! and most package.yml keys are ones we never read, so rejecting a file over one would abort the whole run. +use error_stack::Report; +use serde::de::DeserializeOwned; +use serde::{Deserialize, Deserializer}; +use serde_saphyr::{DuplicateKeyPolicy, MergeKeyPolicy, NonFiniteFloatPolicy, Options, budget::BudgetBreach}; + +/// Default for the `yaml_max_nodes` config key. Real files have at most a few thousand nodes. +pub(crate) const DEFAULT_MAX_NODES: usize = 20_000; + +fn options(max_nodes: usize) -> Options { + serde_saphyr::options! { + // LastWins re-buffers nested mappings once per level, so the default budget lets a 500 KB file take gigabytes. + // Real files nest about 5 deep; the depth limit also prevents stack overflow. + // The other caps grow with `yaml_max_nodes`, so raising it is all a larger file needs. + budget: serde_saphyr::budget! { + max_depth: 16, + max_nodes: max_nodes, + max_events: max_nodes.saturating_mul(50), + max_total_scalar_bytes: max_nodes.saturating_mul(256), + max_recorded_anchor_events: max_nodes.saturating_mul(5), + max_recorded_anchor_bytes: max_nodes.saturating_mul(64), + }, + // The default rejects a repeated key anywhere, including in values we ignore. Last wins, as in Psych. + duplicate_keys: DuplicateKeyPolicy::LastWins, + // serde_yaml treated `<<` as an ordinary key. + merge_keys: MergeKeyPolicy::AsOrdinary, + // Ignored values are still parsed, and the default rejects `.inf`, `.nan` and `1e999` in them. + non_finite_float_policy: NonFiniteFloatPolicy::PassThrough, + // serde_yaml neither decoded nor validated `!!binary`. + ignore_binary_tag_for_string: true, + // serde_yaml rejected YAML 1.1 booleans such as `yes` and `off`. + strict_booleans: true, + // Snippets span several lines, and error_stack prints them twice through `source()`. + with_snippet: false, + } +} + +/// Takes bytes, not a reader: `from_reader` transcodes after a BOM, replacing invalid UTF-8 and accepting UTF-16, unlike serde_yaml. +pub(crate) fn from_slice(bytes: &[u8], max_nodes: usize) -> Result { + serde_saphyr::from_slice_with_options(bytes, options(max_nodes)) +} + +/// [`from_slice`] for team files and package.yml, whose size limits come from the `yaml_max_nodes` config key: a file +/// over them gets a hint naming that key. +pub(crate) fn from_project_slice(bytes: &[u8], max_nodes: usize) -> Result> { + from_slice(bytes, max_nodes).map_err(|error| { + let mut original = &error; + while let serde_saphyr::Error::AliasError { error, .. } = original { + original = error; + } + let too_large = matches!( + original, + serde_saphyr::Error::Budget { + breach: BudgetBreach::Nodes { .. } + | BudgetBreach::Events { .. } + | BudgetBreach::ScalarBytes { .. } + | BudgetBreach::RecordedAnchorEvents { .. } + | BudgetBreach::RecordedAnchorBytes { .. }, + .. + } + ); + let report = Report::new(error); + if too_large { + report.attach(format!( + "raise `yaml_max_nodes` (now {max_nodes}) in the codeowners config to allow larger files" + )) + } else { + report + } + }) +} + +/// Reads an empty, `~` or `null` value as `""` instead of failing. serde_yaml accepted these, keeping `~` and `null` as text. +pub(crate) fn string_or_null<'de, D: Deserializer<'de>>(deserializer: D) -> Result { + Ok(Option::::deserialize(deserializer)?.unwrap_or_default()) +} + +/// [`string_or_null`] for each item of a list. Reads an empty, `~` or `null` list as empty itself, since serde-saphyr#233 asks +/// serde-saphyr to stop doing that for a `Vec`, and `unowned_globs:` with no value is common. +pub(crate) fn strings_or_nulls<'de, D: Deserializer<'de>>(deserializer: D) -> Result, D::Error> { + Ok(Option::>>::deserialize(deserializer)? + .unwrap_or_default() + .into_iter() + .map(Option::unwrap_or_default) + .collect()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::project::deserializers::RubyPackage; + + fn owner(yaml: &[u8]) -> Result, serde_saphyr::Error> { + from_slice::(yaml, DEFAULT_MAX_NODES).map(|package| package.owner) + } + + #[test] + fn test_ignores_unusual_values_in_unknown_keys() { + for unknown in [ + "<<: 5", + "error_budget: .inf", + "sample_rate: .nan", + "huge: 1e999", + "blob: !!binary not base64", + "legacy: !ruby/object:OpenStruct {table: {}}", + ] { + let yaml = format!("owner: TeamA\n{unknown}\n"); + assert_eq!(owner(yaml.as_bytes()).expect(unknown), Some("TeamA".to_string()), "{unknown}"); + } + } + + #[test] + fn test_reads_empty_list_as_empty() { + #[derive(Deserialize)] + struct Globs { + #[serde(deserialize_with = "strings_or_nulls")] + globs: Vec, + } + + for yaml in ["globs:\n", "globs: ~\n", "globs: null\n", "globs: []\n"] { + let parsed: Globs = from_slice(yaml.as_bytes(), DEFAULT_MAX_NODES).expect(yaml); + assert!(parsed.globs.is_empty(), "{yaml:?}"); + } + } + + #[test] + fn test_does_not_apply_merge_keys() { + let yaml = "defaults: &defaults\n owner: TeamA\n<<: *defaults\nmetadata:\n <<: {owner: TeamA}\n"; + let package: RubyPackage = from_slice(yaml.as_bytes(), DEFAULT_MAX_NODES).unwrap(); + assert_eq!(package.owner, None); + assert_eq!(package.metadata.and_then(|metadata| metadata.owner), None); + } + + #[test] + fn test_reads_utf8_bom_before_several_keys() { + assert_eq!( + owner("\u{feff}enforce_dependencies: true\nowner: TeamA\n".as_bytes()).unwrap(), + Some("TeamA".to_string()) + ); + } + + #[test] + fn test_rejects_invalid_utf8_after_bom() { + assert!(owner(b"\xef\xbb\xbfowner: Caf\xe9\n").is_err()); + } + + #[test] + fn test_rejects_deeply_nested_values() { + let owner_at_depth = + |depth: usize| owner(format!("owner: TeamA\nignored: {}{}\n", "[".repeat(depth), "]".repeat(depth)).as_bytes()); + + assert_eq!(owner_at_depth(10).unwrap(), Some("TeamA".to_string())); + assert!(matches!(owner_at_depth(20), Err(serde_saphyr::Error::Budget { .. }))); + } + + #[test] + fn test_rejects_too_many_nodes() { + let owner_with_items = |count: usize, max_nodes: usize| { + let yaml = format!("owner: TeamA\nignored: [{}]\n", vec!["x"; count].join(", ")); + from_slice::(yaml.as_bytes(), max_nodes).map(|package| package.owner) + }; + + assert_eq!(owner_with_items(10_000, DEFAULT_MAX_NODES).unwrap(), Some("TeamA".to_string())); + assert!(matches!( + owner_with_items(100_000, DEFAULT_MAX_NODES), + Err(serde_saphyr::Error::Budget { .. }) + )); + assert_eq!(owner_with_items(100_000, 200_000).unwrap(), Some("TeamA".to_string())); + assert!(matches!(owner_with_items(1_000, 500), Err(serde_saphyr::Error::Budget { .. }))); + } + + #[test] + fn test_scalar_byte_limit_grows_with_max_nodes() { + let yaml = format!("owner: TeamA\nignored: [{}]\n", vec!["x".repeat(400); 15_000].join(", ")); + + let Err(report) = from_project_slice::(yaml.as_bytes(), DEFAULT_MAX_NODES) else { + panic!("15,000 strings of 400 bytes should exceed the default scalar-byte limit"); + }; + assert!(matches!( + report.current_context(), + serde_saphyr::Error::Budget { + breach: BudgetBreach::ScalarBytes { .. }, + .. + } + )); + assert!(format!("{report:?}").contains("raise `yaml_max_nodes`"), "{report:?}"); + assert_eq!( + from_slice::(yaml.as_bytes(), 40_000).unwrap().owner, + Some("TeamA".to_string()) + ); + + let aliased = format!( + "owner: TeamA\nbig: &a \"{}\"\nignored: [{}]\n", + "x".repeat(1_000_000), + ["*a"; 8].join(", ") + ); + let Err(report) = from_project_slice::(aliased.as_bytes(), DEFAULT_MAX_NODES) else { + panic!("eight aliases of a 1 MB string should exceed the default scalar-byte limit"); + }; + assert!(format!("{report:?}").contains("raise `yaml_max_nodes`"), "{report:?}"); + } +} diff --git a/tests/yaml_max_nodes_test.rs b/tests/yaml_max_nodes_test.rs new file mode 100644 index 0000000..77348b9 --- /dev/null +++ b/tests/yaml_max_nodes_test.rs @@ -0,0 +1,82 @@ +use assert_cmd::Command; +use predicates::prelude::*; +use std::{error::Error, fs, path::Path}; +use tempfile::TempDir; + +mod common; + +use common::git_add_all_files; +use common::setup_fixture_repo; + +// 30,000 ignored list items put a package.yml and a team file over the default `yaml_max_nodes` of 20,000. +fn project_with_large_yaml_files(yaml_max_nodes: Option) -> TempDir { + let temp_dir = setup_fixture_repo(Path::new("tests/fixtures/valid_project")); + let root = temp_dir.path(); + let ignored = format!("ignored: [{}]\n", vec!["x"; 30_000].join(", ")); + + fs::write( + root.join("ruby/packages/payroll_flow/package.yml"), + format!("owner: Payroll\n{ignored}"), + ) + .unwrap(); + fs::create_dir_all(root.join("ruby/packages/payroll_flow/app")).unwrap(); + fs::write(root.join("ruby/packages/payroll_flow/app/flow.rb"), "class Flow; end\n").unwrap(); + + let team_path = root.join("config/teams/payroll.yml"); + let team = fs::read_to_string(&team_path).unwrap(); + fs::write(&team_path, format!("{team}{ignored}")).unwrap(); + + if let Some(yaml_max_nodes) = yaml_max_nodes { + let config_path = root.join("config/code_ownership.yml"); + let config = fs::read_to_string(&config_path).unwrap(); + fs::write(&config_path, format!("{config}\nyaml_max_nodes: {yaml_max_nodes}\n")).unwrap(); + } + + git_add_all_files(root); + temp_dir +} + +fn codeowners(project_root: &Path, args: &[&str]) -> Command { + let mut cmd = Command::cargo_bin("codeowners").unwrap(); + cmd.arg("--project-root").arg(project_root).arg("--no-cache").args(args); + cmd +} + +#[test] +fn test_default_yaml_max_nodes_rejects_large_files() -> Result<(), Box> { + let temp_dir = project_with_large_yaml_files(None); + let root = temp_dir.path(); + + codeowners(root, &["generate"]) + .assert() + .failure() + .stderr(predicate::str::contains("budget breached").and(predicate::str::contains("raise `yaml_max_nodes` (now 20000)"))); + codeowners(root, &["for-file", "ruby/packages/payroll_flow/app/flow.rb"]) + .assert() + .stderr(predicate::str::contains("Error reading ruby package").and(predicate::str::contains("budget breached"))); + codeowners(root, &["for-file", "--from-codeowners", "ruby/packages/payroll_flow/app/flow.rb"]) + .assert() + .stderr(predicate::str::contains("Error parsing team file").and(predicate::str::contains("budget breached"))); + + Ok(()) +} + +#[test] +fn test_raised_yaml_max_nodes_accepts_large_files() -> Result<(), Box> { + let temp_dir = project_with_large_yaml_files(Some(100_000)); + let root = temp_dir.path(); + + codeowners(root, &["generate"]).assert().success().stderr(""); + codeowners(root, &["for-file", "ruby/packages/payroll_flow/app/flow.rb"]) + .assert() + .success() + .stdout(predicate::str::contains("Team: Payroll")) + .stderr(""); + codeowners(root, &["for-file", "--from-codeowners", "ruby/packages/payroll_flow/app/flow.rb"]) + .assert() + .success() + .stdout(predicate::str::contains("Team: Payroll")) + .stderr(""); + + Ok(()) +}