Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
55 changes: 55 additions & 0 deletions actions/setup/js/sanitize_content.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,61 @@ describe("sanitize_content.cjs", () => {
});
});

describe("@mention bypass prevention (underscore-prefixed)", () => {
// Security tests for CVE-like vulnerability where underscore before @ could bypass sanitization
// These test cases are from the security report documenting the bypass patterns

it("should neutralize @mentions preceded by underscore in function names", () => {
const result = sanitizeContent("test_@user");
expect(result).toBe("test_`@user`");
});
Comment on lines +134 to +141
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

These new bypass tests only exercise the default path (no allowedAliases), which uses sanitizeContentCore. The underscore-prefix bypass still exists in other code paths that use the old boundary regex (e.g. sanitizeContent() when allowedAliases is non-empty, and other mention regex usages in this package). Add at least one test that calls sanitizeContent with a non-empty allowedAliases list and asserts that an underscore-prefixed disallowed mention (e.g. "ok @Allowed and test_@admin" with allowedAliases ["allowed"]) is still neutralized; then update the corresponding implementation(s) to use the same boundary fix.

Copilot uses AI. Check for mistakes.

it("should neutralize @mentions preceded by underscore in variable names", () => {
const result = sanitizeContent("production_@maintainer");
expect(result).toBe("production_`@maintainer`");
});

it("should neutralize @mentions preceded by underscore with hyphens", () => {
const result = sanitizeContent("validate_@security-team");
expect(result).toBe("validate_`@security-team`");
});

it("should neutralize @mentions preceded by underscore in commands", () => {
const result = sanitizeContent("run_@admin");
expect(result).toBe("run_`@admin`");
});

it("should neutralize @mentions preceded by multiple underscores", () => {
const result = sanitizeContent("My_Project_@owner");
expect(result).toBe("My_Project_`@owner`");
});

it("should neutralize @mentions with just underscore prefix", () => {
const result = sanitizeContent("_@user");
expect(result).toBe("_`@user`");
});

it("should neutralize @mentions preceded by underscore with possessive", () => {
const result = sanitizeContent("is_@user's project");
expect(result).toBe("is_`@user`'s project");
});

it("should neutralize multiple underscore-prefixed @mentions", () => {
const result = sanitizeContent("config_@admin and deploy_@maintainer");
expect(result).toBe("config_`@admin` and deploy_`@maintainer`");
});

it("should neutralize underscore-prefixed org/team mentions", () => {
const result = sanitizeContent("api_@org/team");
expect(result).toBe("api_`@org/team`");
});

it("should handle mixed normal and underscore-prefixed mentions", () => {
const result = sanitizeContent("Hello @user and test_@admin");
expect(result).toBe("Hello `@user` and test_`@admin`");
});
});

describe("@mention allowedAliases", () => {
it("should not neutralize mentions in allowedAliases list", () => {
const result = sanitizeContent("Hello @author", { allowedAliases: ["author"] });
Expand Down
4 changes: 3 additions & 1 deletion actions/setup/js/sanitize_content_core.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,9 @@ function neutralizeCommands(s) {
function neutralizeAllMentions(s) {
// Replace @name or @org/team outside code with `@name`
// No filtering - all mentions are neutralized
return s.replace(/(^|[^\w`])@([A-Za-z0-9](?:[A-Za-z0-9_-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, (m, p1, p2) => {
// Changed [^\w`] to [^A-Za-z0-9`] to include underscore as a valid preceding character
// This prevents bypass patterns like "test_@user" from escaping sanitization
return s.replace(/(^|[^A-Za-z0-9`])@([A-Za-z0-9](?:[A-Za-z0-9_-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, (m, p1, p2) => {
// Log when a mention is escaped to help debug issues
if (typeof core !== "undefined" && core.info) {
core.info(`Escaped mention: @${p2} (not in allowed list)`);
Comment on lines +300 to 305
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

The new comment is misleading: the change doesn’t make underscore a “valid preceding character” for mentions, it makes underscore count as a separator so the regex matches mentions after underscores (fixing the bypass). Please reword to avoid confusion about what the character class is doing.

This issue also appears on line 303 of the same file.

Suggested change
// Changed [^\w`] to [^A-Za-z0-9`] to include underscore as a valid preceding character
// This prevents bypass patterns like "test_@user" from escaping sanitization
return s.replace(/(^|[^A-Za-z0-9`])@([A-Za-z0-9](?:[A-Za-z0-9_-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, (m, p1, p2) => {
// Log when a mention is escaped to help debug issues
if (typeof core !== "undefined" && core.info) {
core.info(`Escaped mention: @${p2} (not in allowed list)`);
// Changed [^\w`] to [^A-Za-z0-9`] to treat underscore as a separator before mentions
// This ensures patterns like "test_@user" are matched and neutralized instead of bypassing
return s.replace(/(^|[^A-Za-z0-9`])@([A-Za-z0-9](?:[A-Za-z0-9_-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, (m, p1, p2) => {
// Log when a mention is neutralized to help debug issues
if (typeof core !== "undefined" && core.info) {
core.info(`Escaped mention: @${p2}`);

Copilot uses AI. Check for mistakes.
Expand Down
Loading