Package, .codeowner and annotation globs aren't escaped, and break at the project root #146

codeowners turns each package and .codeowner into a <directory>/**/** glob, and each annotated file into its path. validate matches files against those globs, and they're written to CODEOWNERS, where for-file --from-codeowners, validate <files> and the gem's CodeOwnership.for_file match against them again. But the directory and file names go into the globs unescaped:

• The package.yml and package.json mappers escape nothing, so [ab] in a package's path is a character class.
• The .codeowner and annotation mappers escape [ and ] (Escaping brackets in team file CODEOWNERS entries #37) but not { and }, so {legacy} is a list of alternatives that matches legacy.
• A package or .codeowner at the project root gets the glob /**/**, which matches no project-relative path, and the CODEOWNERS line //**/**.

So, depending on the name, a file is reported as unowned although its package or .codeowner names a valid team, a file in a sibling directory gets that team, or a file gets a second owner. for-file without --from-codeowners gets every case below right, because it walks up from the file to the nearest package and .codeowner instead of matching globs.

Found while reviewing #135, which doesn't change any of this. Fixing it changes CODEOWNERS output, so it's left out of that PR.

Reproduction

On main (e64429f):

mkdir proj && cd proj && git init -q
mkdir -p config/teams 'packs/[ab]' packs/a 'packs/{legacy}' packs/legacy 'app/{old}' app/old lib
printf 'owned_globs:\n  - "**/*.rb"\n' > config/code_ownership.yml
printf 'name: Foo\ngithub:\n  team: "@org/foo"\n' > config/teams/foo.yml
printf 'name: Bar\ngithub:\n  team: "@org/bar"\n' > config/teams/bar.yml
printf 'owner: Foo\n' > 'packs/[ab]/package.yml'
printf 'owner: Foo\n' > 'packs/{legacy}/package.yml'
printf 'owner: Bar\n' > packs/legacy/package.yml
printf 'Foo\n' > 'app/{old}/.codeowner'
printf 'Bar\n' > app/old/.codeowner
printf '# @team Foo\n' > 'lib/{v2}.rb'
for f in 'packs/[ab]/x.rb' packs/a/y.rb 'packs/{legacy}/x.rb' packs/legacy/z.rb 'app/{old}/x.rb' app/old/z.rb lib/v2.rb; do
  printf 'puts 1\n' > "$f"
done
git add -A && codeowners generate

CODEOWNERS gets:

/app/old/**/** @org/bar
/app/{old}/**/** @org/foo
/packs/[ab]/**/** @org/foo
/packs/legacy/**/** @org/bar
/packs/{legacy}/**/** @org/foo
/config/teams/bar.yml @org/bar
/config/teams/foo.yml @org/foo
/lib/{v2}.rb @org/foo

codeowners validate fails with:

Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways

packs/legacy/z.rb
 owner: Bar
  - Owner defined in `packs/legacy/package.yml` with implicitly owned glob: `packs/legacy/**/**`
 owner: Foo
  - Owner defined in `packs/{legacy}/package.yml` with implicitly owned glob: `packs/{legacy}/**/**`

Some files are missing ownership
- app/{old}/x.rb
- lib/v2.rb
- packs/[ab]/x.rb
- packs/{legacy}/x.rb

lib/v2.rb is listed correctly. packs/a/y.rb has no owner either, but isn't listed, because packs/[ab]/**/** matches it.

At the project root

cd .. && mkdir root && cd root && git init -q
mkdir -p config/teams app
printf 'owned_globs:\n  - "**/*.rb"\n' > config/code_ownership.yml
printf 'name: Foo\ngithub:\n  team: "@org/foo"\n' > config/teams/foo.yml
printf 'Foo\n' > .codeowner
printf 'puts 1\n' > app/x.rb
git add -A && codeowners generate

CODEOWNERS gets //**/** @org/foo. for-file app/x.rb reports Foo, but for-file --from-codeowners app/x.rb reports Unowned, and validate and validate app/x.rb both report app/x.rb as unowned. A package.yml at the root does the same if ruby_package_paths matches the root, as **/* does.

Cause

• PackageMapper::entries and PackageMapper::owner_matchers build format!("{}/**/**", package_root) from the path as it is.
• DirectoryMapper::entries, DirectoryMapper::owner_matchers and TeamFileMapper::entries call escape_brackets, which escapes only [ and ].
• For a file at the root, Package::package_root and DirectoryCodeownersFile::directory_root return an empty path, so the glob is /**/**, and Entry::to_row adds another /.
• validate's owner matchers and the CODEOWNERS lookup both match with fast_glob::glob_match, which reads brackets and braces as patterns.

Effect on code_ownership

CodeOwnership.for_file defaults to from_codeowners: true, which goes through codeowners' CODEOWNERS lookup, so it returns the --from-codeowners column above. validate!(files:) runs validate <files>.

How GitHub reads these lines isn't checked here. Its docs say [ ] ranges don't work in CODEOWNERS and skip a line with invalid syntax, and don't mention braces.

Possible fix

• Escape glob metacharacters in every glob built from a path, with one escaper shared by PackageMapper, DirectoryMapper and TeamFileMapper: at least [, ], { and }, and probably *, ? and \ too. Editing the CODEOWNERS above to /packs/\[ab\]/**/**, /packs/\{legacy\}/**/**, /app/\{old\}/**/** and /lib/\{v2\}.rb makes for-file --from-codeowners agree with for-file for all eight files.
• For the root, use **/**, which CODEOWNERS gets as /**/**. Editing the root project's line to /**/** @org/foo makes for-file --from-codeowners app/x.rb report Foo.
• Repos with such names will see CODEOWNERS change, and validate will ask them to regenerate it, so the release notes should mention it.
• Tests like directory_mapper's test_entries_with_brackets would cover packages, braces and the root.