From d7ea0161f2aff2dd5cc60e71d839afaf0d6d890f Mon Sep 17 00:00:00 2001 From: Sara Strasner Date: Thu, 13 Aug 2026 10:08:57 -0400 Subject: [PATCH] fix: report .codeowner files that reference an unregistered team `DirectoryMapper::entries` looks each directory owner up in the team registry and skips the entry when the name does not resolve. Nothing else reports the name, so a typo'd or renamed team in a `.codeowner` is completely silent: the directory inherits the nearest ancestor owner, `generate` emits no line for it, and `validate` exits 0. That makes the file inert while still looking authoritative, and the ownership it was written to express quietly belongs to whichever team owns the parent directory. Annotations and package ownership are already validated against the registry; this extends the same check to directory ownership, reusing the existing `InvalidTeam` error so output and exit codes are unchanged in shape. Co-Authored-By: Claude Sonnet 4.6 --- src/ownership/validator.rs | 15 +++++++++++ .../.github/CODEOWNERS | 14 +++++++++++ .../app/services/.codeowner | 1 + .../app/services/nested/.codeowner | 1 + .../app/services/nested/nested_file.rb | 2 ++ .../config/code_ownership.yml | 10 ++++++++ .../config/teams/foo.yml | 5 ++++ tests/invalid_directory_codeowner_test.rs | 25 +++++++++++++++++++ 8 files changed, 73 insertions(+) create mode 100644 tests/fixtures/invalid-directory-codeowner/.github/CODEOWNERS create mode 100644 tests/fixtures/invalid-directory-codeowner/app/services/.codeowner create mode 100644 tests/fixtures/invalid-directory-codeowner/app/services/nested/.codeowner create mode 100644 tests/fixtures/invalid-directory-codeowner/app/services/nested/nested_file.rb create mode 100644 tests/fixtures/invalid-directory-codeowner/config/code_ownership.yml create mode 100644 tests/fixtures/invalid-directory-codeowner/config/teams/foo.yml create mode 100644 tests/invalid_directory_codeowner_test.rs diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index 664de3a..39509d9 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -65,6 +65,7 @@ impl Validator { errors.append(&mut self.invalid_team_annotation(&team_names)); errors.append(&mut self.invalid_package_ownership(&team_names)); + errors.append(&mut self.invalid_directory_ownership(&team_names)); errors } @@ -107,6 +108,20 @@ impl Validator { .collect() } + /// `DirectoryMapper::entries` skips unresolvable owners, so the directory silently + /// inherits its ancestor's owner and nothing else reports the bad name. + fn invalid_directory_ownership(&self, team_names: &HashSet<&TeamName>) -> Vec { + self.project + .directory_codeowner_files + .iter() + .filter(|directory_codeowner_file| !team_names.contains(&directory_codeowner_file.owner)) + .map(|directory_codeowner_file| Error::InvalidTeam { + name: directory_codeowner_file.owner.clone(), + path: self.project.relative_path(&directory_codeowner_file.path).to_owned(), + }) + .collect() + } + fn validate_file_ownership(&self) -> Vec { let mut validation_errors = Vec::new(); diff --git a/tests/fixtures/invalid-directory-codeowner/.github/CODEOWNERS b/tests/fixtures/invalid-directory-codeowner/.github/CODEOWNERS new file mode 100644 index 0000000..fed28cf --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/.github/CODEOWNERS @@ -0,0 +1,14 @@ +# STOP! - DO NOT EDIT THIS FILE MANUALLY +# This file was automatically generated by "bin/codeownership validate". +# +# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub +# teams. This is useful when developers create Pull Requests since the +# code/file owner is notified. Reference GitHub docs for more details: +# https://help.github.com/en/articles/about-code-owners + + +# Owner in .codeowner +/app/services/**/** @footeam + +# Team YML ownership +/config/teams/foo.yml @footeam diff --git a/tests/fixtures/invalid-directory-codeowner/app/services/.codeowner b/tests/fixtures/invalid-directory-codeowner/app/services/.codeowner new file mode 100644 index 0000000..bc56c4d --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/app/services/.codeowner @@ -0,0 +1 @@ +Foo diff --git a/tests/fixtures/invalid-directory-codeowner/app/services/nested/.codeowner b/tests/fixtures/invalid-directory-codeowner/app/services/nested/.codeowner new file mode 100644 index 0000000..c2075a5 --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/app/services/nested/.codeowner @@ -0,0 +1 @@ +Web3 diff --git a/tests/fixtures/invalid-directory-codeowner/app/services/nested/nested_file.rb b/tests/fixtures/invalid-directory-codeowner/app/services/nested/nested_file.rb new file mode 100644 index 0000000..d19bcc1 --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/app/services/nested/nested_file.rb @@ -0,0 +1,2 @@ +class NestedFile +end diff --git a/tests/fixtures/invalid-directory-codeowner/config/code_ownership.yml b/tests/fixtures/invalid-directory-codeowner/config/code_ownership.yml new file mode 100644 index 0000000..c76f028 --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/config/code_ownership.yml @@ -0,0 +1,10 @@ +--- +owned_globs: + - "{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx,json,yml}" +unowned_globs: + - config/code_ownership.yml +javascript_package_paths: + - javascript/packages/** +vendored_gems_path: gems +team_file_glob: + - config/teams/**/*.yml \ No newline at end of file diff --git a/tests/fixtures/invalid-directory-codeowner/config/teams/foo.yml b/tests/fixtures/invalid-directory-codeowner/config/teams/foo.yml new file mode 100644 index 0000000..7c3977c --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/config/teams/foo.yml @@ -0,0 +1,5 @@ +name: Foo +github: + team: "@footeam" + members: + - fooer diff --git a/tests/invalid_directory_codeowner_test.rs b/tests/invalid_directory_codeowner_test.rs new file mode 100644 index 0000000..260db32 --- /dev/null +++ b/tests/invalid_directory_codeowner_test.rs @@ -0,0 +1,25 @@ +use indoc::indoc; +use predicates::prelude::*; +use std::error::Error; + +mod common; +use common::OutputStream; +use common::run_codeowners; + +/// A nested `.codeowner` naming an unregistered team, under one naming a real team: +/// ownership falls through to the ancestor, so nothing else reports the bad name. +#[test] +fn test_validate_reports_directory_codeowner_with_invalid_team() -> Result<(), Box> { + run_codeowners( + "invalid-directory-codeowner", + &["validate"], + false, + OutputStream::Stdout, + predicate::str::contains(indoc! {" + Found invalid team annotations + - app/services/nested/.codeowner is referencing an invalid team - 'Web3' + "}), + )?; + + Ok(()) +}