escape globs derived from .codeowner files (#106)

* add a spec for .codeowner in a path containing square bracket chars

* add failing spec for .codeowner file inside dir containing brackets

* escape globs derived from .codeowner files

* bump version 1.36.3 => 1.37.0

* typo

* fix example filenames

Author: schoblaska

code_ownership.gemspec

@@ -1,6 +1,6 @@
 Gem::Specification.new do |spec|
   spec.name          = 'code_ownership'
-  spec.version       = '1.36.3'
+  spec.version       = '1.37.0'
   spec.authors       = ['Gusto Engineers']
   spec.email         = ['[email protected]']
   spec.summary       = 'A gem to help engineering teams declare ownership of code'

lib/code_ownership/private/ownership_mappers/directory_ownership.rb

@@ -48,7 +48,8 @@ def globs_to_owner(files)
             .map(&:cleanpath)
             .each_with_object({}) do |pathname, res|
             owner = owner_for_codeowners_file(pathname)
-            res[pathname.dirname.cleanpath.join('**/**').to_s] = owner
+            glob = glob_for_codeowners_file(pathname)
+            res[glob] = owner
           end
         end
 
@@ -77,7 +78,7 @@ def owner_for_codeowners_file(codeowners_file)
         # Takes a file and finds the relevant `.codeowner` file by walking up the directory
         # structure. Example, given `a/b/c.rb`, this looks for `a/b/.codeowner`, `a/.codeowner`,
         # and `.codeowner` in that order, stopping at the first file to actually exist.
-        # If the parovided file is a directory, it will look for `.codeowner` in that directory and then upwards.
+        # If the provided file is a directory, it will look for `.codeowner` in that directory and then upwards.
         # We do additional caching so that we don't have to check for file existence every time.
         sig { params(file: String).returns(T.nilable(CodeTeams::Team)) }
         def map_file_to_relevant_owner(file)
@@ -123,6 +124,26 @@ def get_team_from_codeowners_file_within_directory(directory)
 
           team
         end
+
+        sig { params(codeowners_file: Pathname).returns(String) }
+        def glob_for_codeowners_file(codeowners_file)
+          unescaped = codeowners_file.dirname.cleanpath.join('**/**').to_s
+
+          # Globs can contain certain regex characters, like "[" and "]".
+          # However, when we are generating a glob from a .codeowner file, we
+          # need to escape bracket characters and interpret them literally.
+          # Otherwise the resulting glob will not actually match the directory
+          # containing the .codeowner file.
+          #
+          # Example
+          # file: "/some/[dir]/.codeowner"
+          # unescaped: "/some/[dir]/**/**"
+          # matches: "/some/d/file"
+          # matches: "/some/i/file"
+          # matches: "/some/r/file"
+          # does not match!: "/some/[dir]/file"
+          unescaped.gsub(/[\[\]]/) { |x| "\\#{x}" }
+        end
       end
     end
   end

spec/lib/code_ownership_spec.rb

@@ -25,6 +25,19 @@
         end
       end
 
+      context 'directory with [] characters containing a .codeowner file' do
+        before do
+          write_file('app/services/[test]/.codeowner', <<~CONTENTS)
+            Bar
+          CONTENTS
+          write_file('app/services/[test]/some_file.rb', '')
+        end
+
+        it 'has no validation errors' do
+          expect { CodeOwnership.validate!(files: ['app/services/[test]/some_file.rb']) }.to_not raise_error
+        end
+      end
+
       context 'file ownership with [] characters' do
         before do
           write_file('app/services/[test]/some_file.ts', <<~TYPESCRIPT)
@@ -161,6 +174,19 @@
       end
     end
 
+    context '.codeowner in a directory with [] characters' do
+      before do
+        write_file('app/javascript/[test]/.codeowner', <<~CONTENTS)
+          Bar
+        CONTENTS
+        write_file('app/javascript/[test]/test.js', '')
+      end
+
+      it 'properly assigns ownership' do
+        expect(CodeOwnership.for_file('app/javascript/[test]/test.js')).to eq CodeTeams.find('Bar')
+      end
+    end
+
     before { create_non_empty_application }
   end