mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-01-22 14:24:13 +08:00
fix: use original title from webhook payload instead of fetched title (#793)
* fix: use original title from webhook payload instead of fetched title - Add extractOriginalTitle() helper to extract title from webhook payload - Add originalTitle parameter to fetchGitHubData() - Update tag mode to pass original title from webhook context - Add tests for extractOriginalTitle and originalTitle parameter This ensures the title used in prompts is the one that existed when the trigger event occurred, rather than a potentially modified title fetched later via GraphQL. * fix: add title sanitization and explicit TOCTOU test - Apply sanitizeContent() to titles in formatContext() for defense-in-depth - Add explicit test documenting TOCTOU prevention for title handling
This commit is contained in:
@@ -3,6 +3,8 @@ import type { Octokits } from "../api/client";
|
||||
import { ISSUE_QUERY, PR_QUERY, USER_QUERY } from "../api/queries/github";
|
||||
import {
|
||||
isIssueCommentEvent,
|
||||
isIssuesEvent,
|
||||
isPullRequestEvent,
|
||||
isPullRequestReviewEvent,
|
||||
isPullRequestReviewCommentEvent,
|
||||
type ParsedGitHubContext,
|
||||
@@ -40,6 +42,31 @@ export function extractTriggerTimestamp(
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the original title from the GitHub webhook payload.
|
||||
* This is the title as it existed when the trigger event occurred.
|
||||
*
|
||||
* @param context - Parsed GitHub context from webhook
|
||||
* @returns The original title string or undefined if not available
|
||||
*/
|
||||
export function extractOriginalTitle(
|
||||
context: ParsedGitHubContext,
|
||||
): string | undefined {
|
||||
if (isIssueCommentEvent(context)) {
|
||||
return context.payload.issue?.title;
|
||||
} else if (isPullRequestEvent(context)) {
|
||||
return context.payload.pull_request?.title;
|
||||
} else if (isPullRequestReviewEvent(context)) {
|
||||
return context.payload.pull_request?.title;
|
||||
} else if (isPullRequestReviewCommentEvent(context)) {
|
||||
return context.payload.pull_request?.title;
|
||||
} else if (isIssuesEvent(context)) {
|
||||
return context.payload.issue?.title;
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Filters comments to only include those that existed in their final state before the trigger time.
|
||||
* This prevents malicious actors from editing comments after the trigger to inject harmful content.
|
||||
@@ -146,6 +173,7 @@ type FetchDataParams = {
|
||||
isPR: boolean;
|
||||
triggerUsername?: string;
|
||||
triggerTime?: string;
|
||||
originalTitle?: string;
|
||||
};
|
||||
|
||||
export type GitHubFileWithSHA = GitHubFile & {
|
||||
@@ -169,6 +197,7 @@ export async function fetchGitHubData({
|
||||
isPR,
|
||||
triggerUsername,
|
||||
triggerTime,
|
||||
originalTitle,
|
||||
}: FetchDataParams): Promise<FetchDataResult> {
|
||||
const [owner, repo] = repository.split("/");
|
||||
if (!owner || !repo) {
|
||||
@@ -354,6 +383,11 @@ export async function fetchGitHubData({
|
||||
triggerDisplayName = await fetchUserDisplayName(octokits, triggerUsername);
|
||||
}
|
||||
|
||||
// Use the original title from the webhook payload if provided
|
||||
if (originalTitle !== undefined) {
|
||||
contextData.title = originalTitle;
|
||||
}
|
||||
|
||||
return {
|
||||
contextData,
|
||||
comments,
|
||||
|
||||
@@ -14,7 +14,8 @@ export function formatContext(
|
||||
): string {
|
||||
if (isPR) {
|
||||
const prData = contextData as GitHubPullRequest;
|
||||
return `PR Title: ${prData.title}
|
||||
const sanitizedTitle = sanitizeContent(prData.title);
|
||||
return `PR Title: ${sanitizedTitle}
|
||||
PR Author: ${prData.author.login}
|
||||
PR Branch: ${prData.headRefName} -> ${prData.baseRefName}
|
||||
PR State: ${prData.state}
|
||||
@@ -24,7 +25,8 @@ Total Commits: ${prData.commits.totalCount}
|
||||
Changed Files: ${prData.files.nodes.length} files`;
|
||||
} else {
|
||||
const issueData = contextData as GitHubIssue;
|
||||
return `Issue Title: ${issueData.title}
|
||||
const sanitizedTitle = sanitizeContent(issueData.title);
|
||||
return `Issue Title: ${sanitizedTitle}
|
||||
Issue Author: ${issueData.author.login}
|
||||
Issue State: ${issueData.state}`;
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ import { prepareMcpConfig } from "../../mcp/install-mcp-server";
|
||||
import {
|
||||
fetchGitHubData,
|
||||
extractTriggerTimestamp,
|
||||
extractOriginalTitle,
|
||||
} from "../../github/data/fetcher";
|
||||
import { createPrompt, generateDefaultPrompt } from "../../create-prompt";
|
||||
import { isEntityContext } from "../../github/context";
|
||||
@@ -78,6 +79,7 @@ export const tagMode: Mode = {
|
||||
const commentId = commentData.id;
|
||||
|
||||
const triggerTime = extractTriggerTimestamp(context);
|
||||
const originalTitle = extractOriginalTitle(context);
|
||||
|
||||
const githubData = await fetchGitHubData({
|
||||
octokits: octokit,
|
||||
@@ -86,6 +88,7 @@ export const tagMode: Mode = {
|
||||
isPR: context.isPR,
|
||||
triggerUsername: context.actor,
|
||||
triggerTime,
|
||||
originalTitle,
|
||||
});
|
||||
|
||||
// Setup branch
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it, jest } from "bun:test";
|
||||
import {
|
||||
extractTriggerTimestamp,
|
||||
extractOriginalTitle,
|
||||
fetchGitHubData,
|
||||
filterCommentsToTriggerTime,
|
||||
filterReviewsToTriggerTime,
|
||||
@@ -9,6 +10,7 @@ import {
|
||||
import {
|
||||
createMockContext,
|
||||
mockIssueCommentContext,
|
||||
mockPullRequestCommentContext,
|
||||
mockPullRequestReviewContext,
|
||||
mockPullRequestReviewCommentContext,
|
||||
mockPullRequestOpenedContext,
|
||||
@@ -63,6 +65,47 @@ describe("extractTriggerTimestamp", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("extractOriginalTitle", () => {
|
||||
it("should extract title from IssueCommentEvent on PR", () => {
|
||||
const title = extractOriginalTitle(mockPullRequestCommentContext);
|
||||
expect(title).toBe("Fix: Memory leak in user service");
|
||||
});
|
||||
|
||||
it("should extract title from PullRequestReviewEvent", () => {
|
||||
const title = extractOriginalTitle(mockPullRequestReviewContext);
|
||||
expect(title).toBe("Refactor: Improve error handling in API layer");
|
||||
});
|
||||
|
||||
it("should extract title from PullRequestReviewCommentEvent", () => {
|
||||
const title = extractOriginalTitle(mockPullRequestReviewCommentContext);
|
||||
expect(title).toBe("Performance: Optimize search algorithm");
|
||||
});
|
||||
|
||||
it("should extract title from pull_request event", () => {
|
||||
const title = extractOriginalTitle(mockPullRequestOpenedContext);
|
||||
expect(title).toBe("Feature: Add user authentication");
|
||||
});
|
||||
|
||||
it("should extract title from issues event", () => {
|
||||
const title = extractOriginalTitle(mockIssueOpenedContext);
|
||||
expect(title).toBe("Bug: Application crashes on startup");
|
||||
});
|
||||
|
||||
it("should return undefined for event without title", () => {
|
||||
const context = createMockContext({
|
||||
eventName: "issue_comment",
|
||||
payload: {
|
||||
comment: {
|
||||
id: 123,
|
||||
body: "test",
|
||||
},
|
||||
} as any,
|
||||
});
|
||||
const title = extractOriginalTitle(context);
|
||||
expect(title).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("filterCommentsToTriggerTime", () => {
|
||||
const createMockComment = (
|
||||
createdAt: string,
|
||||
@@ -945,4 +988,115 @@ describe("fetchGitHubData integration with time filtering", () => {
|
||||
);
|
||||
expect(hasPrBodyInMap).toBe(false);
|
||||
});
|
||||
|
||||
it("should use originalTitle when provided instead of fetched title", async () => {
|
||||
const mockOctokits = {
|
||||
graphql: jest.fn().mockResolvedValue({
|
||||
repository: {
|
||||
pullRequest: {
|
||||
number: 123,
|
||||
title: "Fetched Title From GraphQL",
|
||||
body: "PR body",
|
||||
author: { login: "author" },
|
||||
createdAt: "2024-01-15T10:00:00Z",
|
||||
additions: 10,
|
||||
deletions: 5,
|
||||
state: "OPEN",
|
||||
commits: { totalCount: 1, nodes: [] },
|
||||
files: { nodes: [] },
|
||||
comments: { nodes: [] },
|
||||
reviews: { nodes: [] },
|
||||
},
|
||||
},
|
||||
user: { login: "trigger-user" },
|
||||
}),
|
||||
rest: jest.fn() as any,
|
||||
};
|
||||
|
||||
const result = await fetchGitHubData({
|
||||
octokits: mockOctokits as any,
|
||||
repository: "test-owner/test-repo",
|
||||
prNumber: "123",
|
||||
isPR: true,
|
||||
triggerUsername: "trigger-user",
|
||||
originalTitle: "Original Title From Webhook",
|
||||
});
|
||||
|
||||
expect(result.contextData.title).toBe("Original Title From Webhook");
|
||||
});
|
||||
|
||||
it("should use fetched title when originalTitle is not provided", async () => {
|
||||
const mockOctokits = {
|
||||
graphql: jest.fn().mockResolvedValue({
|
||||
repository: {
|
||||
pullRequest: {
|
||||
number: 123,
|
||||
title: "Fetched Title From GraphQL",
|
||||
body: "PR body",
|
||||
author: { login: "author" },
|
||||
createdAt: "2024-01-15T10:00:00Z",
|
||||
additions: 10,
|
||||
deletions: 5,
|
||||
state: "OPEN",
|
||||
commits: { totalCount: 1, nodes: [] },
|
||||
files: { nodes: [] },
|
||||
comments: { nodes: [] },
|
||||
reviews: { nodes: [] },
|
||||
},
|
||||
},
|
||||
user: { login: "trigger-user" },
|
||||
}),
|
||||
rest: jest.fn() as any,
|
||||
};
|
||||
|
||||
const result = await fetchGitHubData({
|
||||
octokits: mockOctokits as any,
|
||||
repository: "test-owner/test-repo",
|
||||
prNumber: "123",
|
||||
isPR: true,
|
||||
triggerUsername: "trigger-user",
|
||||
});
|
||||
|
||||
expect(result.contextData.title).toBe("Fetched Title From GraphQL");
|
||||
});
|
||||
|
||||
it("should use original title from webhook even if title was edited after trigger", async () => {
|
||||
const mockOctokits = {
|
||||
graphql: jest.fn().mockResolvedValue({
|
||||
repository: {
|
||||
pullRequest: {
|
||||
number: 123,
|
||||
title: "Edited Title (from GraphQL)",
|
||||
body: "PR body",
|
||||
author: { login: "author" },
|
||||
createdAt: "2024-01-15T10:00:00Z",
|
||||
lastEditedAt: "2024-01-15T12:30:00Z", // Edited after trigger
|
||||
additions: 10,
|
||||
deletions: 5,
|
||||
state: "OPEN",
|
||||
commits: { totalCount: 1, nodes: [] },
|
||||
files: { nodes: [] },
|
||||
comments: { nodes: [] },
|
||||
reviews: { nodes: [] },
|
||||
},
|
||||
},
|
||||
user: { login: "trigger-user" },
|
||||
}),
|
||||
rest: jest.fn() as any,
|
||||
};
|
||||
|
||||
const result = await fetchGitHubData({
|
||||
octokits: mockOctokits as any,
|
||||
repository: "test-owner/test-repo",
|
||||
prNumber: "123",
|
||||
isPR: true,
|
||||
triggerUsername: "trigger-user",
|
||||
triggerTime: "2024-01-15T12:00:00Z",
|
||||
originalTitle: "Original Title (from webhook at trigger time)",
|
||||
});
|
||||
|
||||
expect(result.contextData.title).toBe(
|
||||
"Original Title (from webhook at trigger time)",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user