diff --git a/.changeset/provider-comment-untrusted-framing.md b/.changeset/provider-comment-untrusted-framing.md new file mode 100644 index 000000000..8b2eb4111 --- /dev/null +++ b/.changeset/provider-comment-untrusted-framing.md @@ -0,0 +1,5 @@ +--- +"@roomote/web": patch +--- + +Apply the untrusted-content prompt framing to GitLab, Bitbucket, Azure DevOps, and Gitea comment follow-up messages, wrapping the triggering comment in a mention-request block and appending the shared injection-resistance policy diff --git a/apps/api/src/handlers/ado/__tests__/handleComment.test.ts b/apps/api/src/handlers/ado/__tests__/handleComment.test.ts index 1bf09f642..89c2ea2b7 100644 --- a/apps/api/src/handlers/ado/__tests__/handleComment.test.ts +++ b/apps/api/src/handlers/ado/__tests__/handleComment.test.ts @@ -20,9 +20,16 @@ const { mockSteerMessageToTask: vi.fn(), })); +// Prompt-framing fakes use distinctive markers so tests can assert the +// handler routes each piece of text through the right builder; the real +// escaping/wrapping behavior is unit-tested in @roomote/cloud-agents. vi.mock('@roomote/cloud-agents/server', () => ({ enqueueTask: mockEnqueueTask, getTaskUrl: mockGetTaskUrl, + buildMentionRequestBlock: (text: string) => + `${text}`, + buildUntrustedContentPolicy: () => '', + escapeTaskContextText: (value: string) => value, })); vi.mock('@roomote/ado', () => ({ @@ -354,10 +361,17 @@ describe('handleAdoComment', () => { expect.objectContaining({ taskId: 'task-existing', userId: 'user-1', - message: expect.stringContaining('mentioned Roomote in a comment'), + message: expect.stringContaining( + '@roomote please review this', + ), senderMode: 'github_pr_follow_up', }), ); + expect(mockSteerMessageToTask).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining(''), + }), + ); expect(mockEnqueueTask).not.toHaveBeenCalled(); expect(mockCreateAdoPullRequestComment).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/apps/api/src/handlers/ado/handleComment.ts b/apps/api/src/handlers/ado/handleComment.ts index 860fe0bc3..d3377177b 100644 --- a/apps/api/src/handlers/ado/handleComment.ts +++ b/apps/api/src/handlers/ado/handleComment.ts @@ -1,4 +1,10 @@ -import { enqueueTask, getTaskUrl } from '@roomote/cloud-agents/server'; +import { + buildMentionRequestBlock, + buildUntrustedContentPolicy, + enqueueTask, + escapeTaskContextText, + getTaskUrl, +} from '@roomote/cloud-agents/server'; import { findActiveGitHubPrReviewTask, findReusableGitHubPrFollowUpOwner, @@ -207,14 +213,6 @@ function buildTaskStartFailedComment(): string { return 'I saw the mention, but I could not start a task for this pull request right now. Please try again in a moment.'; } -function formatQuotedText(text: string): string { - return text - .trim() - .split('\n') - .map((line) => `> ${line}`) - .join('\n'); -} - function buildExistingTaskFollowUpMessage({ repoFullName, pullRequest, @@ -227,8 +225,7 @@ function buildExistingTaskFollowUpMessage({ commentBody: string; }): string { const lines = [ - `${commenter} mentioned Roomote in a comment on Azure DevOps pull request #${pullRequest.pullRequestId} (${pullRequest.title}) in ${repoFullName}:`, - formatQuotedText(commentBody), + `${commenter} mentioned Roomote in a comment on Azure DevOps pull request #${pullRequest.pullRequestId} (${escapeTaskContextText(pullRequest.title)}) in ${repoFullName}.`, '', 'Please act on this comment as a follow-up to your existing work on this pull request.', ]; @@ -238,6 +235,14 @@ function buildExistingTaskFollowUpMessage({ lines.push(`The pull request source branch is \`${branchName}\`.`); } + lines.push( + '', + 'Mention comment (the request to act on):', + buildMentionRequestBlock(commentBody), + '', + buildUntrustedContentPolicy(), + ); + return lines.join('\n'); } diff --git a/apps/api/src/handlers/bitbucket/__tests__/handleComment.test.ts b/apps/api/src/handlers/bitbucket/__tests__/handleComment.test.ts index 5ff72662b..f42f951ef 100644 --- a/apps/api/src/handlers/bitbucket/__tests__/handleComment.test.ts +++ b/apps/api/src/handlers/bitbucket/__tests__/handleComment.test.ts @@ -4,15 +4,30 @@ const { mockCreateBitbucketPullRequestComment, mockGetBitbucketAutomationTargets, mockEnqueueTask, + mockFindActiveGitHubPrReviewTask, + mockFindReusableGitHubPrFollowUpOwner, + mockSendMessageToTask, + mockSteerMessageToTask, } = vi.hoisted(() => ({ mockCreateBitbucketPullRequestComment: vi.fn(), mockGetBitbucketAutomationTargets: vi.fn(), mockEnqueueTask: vi.fn(), + mockFindActiveGitHubPrReviewTask: vi.fn(), + mockFindReusableGitHubPrFollowUpOwner: vi.fn(), + mockSendMessageToTask: vi.fn(), + mockSteerMessageToTask: vi.fn(), })); +// Prompt-framing fakes use distinctive markers so tests can assert the +// handler routes each piece of text through the right builder; the real +// escaping/wrapping behavior is unit-tested in @roomote/cloud-agents. vi.mock('@roomote/cloud-agents/server', () => ({ enqueueTask: mockEnqueueTask, getTaskUrl: vi.fn(), + buildMentionRequestBlock: (text: string) => + `${text}`, + buildUntrustedContentPolicy: () => '', + escapeTaskContextText: (value: string) => value, })); vi.mock('@roomote/bitbucket', () => ({ @@ -24,11 +39,16 @@ vi.mock('@roomote/db/server', async (importOriginal) => { return { ...actual, - findActiveGitHubPrReviewTask: vi.fn(), - findReusableGitHubPrFollowUpOwner: vi.fn(), + findActiveGitHubPrReviewTask: mockFindActiveGitHubPrReviewTask, + findReusableGitHubPrFollowUpOwner: mockFindReusableGitHubPrFollowUpOwner, }; }); +vi.mock('../../tasks/sendMessageToTask', () => ({ + sendMessageToTask: mockSendMessageToTask, + steerMessageToTask: mockSteerMessageToTask, +})); + vi.mock('../getBitbucketAutomationTargets', async () => { const actual = await vi.importActual< typeof import('../getBitbucketAutomationTargets') @@ -40,6 +60,8 @@ vi.mock('../getBitbucketAutomationTargets', async () => { }; }); +import { RunStatus } from '@roomote/types'; + import { handleBitbucketComment } from '../handleComment'; import type { BitbucketPullRequestCommentWebhook } from '../types'; @@ -74,12 +96,18 @@ describe('handleBitbucketComment', () => { mockCreateBitbucketPullRequestComment.mockReset(); mockGetBitbucketAutomationTargets.mockReset(); mockEnqueueTask.mockReset(); + mockFindActiveGitHubPrReviewTask.mockReset(); + mockFindReusableGitHubPrFollowUpOwner.mockReset(); + mockSendMessageToTask.mockReset(); + mockSteerMessageToTask.mockReset(); mockGetBitbucketAutomationTargets.mockResolvedValue({ status: 'error', code: 'account_link_required', message: 'Bitbucket user alice is not linked', }); + mockFindActiveGitHubPrReviewTask.mockResolvedValue(null); + mockFindReusableGitHubPrFollowUpOwner.mockResolvedValue(null); }); it('propagates a failed account-link response so the webhook is recorded as failed', async () => { @@ -159,6 +187,55 @@ describe('handleBitbucketComment', () => { expect(mockCreateBitbucketPullRequestComment).toHaveBeenCalledOnce(); }); + it('routes mentions into a reusable active task with untrusted-content framing', async () => { + mockGetBitbucketAutomationTargets.mockResolvedValue({ + status: 'ok', + targets: [ + { + id: 'bitbucket:pr_review:repo-1', + workflow: 'pr_review', + settings: null, + repo: { + id: 'repo-1', + host: 'bitbucket.org', + }, + repositoryIds: ['repo-1'], + userId: 'user-1', + }, + ], + }); + mockFindReusableGitHubPrFollowUpOwner.mockResolvedValue({ + taskId: 'task-existing', + status: RunStatus.Running, + taskPhase: 'running', + }); + mockSteerMessageToTask.mockResolvedValue({ success: true, result: {} }); + mockCreateBitbucketPullRequestComment.mockResolvedValue({ id: 7 }); + + const result = await handleBitbucketComment( + makeCommentPayload(), + 'pullrequest:comment_created', + ); + + expect(result).toEqual({ status: 'ok', message: 'active_pr_owner_routed' }); + expect(mockSteerMessageToTask).toHaveBeenCalledWith( + expect.objectContaining({ + taskId: 'task-existing', + userId: 'user-1', + message: expect.stringContaining( + '@roomote review this please', + ), + senderMode: 'github_pr_follow_up', + }), + ); + expect(mockSteerMessageToTask).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining(''), + }), + ); + expect(mockEnqueueTask).not.toHaveBeenCalled(); + }); + it('logs enqueue failures and posts a queue-specific response', async () => { mockGetBitbucketAutomationTargets.mockResolvedValue({ status: 'ok', diff --git a/apps/api/src/handlers/bitbucket/handleComment.ts b/apps/api/src/handlers/bitbucket/handleComment.ts index b69a45c12..e024ab176 100644 --- a/apps/api/src/handlers/bitbucket/handleComment.ts +++ b/apps/api/src/handlers/bitbucket/handleComment.ts @@ -1,4 +1,10 @@ -import { enqueueTask, getTaskUrl } from '@roomote/cloud-agents/server'; +import { + buildMentionRequestBlock, + buildUntrustedContentPolicy, + enqueueTask, + escapeTaskContextText, + getTaskUrl, +} from '@roomote/cloud-agents/server'; import { findActiveGitHubPrReviewTask, findReusableGitHubPrFollowUpOwner, @@ -141,14 +147,6 @@ function buildTaskStartFailedComment(): string { return 'I saw the mention, but Roomote could not queue a review task for this pull request. Please try again in a moment. If it keeps failing, an administrator should check the deployment logs.'; } -function formatQuotedText(text: string): string { - return text - .trim() - .split('\n') - .map((line) => `> ${line}`) - .join('\n'); -} - function buildExistingTaskFollowUpMessage({ repoFullName, pullRequestNumber, @@ -165,8 +163,7 @@ function buildExistingTaskFollowUpMessage({ commentBody: string; }): string { const lines = [ - `${commenter} mentioned Roomote in a comment on Bitbucket pull request #${pullRequestNumber} (${pullRequestTitle}) in ${repoFullName}:`, - formatQuotedText(commentBody), + `${commenter} mentioned Roomote in a comment on Bitbucket pull request #${pullRequestNumber} (${escapeTaskContextText(pullRequestTitle)}) in ${repoFullName}.`, '', 'Please act on this comment as a follow-up to your existing work on this pull request.', ]; @@ -175,6 +172,14 @@ function buildExistingTaskFollowUpMessage({ lines.push(`The pull request source branch is \`${headRef}\`.`); } + lines.push( + '', + 'Mention comment (the request to act on):', + buildMentionRequestBlock(commentBody), + '', + buildUntrustedContentPolicy(), + ); + return lines.join('\n'); } diff --git a/apps/api/src/handlers/gitea/__tests__/handleComment.test.ts b/apps/api/src/handlers/gitea/__tests__/handleComment.test.ts index fc1a480f0..2c4930da7 100644 --- a/apps/api/src/handlers/gitea/__tests__/handleComment.test.ts +++ b/apps/api/src/handlers/gitea/__tests__/handleComment.test.ts @@ -20,9 +20,16 @@ const { mockSteerMessageToTask: vi.fn(), })); +// Prompt-framing fakes use distinctive markers so tests can assert the +// handler routes each piece of text through the right builder; the real +// escaping/wrapping behavior is unit-tested in @roomote/cloud-agents. vi.mock('@roomote/cloud-agents/server', () => ({ enqueueTask: mockEnqueueTask, getTaskUrl: mockGetTaskUrl, + buildMentionRequestBlock: (text: string) => + `${text}`, + buildUntrustedContentPolicy: () => '', + escapeTaskContextText: (value: string) => value, })); vi.mock('@roomote/gitea', () => ({ @@ -279,7 +286,14 @@ describe('handleGiteaComment', () => { expect.objectContaining({ taskId: 'task-existing', userId: 'user-1', - message: expect.stringContaining('mentioned Roomote in a comment'), + message: expect.stringContaining( + '@roomote please review this', + ), + }), + ); + expect(mockSteerMessageToTask).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining(''), }), ); expect(mockEnqueueTask).not.toHaveBeenCalled(); diff --git a/apps/api/src/handlers/gitea/handleComment.ts b/apps/api/src/handlers/gitea/handleComment.ts index 4c546434e..1313c9ece 100644 --- a/apps/api/src/handlers/gitea/handleComment.ts +++ b/apps/api/src/handlers/gitea/handleComment.ts @@ -1,4 +1,10 @@ -import { enqueueTask, getTaskUrl } from '@roomote/cloud-agents/server'; +import { + buildMentionRequestBlock, + buildUntrustedContentPolicy, + enqueueTask, + escapeTaskContextText, + getTaskUrl, +} from '@roomote/cloud-agents/server'; import { findActiveGitHubPrReviewTask, findReusableGitHubPrFollowUpOwner, @@ -184,14 +190,6 @@ function buildTaskStartFailedComment(): string { return 'I saw the mention, but I could not start a task for this pull request right now. Please try again in a moment.'; } -function formatQuotedText(text: string): string { - return text - .trim() - .split('\n') - .map((line) => `> ${line}`) - .join('\n'); -} - function buildExistingTaskFollowUpMessage({ repoFullName, pullRequest, @@ -204,8 +202,7 @@ function buildExistingTaskFollowUpMessage({ commentBody: string; }): string { const lines = [ - `${commenter} mentioned Roomote in a comment on Gitea pull request #${pullRequest.number} (${pullRequest.title}) in ${repoFullName}:`, - formatQuotedText(commentBody), + `${commenter} mentioned Roomote in a comment on Gitea pull request #${pullRequest.number} (${escapeTaskContextText(pullRequest.title)}) in ${repoFullName}.`, '', 'Please act on this comment as a follow-up to your existing work on this pull request.', ]; @@ -216,6 +213,14 @@ function buildExistingTaskFollowUpMessage({ ); } + lines.push( + '', + 'Mention comment (the request to act on):', + buildMentionRequestBlock(commentBody), + '', + buildUntrustedContentPolicy(), + ); + return lines.join('\n'); } diff --git a/apps/api/src/handlers/gitlab/__tests__/handleNote.test.ts b/apps/api/src/handlers/gitlab/__tests__/handleNote.test.ts index 5c7e0ce09..0be87e756 100644 --- a/apps/api/src/handlers/gitlab/__tests__/handleNote.test.ts +++ b/apps/api/src/handlers/gitlab/__tests__/handleNote.test.ts @@ -26,6 +26,9 @@ const { mockDbSelect: vi.fn(), })); +// Prompt-framing fakes use distinctive markers so tests can assert the +// handler routes each piece of text through the right builder; the real +// escaping/wrapping behavior is unit-tested in @roomote/cloud-agents. vi.mock('@roomote/cloud-agents/server', () => ({ enqueueTask: mockEnqueueTask, getTaskUrl: mockGetTaskUrl, @@ -372,9 +375,17 @@ describe('handleGitLabNote', () => { expect.objectContaining({ taskId: 'owner-task', userId: 'user-1', + message: expect.stringContaining( + 'Hey @roomote please take a look', + ), senderMode: 'github_pr_follow_up', }), ); + expect(mockSteerMessageToTask).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining(''), + }), + ); expect(mockEnqueueTask).not.toHaveBeenCalled(); expect(mockCreateGitLabMergeRequestNote).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/apps/api/src/handlers/gitlab/handleNote.ts b/apps/api/src/handlers/gitlab/handleNote.ts index dcb406f44..13e719f3e 100644 --- a/apps/api/src/handlers/gitlab/handleNote.ts +++ b/apps/api/src/handlers/gitlab/handleNote.ts @@ -229,14 +229,6 @@ function buildTaskStartFailedNote(surface: 'merge request' | 'issue'): string { return `I saw the mention, but I could not start a task for this ${surface} right now. Please try again in a moment.`; } -function formatQuotedText(text: string): string { - return text - .trim() - .split('\n') - .map((line) => `> ${line}`) - .join('\n'); -} - function buildExistingMrTaskFollowUpMessage({ repoFullName, mergeRequest, @@ -249,8 +241,7 @@ function buildExistingMrTaskFollowUpMessage({ noteBody: string; }): string { const lines = [ - `${commenter} mentioned Roomote in a comment on GitLab merge request !${mergeRequest.iid} (${mergeRequest.title}) in ${repoFullName}:`, - formatQuotedText(noteBody), + `${commenter} mentioned Roomote in a comment on GitLab merge request !${mergeRequest.iid} (${escapeTaskContextText(mergeRequest.title)}) in ${repoFullName}.`, '', 'Please act on this comment as a follow-up to your existing work on this merge request.', ]; @@ -261,6 +252,14 @@ function buildExistingMrTaskFollowUpMessage({ ); } + lines.push( + '', + 'Mention comment (the request to act on):', + buildMentionRequestBlock(noteBody), + '', + buildUntrustedContentPolicy(), + ); + return lines.join('\n'); }