[jsweep] Clean check_permissions_utils.cjs (#7985)

Author: github-actions[bot]

.changeset/patch-modernize-check-permissions-utils.md

@@ -11,17 +11,15 @@
  * @returns {string[]} Array of required permission levels
  */
 function parseRequiredPermissions() {
-  const requiredPermissionsEnv = process.env.GH_AW_REQUIRED_ROLES;
-  return requiredPermissionsEnv ? requiredPermissionsEnv.split(",").filter(p => p.trim() !== "") : [];
+  return process.env.GH_AW_REQUIRED_ROLES?.split(",").filter(p => p.trim()) ?? [];
 }
 
 /**
  * Parse allowed bot identifiers from environment variable
  * @returns {string[]} Array of allowed bot identifiers
  */
 function parseAllowedBots() {
-  const allowedBotsEnv = process.env.GH_AW_ALLOWED_BOTS;
-  return allowedBotsEnv ? allowedBotsEnv.split(",").filter(b => b.trim() !== "") : [];
+  return process.env.GH_AW_ALLOWED_BOTS?.split(",").filter(b => b.trim()) ?? [];
 }
 
 /**
@@ -55,17 +53,17 @@ async function checkBotStatus(actor, owner, repo) {
       return { isBot: true, isActive: true };
     } catch (botError) {
       // If we get a 404, the bot is not installed/active on this repository
-      if (typeof botError === "object" && botError !== null && "status" in botError && botError.status === 404) {
+      if (botError?.status === 404) {
         core.warning(`Bot '${actor}' is not active/installed on ${owner}/${repo}`);
         return { isBot: true, isActive: false };
       }
       // For other errors, we'll treat as inactive to be safe
-      const errorMessage = botError instanceof Error ? botError.message : String(botError);
+      const errorMessage = botError?.message ?? String(botError);
       core.warning(`Failed to check bot status: ${errorMessage}`);
       return { isBot: true, isActive: false, error: errorMessage };
     }
   } catch (error) {
-    const errorMessage = error instanceof Error ? error.message : String(error);
+    const errorMessage = error?.message ?? String(error);
     core.warning(`Error checking bot status: ${errorMessage}`);
     return { isBot: false, isActive: false, error: errorMessage };
   }
@@ -94,17 +92,17 @@ async function checkRepositoryPermission(actor, owner, repo, requiredPermissions
     core.info(`Repository permission level: ${permission}`);
 
     // Check if user has one of the required permission levels
-    for (const requiredPerm of requiredPermissions) {
-      if (permission === requiredPerm || (requiredPerm === "maintainer" && permission === "maintain")) {
-        core.info(`✅ User has ${permission} access to repository`);
-        return { authorized: true, permission: permission };
-      }
+    const hasPermission = requiredPermissions.some(requiredPerm => permission === requiredPerm || (requiredPerm === "maintainer" && permission === "maintain"));
+
+    if (hasPermission) {
+      core.info(`✅ User has ${permission} access to repository`);
+      return { authorized: true, permission };
     }
 
     core.warning(`User permission '${permission}' does not meet requirements: ${requiredPermissions.join(", ")}`);
-    return { authorized: false, permission: permission };
+    return { authorized: false, permission };
   } catch (repoError) {
-    const errorMessage = repoError instanceof Error ? repoError.message : String(repoError);
+    const errorMessage = repoError?.message ?? String(repoError);
     core.warning(`Repository permission check failed: ${errorMessage}`);
     return { authorized: false, error: errorMessage };
   }

actions/setup/js/check_permissions_utils.cjs

@@ -28,29 +28,76 @@ global.github = mockGithub;
 
 describe("check_permissions_utils", () => {
   let parseRequiredPermissions;
+  let parseAllowedBots;
   let checkRepositoryPermission;
+  let checkBotStatus;
   let originalEnv;
 
   beforeEach(async () => {
     // Reset all mocks
     vi.clearAllMocks();
 
     // Store original environment
-    originalEnv = process.env.GH_AW_REQUIRED_ROLES;
+    originalEnv = {
+      GH_AW_REQUIRED_ROLES: process.env.GH_AW_REQUIRED_ROLES,
+      GH_AW_ALLOWED_BOTS: process.env.GH_AW_ALLOWED_BOTS,
+    };
 
     // Import the module functions
     const module = await import("./check_permissions_utils.cjs");
     parseRequiredPermissions = module.parseRequiredPermissions;
+    parseAllowedBots = module.parseAllowedBots;
     checkRepositoryPermission = module.checkRepositoryPermission;
+    checkBotStatus = module.checkBotStatus;
   });
 
   afterEach(() => {
     // Restore original environment
-    if (originalEnv !== undefined) {
-      process.env.GH_AW_REQUIRED_ROLES = originalEnv;
-    } else {
-      delete process.env.GH_AW_REQUIRED_ROLES;
-    }
+    Object.keys(originalEnv).forEach(key => {
+      if (originalEnv[key] !== undefined) {
+        process.env[key] = originalEnv[key];
+      } else {
+        delete process.env[key];
+      }
+    });
+  });
+
+  describe("parseAllowedBots", () => {
+    it("should parse comma-separated bot identifiers", () => {
+      process.env.GH_AW_ALLOWED_BOTS = "dependabot[bot],renovate[bot],github-actions[bot]";
+      const result = parseAllowedBots();
+      expect(result).toEqual(["dependabot[bot]", "renovate[bot]", "github-actions[bot]"]);
+    });
+
+    it("should filter out empty strings", () => {
+      process.env.GH_AW_ALLOWED_BOTS = "dependabot[bot],,renovate[bot],";
+      const result = parseAllowedBots();
+      expect(result).toEqual(["dependabot[bot]", "renovate[bot]"]);
+    });
+
+    it("should filter out whitespace-only entries", () => {
+      process.env.GH_AW_ALLOWED_BOTS = "dependabot[bot], ,renovate[bot]";
+      const result = parseAllowedBots();
+      expect(result).toEqual(["dependabot[bot]", "renovate[bot]"]);
+    });
+
+    it("should return empty array when env var is not set", () => {
+      delete process.env.GH_AW_ALLOWED_BOTS;
+      const result = parseAllowedBots();
+      expect(result).toEqual([]);
+    });
+
+    it("should return empty array when env var is empty string", () => {
+      process.env.GH_AW_ALLOWED_BOTS = "";
+      const result = parseAllowedBots();
+      expect(result).toEqual([]);
+    });
+
+    it("should handle single bot identifier", () => {
+      process.env.GH_AW_ALLOWED_BOTS = "dependabot[bot]";
+      const result = parseAllowedBots();
+      expect(result).toEqual(["dependabot[bot]"]);
+    });
   });
 
   describe("parseRequiredPermissions", () => {
@@ -222,4 +269,109 @@ describe("check_permissions_utils", () => {
       expect(successLog[0]).toContain("write");
     });
   });
+
+  describe("checkBotStatus", () => {
+    it("should identify bot by [bot] suffix", async () => {
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({
+        data: { permission: "write" },
+      });
+
+      const result = await checkBotStatus("dependabot[bot]", "testowner", "testrepo");
+
+      expect(result).toEqual({
+        isBot: true,
+        isActive: true,
+      });
+
+      expect(mockCore.info).toHaveBeenCalledWith("Checking if bot 'dependabot[bot]' is active on testowner/testrepo");
+      expect(mockCore.info).toHaveBeenCalledWith("Bot 'dependabot[bot]' is active with permission level: write");
+    });
+
+    it("should return false for non-bot users", async () => {
+      const result = await checkBotStatus("regularuser", "testowner", "testrepo");
+
+      expect(result).toEqual({
+        isBot: false,
+        isActive: false,
+      });
+
+      expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).not.toHaveBeenCalled();
+    });
+
+    it("should handle 404 error for inactive bot", async () => {
+      const apiError = { status: 404, message: "Not Found" };
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(apiError);
+
+      const result = await checkBotStatus("renovate[bot]", "testowner", "testrepo");
+
+      expect(result).toEqual({
+        isBot: true,
+        isActive: false,
+      });
+
+      expect(mockCore.warning).toHaveBeenCalledWith("Bot 'renovate[bot]' is not active/installed on testowner/testrepo");
+    });
+
+    it("should handle other API errors", async () => {
+      const apiError = new Error("API rate limit exceeded");
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(apiError);
+
+      const result = await checkBotStatus("github-actions[bot]", "testowner", "testrepo");
+
+      expect(result).toEqual({
+        isBot: true,
+        isActive: false,
+        error: "API rate limit exceeded",
+      });
+
+      expect(mockCore.warning).toHaveBeenCalledWith("Failed to check bot status: API rate limit exceeded");
+    });
+
+    it("should handle non-Error API failures", async () => {
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue("String error");
+
+      const result = await checkBotStatus("bot[bot]", "testowner", "testrepo");
+
+      expect(result).toEqual({
+        isBot: true,
+        isActive: false,
+        error: "String error",
+      });
+
+      expect(mockCore.warning).toHaveBeenCalledWith("Failed to check bot status: String error");
+    });
+
+    it("should handle unexpected errors gracefully", async () => {
+      // Simulate an error during bot detection
+      const unexpectedError = new Error("Unexpected error");
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockImplementation(() => {
+        throw unexpectedError;
+      });
+
+      const result = await checkBotStatus("test[bot]", "testowner", "testrepo");
+
+      expect(result).toEqual({
+        isBot: true,
+        isActive: false,
+        error: "Unexpected error",
+      });
+    });
+
+    it("should verify bot is installed on repository", async () => {
+      mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({
+        data: { permission: "admin" },
+      });
+
+      const result = await checkBotStatus("dependabot[bot]", "testowner", "testrepo");
+
+      expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({
+        owner: "testowner",
+        repo: "testrepo",
+        username: "dependabot[bot]",
+      });
+
+      expect(result.isBot).toBe(true);
+      expect(result.isActive).toBe(true);
+    });
+  });
 });

actions/setup/js/check_permissions_utils.test.cjs

@@ -22,10 +22,10 @@
     "test:js-watch": "vitest",
     "test:js-coverage": "vitest run --coverage",
     "format:terser": "find . -name '*.cjs' -type f -not -path './node_modules/*' | while read file; do if ! grep -qE '^(await |/// <reference|// @ts-check)|await import' \"$file\"; then npx terser \"$file\" -b -c dead_code=true,unused=true,collapse_vars=true --ecma 2022 -o \"$file.tmp\" && mv \"$file.tmp\" \"$file\"; fi; done",
-    "format:cjs": "npx prettier --write '**/*.cjs' '**/*.ts' '**/*.json'",
-    "lint:cjs": "npx prettier --check '**/*.cjs' '**/*.ts' '**/*.json'",
+    "format:cjs": "npx prettier --write '**/*.cjs' '**/*.ts' '**/*.json' --ignore-path ../../../.prettierignore",
+    "lint:cjs": "npx prettier --check '**/*.cjs' '**/*.ts' '**/*.json' --ignore-path ../../../.prettierignore",
     "format:schema": "npx prettier --write ../../../pkg/workflow/schemas/github-workflow.json --ignore-path /dev/null",
-    "format:pkg-json": "npx prettier --write '../../../**/*.json' '!../../../pkg/workflow/js/**/*.json' '!../../../**/package.json' '!../../../**/package-lock.json'",
-    "check:pkg-json": "npx prettier --check '../../../**/*.json' '!../../../pkg/workflow/js/**/*.json' '!../../../**/package.json' '!../../../**/package-lock.json'"
+    "format:pkg-json": "npx prettier --write '../../../**/*.json' '!../../../pkg/workflow/js/**/*.json' --ignore-path ../../../.prettierignore",
+    "check:pkg-json": "npx prettier --check '../../../**/*.json' '!../../../pkg/workflow/js/**/*.json' --ignore-path ../../../.prettierignore"
   }
 }