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
5 changes: 5 additions & 0 deletions .changeset/patch-stable-island-markers.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions actions/setup/js/fuzz_update_body_harness.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,18 @@ const { updateBody } = require("./update_pr_description_helpers.cjs");
* @param {string} operation - Operation type: "append", "prepend", "replace", or "replace-island"
* @param {string} workflowName - Name of the workflow
* @param {string} runUrl - URL of the workflow run
* @param {number} runId - Workflow run ID
* @param {string} workflowId - Workflow ID (stable identifier across runs)
* @returns {{result: string, error: string | null}} Result object
*/
function testUpdateBody(currentBody, newContent, operation, workflowName, runUrl, runId) {
function testUpdateBody(currentBody, newContent, operation, workflowName, runUrl, workflowId) {
try {
const result = updateBody({
currentBody,
newContent,
operation,
workflowName,
runUrl,
runId,
workflowId,
});
return { result, error: null };
} catch (err) {
Expand All @@ -55,9 +55,9 @@ if (require.main === module) {

process.stdin.on("end", () => {
try {
// Parse input as JSON: { currentBody, newContent, operation, workflowName, runUrl, runId }
const { currentBody, newContent, operation, workflowName, runUrl, runId } = JSON.parse(input);
const result = testUpdateBody(currentBody || "", newContent || "", operation || "append", workflowName || "Test Workflow", runUrl || "https://github.com/test/actions/runs/123", runId || 123);
// Parse input as JSON: { currentBody, newContent, operation, workflowName, runUrl, workflowId }
const { currentBody, newContent, operation, workflowName, runUrl, workflowId } = JSON.parse(input);
const result = testUpdateBody(currentBody || "", newContent || "", operation || "append", workflowName || "Test Workflow", runUrl || "https://github.com/test/actions/runs/123", workflowId || "test-workflow");
process.stdout.write(JSON.stringify(result));
process.exit(0);
} catch (err) {
Expand Down
3 changes: 2 additions & 1 deletion actions/setup/js/update_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ async function executeIssueUpdate(github, context, issueNumber, updateData) {

// Get workflow run URL for AI attribution
const workflowName = process.env.GH_AW_WORKFLOW_NAME || "GitHub Agentic Workflow";
const workflowId = process.env.GH_AW_WORKFLOW_ID || "";
const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;

// Use helper to update body (handles all operations including replace)
Expand All @@ -61,7 +62,7 @@ async function executeIssueUpdate(github, context, issueNumber, updateData) {
operation,
workflowName,
runUrl,
runId: context.runId,
workflowId,
includeFooter, // Pass footer flag to helper
});

Expand Down
21 changes: 11 additions & 10 deletions actions/setup/js/update_issue.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ describe("update_issue.cjs - footer support", () => {

// Reset environment variables
process.env.GH_AW_WORKFLOW_NAME = "Test Workflow";
process.env.GH_AW_WORKFLOW_ID = "test-workflow";

// Reset mock implementations
mockGithub.rest.issues.get.mockResolvedValue({
Expand Down Expand Up @@ -209,17 +210,17 @@ describe("update_issue.cjs - footer support", () => {
});

const newContent = "Island content";
const expectedBody = `Original content\n\n---\n\n<!-- gh-aw-island-start:12345 -->\n${newContent}\n\n> AI generated by [Test Workflow](https://github.com/testowner/testrepo/actions/runs/12345)\n<!-- gh-aw-island-end:12345 -->`;
const expectedBody = `Original content\n\n---\n\n<!-- gh-aw-island-start:test-workflow -->\n${newContent}\n\n> AI generated by [Test Workflow](https://github.com/testowner/testrepo/actions/runs/12345)\n<!-- gh-aw-island-end:test-workflow -->`;

expect(expectedBody).toContain("Original content");
expect(expectedBody).toContain("Island content");
expect(expectedBody).toContain("<!-- gh-aw-island-start:12345 -->");
expect(expectedBody).toContain("<!-- gh-aw-island-end:12345 -->");
expect(expectedBody).toContain("<!-- gh-aw-island-start:test-workflow -->");
expect(expectedBody).toContain("<!-- gh-aw-island-end:test-workflow -->");
expect(expectedBody).toContain("> AI generated by");
});

it("should replace existing island content", async () => {
const existingBody = "Before\n<!-- gh-aw-island-start:12345 -->\nOld island\n<!-- gh-aw-island-end:12345 -->\nAfter";
const existingBody = "Before\n<!-- gh-aw-island-start:test-workflow -->\nOld island\n<!-- gh-aw-island-end:test-workflow -->\nAfter";
mockGithub.rest.issues.get.mockResolvedValueOnce({
data: {
number: 100,
Expand All @@ -230,7 +231,7 @@ describe("update_issue.cjs - footer support", () => {
});

const newContent = "New island";
const expectedBody = `Before\n<!-- gh-aw-island-start:12345 -->\n${newContent}\n\n> AI generated by [Test Workflow](https://github.com/testowner/testrepo/actions/runs/12345)\n<!-- gh-aw-island-end:12345 -->\nAfter`;
const expectedBody = `Before\n<!-- gh-aw-island-start:test-workflow -->\n${newContent}\n\n> AI generated by [Test Workflow](https://github.com/testowner/testrepo/actions/runs/12345)\n<!-- gh-aw-island-end:test-workflow -->\nAfter`;

expect(expectedBody).toContain("Before");
expect(expectedBody).toContain("After");
Expand Down Expand Up @@ -456,17 +457,17 @@ describe("update_issue.cjs - footer support", () => {
});

const newContent = "New island";
// Should append because island with run ID 12345 doesn't exist
const expectedBody = `${existingBody}\n\n---\n\n<!-- gh-aw-island-start:12345 -->\n${newContent}\n\n> AI generated by [Test Workflow](https://github.com/testowner/testrepo/actions/runs/12345)\n<!-- gh-aw-island-end:12345 -->`;
// Should append because island with workflow test-workflow doesn't exist
const expectedBody = `${existingBody}\n\n---\n\n<!-- gh-aw-island-start:test-workflow -->\n${newContent}\n\n> AI generated by [Test Workflow](https://github.com/testowner/testrepo/actions/runs/12345)\n<!-- gh-aw-island-end:test-workflow -->`;

expect(expectedBody).toContain("Other island");
expect(expectedBody).toContain("New island");
expect(expectedBody).toContain("<!-- gh-aw-island-start:99999 -->");
expect(expectedBody).toContain("<!-- gh-aw-island-start:12345 -->");
expect(expectedBody).toContain("<!-- gh-aw-island-start:test-workflow -->");
});

it("should preserve content outside island when replacing", async () => {
const existingBody = "# Title\n\n<!-- gh-aw-island-start:12345 -->\nOld\n<!-- gh-aw-island-end:12345 -->\n\n## Footer";
const existingBody = "# Title\n\n<!-- gh-aw-island-start:test-workflow -->\nOld\n<!-- gh-aw-island-end:test-workflow -->\n\n## Footer";
mockGithub.rest.issues.get.mockResolvedValueOnce({
data: {
number: 100,
Expand All @@ -477,7 +478,7 @@ describe("update_issue.cjs - footer support", () => {
});

const newContent = "Updated content";
const expectedBody = `# Title\n\n<!-- gh-aw-island-start:12345 -->\n${newContent}\n\n> AI generated by [Test Workflow](https://github.com/testowner/testrepo/actions/runs/12345)\n<!-- gh-aw-island-end:12345 -->\n\n## Footer`;
const expectedBody = `# Title\n\n<!-- gh-aw-island-start:test-workflow -->\n${newContent}\n\n> AI generated by [Test Workflow](https://github.com/testowner/testrepo/actions/runs/12345)\n<!-- gh-aw-island-end:test-workflow -->\n\n## Footer`;

expect(expectedBody).toContain("# Title");
expect(expectedBody).toContain("## Footer");
Expand Down
40 changes: 20 additions & 20 deletions actions/setup/js/update_pr_description_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,31 @@ function buildAIFooter(workflowName, runUrl) {

/**
* Build the island start marker for replace-island mode
* @param {number} runId - Workflow run ID
* @param {string} workflowId - Workflow ID (stable identifier across runs)
* @returns {string} Island start marker
*/
function buildIslandStartMarker(runId) {
return `<!-- gh-aw-island-start:${runId} -->`;
function buildIslandStartMarker(workflowId) {
return `<!-- gh-aw-island-start:${workflowId} -->`;
Comment on lines +28 to +29
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.

workflowId is interpolated directly into an HTML comment marker. If it contains -->, newlines, or other unexpected characters, it can break marker structure and potentially enable marker spoofing/injection in the body. Recommend sanitizing/normalizing workflowId to a safe character set (and trimming) before building markers (e.g., restrict to [A-Za-z0-9._-] and replace others), and documenting the allowed format.

Copilot uses AI. Check for mistakes.
}

/**
* Build the island end marker for replace-island mode
* @param {number} runId - Workflow run ID
* @param {string} workflowId - Workflow ID (stable identifier across runs)
* @returns {string} Island end marker
*/
function buildIslandEndMarker(runId) {
return `<!-- gh-aw-island-end:${runId} -->`;
function buildIslandEndMarker(workflowId) {
return `<!-- gh-aw-island-end:${workflowId} -->`;
Comment on lines +37 to +38
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.

workflowId is interpolated directly into an HTML comment marker. If it contains -->, newlines, or other unexpected characters, it can break marker structure and potentially enable marker spoofing/injection in the body. Recommend sanitizing/normalizing workflowId to a safe character set (and trimming) before building markers (e.g., restrict to [A-Za-z0-9._-] and replace others), and documenting the allowed format.

Copilot uses AI. Check for mistakes.
}

/**
* Find and extract island content from body
* @param {string} body - The body content to search
* @param {number} runId - Workflow run ID
* @param {string} workflowId - Workflow ID (stable identifier across runs)
* @returns {{found: boolean, startIndex: number, endIndex: number}} Island location info
*/
function findIsland(body, runId) {
const startMarker = buildIslandStartMarker(runId);
const endMarker = buildIslandEndMarker(runId);
function findIsland(body, workflowId) {
const startMarker = buildIslandStartMarker(workflowId);
const endMarker = buildIslandEndMarker(workflowId);

const startIndex = body.indexOf(startMarker);
if (startIndex === -1) {
Expand All @@ -70,12 +70,12 @@ function findIsland(body, runId) {
* @param {string} params.operation - Operation type: "append", "prepend", "replace", or "replace-island"
* @param {string} params.workflowName - Name of the workflow
* @param {string} params.runUrl - URL of the workflow run
* @param {number} params.runId - Workflow run ID
* @param {string} params.workflowId - Workflow ID (stable identifier across runs)
* @param {boolean} [params.includeFooter=true] - Whether to include AI-generated footer (default: true)
* @returns {string} Updated body content
*/
function updateBody(params) {
const { currentBody, newContent, operation, workflowName, runUrl, runId, includeFooter = true } = params;
const { currentBody, newContent, operation, workflowName, runUrl, workflowId, includeFooter = true } = params;
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.

replace-island now fully depends on workflowId, but nothing here enforces that it’s non-empty. If callers pass \"\" (which they currently do as a default), all workflows without GH_AW_WORKFLOW_ID will share the same markers (<!-- gh-aw-island-start: -->), causing unrelated runs/workflows to overwrite each other’s islands. Consider failing fast for replace-island when workflowId is falsy (with an actionable message), or deriving a stable fallback identifier instead of allowing the empty string.

Copilot uses AI. Check for mistakes.
const aiFooter = includeFooter ? buildAIFooter(workflowName, runUrl) : "";

if (operation === "replace") {
Expand All @@ -85,24 +85,24 @@ function updateBody(params) {
}

if (operation === "replace-island") {
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.

replace-island now fully depends on workflowId, but nothing here enforces that it’s non-empty. If callers pass \"\" (which they currently do as a default), all workflows without GH_AW_WORKFLOW_ID will share the same markers (<!-- gh-aw-island-start: -->), causing unrelated runs/workflows to overwrite each other’s islands. Consider failing fast for replace-island when workflowId is falsy (with an actionable message), or deriving a stable fallback identifier instead of allowing the empty string.

Suggested change
if (operation === "replace-island") {
if (operation === "replace-island") {
if (!workflowId) {
throw new Error(
"updateBody: 'workflowId' must be a non-empty string for 'replace-island' operation. " +
"Set GH_AW_WORKFLOW_ID or pass a stable workflowId value."
);
}

Copilot uses AI. Check for mistakes.
// Try to find existing island for this run ID
const island = findIsland(currentBody, runId);
// Try to find existing island for this workflow ID
const island = findIsland(currentBody, workflowId);

if (island.found) {
// Replace the island content
core.info(`Operation: replace-island (updating existing island for run ${runId})`);
const startMarker = buildIslandStartMarker(runId);
const endMarker = buildIslandEndMarker(runId);
core.info(`Operation: replace-island (updating existing island for workflow ${workflowId})`);
const startMarker = buildIslandStartMarker(workflowId);
const endMarker = buildIslandEndMarker(workflowId);
const islandContent = `${startMarker}\n${newContent}${aiFooter}\n${endMarker}`;

const before = currentBody.substring(0, island.startIndex);
const after = currentBody.substring(island.endIndex);
return before + islandContent + after;
} else {
// Island not found, fall back to append mode
core.info(`Operation: replace-island (island not found for run ${runId}, falling back to append)`);
const startMarker = buildIslandStartMarker(runId);
const endMarker = buildIslandEndMarker(runId);
core.info(`Operation: replace-island (island not found for workflow ${workflowId}, falling back to append)`);
const startMarker = buildIslandStartMarker(workflowId);
const endMarker = buildIslandEndMarker(workflowId);
const islandContent = `${startMarker}\n${newContent}${aiFooter}\n${endMarker}`;
const appendSection = `\n\n---\n\n${islandContent}`;
return currentBody + appendSection;
Expand Down
Loading
Loading