refactor: improve bot permission validation readability

- Reduce nesting in bot validation block
- Add missing test cases for admin permissions
- Test edge cases: null user type, missing permissions object
- All tests pass (283/283)

Author: lroolle

src/github/validation/permissions.ts

@@ -37,6 +37,7 @@ export async function checkWritePermissions(
     if (actorType === "Bot") {
       core.info(`GitHub App detected: ${actor}, checking write permissions`);
 
+      // Try collaborator permissions first
       try {
         const response = await octokit.repos.getCollaboratorPermissionLevel({
           owner: repository.owner,
@@ -47,42 +48,42 @@ export async function checkWritePermissions(
         const permissionLevel = response.data.permission;
         core.info(`App permission level: ${permissionLevel}`);
 
-        if (permissionLevel === "admin" || permissionLevel === "write") {
+        const hasWriteAccess = permissionLevel === "admin" || permissionLevel === "write";
+        if (hasWriteAccess) {
           core.info(`App has write access: ${permissionLevel}`);
           return true;
-        } else {
-          core.warning(`App has insufficient permissions: ${permissionLevel}`);
-          return false;
         }
+        
+        core.warning(`App has insufficient permissions: ${permissionLevel}`);
+        return false;
       } catch (error) {
         core.warning(
           `Could not check collaborator permissions for bot, checking app installation: ${error}`,
         );
+      }
 
-        try {
-          const installation = await octokit.apps.getRepoInstallation({
-            owner: repository.owner,
-            repo: repository.repo,
-          });
+      // Fallback to app installation permissions
+      try {
+        const installation = await octokit.apps.getRepoInstallation({
+          owner: repository.owner,
+          repo: repository.repo,
+        });
 
-          core.info(`App installation found: ${installation.data.id}`);
+        core.info(`App installation found: ${installation.data.id}`);
 
-          const permissions = installation.data.permissions;
-          if (
-            permissions &&
-            (permissions.contents === "write" ||
-              permissions.contents === "admin")
-          ) {
-            core.info(`App has write permissions via installation`);
-            return true;
-          } else {
-            core.warning(`App lacks write permissions in installation`);
-            return false;
-          }
-        } catch (installationError) {
-          core.warning(`App lacks repository access: ${installationError}`);
-          return false;
+        const permissions = installation.data.permissions;
+        const hasWriteAccess = permissions?.contents === "write" || permissions?.contents === "admin";
+        
+        if (hasWriteAccess) {
+          core.info(`App has write permissions via installation`);
+          return true;
         }
+        
+        core.warning(`App lacks write permissions in installation`);
+        return false;
+      } catch (installationError) {
+        core.warning(`App lacks repository access: ${installationError}`);
+        return false;
       }
     }
 

test/permissions-validation.test.ts

@@ -244,4 +244,120 @@ describe("checkWritePermissions", () => {
       "Failed to check permissions for test-user:",
     );
   });
+
+  test("should grant write permissions to users with admin access", async () => {
+    const mockOctokit = {
+      users: {
+        getByUsername: async () => ({
+          data: { type: "User" },
+        }),
+      },
+      repos: {
+        getCollaboratorPermissionLevel: async () => ({
+          data: { permission: "admin" },
+        }),
+      },
+    } as any;
+
+    const context = createContext("admin-user");
+    const result = await checkWritePermissions(mockOctokit, context);
+
+    expect(result).toBe(true);
+  });
+
+  test("should grant write permissions to bots with admin access via collaborator", async () => {
+    const mockOctokit = {
+      users: {
+        getByUsername: async () => ({
+          data: { type: "Bot" },
+        }),
+      },
+      repos: {
+        getCollaboratorPermissionLevel: async () => ({
+          data: { permission: "admin" },
+        }),
+      },
+    } as any;
+
+    const context = createContext("admin-bot");
+    const result = await checkWritePermissions(mockOctokit, context);
+
+    expect(result).toBe(true);
+  });
+
+  test("should grant write permissions to bots with admin access via installation", async () => {
+    const mockOctokit = {
+      users: {
+        getByUsername: async () => ({
+          data: { type: "Bot" },
+        }),
+      },
+      repos: {
+        getCollaboratorPermissionLevel: async () => {
+          throw new Error("Not found");
+        },
+      },
+      apps: {
+        getRepoInstallation: async () => ({
+          data: {
+            id: 123,
+            permissions: { contents: "admin" },
+          },
+        }),
+      },
+    } as any;
+
+    const context = createContext("admin-bot");
+    const result = await checkWritePermissions(mockOctokit, context);
+
+    expect(result).toBe(true);
+  });
+
+  test("should continue when getBotActorType fails", async () => {
+    const mockOctokit = {
+      users: {
+        getByUsername: async () => {
+          throw new Error("User not found");
+        },
+      },
+      repos: {
+        getCollaboratorPermissionLevel: async () => ({
+          data: { permission: "write" },
+        }),
+      },
+    } as any;
+
+    const context = createContext("unknown-user");
+    const result = await checkWritePermissions(mockOctokit, context);
+
+    expect(result).toBe(true);
+  });
+
+  test("should handle installation without permissions object", async () => {
+    const mockOctokit = {
+      users: {
+        getByUsername: async () => ({
+          data: { type: "Bot" },
+        }),
+      },
+      repos: {
+        getCollaboratorPermissionLevel: async () => {
+          throw new Error("Not found");
+        },
+      },
+      apps: {
+        getRepoInstallation: async () => ({
+          data: {
+            id: 123,
+            // permissions is undefined
+          },
+        }),
+      },
+    } as any;
+
+    const context = createContext("bot-no-perms");
+    const result = await checkWritePermissions(mockOctokit, context);
+
+    expect(result).toBe(false);
+  });
 });