Skip to content

Commit 361e57b

Browse files
committed
fix(linear): authorize comment triggers before acting (review MEDIUM security)
The issue→task path gated on lookupPlatformUser; the comment path did NOT. Any workspace member or guest who could post @bgagent could approve/reject plans, drive plan commands, and start code-pushing agent runs — all attributed to and BILLED against the original requester. handleCommentTrigger now resolves the commenter (payload.actor.id) to a platform user BEFORE any verdict/command/ dispatch; an unmapped commenter gets a ❓ reaction + a one-line 'link your account first' reply and the trigger is refused. The near-miss-mention nudge stays ahead of the gate (a nudge is not a privileged action); an app-actor commenter is correctly unmapped (the app can't self-authorize). Tests: unmapped-commenter-blocked regression + updated the shared ddbSend mocks to return a mapped commenter (linear_identity Get) so the actionable paths pass. cdk typecheck + 86 comment-path tests green.
1 parent 43d0c97 commit 361e57b

2 files changed

Lines changed: 59 additions & 1 deletion

File tree

cdk/src/handlers/linear-webhook-processor.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,6 +1450,35 @@ async function handleCommentTrigger(payload: LinearCommentEvent): Promise<void>
14501450
// actual comment the human wrote (reactions work at any thread depth).
14511451
const replyTargetId = payload.data.parentId ?? commentId;
14521452

1453+
// AUTHORIZATION (review MEDIUM): the issue→task path gates on lookupPlatformUser
1454+
// (a Linear actor with no linked ABCA user can't create tasks). The COMMENT
1455+
// path did NOT — so ANY workspace member or guest who can post @bgagent could
1456+
// approve/reject plans, drive plan commands, and START code-pushing agent runs,
1457+
// all attributed to and BILLED against the original requester. Resolve the
1458+
// commenter to a platform user BEFORE any verdict/command/dispatch. Unmapped →
1459+
// ❓ + a one-line reply, then stop. (The bot's own comments never carry the
1460+
// mention token, so they don't reach here; and an app-actor commenter is
1461+
// likewise unmapped, which is correct — the app can't authorize itself.)
1462+
const commenterId = payload.actor?.id;
1463+
const commenterPlatformUserId = commenterId
1464+
? await lookupPlatformUser(workspaceId, commenterId)
1465+
: null;
1466+
if (!commenterPlatformUserId) {
1467+
logger.warn('A6 comment: commenter has no linked platform user — refusing to act on the trigger', {
1468+
linear_workspace_id: workspaceId, linear_user_id: commenterId, linear_issue_id: commentedIssueId,
1469+
});
1470+
const feedbackCtx = { linearWorkspaceId: workspaceId, registryTableName: WORKSPACE_REGISTRY_TABLE };
1471+
await reactToComment(feedbackCtx, commentId, EMOJI_NEEDS_INPUT);
1472+
try {
1473+
await upsertThreadedReply(
1474+
feedbackCtx, commentedIssueId, replyTargetId,
1475+
'I can only act on `@bgagent` requests from a linked ABCA user. Link your Linear '
1476+
+ 'account first (ask your ABCA admin / run `bgagent linear link`), then re-comment.',
1477+
);
1478+
} catch { /* best-effort reply */ }
1479+
return;
1480+
}
1481+
14531482
// #299 Mode B: a comment on a parent that has a PENDING plan (proposed but not
14541483
// yet executed). Checked BEFORE A6 routing because NO orchestration is seeded
14551484
// yet — the parent has only a pending-plan row, so loadOrchestration misses it.

cdk/test/handlers/linear-webhook-processor-orchestration.test.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,12 @@ describe('linear-webhook-processor — #247 A6 comment trigger', () => {
450450
return { Items: opts.standalone ? [opts.standalone] : [] }; // resolveTaskByLinearIssue
451451
}
452452
if (cmd._type === 'Query') return { Items: [meta, child] }; // loadOrchestration
453+
// Comment-trigger authorization: lookupPlatformUser Gets the user-mapping
454+
// row keyed on linear_identity. Return a mapped commenter so the auth gate
455+
// passes (the un-mapped case is covered by its own dedicated test).
456+
if (cmd._type === 'Get' && (cmd.input.Key as { linear_identity?: string })?.linear_identity) {
457+
return { Item: { platform_user_id: 'commenter-user', status: 'active' } };
458+
}
453459
if (cmd._type === 'Get') return { Item: opts.prUrl ? { pr_url: opts.prUrl } : {} };
454460
return {};
455461
});
@@ -462,6 +468,10 @@ describe('linear-webhook-processor — #247 A6 comment trigger', () => {
462468
if (cmd._type === 'Query' && cmd.input.IndexName === 'LinearIssueIndex') {
463469
return { Items: standalone ? [standalone] : [] };
464470
}
471+
// Comment-trigger authorization: the commenter resolves to a mapped user.
472+
if (cmd._type === 'Get' && (cmd.input.Key as { linear_identity?: string })?.linear_identity) {
473+
return { Item: { platform_user_id: 'commenter-user', status: 'active' } };
474+
}
465475
return {};
466476
});
467477
}
@@ -562,6 +572,23 @@ describe('linear-webhook-processor — #247 A6 comment trigger', () => {
562572
expect(createTaskCoreMock).not.toHaveBeenCalled();
563573
});
564574

575+
test('review AUTH: an UNMAPPED commenter cannot drive a dispatch (❓ + reply, no task)', async () => {
576+
// Even with a fully actionable iteration target, a commenter with NO linked
577+
// platform user must not be able to start a code-pushing run billed to the
578+
// requester. The mapping Get returns nothing → the gate blocks before dispatch.
579+
fetchIssueParentIdMock.mockResolvedValue(null);
580+
ddbSend.mockImplementation(async (cmd: { _type: string; input: Record<string, unknown> }) => {
581+
if (cmd._type === 'Query' && cmd.input.IndexName === 'LinearIssueIndex') {
582+
return { Items: [{ task_id: 'task-solo', user_id: 'u-solo', repo: 'o/r', pr_number: 99 }] };
583+
}
584+
// No user-mapping row for the commenter (linear_identity Get → empty).
585+
return {};
586+
});
587+
await handler(eventWith(comment()));
588+
expect(createTaskCoreMock).not.toHaveBeenCalled(); // blocked by auth
589+
expect(reactToCommentMock).toHaveBeenCalledWith(expect.anything(), 'comment-1', 'question');
590+
});
591+
565592
test('bare @bgagent (no text) → falls back to a generic iteration instruction', async () => {
566593
mockOrchWithChild({ subIssueId: 'sub-issue-1', childTaskId: 'task-sub-1', prUrl: 'https://github.com/o/r/pull/7' });
567594
await handler(eventWith(comment({ data: { id: 'c3', body: '@bgagent', issueId: 'sub-issue-1' } })));
@@ -721,7 +748,9 @@ describe('linear-webhook-processor — #247 A6 comment trigger', () => {
721748
if (cmd._type === 'Query' && cmd.input.IndexName === 'LinearIssueIndex') return { Items: [] };
722749
if (cmd._type === 'Query') return { Items: [meta, footer, news] }; // loadOrchestration (parent's own)
723750
if (cmd._type === 'Get') {
724-
const key = cmd.input.Key as { task_id?: string; sub_issue_id?: string };
751+
const key = cmd.input.Key as { task_id?: string; sub_issue_id?: string; linear_identity?: string };
752+
// Comment-trigger authorization: mapped commenter (auth gate passes).
753+
if (key.linear_identity) return { Item: { platform_user_id: 'commenter-user', status: 'active' } };
725754
// Mode B getPendingPlan Get is keyed on the #pending-plan SK — no plan
726755
// on this epic, so return no item (matches prod; a verdict-shaped
727756
// comment like "ship it" then falls through to the A6 no-match path).

0 commit comments

Comments
 (0)