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
9 changes: 6 additions & 3 deletions actions/setup/js/create_discussion.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
const HANDLER_TYPE = "create_discussion";

const { getTrackerID } = require("./get_tracker_id.cjs");
const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs");
const { replaceTemporaryIdReferences } = require("./temporary_id.cjs");
const { parseAllowedRepos, getDefaultTargetRepo, validateRepo, parseRepoSlug } = require("./repo_helpers.cjs");
const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_title.cjs");
Expand Down Expand Up @@ -343,9 +344,11 @@ async function main(config = {}) {
title = item.body || "Discussion";
}

if (titlePrefix && !title.startsWith(titlePrefix)) {
title = titlePrefix + title;
}
// Sanitize title for Unicode security and remove any duplicate prefixes
title = sanitizeTitle(title, titlePrefix);

// Apply title prefix (only if it doesn't already exist)
title = applyTitlePrefix(title, titlePrefix);

// Build body
let bodyLines = processedBody.split("\n");
Expand Down
10 changes: 6 additions & 4 deletions actions/setup/js/create_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ function resetIssuesToAssignCopilot() {
}

const { sanitizeLabelContent } = require("./sanitize_label_content.cjs");
const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs");
const { generateFooter, generateWorkflowIdMarker } = require("./generate_footer.cjs");
const { getTrackerID } = require("./get_tracker_id.cjs");
const { generateTemporaryId, isTemporaryId, normalizeTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs");
Expand Down Expand Up @@ -384,10 +385,11 @@ async function main(config = {}) {
title = message.body ?? "Agent Output";
}

// Apply title prefix
if (titlePrefix && !title.startsWith(titlePrefix)) {
title = titlePrefix + title;
}
// Sanitize title for Unicode security and remove any duplicate prefixes
title = sanitizeTitle(title, titlePrefix);

// Apply title prefix (only if it doesn't already exist)
title = applyTitlePrefix(title, titlePrefix);

// Add parent reference
if (effectiveParentIssueNumber) {
Expand Down
10 changes: 6 additions & 4 deletions actions/setup/js/create_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const { updateActivationComment } = require("./update_activation_comment.cjs");
const { getTrackerID } = require("./get_tracker_id.cjs");
const { addExpirationComment } = require("./expiration_helpers.cjs");
const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_title.cjs");
const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");
const { replaceTemporaryIdReferences } = require("./temporary_id.cjs");
const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs");
Expand Down Expand Up @@ -350,10 +351,11 @@ async function main(config = {}) {
title = "Agent Output";
}

// Apply title prefix from config
if (titlePrefix && !title.startsWith(titlePrefix)) {
title = titlePrefix + title;
}
// Sanitize title for Unicode security and remove any duplicate prefixes
title = sanitizeTitle(title, titlePrefix);

// Apply title prefix (only if it doesn't already exist)
title = applyTitlePrefix(title, titlePrefix);

// Add AI disclaimer with workflow name and run url
const workflowName = process.env.GH_AW_WORKFLOW_NAME || "Workflow";
Expand Down
83 changes: 83 additions & 0 deletions actions/setup/js/sanitize_title.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// @ts-check
/**
* Title sanitization utilities for issues, discussions, and pull requests
* @module sanitize_title
*/

const { hardenUnicodeText } = require("./sanitize_content_core.cjs");

/**
* Sanitizes a title by applying Unicode security hardening and preventing duplicate prefixes
* @param {string} title - The title to sanitize
* @param {string} [titlePrefix] - Optional prefix that may need to be added
* @returns {string} The sanitized title
*/
function sanitizeTitle(title, titlePrefix = "") {
if (!title || typeof title !== "string") {
return "";
}

let sanitized = title.trim();

// Apply Unicode security hardening (NFC normalization, zero-width removal, etc.)
sanitized = hardenUnicodeText(sanitized);

// If a prefix is provided, remove any existing occurrences to avoid duplication
if (titlePrefix && titlePrefix.trim()) {
const cleanPrefix = titlePrefix.trim();

// First, check for and remove variations with common separators that agents might add
// This prevents issues like "[Agent]: Title" being treated as duplicate
// Get the prefix without any trailing space for separator checking
const prefixWithoutSpace = cleanPrefix.replace(/\s+$/, "");
const separatorPatterns = [":", " -", " |"];

let foundSeparator = false;
for (const separator of separatorPatterns) {
const variation = prefixWithoutSpace + separator;
if (sanitized.startsWith(variation)) {
sanitized = sanitized.substring(variation.length).trim();
foundSeparator = true;
break; // Only remove one separator variation
}
}

// If no separator was found, remove the exact prefix (case-sensitive match)
// Keep removing until we no longer find the prefix at the start
if (!foundSeparator) {
while (sanitized.startsWith(cleanPrefix)) {
sanitized = sanitized.substring(cleanPrefix.length).trim();
}
}
}

return sanitized;
}

/**
* Applies a title prefix to a sanitized title
* This should be called after sanitizeTitle() to ensure the title is clean before prefixing
* @param {string} sanitizedTitle - The already-sanitized title
* @param {string} titlePrefix - The prefix to add
* @returns {string} The title with prefix applied
*/
function applyTitlePrefix(sanitizedTitle, titlePrefix) {
if (!titlePrefix || !titlePrefix.trim()) {
return sanitizedTitle;
}

const cleanTitle = sanitizedTitle.trim();

// Only add prefix if title doesn't already start with it
// The titlePrefix parameter is used as-is (not trimmed) to preserve any trailing space
if (cleanTitle && !cleanTitle.startsWith(titlePrefix)) {
return titlePrefix + cleanTitle;
}

return cleanTitle;
}

module.exports = {
sanitizeTitle,
applyTitlePrefix,
};
196 changes: 196 additions & 0 deletions actions/setup/js/sanitize_title.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
// @ts-check
import { describe, it, expect } from "vitest";

const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs");

describe("sanitize_title", () => {
describe("sanitizeTitle", () => {
describe("basic sanitization", () => {
it("should return empty string for null/undefined", () => {
expect(sanitizeTitle(null)).toBe("");
expect(sanitizeTitle(undefined)).toBe("");
expect(sanitizeTitle("")).toBe("");
});

it("should trim whitespace", () => {
expect(sanitizeTitle(" Test Title ")).toBe("Test Title");
expect(sanitizeTitle("\n\tTest Title\n\t")).toBe("Test Title");
});

it("should handle normal ASCII titles", () => {
expect(sanitizeTitle("Bug Report")).toBe("Bug Report");
expect(sanitizeTitle("Feature Request: Add new feature")).toBe("Feature Request: Add new feature");
});
});

describe("Unicode security hardening", () => {
it("should apply NFC normalization", () => {
// Composed vs decomposed é (U+00E9 vs U+0065 U+0301)
const composed = "café";
const decomposed = "café"; // Using combining character
expect(sanitizeTitle(composed)).toBe(sanitizeTitle(decomposed));
});

it("should strip zero-width characters", () => {
expect(sanitizeTitle("Test\u200BTitle")).toBe("TestTitle");
expect(sanitizeTitle("Test\u200CTitle")).toBe("TestTitle");
expect(sanitizeTitle("Test\u200DTitle")).toBe("TestTitle");
expect(sanitizeTitle("Test\u2060Title")).toBe("TestTitle");
expect(sanitizeTitle("Test\uFEFFTitle")).toBe("TestTitle");
});

it("should remove bidirectional override controls", () => {
expect(sanitizeTitle("Test\u202ATitle")).toBe("TestTitle");
expect(sanitizeTitle("Test\u202BTitle")).toBe("TestTitle");
expect(sanitizeTitle("Test\u202CTitle")).toBe("TestTitle");
expect(sanitizeTitle("Test\u202DTitle")).toBe("TestTitle");
expect(sanitizeTitle("Test\u202ETitle")).toBe("TestTitle");
expect(sanitizeTitle("Test\u2066Title")).toBe("TestTitle");
expect(sanitizeTitle("Test\u2067Title")).toBe("TestTitle");
expect(sanitizeTitle("Test\u2068Title")).toBe("TestTitle");
expect(sanitizeTitle("Test\u2069Title")).toBe("TestTitle");
});

it("should convert fullwidth ASCII to standard ASCII", () => {
// Fullwidth brackets
expect(sanitizeTitle("[Test]")).toBe("[Test]");
// Fullwidth letters and numbers (note: fullwidth space U+3000 is not converted by hardenUnicodeText)
expect(sanitizeTitle("Test 123")).toBe("Test 123");
// Mix of fullwidth and normal
expect(sanitizeTitle("Test[Agent]")).toBe("Test[Agent]");
});

it("should handle complex Unicode attacks", () => {
// Combining zero-width spaces with bidirectional overrides
const malicious = "Test\u200B\u202ETi\u200Ctle\u202C\uFEFF";
expect(sanitizeTitle(malicious)).toBe("TestTitle");
});
});

describe("duplicate prefix removal", () => {
it("should remove exact prefix match", () => {
expect(sanitizeTitle("[Agent] Fix bug", "[Agent] ")).toBe("Fix bug");
expect(sanitizeTitle("🤖 Fix bug", "🤖 ")).toBe("Fix bug");
expect(sanitizeTitle("[WIP] Update docs", "[WIP] ")).toBe("Update docs");
});

it("should remove prefix with colon separator", () => {
expect(sanitizeTitle("[Agent]: Fix bug", "[Agent] ")).toBe("Fix bug");
expect(sanitizeTitle("Agent: Fix bug", "Agent ")).toBe("Fix bug");
});

it("should remove prefix with dash separator", () => {
expect(sanitizeTitle("[Agent] - Fix bug", "[Agent] ")).toBe("Fix bug");
expect(sanitizeTitle("Agent - Fix bug", "Agent ")).toBe("Fix bug");
});

it("should remove prefix with pipe separator", () => {
expect(sanitizeTitle("[Agent] | Fix bug", "[Agent] ")).toBe("Fix bug");
expect(sanitizeTitle("Agent | Fix bug", "Agent ")).toBe("Fix bug");
});

it("should not remove prefix from middle of title", () => {
expect(sanitizeTitle("Fix [Agent] bug", "[Agent] ")).toBe("Fix [Agent] bug");
expect(sanitizeTitle("Update Agent feature", "Agent ")).toBe("Update Agent feature");
});

it("should handle case-sensitive prefix matching", () => {
expect(sanitizeTitle("[AGENT] Fix bug", "[Agent] ")).toBe("[AGENT] Fix bug");
expect(sanitizeTitle("[agent] Fix bug", "[Agent] ")).toBe("[agent] Fix bug");
});

it("should handle empty or whitespace-only prefix", () => {
expect(sanitizeTitle("Test Title", "")).toBe("Test Title");
expect(sanitizeTitle("Test Title", " ")).toBe("Test Title");
});

it("should prevent double prefix from agent confusion", () => {
// Agent might generate "[Agent] [Agent] Title" if confused
// sanitizeTitle removes all instances of the prefix, so both get removed
expect(sanitizeTitle("[Agent] [Agent] Fix bug", "[Agent] ")).toBe("Fix bug");
});

it("should handle Unicode in prefixes", () => {
// sanitizeTitle removes all instances of the prefix
expect(sanitizeTitle("🤖 🤖 Fix bug", "🤖 ")).toBe("Fix bug");
// After hardening, fullwidth brackets become regular brackets, then get removed as prefix
expect(sanitizeTitle("[Agent]Fix bug", "[Agent] ")).toBe("Fix bug");
});
});

describe("combined sanitization and prefix removal", () => {
it("should apply Unicode hardening before prefix removal", () => {
const title = "[Agent]\u200BFix\u202Ebug\u202C";
// After hardening: "[Agent]Fixbug", then "[Agent]" prefix removed (no space), leaving "Fixbug"
expect(sanitizeTitle(title, "[Agent] ")).toBe("Fixbug");
});

it("should handle malicious prefix injection attempts", () => {
// Attacker tries to inject prefix with invisible characters
// After hardening: "[Agent] [Agent] Fix bug", then both prefixes get removed
const title = "[Agent]\u200B\u202A [Agent]\u202C Fix bug";
expect(sanitizeTitle(title, "[Agent] ")).toBe("Fix bug");
});

it("should preserve legitimate content after sanitization", () => {
const title = "[Agent] Fix bug #123: Update configuration";
expect(sanitizeTitle(title, "[Agent] ")).toBe("Fix bug #123: Update configuration");
});
});
});

describe("applyTitlePrefix", () => {
it("should add prefix to clean title", () => {
expect(applyTitlePrefix("Fix bug", "[Agent] ")).toBe("[Agent] Fix bug");
expect(applyTitlePrefix("Update docs", "🤖 ")).toBe("🤖 Update docs");
});

it("should not duplicate prefix if already present", () => {
expect(applyTitlePrefix("[Agent] Fix bug", "[Agent] ")).toBe("[Agent] Fix bug");
expect(applyTitlePrefix("🤖 Fix bug", "🤖 ")).toBe("🤖 Fix bug");
});

it("should handle empty prefix", () => {
expect(applyTitlePrefix("Fix bug", "")).toBe("Fix bug");
expect(applyTitlePrefix("Fix bug", " ")).toBe("Fix bug");
});

it("should trim inputs", () => {
// applyTitlePrefix should use titlePrefix as-is, but the title is trimmed
expect(applyTitlePrefix(" Fix bug ", " [Agent] ")).toBe(" [Agent] Fix bug");
});

it("should handle empty title", () => {
expect(applyTitlePrefix("", "[Agent] ")).toBe("");
expect(applyTitlePrefix(" ", "[Agent] ")).toBe("");
});
});

describe("integration scenarios", () => {
it("should handle typical workflow: sanitize then apply prefix", () => {
const rawTitle = "[Agent]\u200BFix\u202Ebug #123\u202C";
const sanitized = sanitizeTitle(rawTitle, "[Agent] ");
const final = applyTitlePrefix(sanitized, "[Agent] ");
// After sanitization, prefix is removed, so we need to apply it again
expect(final).toBe("[Agent] Fixbug #123");
});

it("should prevent agent-generated duplicate prefixes", () => {
// Agent generates title with prefix already included
const agentTitle = "[Agent] Update configuration";
const sanitized = sanitizeTitle(agentTitle, "[Agent] ");
const final = applyTitlePrefix(sanitized, "[Agent] ");
// Prefix gets removed during sanitization, then re-applied
expect(final).toBe("[Agent] Update configuration");
});

it("should handle fullwidth brackets in agent output", () => {
// Agent uses fullwidth brackets (common in some locales)
const agentTitle = "[Agent]Fix critical bug";
const sanitized = sanitizeTitle(agentTitle, "[Agent] ");
const final = applyTitlePrefix(sanitized, "[Agent] ");
// Fullwidth brackets get converted to ASCII "[Agent]Fix", then "[Agent]" removed leaving "Fix critical bug"
expect(final).toBe("[Agent] Fix critical bug");
});
});
});
4 changes: 3 additions & 1 deletion actions/setup/js/update_discussion.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

const { isDiscussionContext, getDiscussionNumber } = require("./update_context_helpers.cjs");
const { createUpdateHandlerFactory } = require("./update_handler_factory.cjs");
const { sanitizeTitle } = require("./sanitize_title.cjs");

/**
* Execute the discussion update API call using GraphQL
Expand Down Expand Up @@ -126,7 +127,8 @@ function buildDiscussionUpdateData(item, config) {
const updateData = {};

if (item.title !== undefined) {
updateData.title = item.title;
// Sanitize title for Unicode security (no prefix handling needed for updates)
updateData.title = sanitizeTitle(item.title);
}
if (item.body !== undefined) {
updateData.body = item.body;
Expand Down
4 changes: 3 additions & 1 deletion actions/setup/js/update_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const { resolveTarget } = require("./safe_output_helpers.cjs");
const { createUpdateHandlerFactory } = require("./update_handler_factory.cjs");
const { updateBody } = require("./update_pr_description_helpers.cjs");
const { loadTemporaryProjectMap, replaceTemporaryProjectReferences } = require("./temporary_id.cjs");
const { sanitizeTitle } = require("./sanitize_title.cjs");

/**
* Execute the issue update API call
Expand Down Expand Up @@ -109,7 +110,8 @@ function buildIssueUpdateData(item, config) {
const updateData = {};

if (item.title !== undefined) {
updateData.title = item.title;
// Sanitize title for Unicode security (no prefix handling needed for updates)
updateData.title = sanitizeTitle(item.title);
}
if (item.body !== undefined) {
// Store operation information for consistent footer/append behavior.
Expand Down
Loading
Loading