Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion actions/setup/js/assign_to_user.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const HANDLER_TYPE = "assign_to_user";
async function main(config = {}) {
// Extract configuration
const allowedAssignees = config.allowed || [];
const blockedAssignees = config.blocked || [];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition of blockedAssignees support. Consider adding a comment here explaining that blocked takes precedence over allowed to clarify the filtering priority for future readers.

const maxCount = config.max || 10;
const unassignFirst = config.unassign_first || false;
const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config);
Expand All @@ -32,6 +33,9 @@ async function main(config = {}) {
if (allowedAssignees.length > 0) {
core.info(`Allowed assignees: ${allowedAssignees.join(", ")}`);
}
if (blockedAssignees.length > 0) {
core.info(`Blocked assignees: ${blockedAssignees.join(", ")}`);
}
core.info(`Default target repo: ${defaultTargetRepo}`);
if (allowedRepos.size > 0) {
core.info(`Allowed repos: ${Array.from(allowedRepos).join(", ")}`);
Expand Down Expand Up @@ -89,7 +93,7 @@ async function main(config = {}) {
core.info(`Requested assignees: ${JSON.stringify(requestedAssignees)}`);

// Use shared helper to filter, sanitize, dedupe, and limit
const uniqueAssignees = processItems(requestedAssignees, allowedAssignees, maxCount);
const uniqueAssignees = processItems(requestedAssignees, allowedAssignees, maxCount, blockedAssignees);

if (uniqueAssignees.length === 0) {
core.info("No assignees to add");
Expand Down
102 changes: 102 additions & 0 deletions actions/setup/js/assign_to_user.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -483,4 +483,106 @@ describe("assign_to_user (Handler Factory Architecture)", () => {
assignees: ["new-user1"],
});
});

describe("blocked patterns", () => {
it("should filter out blocked users by exact match", async () => {
const { main } = require("./assign_to_user.cjs");
const handler = await main({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good test coverage for blocked patterns. It would also be worth adding a test case that verifies an empty blocked array (the default) doesn't filter any valid assignees.

max: 10,
blocked: ["copilot", "admin"],
});

mockGithub.rest.issues.addAssignees.mockResolvedValue({});

const message = {
type: "assign_to_user",
assignees: ["user1", "copilot", "admin", "user2"],
};

const result = await handler(message, {});

expect(result.success).toBe(true);
expect(result.assigneesAdded).toEqual(["user1", "user2"]);
expect(mockGithub.rest.issues.addAssignees).toHaveBeenCalledWith({
owner: "test-owner",
repo: "test-repo",
issue_number: 123,
assignees: ["user1", "user2"],
});
});

it("should filter out blocked users by pattern", async () => {
const { main } = require("./assign_to_user.cjs");
const handler = await main({
max: 10,
blocked: ["*[bot]"],
});

mockGithub.rest.issues.addAssignees.mockResolvedValue({});

const message = {
type: "assign_to_user",
assignees: ["user1", "dependabot[bot]", "github-actions[bot]", "user2"],
};

const result = await handler(message, {});

expect(result.success).toBe(true);
expect(result.assigneesAdded).toEqual(["user1", "user2"]);
expect(mockGithub.rest.issues.addAssignees).toHaveBeenCalledWith({
owner: "test-owner",
repo: "test-repo",
issue_number: 123,
assignees: ["user1", "user2"],
});
});

it("should combine allowed and blocked filters", async () => {
const { main } = require("./assign_to_user.cjs");
const handler = await main({
max: 10,
allowed: ["user1", "user2", "copilot", "github-actions[bot]"],
blocked: ["copilot", "*[bot]"],
});

mockGithub.rest.issues.addAssignees.mockResolvedValue({});

const message = {
type: "assign_to_user",
assignees: ["user1", "user2", "copilot", "github-actions[bot]", "unauthorized"],
};

const result = await handler(message, {});

expect(result.success).toBe(true);
// Should only include user1 and user2 (allowed and not blocked)
expect(result.assigneesAdded).toEqual(["user1", "user2"]);
expect(mockGithub.rest.issues.addAssignees).toHaveBeenCalledWith({
owner: "test-owner",
repo: "test-repo",
issue_number: 123,
assignees: ["user1", "user2"],
});
});

it("should return success with empty array when all assignees are blocked", async () => {
const { main } = require("./assign_to_user.cjs");
const handler = await main({
max: 10,
blocked: ["*[bot]"],
});

const message = {
type: "assign_to_user",
assignees: ["dependabot[bot]", "github-actions[bot]"],
};

const result = await handler(message, {});

expect(result.success).toBe(true);
expect(result.assigneesAdded).toEqual([]);
expect(result.message).toContain("No valid assignees found");
expect(mockGithub.rest.issues.addAssignees).not.toHaveBeenCalled();
});
});
});
61 changes: 61 additions & 0 deletions actions/setup/js/glob_pattern_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,69 @@ function matchesGlobPattern(filePath, fileGlobFilter) {
return patterns.some(pattern => pattern.test(filePath));
}

/**

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot deduplicate with globPatternToRegex

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deduplicated in commit 0626e1a. The simpleGlobToRegex() function now reuses globPatternToRegex() with pathMode: false option, sharing the same escaping logic through a new escapeRegexChars() helper.

* Convert a simple glob pattern to a RegExp (for non-path matching)
* @param {string} pattern - Glob pattern (e.g., "copilot", "*[bot]")
* @param {boolean} caseSensitive - Whether matching should be case-sensitive (default: false)
* @returns {RegExp} - Regular expression that matches the pattern
*
* Supports:
* - * matches any characters (not limited to non-slash like globPatternToRegex)
* - Escapes special regex characters except *
* - Case-insensitive by default
*
* @example
* const regex = simpleGlobToRegex("*[bot]");
* regex.test("dependabot[bot]"); // true
* regex.test("github-actions[bot]"); // true
*
* @example
* const regex = simpleGlobToRegex("copilot");
* regex.test("copilot"); // true
* regex.test("Copilot"); // true (case-insensitive)
*/
function simpleGlobToRegex(pattern, caseSensitive = false) {
// Escape special regex characters except *
const regexPattern = pattern
.replace(/[.+?^${}()|[\]\\]/g, "\\$&") // Escape special regex chars
.replace(/\*/g, ".*"); // Convert * to .*

return new RegExp(`^${regexPattern}$`, caseSensitive ? "" : "i");
}

/**
* Check if a string matches a simple glob pattern
* @param {string} str - String to test (e.g., "copilot", "dependabot[bot]")
* @param {string} pattern - Glob pattern (e.g., "copilot", "*[bot]")
* @param {boolean} caseSensitive - Whether matching should be case-sensitive (default: false)
* @returns {boolean} - True if the string matches the pattern
*
* @example
* matchesSimpleGlob("dependabot[bot]", "*[bot]"); // true
* matchesSimpleGlob("copilot", "copilot"); // true
* matchesSimpleGlob("Copilot", "copilot"); // true (case-insensitive by default)
* matchesSimpleGlob("alice", "*[bot]"); // false
*/
function matchesSimpleGlob(str, pattern, caseSensitive = false) {
if (!str || !pattern) {
return false;
}

// Exact match check (case-insensitive by default)
if (!caseSensitive && str.toLowerCase() === pattern.toLowerCase()) {
return true;
} else if (caseSensitive && str === pattern) {
return true;
}

const regex = simpleGlobToRegex(pattern, caseSensitive);
return regex.test(str);
}

module.exports = {
globPatternToRegex,
parseGlobPatterns,
matchesGlobPattern,
simpleGlobToRegex,
matchesSimpleGlob,
};
89 changes: 88 additions & 1 deletion actions/setup/js/glob_pattern_helpers.test.cjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from "vitest";
import { globPatternToRegex, parseGlobPatterns, matchesGlobPattern } from "./glob_pattern_helpers.cjs";
import { globPatternToRegex, parseGlobPatterns, matchesGlobPattern, simpleGlobToRegex, matchesSimpleGlob } from "./glob_pattern_helpers.cjs";

describe("glob_pattern_helpers.cjs", () => {
describe("globPatternToRegex", () => {
Expand Down Expand Up @@ -358,4 +358,91 @@
expect(patterns.some(p => p.test("dir/history.jsonl"))).toBe(false);
});
});

describe("simpleGlobToRegex", () => {
it("should match exact patterns without wildcards", () => {
const regex = simpleGlobToRegex("copilot");

expect(regex.test("copilot")).toBe(true);
expect(regex.test("Copilot")).toBe(true); // Case-insensitive by default
expect(regex.test("alice")).toBe(false);
});

it("should match wildcard patterns", () => {
const regex = simpleGlobToRegex("*[bot]");

expect(regex.test("dependabot[bot]")).toBe(true);
expect(regex.test("github-actions[bot]")).toBe(true);
expect(regex.test("renovate[bot]")).toBe(true);
expect(regex.test("alice")).toBe(false);
expect(regex.test("bot-user")).toBe(false);
});

it("should handle wildcards at different positions", () => {
const prefixRegex = simpleGlobToRegex("github-*");
const suffixRegex = simpleGlobToRegex("*-bot");

expect(prefixRegex.test("github-actions")).toBe(true);
expect(prefixRegex.test("github-bot")).toBe(true);
expect(prefixRegex.test("gitlab-actions")).toBe(false);

expect(suffixRegex.test("my-bot")).toBe(true);
expect(suffixRegex.test("github-bot")).toBe(true);
expect(suffixRegex.test("bot-user")).toBe(false);
});

it("should respect case sensitivity flag", () => {
const caseSensitiveRegex = simpleGlobToRegex("Copilot", true);
const caseInsensitiveRegex = simpleGlobToRegex("Copilot", false);

expect(caseSensitiveRegex.test("Copilot")).toBe(true);
expect(caseSensitiveRegex.test("copilot")).toBe(false);

expect(caseInsensitiveRegex.test("Copilot")).toBe(true);
expect(caseInsensitiveRegex.test("copilot")).toBe(true);
expect(caseInsensitiveRegex.test("COPILOT")).toBe(true);
});

it("should escape special regex characters", () => {
const regex = simpleGlobToRegex("user.name");

expect(regex.test("user.name")).toBe(true);
expect(regex.test("user_name")).toBe(false);

Check failure on line 410 in actions/setup/js/glob_pattern_helpers.test.cjs

View workflow job for this annotation

GitHub Actions / copilot

glob_pattern_helpers.test.cjs > glob_pattern_helpers.cjs > simpleGlobToRegex > should escape special regex characters

AssertionError: expected true to be false // Object.is equality - Expected + Received - false + true ❯ glob_pattern_helpers.test.cjs:410:39
expect(regex.test("username")).toBe(false);
});
});

describe("matchesSimpleGlob", () => {
it("should match exact usernames", () => {
expect(matchesSimpleGlob("copilot", "copilot")).toBe(true);
expect(matchesSimpleGlob("Copilot", "copilot")).toBe(true); // Case-insensitive
expect(matchesSimpleGlob("alice", "copilot")).toBe(false);
});

it("should match wildcard patterns for bot accounts", () => {
expect(matchesSimpleGlob("dependabot[bot]", "*[bot]")).toBe(true);
expect(matchesSimpleGlob("github-actions[bot]", "*[bot]")).toBe(true);
expect(matchesSimpleGlob("renovate[bot]", "*[bot]")).toBe(true);
expect(matchesSimpleGlob("alice", "*[bot]")).toBe(false);
});

it("should handle empty or null inputs", () => {
expect(matchesSimpleGlob("", "copilot")).toBe(false);
expect(matchesSimpleGlob("copilot", "")).toBe(false);
expect(matchesSimpleGlob(null, "copilot")).toBe(false);
expect(matchesSimpleGlob("copilot", null)).toBe(false);
});

it("should respect case sensitivity flag", () => {
expect(matchesSimpleGlob("Copilot", "copilot", false)).toBe(true);
expect(matchesSimpleGlob("Copilot", "copilot", true)).toBe(false);
expect(matchesSimpleGlob("copilot", "copilot", true)).toBe(true);
});

it("should match wildcards at various positions", () => {
expect(matchesSimpleGlob("github-actions-bot", "github-*")).toBe(true);
expect(matchesSimpleGlob("my-bot", "*-bot")).toBe(true);
expect(matchesSimpleGlob("test-user-123", "test-*-123")).toBe(true);
});
});
});
28 changes: 28 additions & 0 deletions actions/setup/js/safe_output_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,39 @@ function extractAssignees(message) {
return requestedAssignees;
}

/**
* Check if a username matches a blocked pattern
* Supports exact matching and glob-style patterns (e.g., "*[bot]")
* @param {string} username - The username to check
* @param {string} pattern - The pattern to match against (e.g., "copilot", "*[bot]")
* @returns {boolean} True if username matches the blocked pattern
*/
function matchesBlockedPattern(username, pattern) {
Comment thread
pelikhan marked this conversation as resolved.
const { matchesSimpleGlob } = require("./glob_pattern_helpers.cjs");
return matchesSimpleGlob(username, pattern);
}

/**
* Check if a username is blocked by any pattern in the blocked list
* @param {string} username - The username to check
* @param {string[]|undefined} blockedPatterns - Array of blocked patterns (e.g., ["copilot", "*[bot]"])
* @returns {boolean} True if username is blocked
*/
function isUsernameBlocked(username, blockedPatterns) {
if (!blockedPatterns || blockedPatterns.length === 0) {
return false;
}

return blockedPatterns.some(pattern => matchesBlockedPattern(username, pattern));
}

module.exports = {
parseAllowedItems,
parseMaxCount,
resolveTarget,
loadCustomSafeOutputJobTypes,
resolveIssueNumber,
extractAssignees,
matchesBlockedPattern,
isUsernameBlocked,
};
Loading