validate <files> and for-file don't normalize the paths they're given #145

validate <files>, gv <files> and for-file match each path as it was typed. The only cleanup is stripping the project root from an absolute path, and the CLI canonicalizes the root first. So a path that names the right file in another form (a leading ./, a .. segment, or an absolute path through a symlink) is compared against owned_globs, unowned_globs and CODEOWNERS as a different string. Depending on the config, a file that CODEOWNERS assigns is reported as unowned, or a file with no owner is skipped and passes.

Found while working on #135, which makes one case worse (below). Fixing it changes how every listed file is looked up, so it's left out of that PR.

Reproduction

On main (e64429f), on macOS, where /tmp is a symlink to /private/tmp:

mkdir /tmp/proj && cd /tmp/proj && git init -q
mkdir -p app config/teams
printf 'owned_globs:\n  - "**/*.rb"\n' > config/code_ownership.yml
printf 'name: Valid\ngithub:\n  team: "@org/valid"\n' > config/teams/valid.yml
printf '# @team Valid\n' > app/valid.rb
printf 'puts 1\n' > app/unowned.rb
git add -A && codeowners generate

With owned_globs scoped to directories, as in the README's example, the opposite happens:

printf 'owned_globs:\n  - "{app,config}/**/*.rb"\n' > config/code_ownership.yml
codeowners generate

./app/unowned.rb and the absolute paths don't match {app,config}/**/*.rb, so validate drops them before checking, and for-file ignores the annotation.

With allow_unowned_globs (#135)

In #135, validate <files> excuses an allowed file only if nothing names an owner for it, and that check gets the same path. On its branch (f15fff0):

printf 'owned_globs:\n  - "**/*.rb"\nallow_unowned_globs:\n  - "**/*"\n' > config/code_ownership.yml
printf '# @team Typo\n' > app/typo.rb   # Typo isn't a team
git add app/typo.rb && codeowners generate

On main, which ignores the key, all four are reported. With the key, ./app/valid.rb passes, but only because the owner check misses it as well.

Cause

• Runner::validate_files strips the project root only from an absolute path before matching owned_globs and unowned_globs, and passes the path as typed to the CODEOWNERS lookup.
• teams_for_files_from_codeowners does the same, and Parser::teams_from_files_paths prefixes /, so /./app/valid.rb is matched against CODEOWNERS.
• Args::absolute_project_root canonicalizes the project root, so an absolute path through a symlink never starts with it.

for-file without --from-codeowners copes with ./ because find_file_owners joins the path onto the root first, and strip_prefix compares components, which drops .. It doesn't handle the symlinked path.

Effect on code_ownership

The gem passes paths through as given. bin/codeownership validate <files> hands its arguments to validate. validate --diff takes its paths from git diff --staged --name-only, which prints them without ./ or .., so it isn't affected. CodeOwnership.for_file and teams_for_files_from_codeowners pass theirs to RustCodeOwners.teams_for_files unchanged.

Possible fix

Normalize each path once, before any matching, and use the result for the glob filters, the CODEOWNERS lookup and #135's owner check: join a relative path onto the project root, clean up . and .. (cli.rs already uses path_clean's clean()), and if the result doesn't start with the canonical root, canonicalize its parent directory (when it exists) and strip the root from that. Canonicalizing the file itself would follow a symlinked file to its target, which isn't the path CODEOWNERS matches. Results and error messages should keep the path as given.

Doing this in teams_for_files_from_codeowners would also fix for-file --from-codeowners and the gem's lookups. The gem's TeamFinder.for_file, used with from_codeowners: false, returns nil for any path starting with ./, so it may be worth deciding whether those lookups should accept these forms at all.