providing some validation on codeowners to help detect invalid rules (#29)

Author: jjmschofield

README.md

@@ -12,6 +12,7 @@ Things it does:
 * Output ownership information of staged files
 * Outputs lots of lovely stats
 * Outputs handy formats for integrations (CSV and JSONL)
+* Validates that the provided owners are in the correct format for github
 
 ## Installation
 Install via npm globally then run

src/lib/OwnershipEngine.test.ts

@@ -1,7 +1,16 @@
+import * as fs from 'fs';
+
 import { OwnershipEngine } from './OwnershipEngine';
 import { FileOwnershipMatcher } from './types';
 
+jest.mock('fs');
+const readFileSyncMock = fs.readFileSync as jest.Mock;
+
 describe('OwnershipEngine', () => {
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
   describe('calcFileOwnership', () => {
     const createFileOwnershipMatcher = (path: string, owners: string[]): FileOwnershipMatcher => {
       return {
@@ -50,4 +59,81 @@ describe('OwnershipEngine', () => {
       expect(result).not.toContainEqual(unexpectedOwner);
     });
   });
+
+  describe('FromCodeownersFile', () => {
+    it('should not throw when provided valid owners', () => {
+      // Arrange
+      const codeowners = 'some/path @global-owner1 @org/octocat docs@example.com';
+
+      readFileSyncMock.mockReturnValue(Buffer.from(codeowners));
+
+      // Assert
+      expect(() => OwnershipEngine.FromCodeownersFile('some/codeowners/file')).not.toThrow();
+    });
+
+    it('should throw when provided an invalid owner', () => {
+      // Arrange
+      const rulePath = 'some/path';
+      const owner = '.not@valid-owner';
+
+      const expectedError = new Error(`${owner} is not a valid owner name in rule ${rulePath} ${owner}`);
+
+      const codeowners = `${rulePath} ${owner}`;
+
+      readFileSyncMock.mockReturnValue(Buffer.from(codeowners));
+
+      // Assert
+      expect(() => OwnershipEngine.FromCodeownersFile('some/codeowners/file'))
+        .toThrowError(expectedError);
+    });
+
+    it('should throw when provided an invalid github user as an owner', () => {
+      // Arrange
+      const rulePath = 'some/path';
+      const owner = 'invalid-owner';
+
+      const expectedError = new Error(`${owner} is not a valid owner name in rule ${rulePath} ${owner}`);
+
+      const codeowners = `${rulePath} ${owner}`;
+
+      readFileSyncMock.mockReturnValue(Buffer.from(codeowners));
+
+      // Assert
+      expect(() => OwnershipEngine.FromCodeownersFile('some/codeowners/file'))
+        .toThrowError(expectedError);
+    });
+
+    it('should throw when provided an invalid email address as an owner', () => {
+      // Arrange
+      const rulePath = 'some/path';
+      const owner = 'invalid-owner@nowhere';
+
+      const expectedError = new Error(`${owner} is not a valid owner name in rule ${rulePath} ${owner}`);
+
+      const codeowners = `${rulePath} ${owner}`;
+
+      readFileSyncMock.mockReturnValue(Buffer.from(codeowners));
+
+      // Assert
+      expect(() => OwnershipEngine.FromCodeownersFile('some/codeowners/file'))
+        .toThrowError(expectedError);
+    });
+
+    it('should throw when provided at least one invalid owner', () => {
+      // Arrange
+      const rulePath = 'some/path';
+      const valid = 'valid@owner.com';
+      const owner = '@invalid-owner*';
+
+      const expectedError = new Error(`${owner} is not a valid owner name in rule ${rulePath} ${valid} ${owner}`);
+
+      const codeowners = `${rulePath} ${valid} ${owner}`;
+
+      readFileSyncMock.mockReturnValue(Buffer.from(codeowners));
+
+      // Assert
+      expect(() => OwnershipEngine.FromCodeownersFile('some/codeowners/file'))
+        .toThrowError(expectedError);
+    });
+  });
 });

src/lib/OwnershipEngine.ts

@@ -61,10 +61,15 @@ const createMatcherCodeownersRule = (rule: string) => {
   // Remaining parts are expected to be team names (if any)
   if (parts.length > 1) {
     teamNames = parts.slice(1, parts.length);
+    for(const name of teamNames){
+      if(!codeOwnerRegex.test(name)){
+        throw new Error(`${name} is not a valid owner name in rule ${rule}`);
+      }
+    }
   }
 
   // Create an `ignore` matcher to ape github behaviour
-  const match : any = ignore().add(path);
+  const match: any = ignore().add(path);
 
   // Return our complete matcher
   return {
@@ -73,3 +78,6 @@ const createMatcherCodeownersRule = (rule: string) => {
     match: match.ignores.bind(match),
   };
 };
+
+// ensures that only the following patterns are allowed @octocat @octocat/kitty docs@example.com
+const codeOwnerRegex = /(^@[a-zA-Z0-9_\-/]*$)|(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])/;