Skip to content
Closed
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
2 changes: 1 addition & 1 deletion .github/workflows/smoke-gemini.lock.yml

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

9 changes: 6 additions & 3 deletions actions/setup/js/collect_ndjson_output.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,7 @@ describe("collect_ndjson_output.cjs", () => {
parsedOutput = JSON.parse(outputCall[1]);
expect(parsedOutput.items[0].body).toBe("Hey `@username` and `@org/team`, check this out! But preserve email@domain.com");
}),
it("should neutralize bot trigger phrases", async () => {
it("should not escape issue-closing keywords in output", async () => {
const testFile = "/tmp/gh-aw/test-ndjson-output.txt",
ndjsonContent = '{"type": "create_issue", "title": "Bot Trigger Test", "body": "This fixes #123 and closes #456, also resolves #789"}';
(fs.writeFileSync(testFile, ndjsonContent), (process.env.GH_AW_SAFE_OUTPUTS = testFile));
Expand All @@ -1228,7 +1228,7 @@ describe("collect_ndjson_output.cjs", () => {
(fs.mkdirSync("/opt/gh-aw/safeoutputs", { recursive: !0 }), fs.writeFileSync(configPath, __config), await eval(`(async () => { ${collectScript}; await main(); })()`));
const outputCall = mockCore.setOutput.mock.calls.find(call => "output" === call[0]),
parsedOutput = JSON.parse(outputCall[1]);
expect(parsedOutput.items[0].body).toBe("This `fixes #123` and `closes #456`, also `resolves #789`");
expect(parsedOutput.items[0].body).toBe("This fixes #123 and closes #456, also resolves #789");
}),
it("should remove ANSI escape sequences", async () => {
const testFile = "/tmp/gh-aw/test-ndjson-output.txt",
Expand Down Expand Up @@ -1488,7 +1488,10 @@ describe("collect_ndjson_output.cjs", () => {
outputCall = setOutputCalls.find(call => "output" === call[0]);
expect(outputCall).toBeDefined();
const parsedOutput = JSON.parse(outputCall[1]);
(expect(parsedOutput.items).toHaveLength(1), expect(parsedOutput.items[0].message).toContain("`@mention`"), expect(parsedOutput.items[0].message).toContain("`fixes #123`"));
(expect(parsedOutput.items).toHaveLength(1),
expect(parsedOutput.items[0].message).toContain("`@mention`"),
expect(parsedOutput.items[0].message).not.toContain("`fixes #123`"),
expect(parsedOutput.items[0].message).toContain("fixes #123"));
}),
it("should handle multiple noop messages", async () => {
const testFile = "/tmp/gh-aw/test-ndjson-output.txt",
Expand Down
6 changes: 3 additions & 3 deletions actions/setup/js/compute_text.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ const mockCore = {
const result = sanitizeIncomingTextFunction("Hello @user and @org/team");
(expect(result).toContain("`@user`"), expect(result).toContain("`@org/team`"));
}),
it("should neutralize bot trigger phrases", () => {
it("should not escape issue-closing keywords in incoming text", () => {
const result = sanitizeIncomingTextFunction("This fixes #123 and closes #456");
(expect(result).toContain("`fixes #123`"), expect(result).toContain("`closes #456`"));
(expect(result).toContain("fixes #123"), expect(result).toContain("closes #456"), expect(result).not.toContain("`fixes #123`"), expect(result).not.toContain("`closes #456`"));
}),
it("should remove control characters", () => {
const result = sanitizeIncomingTextFunction("Hello\0\bworld");
Expand Down Expand Up @@ -203,7 +203,7 @@ const mockCore = {
it("should sanitize extracted text before output", async () => {
((mockContext.eventName = "issues"), (mockContext.payload = { issue: { title: "Test @user fixes #123", body: "Visit https://evil.com" } }), await testMain());
const outputCall = mockCore.setOutput.mock.calls[0];
(expect(outputCall[1]).toContain("`@user`"), expect(outputCall[1]).toContain("`fixes #123`"), expect(outputCall[1]).toContain("/redacted"));
(expect(outputCall[1]).toContain("`@user`"), expect(outputCall[1]).not.toContain("`fixes #123`"), expect(outputCall[1]).toContain("fixes #123"), expect(outputCall[1]).toContain("/redacted"));
}),
it("should handle missing title and body gracefully", async () => {
((mockContext.eventName = "issues"),
Expand Down
4 changes: 0 additions & 4 deletions actions/setup/js/sanitize_content.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const {
neutralizeGitHubReferences,
removeXmlComments,
convertXmlTags,
neutralizeBotTriggers,
applyTruncation,
hardenUnicodeText,
} = require("./sanitize_content_core.cjs");
Expand Down Expand Up @@ -102,9 +101,6 @@ function sanitizeContent(content, maxLengthOrOptions) {
// Neutralize GitHub references if restrictions are configured
sanitized = neutralizeGitHubReferences(sanitized, allowedGitHubRefs);

// Neutralize bot triggers
sanitized = neutralizeBotTriggers(sanitized);

// Balance markdown code regions to fix improperly nested fences
// This repairs markdown where AI models generate nested code blocks at the same indentation
sanitized = balanceCodeRegions(sanitized);
Expand Down
25 changes: 13 additions & 12 deletions actions/setup/js/sanitize_content.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -745,33 +745,33 @@ describe("sanitize_content.cjs", () => {
});
});

describe("bot trigger neutralization", () => {
it("should neutralize 'fixes #123' patterns", () => {
describe("bot trigger passthrough", () => {
it("should not escape 'fixes #123' patterns", () => {
const result = sanitizeContent("This fixes #123");
expect(result).toBe("This `fixes #123`");
expect(result).toBe("This fixes #123");
});

it("should neutralize 'closes #456' patterns", () => {
it("should not escape 'closes #456' patterns", () => {
const result = sanitizeContent("PR closes #456");
expect(result).toBe("PR `closes #456`");
expect(result).toBe("PR closes #456");
});

it("should neutralize 'resolves #789' patterns", () => {
it("should not escape 'resolves #789' patterns", () => {
const result = sanitizeContent("This resolves #789");
expect(result).toBe("This `resolves #789`");
expect(result).toBe("This resolves #789");
});

it("should handle various bot trigger verbs", () => {
it("should not escape various issue-closing verbs", () => {
const triggers = ["fix", "fixes", "close", "closes", "resolve", "resolves"];
triggers.forEach(verb => {
const result = sanitizeContent(`This ${verb} #123`);
expect(result).toBe(`This \`${verb} #123\``);
expect(result).toBe(`This ${verb} #123`);
});
});

it("should neutralize alphanumeric issue references", () => {
it("should not escape alphanumeric issue references", () => {
const result = sanitizeContent("fixes #abc123def");
expect(result).toBe("`fixes #abc123def`");
expect(result).toBe("fixes #abc123def");
});
});

Expand Down Expand Up @@ -1063,7 +1063,8 @@ describe("sanitize_content.cjs", () => {
expect(result).toContain("https://github.com");
expect(result).not.toContain("<script>");
expect(result).toContain("(script)");
expect(result).toContain("`fixes #123`");
expect(result).not.toContain("`fixes #123`");
expect(result).toContain("fixes #123");
expect(result).not.toContain("\x1b[31m");
expect(result).toContain("Red text");
});
Expand Down
3 changes: 0 additions & 3 deletions actions/setup/js/sanitize_content_core.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -767,9 +767,6 @@ function sanitizeContentCore(content, maxLength) {
// Neutralize GitHub references if restrictions are configured
sanitized = neutralizeGitHubReferences(sanitized, allowedGitHubRefs);

// Neutralize common bot trigger phrases
sanitized = neutralizeBotTriggers(sanitized);

// Neutralize template syntax delimiters (defense-in-depth)
// This prevents potential issues if content is processed by downstream template engines
sanitized = neutralizeTemplateDelimiters(sanitized);
Expand Down
25 changes: 19 additions & 6 deletions actions/setup/js/sanitize_output.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,14 @@ const mockCore = {
const result = sanitizeContentFunction("Check `@user` in code and @realuser outside");
(expect(result).toContain("`@user`"), expect(result).toContain("`@realuser`"));
}),
it("should neutralize bot trigger phrases", () => {
it("should not escape issue-closing keywords", () => {
const result = sanitizeContentFunction("This fixes #123 and closes #456. Also resolves #789");
(expect(result).toContain("`fixes #123`"), expect(result).toContain("`closes #456`"), expect(result).toContain("`resolves #789`"));
(expect(result).toContain("fixes #123"),
expect(result).toContain("closes #456"),
expect(result).toContain("resolves #789"),
expect(result).not.toContain("`fixes #123`"),
expect(result).not.toContain("`closes #456`"),
expect(result).not.toContain("`resolves #789`"));
}),
it("should remove control characters except newlines and tabs", () => {
const result = sanitizeContentFunction("Hello\0world\f\nNext line\tbad");
Expand Down Expand Up @@ -169,7 +174,8 @@ const mockCore = {
"\n# Issue Report by @user\n\nThis fixes #123 and has links:\n- HTTP: http://bad.com (should be blocked)\n- HTTPS: https://github.com/repo (should be preserved)\n- JavaScript: javascript:alert('xss') (should be blocked)\n\n<script>alert(\"xss\")<\/script>\n\nSpecial chars: \0 & \"quotes\" 'apostrophes'\n ".trim(),
result = sanitizeContentFunction(input);
(expect(result).toContain("`@user`"),
expect(result).toContain("`fixes #123`"),
expect(result).not.toContain("`fixes #123`"),
expect(result).toContain("fixes #123"),
expect(result).toContain("(redacted)"),
expect(result).toContain("https://github.com/repo"),
expect(result).not.toContain("http://bad.com"),
Expand Down Expand Up @@ -207,9 +213,15 @@ const mockCore = {
const result = sanitizeContentFunction("Code: `@user.method()` and normal @user mention");
(expect(result).toContain("`@user.method()`"), expect(result).toContain("`@user`"));
}),
it("should handle various bot trigger phrase formats", () => {
it("should not escape various issue-closing phrase formats", () => {
const result = sanitizeContentFunction("Fix #123, close #abc, FIXES #XYZ, resolves #456, fixes #789");
(expect(result).toContain("`Fix #123`"), expect(result).toContain("`close #abc`"), expect(result).toContain("`FIXES #XYZ`"), expect(result).toContain("`resolves #456`"), expect(result).toContain("`fixes #789`"));
(expect(result).toContain("Fix #123"),
expect(result).toContain("close #abc"),
expect(result).toContain("FIXES #XYZ"),
expect(result).toContain("resolves #456"),
expect(result).toContain("fixes #789"),
expect(result).not.toContain("`Fix #123`"),
expect(result).not.toContain("`close #abc`"));
}),
it("should handle edge cases in protocol filtering", () => {
const result = sanitizeContentFunction(
Expand Down Expand Up @@ -355,7 +367,8 @@ const mockCore = {
expect(outputCall).toBeDefined();
const sanitizedOutput = outputCall[1];
(expect(sanitizedOutput).toContain("`@user`"),
expect(sanitizedOutput).toContain("`fixes #123`"),
expect(sanitizedOutput).not.toContain("`fixes #123`"),
expect(sanitizedOutput).toContain("fixes #123"),
expect(sanitizedOutput).toContain("/redacted"),
expect(sanitizedOutput).toContain("https://github.com/repo"),
fs.unlinkSync(testFile));
Expand Down