diff --git a/cdk/src/handlers/linear-webhook-processor.ts b/cdk/src/handlers/linear-webhook-processor.ts index 22e196b33..533675d0d 100644 --- a/cdk/src/handlers/linear-webhook-processor.ts +++ b/cdk/src/handlers/linear-webhook-processor.ts @@ -46,7 +46,7 @@ import { renderTaskLookupFailedNudge, renderWrongMentionNudge, } from './shared/linear-notes'; -import { resolveLinearOauthToken } from './shared/linear-oauth-resolver'; +import { resolveLinearOauthToken, resolveSoleActiveLinearWorkspace } from './shared/linear-oauth-resolver'; import { fetchIssueParentId } from './shared/linear-subissue-fetch'; import { lookupTaskByLinearIssue, prNumberFromTask } from './shared/linear-task-by-issue'; import { logger } from './shared/logger'; @@ -623,6 +623,16 @@ interface LinearCommentEvent { interface ProcessorEvent { readonly raw_body: string; + /** + * Whether the stack-wide secret — bound to no workspace — is what verified this + * delivery, as reported by the receiver. + * + * On that path the body's `organizationId` is claimed rather than attested, so it must + * not be used to select a tenant. Absent is read as `false`, which is correct for the + * per-workspace path and is also what an in-flight invocation from a previous version + * looks like during a deploy. + */ + readonly verified_via_stack_wide?: boolean; } /** @@ -690,6 +700,33 @@ export async function handler(event: ProcessorEvent): Promise { return; } + // A delivery verified by the stack-wide secret carries no proof of WHICH workspace + // sent it — that secret is bound to none of them — so the body's `organizationId` + // is a claim. Replace it with the only workspace it could mean, and drop the delivery + // when there is no single answer. + // + // Rewritten on the payload rather than threaded as a parameter because the workspace + // id is read from ~6 places downstream (task attribution, feedback, the comment path). + // Passing it alongside would leave every one of those a site where the claimed value + // could still be picked up by mistake; overwriting the untrusted field means the + // attested value is the only one reachable. + if (event.verified_via_stack_wide) { + const bound = await resolveSoleActiveLinearWorkspace(ddb, WORKSPACE_REGISTRY_TABLE); + if (!bound) { + logger.warn('Dropping stack-wide-verified Linear delivery: cannot determine the sending workspace', { + claimed_workspace_id: payload.organizationId, + }); + return; + } + if (payload.organizationId && payload.organizationId !== bound) { + logger.warn('Ignoring body organizationId on a stack-wide-verified delivery; binding to the sole active workspace', { + claimed_workspace_id: payload.organizationId, + bound_workspace_id: bound, + }); + } + (payload as { organizationId?: string }).organizationId = bound; + } + // A Comment with an @bgagent mention on an orchestrated sub-issue // re-iterates that sub-issue's PR (the reconciler then cascades the // re-stack). Handled on a separate path from Issue → task creation. @@ -721,6 +758,38 @@ export async function handler(event: ProcessorEvent): Promise { mappingItem = mapping.Item; } } + + // The mapping table is keyed on the project id alone, so `projectId` selects a + // repository on its own — and it arrives in the request body. Check the mapping + // against the workspace the delivery claims to be from, so naming another + // workspace's project cannot steer a task at that workspace's repository. + // + // Drop rather than reply. The reply would go to the sender's own workspace, and + // "that project belongs to someone else" both confirms the project exists and tells + // a prober the attempt was seen. The log line is the diagnostic surface instead. + const mappedWorkspaceId = mappingItem?.linear_workspace_id as string | undefined; + if (mappingItem && mappedWorkspaceId && mappedWorkspaceId !== payload.organizationId) { + logger.warn('Linear project is mapped to a different workspace than this webhook — dropping', { + issue_id: issue.id, + linear_project_id: projectId, + event_workspace_id: payload.organizationId, + mapped_workspace_id: mappedWorkspaceId, + }); + return; + } + if (mappingItem && !mappedWorkspaceId) { + // Allowed for now: rows written before the owning workspace was recorded have + // nothing to check against, and rejecting them would break working installs on + // deploy. Re-running `bgagent linear onboard-project` records the owner, and + // `bgagent platform doctor` reports what is left (failing once more than one + // workspace is active), which is what makes it safe to turn this into a rejection later. + logger.warn('Linear project mapping records no owning workspace — cannot verify the tenant', { + issue_id: issue.id, + linear_project_id: projectId, + event_workspace_id: payload.organizationId, + }); + } + const labelFilter = (mappingItem?.label_filter as string | undefined) ?? DEFAULT_LABEL_FILTER; // ``:help`` — post a one-time explainer of what the trigger labels do @@ -1840,6 +1909,33 @@ async function handleNearMissMention(payload: LinearCommentEvent): Promise * a non-orchestration comment, a missing mention, or an un-started sub-issue is * a clean no-op (no failure comment — comments are conversational). */ +/** + * Whether the work a comment would act on belongs to the workspace that sent it. + * + * The comment path finds its target by issue id: an orchestration (keyed on a hash of + * that id) or the newest task on it. Those lookups are not scoped to a workspace, and + * the work that follows runs on the target's repo as its original requester, so the + * target's recorded workspace is matched against the delivery's here. This keeps the + * comment path consistent with the project-mapping check in the handler. + * + * A target that records no workspace is not acted on: every orchestration and Linear + * task records one, so absence means the row is not one this path should act on. + * No reply is posted, matching the project-mapping check. + */ +function commentTargetBelongsToWorkspace( + ownerWorkspaceId: string | undefined, + workspaceId: string, + issueId: string, +): boolean { + if (ownerWorkspaceId && ownerWorkspaceId === workspaceId) return true; + logger.warn('Comment trigger: commented issue belongs to a different workspace than this webhook — dropping', { + linear_issue_id: issueId, + event_workspace_id: workspaceId, + owner_workspace_id: ownerWorkspaceId, + }); + return false; +} + async function handleCommentTrigger(payload: LinearCommentEvent): Promise { // Orchestration must be enabled + a workspace token resolvable. if (!ORCHESTRATION_TABLE || !WORKSPACE_REGISTRY_TABLE) { @@ -1919,6 +2015,7 @@ async function handleCommentTrigger(payload: LinearCommentEvent): Promise const ownOrchestrationId = deriveOrchestrationId(commentedIssueId); const parentSnapshot = await loadOrchestration(ddb, ORCHESTRATION_TABLE, ownOrchestrationId); if (parentSnapshot && parentSnapshot.meta.parent_issue_ref === commentedIssueId) { + if (!commentTargetBelongsToWorkspace(parentSnapshot.meta.credentials_ref, workspaceId, commentedIssueId)) return; await handleParentEpicCommentTrigger({ orchestrationId: ownOrchestrationId, snapshot: parentSnapshot, @@ -1968,6 +2065,7 @@ async function handleCommentTrigger(payload: LinearCommentEvent): Promise ? await loadOrchestration(ddb, ORCHESTRATION_TABLE, orchestrationId) : null; const child = snapshot?.children.find((c) => c.sub_issue_id === commentedIssueId); + if (snapshot && !commentTargetBelongsToWorkspace(snapshot.meta.credentials_ref, workspaceId, commentedIssueId)) return; if (!snapshot || !child || !child.child_task_id) { await handleStandaloneCommentTrigger({ subIssueId: commentedIssueId, @@ -2473,6 +2571,7 @@ async function handleStandaloneCommentTrigger(args: { return; } const task = lookup.task; + if (!commentTargetBelongsToWorkspace(task.linear_workspace_id, workspaceId, issueId)) return; const prNumber = prNumberFromTask(task); if (prNumber === null || !task.repo) { // Clarify-resume: a task with no PR MIGHT be a clarify-HOLD (a diff --git a/cdk/src/handlers/linear-webhook.ts b/cdk/src/handlers/linear-webhook.ts index 0cba78c70..2f4f747c8 100644 --- a/cdk/src/handlers/linear-webhook.ts +++ b/cdk/src/handlers/linear-webhook.ts @@ -22,6 +22,7 @@ import { InvokeCommand, LambdaClient } from '@aws-sdk/client-lambda'; import { DeleteCommand, PutCommand } from '@aws-sdk/lib-dynamodb'; import type { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'; import { + countActiveLinearWorkspaces, isWebhookTimestampFresh, verifyLinearRequest, verifyLinearRequestForWorkspace, @@ -133,18 +134,48 @@ export async function handler(event: APIGatewayProxyEvent): Promise', + }); + return jsonResponse(401, { error: 'Workspace signing secret is not its own' }); } // 'no-per-workspace-secret' falls through to the stack-wide path // below — back-compat for installs predating per-workspace secrets. } + // Whether the stack-wide secret was what verified this delivery. Forwarded to the + // processor, which routes from body-supplied identifiers that a tenant-less secret + // cannot attest to. + let verifiedViaStackWide = false; + if (!verified) { + // The stack-wide secret is bound to no workspace, so on a stack with more than one + // it cannot say which tenant sent this — and the body's `organizationId` is the + // attacker's to choose. Refuse rather than guess. Single-workspace installs keep + // the fallback: there, the only tenant it could mean is the only tenant there is. + const activeWorkspaces = await countActiveLinearWorkspaces(WORKSPACE_REGISTRY_TABLE); + if (activeWorkspaces > 1) { + logger.warn('Refusing the stack-wide fallback on a multi-workspace stack', { + linear_workspace_id: payload.organizationId, + active_workspace_count: activeWorkspaces, + remedy: 'bgagent linear update-webhook-secret ', + }); + return jsonResponse(401, { error: 'Per-workspace signing secret required' }); + } + if (!await verifyLinearRequest(WEBHOOK_SECRET_ARN, signature, event.body)) { logger.warn('Invalid Linear webhook signature', { linear_workspace_id: payload.organizationId, }); return jsonResponse(401, { error: 'Invalid signature' }); } + verifiedViaStackWide = true; // Stack-wide fallback succeeded. Log positively so operators // diagnosing a per-workspace verification regression have a // breadcrumb that says "this workspace is verifying via the @@ -244,7 +275,13 @@ export async function handler(event: APIGatewayProxyEvent): Promise { + if (!registryTableName) return undefined; + const active: string[] = []; + let lastKey: Record | undefined; + do { + const page = await ddbClient.send(new ScanCommand({ + TableName: registryTableName, + ProjectionExpression: 'linear_workspace_id, #s', + ExpressionAttributeNames: { '#s': 'status' }, + ExclusiveStartKey: lastKey, + ConsistentRead: true, + })); + for (const item of page.Items ?? []) { + if (item.status === 'active' && typeof item.linear_workspace_id === 'string') { + active.push(item.linear_workspace_id); + } + } + lastKey = page.LastEvaluatedKey; + if (active.length > 1) break; + } while (lastKey); + + if (active.length === 1) return active[0]; + logger.warn('Cannot bind a stack-wide-verified Linear delivery: registry does not have exactly one active workspace', { + active_workspace_count: active.length, + }); + return undefined; +} + /** Test-only: clear all caches. */ export function _resetCachesForTesting(): void { registryCache.clear(); diff --git a/cdk/src/handlers/shared/linear-task-by-issue.ts b/cdk/src/handlers/shared/linear-task-by-issue.ts index bcb84d99e..d6bbeeb72 100644 --- a/cdk/src/handlers/shared/linear-task-by-issue.ts +++ b/cdk/src/handlers/shared/linear-task-by-issue.ts @@ -33,6 +33,11 @@ export interface LinearIssueTask { readonly pr_url?: string; readonly pr_number?: number; readonly status?: string; + /** + * The Linear workspace the task was created for, from its `channel_metadata`. + * Callers acting on a webhook match this against the delivery's workspace. + */ + readonly linear_workspace_id?: string; } /** @@ -91,6 +96,9 @@ export async function lookupTaskByLinearIssue( ...(item.pr_url !== undefined && { pr_url: item.pr_url as string }), ...(item.pr_number !== undefined && { pr_number: item.pr_number as number }), ...(item.status !== undefined && { status: item.status as string }), + ...(typeof item.channel_metadata?.linear_workspace_id === 'string' && { + linear_workspace_id: item.channel_metadata.linear_workspace_id as string, + }), }, }; } catch (err) { diff --git a/cdk/src/handlers/shared/linear-verify.ts b/cdk/src/handlers/shared/linear-verify.ts index bc60db78a..29fde6c6d 100644 --- a/cdk/src/handlers/shared/linear-verify.ts +++ b/cdk/src/handlers/shared/linear-verify.ts @@ -19,6 +19,7 @@ import * as crypto from 'crypto'; import { GetSecretValueCommand, SecretsManagerClient } from '@aws-sdk/client-secrets-manager'; +import { ScanCommand } from '@aws-sdk/lib-dynamodb'; import { isUsableHmacSecret } from './hmac-secret'; import { getOauthSecretStrict, getRegistryRowStrict } from './linear-oauth-resolver'; import { logger } from './logger'; @@ -35,6 +36,72 @@ const CACHE_TTL_MS = CACHE_TTL_MINUTES * 60 * 1000; /** Maximum age of a Linear webhookTimestamp (ms) before it is rejected (replay protection). */ export const MAX_WEBHOOK_TIMESTAMP_AGE_MS = 60 * 1000; +/** + * Cached count of active workspaces, capped at 2. + * + * Capped because nothing needs the true total — every decision that reads it only asks + * "is there more than one tenant on this stack", so the Scan can stop at the second hit. + * + * Cached separately from `secretCache` because it is not keyed by anything: it is one + * table-wide fact, re-derived by a Scan that would otherwise run on every delivery. + * A short TTL is the tradeoff — onboarding a second workspace takes up to this long to + * start being enforced, which is acceptable because the CLI refuses to create the + * shared-secret state in the first place. + */ +let activeWorkspaceCountCache: { count: number; expiresAt: number } | undefined; + +/** + * How many active Linear workspaces this stack has, saturating at 2. + * + * The distinction that matters is one tenant versus more than one. With a single + * workspace a secret that is not bound to a tenant still identifies the only tenant + * there is, so neither the stack-wide fallback nor a shared secret can cross a + * boundary. With two or more, both can. + * + * Returns 1 when the registry cannot be read, and says so in the log. That is the + * permissive answer, chosen deliberately: a DynamoDB throttle must not start rejecting + * every delivery on a healthy single-workspace install. The callers that act on this + * are hardening an already-verified signature, not standing in for one. + */ +export async function countActiveLinearWorkspaces(registryTableName: string | undefined): Promise { + if (!registryTableName) return 1; + const now = Date.now(); + if (activeWorkspaceCountCache && activeWorkspaceCountCache.expiresAt > now) { + return activeWorkspaceCountCache.count; + } + + try { + let count = 0; + let lastKey: Record | undefined; + do { + const page = await ddb.send(new ScanCommand({ + TableName: registryTableName, + ProjectionExpression: 'linear_workspace_id, #s', + ExpressionAttributeNames: { '#s': 'status' }, + ExclusiveStartKey: lastKey, + })); + for (const item of page.Items ?? []) { + if (item.status === 'active') count += 1; + } + lastKey = page.LastEvaluatedKey; + if (count > 1) break; + } while (lastKey); + + activeWorkspaceCountCache = { count, expiresAt: now + CACHE_TTL_MS }; + return count; + } catch (err) { + logger.warn('Could not count active Linear workspaces — assuming a single-workspace stack', { + error: err instanceof Error ? err.message : String(err), + }); + return 1; + } +} + +/** Drop the cached workspace count. Exported for tests. */ +export function _resetActiveWorkspaceCountCache(): void { + activeWorkspaceCountCache = undefined; +} + /** * Fetch a secret from Secrets Manager with in-memory caching. * @param secretId - the full Secrets Manager secret ID or ARN. @@ -220,7 +287,7 @@ export async function verifyLinearRequestForWorkspace( linearWorkspaceId: string, signature: string, body: string, -): Promise<'verified' | 'mismatch' | 'revoked' | 'no-per-workspace-secret'> { +): Promise<'verified' | 'mismatch' | 'revoked' | 'no-per-workspace-secret' | 'shared-secret'> { const row = await getRegistryRowStrict(ddb, registryTableName, linearWorkspaceId); if (!row) { return 'no-per-workspace-secret'; @@ -232,7 +299,19 @@ export async function verifyLinearRequestForWorkspace( if (!stored || !stored.webhook_signing_secret) { return 'no-per-workspace-secret'; } - return verifyLinearSignature(stored.webhook_signing_secret, signature, body) - ? 'verified' - : 'mismatch'; + if (!verifyLinearSignature(stored.webhook_signing_secret, signature, body)) { + return 'mismatch'; + } + + // The signature matched — but matching a secret this workspace does not exclusively + // hold proves only that the sender knows a secret SOME workspace on this stack holds. + // A workspace onboarded by an older release can be carrying a copy of the first + // workspace's secret, and the routing values in the body are read from whichever + // workspace the sender names. Checked only when another tenant exists to impersonate, + // so a single-workspace install — where the same secret cannot cross a boundary and + // where an unrecorded provenance is the normal state — is untouched. + if (row.webhook_secret_owned !== true && await countActiveLinearWorkspaces(registryTableName) > 1) { + return 'shared-secret'; + } + return 'verified'; } diff --git a/cdk/test/handlers/linear-webhook-processor-orchestration.test.ts b/cdk/test/handlers/linear-webhook-processor-orchestration.test.ts index e123e829b..134e05aca 100644 --- a/cdk/test/handlers/linear-webhook-processor-orchestration.test.ts +++ b/cdk/test/handlers/linear-webhook-processor-orchestration.test.ts @@ -688,7 +688,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { sub_issue_id: '#meta', orchestration_id: 'orch_x', parent_linear_issue_id: 'PARENT', - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', repo: 'o/r', child_count: 1, platform_user_id: 'release-user', @@ -700,12 +700,12 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { child_status: 'succeeded', repo: 'o/r', parent_linear_issue_id: 'PARENT', - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', }; if (opts.childTaskId) child.child_task_id = opts.childTaskId; ddbSend.mockImplementation(async (cmd: { _type: string; input: Record }) => { if (cmd._type === 'Query' && cmd.input.IndexName === 'LinearIssueIndex') { - return { Items: opts.standalone ? [opts.standalone] : [] }; // resolveTaskByLinearIssue + return { Items: opts.standalone ? [{ channel_metadata: { linear_workspace_id: 'org-1' }, ...opts.standalone }] : [] }; // resolveTaskByLinearIssue } if (cmd._type === 'Query') return { Items: [meta, child] }; // loadOrchestration // Comment-trigger authorization: lookupPlatformUser Gets the user-mapping @@ -724,7 +724,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { fetchIssueParentIdMock.mockResolvedValue(LOOKUP_ABSENT); // no parent ⇒ not a sub-issue ddbSend.mockImplementation(async (cmd: { _type: string; input: Record }) => { if (cmd._type === 'Query' && cmd.input.IndexName === 'LinearIssueIndex') { - return { Items: standalone ? [standalone] : [] }; + return { Items: standalone ? [{ channel_metadata: { linear_workspace_id: 'org-1' }, ...standalone }] : [] }; } // Comment-trigger authorization: the commenter resolves to a mapped user. if (cmd._type === 'Get' && (cmd.input.Key as { linear_identity?: string })?.linear_identity) { @@ -746,6 +746,53 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { upsertThreadedReplyMock.mockReset().mockResolvedValue('reply-1'); }); + // A comment only acts on work recorded under the workspace the delivery came from. + // The other fixtures in this file are single-workspace (org-1). + describe('a comment cannot act on another workspace\'s work', () => { + function expectSilentDrop(): void { + expect(createTaskCoreMock).not.toHaveBeenCalled(); + // No ack or reply, matching the project-mapping check. + expect(reactToCommentMock).not.toHaveBeenCalled(); + expect(upsertThreadedReplyMock).not.toHaveBeenCalled(); + expect(replyToCommentMock).not.toHaveBeenCalled(); + } + + test('a sub-issue of another workspace\'s orchestration starts no iteration', async () => { + mockOrchWithChild({ subIssueId: 'sub-issue-1', childTaskId: 'task-sub-1', prUrl: 'https://github.com/o/r/pull/42' }); + await handler(eventWith(comment({ organizationId: 'org-2' }))); + expectSilentDrop(); + }); + + test('another workspace\'s parent epic starts no iteration, even naming a child', async () => { + // A comment on the epic itself is routed without a Linear API call, so the + // workspace match is checked on this path too. + mockOrchWithChild({ subIssueId: 'sub-issue-1', childTaskId: 'task-sub-1', prUrl: 'https://github.com/o/r/pull/42' }); + await handler(eventWith(comment({ + organizationId: 'org-2', + data: { id: 'comment-1', body: '@bgagent sub-issue-1: change the timeout', issueId: 'PARENT' }, + }))); + expectSilentDrop(); + }); + + test('a standalone task from another workspace starts no iteration', async () => { + mockStandaloneOnly({ task_id: 'task-p', user_id: 'other-user', repo: 'o2/r2', pr_number: 7 }); + await handler(eventWith(comment({ organizationId: 'org-2' }))); + expectSilentDrop(); + }); + + test('a standalone task that records no workspace is not acted on', async () => { + mockStandaloneOnly({ + task_id: 'task-p', + user_id: 'other-user', + repo: 'o2/r2', + pr_number: 7, + channel_metadata: {}, + } as Parameters[0]); + await handler(eventWith(comment())); + expectSilentDrop(); + }); + }); + test('@bgagent on a started sub-issue → pr-iteration task on its PR with cascade marker', async () => { mockOrchWithChild({ subIssueId: 'sub-issue-1', childTaskId: 'task-sub-1', prUrl: 'https://github.com/o/r/pull/42' }); await handler(eventWith(comment())); @@ -793,7 +840,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { sub_issue_id: '#meta', orchestration_id: 'orch_x', parent_linear_issue_id: 'PARENT', - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', repo: 'o/r', child_count: 2, platform_user_id: 'release-user', @@ -807,7 +854,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { child_status: 'succeeded', repo: 'o/r', parent_linear_issue_id: 'PARENT', - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', child_task_id: 'task-sub-1', }; const badSibling = { @@ -817,7 +864,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { child_status: 'failed', repo: 'o/r', parent_linear_issue_id: 'PARENT', - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', child_task_id: 'task-bad-1', }; const claimed = new Set(); @@ -1129,7 +1176,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { sub_issue_id: '#meta', orchestration_id: 'orch_x', parent_linear_issue_id: parentIssueId, - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', repo: 'o/r', child_count: 2, platform_user_id: 'release-user', @@ -1141,7 +1188,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { child_status: 'succeeded', repo: 'o/r', parent_linear_issue_id: parentIssueId, - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', linear_identifier: 'ABCA-305', title: 'Add a site-wide footer', child_task_id: 'task-footer', @@ -1153,7 +1200,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { child_status: 'succeeded', repo: 'o/r', parent_linear_issue_id: parentIssueId, - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', linear_identifier: 'ABCA-306', title: 'Add a newsletter signup section', child_task_id: 'task-news', @@ -1198,13 +1245,13 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { child_status: 'succeeded', repo: 'o/r', parent_linear_issue_id: parentIssueId, - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', }; const meta = { orchestration_id: 'orch_x', sub_issue_id: '#meta', parent_linear_issue_id: parentIssueId, - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', repo: 'o/r', child_count: 3, platform_user_id: 'u1', @@ -1446,7 +1493,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { sub_issue_id: '#meta', orchestration_id: 'orch_f', parent_linear_issue_id: parentIssueId, - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', repo: 'o/r', child_count: 2, platform_user_id: 'release-user', @@ -1459,7 +1506,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { child_status: 'succeeded', repo: 'o/r', parent_linear_issue_id: parentIssueId, - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', linear_identifier: 'ABCA-1', title: 'Step A', child_task_id: 'task-ok', @@ -1471,7 +1518,7 @@ describe('linear-webhook-processor — @bgagent comment trigger', () => { child_status: 'failed', repo: 'o/r', parent_linear_issue_id: parentIssueId, - linear_workspace_id: 'WS', + linear_workspace_id: 'org-1', linear_identifier: 'ABCA-2', title: 'Step B', child_task_id: 'task-bad-1', diff --git a/cdk/test/handlers/linear-webhook-processor.test.ts b/cdk/test/handlers/linear-webhook-processor.test.ts index 90dc63ae4..31ecb7121 100644 --- a/cdk/test/handlers/linear-webhook-processor.test.ts +++ b/cdk/test/handlers/linear-webhook-processor.test.ts @@ -64,8 +64,10 @@ jest.mock('../../src/handlers/shared/linear-attachments', () => { }); const resolveLinearOauthTokenMock = jest.fn(); +const resolveSoleActiveLinearWorkspaceMock = jest.fn(); jest.mock('../../src/handlers/shared/linear-oauth-resolver', () => ({ resolveLinearOauthToken: (...args: unknown[]) => resolveLinearOauthTokenMock(...args), + resolveSoleActiveLinearWorkspace: (...args: unknown[]) => resolveSoleActiveLinearWorkspaceMock(...args), })); const probeLinearIssueContextMock = jest.fn(); @@ -90,6 +92,7 @@ process.env.GUARDRAIL_VERSION = '1'; import { handler } from '../../src/handlers/linear-webhook-processor'; import { LinearAttachmentError } from '../../src/handlers/shared/linear-attachments'; +import { logger } from '../../src/handlers/shared/logger'; function eventWith(payload: Record): { raw_body: string } { return { raw_body: JSON.stringify(payload) }; @@ -134,6 +137,7 @@ describe('linear-webhook-processor handler', () => { cleanupPreScreenedAttachmentsMock.mockReset(); cleanupPreScreenedAttachmentsMock.mockResolvedValue(undefined); resolveLinearOauthTokenMock.mockReset(); + resolveSoleActiveLinearWorkspaceMock.mockReset(); // Default: workspace IS resolvable (active registry row + valid // OAuth bundle). The processor early-returns when this resolves to // null — see "Linear workspace not resolvable from registry — @@ -311,6 +315,138 @@ describe('linear-webhook-processor handler', () => { }); }); + // The mapping table is keyed on the project id alone and `projectId` arrives in the + // request body, so the row's owning workspace is the only thing standing between a + // signed event and another tenant's repository. + describe('project → workspace binding', () => { + test('drops an event whose project is mapped to a different workspace', async () => { + ddbSend.mockResolvedValueOnce({ + Item: { repo: 'victim/repo', status: 'active', linear_workspace_id: 'org-victim' }, + }); + + await handler(eventWith(issue())); + + expect(createTaskCoreMock).not.toHaveBeenCalled(); + // Silent on the wire: replying would confirm the project exists to whoever named + // it, and the reply would land in the sender's own workspace. + expect(reportIssueFailureMock).not.toHaveBeenCalled(); + }); + + test('proceeds when the mapping names the workspace the event came from', async () => { + ddbSend + .mockResolvedValueOnce({ + Item: { repo: 'org/repo', status: 'active', linear_workspace_id: 'org-1' }, + }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + createTaskCoreMock.mockResolvedValueOnce({ statusCode: 201, body: JSON.stringify({ data: { task_id: 'T1' } }) }); + + await handler(eventWith(issue())); + + expect(createTaskCoreMock).toHaveBeenCalledTimes(1); + expect(createTaskCoreMock.mock.calls[0][0].repo).toBe('org/repo'); + }); + + test('still proceeds when the mapping predates the owning-workspace field', async () => { + // Back-compat: rejecting unbacked rows on deploy would break every install that + // has not run the backfill yet. Enforcement for these is a later, gated step. + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => undefined); + try { + ddbSend + .mockResolvedValueOnce({ Item: { repo: 'org/repo', status: 'active' } }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + createTaskCoreMock.mockResolvedValueOnce({ statusCode: 201, body: JSON.stringify({ data: { task_id: 'T1' } }) }); + + await handler(eventWith(issue())); + + expect(createTaskCoreMock).toHaveBeenCalledTimes(1); + // The warning is the only signal that this install still has rows to backfill, + // so it is asserted rather than left to the source comment. + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('records no owning workspace'), + expect.objectContaining({ linear_project_id: 'project-1' }), + ); + } finally { + warnSpy.mockRestore(); + } + }); + + // Asserted on the log rather than on `createTaskCore`, because an event with no + // workspace id never creates a task anyway — it fails later at attribution. Only + // the guard's own line proves the drop happened HERE, at the binding check. + test('drops at the binding check when the event carries no workspace id', async () => { + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => undefined); + try { + ddbSend.mockResolvedValueOnce({ + Item: { repo: 'org/repo', status: 'active', linear_workspace_id: 'org-1' }, + }); + + await handler(eventWith(issue({ organizationId: undefined }))); + + expect(createTaskCoreMock).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('mapped to a different workspace'), + expect.objectContaining({ mapped_workspace_id: 'org-1' }), + ); + } finally { + warnSpy.mockRestore(); + } + }); + }); + + // On the stack-wide path the body's organizationId is claimed, not attested, so it is + // replaced with the only workspace it could mean. + describe('binding a stack-wide-verified delivery', () => { + test('ignores the claimed organizationId and binds to the sole active workspace', async () => { + resolveSoleActiveLinearWorkspaceMock.mockResolvedValue('org-real'); + ddbSend + .mockResolvedValueOnce({ + Item: { repo: 'org/repo', status: 'active', linear_workspace_id: 'org-real' }, + }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + createTaskCoreMock.mockResolvedValueOnce({ statusCode: 201, body: JSON.stringify({ data: { task_id: 'T1' } }) }); + + await handler({ + ...eventWith(issue({ organizationId: 'org-attacker-claimed' })), + verified_via_stack_wide: true, + }); + + // Bound, not claimed: the mapping is owned by org-real, so a processor that kept + // the claimed value would have dropped this at the project-binding check. + expect(createTaskCoreMock).toHaveBeenCalledTimes(1); + expect(createTaskCoreMock.mock.calls[0][1].channelMetadata).toMatchObject({ + linear_workspace_id: 'org-real', + }); + }); + + test('drops the delivery when the registry has no single active workspace', async () => { + // Two active workspaces means the resolver has no single answer to give. + resolveSoleActiveLinearWorkspaceMock.mockResolvedValue(undefined); + + await handler({ ...eventWith(issue()), verified_via_stack_wide: true }); + + expect(createTaskCoreMock).not.toHaveBeenCalled(); + }); + + test('leaves the organizationId alone when a per-workspace secret verified it', async () => { + // The attested value must survive. Rebinding here would silently repoint a + // correctly-verified delivery at whichever workspace happens to be the only + // active one. + ddbSend + .mockResolvedValueOnce({ + Item: { repo: 'org/repo', status: 'active', linear_workspace_id: 'org-1' }, + }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + createTaskCoreMock.mockResolvedValueOnce({ statusCode: 201, body: JSON.stringify({ data: { task_id: 'T1' } }) }); + + await handler(eventWith(issue())); + + expect(createTaskCoreMock).toHaveBeenCalledTimes(1); + expect(createTaskCoreMock.mock.calls[0][1].channelMetadata).toMatchObject({ + linear_workspace_id: 'org-1', + }); + }); + }); + test('fires on update when labelIds newly include the trigger label', async () => { ddbSend .mockResolvedValueOnce({ Item: { repo: 'org/repo', status: 'active' } }) diff --git a/cdk/test/handlers/linear-webhook-tenant-binding.test.ts b/cdk/test/handlers/linear-webhook-tenant-binding.test.ts new file mode 100644 index 000000000..2a21f7ceb --- /dev/null +++ b/cdk/test/handlers/linear-webhook-tenant-binding.test.ts @@ -0,0 +1,241 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +// A signing secret must attest WHICH tenant sent a delivery, because the processor +// routes from identifiers in the body. Two secrets fail that: the stack-wide one, which +// is bound to no workspace, and a per-workspace one that is a copy of another +// workspace's. Both are only dangerous once a second tenant exists to impersonate, +// which is what these tests pivot on. +// +// A separate file from linear-webhook.test.ts because the registry table name is read +// into a module-level const at import time, and these cases need it set. +import * as crypto from 'crypto'; +import type { APIGatewayProxyEvent } from 'aws-lambda'; + +const ddbSend = jest.fn(); +jest.mock('@aws-sdk/client-dynamodb', () => ({ DynamoDBClient: jest.fn(() => ({})) })); +jest.mock('@aws-sdk/lib-dynamodb', () => ({ + DynamoDBDocumentClient: { from: jest.fn(() => ({ send: ddbSend })) }, + PutCommand: jest.fn((input: unknown) => ({ _type: 'Put', input })), + DeleteCommand: jest.fn((input: unknown) => ({ _type: 'Delete', input })), + GetCommand: jest.fn((input: unknown) => ({ _type: 'Get', input })), + ScanCommand: jest.fn((input: unknown) => ({ _type: 'Scan', input })), +})); + +const lambdaSend = jest.fn(); +jest.mock('@aws-sdk/client-lambda', () => ({ + LambdaClient: jest.fn(() => ({ send: lambdaSend })), + InvokeCommand: jest.fn((input: unknown) => ({ _type: 'Invoke', input })), +})); + +const smSend = jest.fn(); +jest.mock('@aws-sdk/client-secrets-manager', () => ({ + SecretsManagerClient: jest.fn(() => ({ send: smSend })), + GetSecretValueCommand: jest.fn((input: unknown) => ({ _type: 'GetSecretValue', input })), +})); + +const REGISTRY = 'LinearWorkspaceRegistry'; +const OAUTH_ARN = 'arn:aws:secretsmanager:us-east-1:123:secret:bgagent-linear-oauth-acme'; +const STACK_WIDE_ARN = 'arn:aws:secretsmanager:us-east-1:123:secret:bgagent/linear/webhook-XYZ'; +process.env.LINEAR_WEBHOOK_SECRET_ARN = STACK_WIDE_ARN; +process.env.LINEAR_WEBHOOK_DEDUP_TABLE_NAME = 'LinearDedup'; +process.env.LINEAR_WEBHOOK_PROCESSOR_FUNCTION_NAME = 'linear-processor'; +process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME = REGISTRY; + +import { handler } from '../../src/handlers/linear-webhook'; +import { _resetCachesForTesting } from '../../src/handlers/shared/linear-oauth-resolver'; +import { _resetActiveWorkspaceCountCache, invalidateLinearSecretCache } from '../../src/handlers/shared/linear-verify'; + +const OWN_SECRET = 'lin_wh_acmeOwnSecret'; +const STACK_WIDE_SECRET = 'lin_wh_firstWorkspaceSecret'; +const WS_ACME = 'org-acme'; + +function bundle(signingSecret: string): string { + return JSON.stringify({ + access_token: '', + refresh_token: '', + expires_at: '', + scope: '', + client_id: 'cid', + client_secret: 'csec', + workspace_id: WS_ACME, + workspace_slug: 'acme', + installed_at: '2026-09-01T00:00:00.000Z', + updated_at: '2026-09-01T00:00:00.000Z', + installed_by_platform_user_id: 'u-1', + webhook_signing_secret: signingSecret, + }); +} + +function body(): string { + return JSON.stringify({ + action: 'update', + type: 'Issue', + organizationId: WS_ACME, + webhookTimestamp: Date.now(), + data: { id: 'issue-1', identifier: 'ABC-1', title: 't', projectId: 'proj-1' }, + }); +} + +function event(raw: string, key: string): APIGatewayProxyEvent { + return { + body: raw, + headers: { 'linear-signature': crypto.createHmac('sha256', key).update(raw).digest('hex') }, + httpMethod: 'POST', + } as unknown as APIGatewayProxyEvent; +} + +/** + * Wire DynamoDB for a registry holding `activeCount` workspaces. + * + * `Get` answers the row for the workspace under test; `Scan` answers the count. `Put` is + * the dedup claim, which must succeed or the receiver stops before verification matters. + */ +function withRegistry(opts: { activeCount: number; row?: Record | null }) { + const selfRow = opts.row === undefined + ? { linear_workspace_id: WS_ACME, workspace_slug: 'acme', oauth_secret_arn: OAUTH_ARN, status: 'active' } + : opts.row; + // `activeCount` is the total the Scan reports. A workspace with no registry row of its + // own still shares the stack with the others, so the count must not shrink with it — + // getting this wrong is what made the multi-workspace fallback case look like it + // passed when the receiver had only ever seen one workspace. + const otherCount = opts.activeCount - (selfRow ? 1 : 0); + const others = Array.from({ length: Math.max(0, otherCount) }, (_, i) => ({ + linear_workspace_id: `org-other-${i}`, status: 'active', + })); + + ddbSend.mockImplementation((cmd: { _type: string }) => { + if (cmd._type === 'Get') return Promise.resolve({ Item: selfRow ?? undefined }); + if (cmd._type === 'Scan') { + return Promise.resolve({ + Items: [...(selfRow ? [{ linear_workspace_id: WS_ACME, status: 'active' }] : []), ...others], + }); + } + return Promise.resolve({}); + }); +} + +beforeEach(() => { + jest.clearAllMocks(); + invalidateLinearSecretCache(); + _resetCachesForTesting(); + _resetActiveWorkspaceCountCache(); + smSend.mockImplementation((cmd: { _type: string; input: { SecretId?: string } }) => { + if (cmd.input.SecretId === STACK_WIDE_ARN) return Promise.resolve({ SecretString: STACK_WIDE_SECRET }); + return Promise.resolve({ SecretString: bundle(OWN_SECRET) }); + }); + lambdaSend.mockResolvedValue({}); +}); + +describe('a secret shared with another workspace is not proof of this one', () => { + test('rejects when the row does not record the secret as its own and a second tenant exists', async () => { + withRegistry({ activeCount: 2 }); + + const res = await handler(event(body(), OWN_SECRET)); + + expect(res.statusCode).toBe(401); + expect(JSON.parse(res.body).error).toMatch(/not its own/); + expect(lambdaSend).not.toHaveBeenCalled(); + }); + + test('accepts the very same request once ownership is recorded', async () => { + // The signature, the secret and the body are identical to the case above — only the + // provenance flag differs. That isolates the flag as the thing being tested rather + // than some incidental difference in the request. + withRegistry({ + activeCount: 2, + row: { + linear_workspace_id: WS_ACME, + workspace_slug: 'acme', + oauth_secret_arn: OAUTH_ARN, + status: 'active', + webhook_secret_owned: true, + }, + }); + + const res = await handler(event(body(), OWN_SECRET)); + + expect(res.statusCode).toBe(200); + expect(lambdaSend).toHaveBeenCalled(); + }); + + test('accepts an unrecorded provenance on a single-workspace stack', async () => { + // Absent provenance is the normal state for every row written before it existed. + // With one tenant the same secret cannot cross a boundary, so rejecting here would + // break healthy installs to no benefit. + withRegistry({ activeCount: 1 }); + + const res = await handler(event(body(), OWN_SECRET)); + + expect(res.statusCode).toBe(200); + }); +}); + +describe('the stack-wide fallback cannot name a tenant among several', () => { + test('refuses the fallback when more than one workspace is active', async () => { + // No per-workspace secret for this workspace, so verification falls through — and on + // a multi-tenant stack the stack-wide secret cannot say who sent this. + withRegistry({ activeCount: 2, row: null }); + + const res = await handler(event(body(), STACK_WIDE_SECRET)); + + expect(res.statusCode).toBe(401); + expect(JSON.parse(res.body).error).toMatch(/Per-workspace signing secret required/); + expect(lambdaSend).not.toHaveBeenCalled(); + }); + + test('keeps the fallback working for a single-workspace install', async () => { + withRegistry({ activeCount: 1, row: null }); + + const res = await handler(event(body(), STACK_WIDE_SECRET)); + + expect(res.statusCode).toBe(200); + }); + + test('tells the processor the fallback is what verified the delivery', async () => { + withRegistry({ activeCount: 1, row: null }); + + await handler(event(body(), STACK_WIDE_SECRET)); + + const invoked = lambdaSend.mock.calls[0][0] as { input: { Payload: Uint8Array } }; + const forwarded = JSON.parse(new TextDecoder().decode(invoked.input.Payload)) as Record; + expect(forwarded.verified_via_stack_wide).toBe(true); + }); + + test('reports false when a per-workspace secret verified the delivery', async () => { + // The processor binds the workspace only on the stack-wide path, so a false positive + // here would make it discard a correctly attested organizationId and rebind it. + withRegistry({ + activeCount: 1, + row: { + linear_workspace_id: WS_ACME, + workspace_slug: 'acme', + oauth_secret_arn: OAUTH_ARN, + status: 'active', + webhook_secret_owned: true, + }, + }); + + await handler(event(body(), OWN_SECRET)); + + const invoked = lambdaSend.mock.calls[0][0] as { input: { Payload: Uint8Array } }; + const forwarded = JSON.parse(new TextDecoder().decode(invoked.input.Payload)) as Record; + expect(forwarded.verified_via_stack_wide).toBe(false); + }); +}); diff --git a/cdk/test/handlers/shared/linear-oauth-resolver.test.ts b/cdk/test/handlers/shared/linear-oauth-resolver.test.ts index 73b1d16dc..6a12bfa10 100644 --- a/cdk/test/handlers/shared/linear-oauth-resolver.test.ts +++ b/cdk/test/handlers/shared/linear-oauth-resolver.test.ts @@ -23,6 +23,8 @@ import { clearWorkspaceRevocation, getOauthSecret, getOauthSecretStrict, + getRegistryRowStrict, + resolveSoleActiveLinearWorkspace, invalidateLinearOauthCache, isRefreshTokenRejection, isTokenExpiring, @@ -1323,3 +1325,94 @@ describe('a vault-managed bundle carries no grant, and must still verify webhook expect(fetched).toBeNull(); }); }); + +describe('webhook_secret_owned — provenance, not value equality', () => { + beforeEach(() => _resetCachesForTesting()); + + function rowWith(extra: Record) { + return { + send: jest.fn().mockResolvedValue({ + Item: { + linear_workspace_id: 'ws-1', + workspace_slug: 'acme', + oauth_secret_arn: 'arn:secret:acme', + status: 'active', + ...extra, + }, + }), + } as unknown as Parameters[0]; + } + + test('carries a recorded true through to the reader', async () => { + const row = await getRegistryRowStrict(rowWith({ webhook_secret_owned: true }), 'registry', 'ws-1'); + expect(row?.webhook_secret_owned).toBe(true); + }); + + test('leaves the field absent when the row predates it', async () => { + // Absent must not read as `false`: the reader distinguishes "not proven" from + // "proven not owned", and every row written before this field is the former. + const row = await getRegistryRowStrict(rowWith({}), 'registry', 'ws-1'); + expect(row?.webhook_secret_owned).toBeUndefined(); + }); + + test.each([['false-ish string', 'true'], ['number', 1], ['explicit false', false]] as const)( + 'refuses to treat a %s as proof of ownership', + async (_label, value) => { + // Only a literal boolean `true` counts. A truthy-but-not-true value reaching this + // field would otherwise silently grant the workspace the same standing as one the + // operator actually supplied a secret for. + const row = await getRegistryRowStrict(rowWith({ webhook_secret_owned: value }), 'registry', 'ws-1'); + expect(row?.webhook_secret_owned).toBeUndefined(); + }, + ); +}); + +describe('resolveSoleActiveLinearWorkspace — bind only when the answer is unambiguous', () => { + function scanning(pages: Array<{ Items: Array>; LastEvaluatedKey?: Record }>) { + let i = 0; + return { send: jest.fn(async () => pages[i++]) } as unknown as Parameters[0]; + } + + test('returns the id when exactly one workspace is active', async () => { + const ddb = scanning([{ Items: [{ linear_workspace_id: 'org-only', status: 'active' }] }]); + await expect(resolveSoleActiveLinearWorkspace(ddb, 'registry')).resolves.toBe('org-only'); + }); + + test('returns undefined when two are active, rather than picking the first', async () => { + const ddb = scanning([{ + Items: [ + { linear_workspace_id: 'org-a', status: 'active' }, + { linear_workspace_id: 'org-b', status: 'active' }, + ], + }]); + await expect(resolveSoleActiveLinearWorkspace(ddb, 'registry')).resolves.toBeUndefined(); + }); + + test('returns undefined on an empty registry', async () => { + await expect(resolveSoleActiveLinearWorkspace(scanning([{ Items: [] }]), 'registry')).resolves.toBeUndefined(); + }); + + test('ignores revoked rows when deciding whether the answer is unique', async () => { + const ddb = scanning([{ + Items: [ + { linear_workspace_id: 'org-live', status: 'active' }, + { linear_workspace_id: 'org-dead', status: 'revoked' }, + ], + }]); + await expect(resolveSoleActiveLinearWorkspace(ddb, 'registry')).resolves.toBe('org-live'); + }); + + test('follows pagination before concluding the answer is unique', async () => { + // A single-page read would report org-a as the sole workspace and bind deliveries to + // it, on a stack that actually has two tenants. + const ddb = scanning([ + { Items: [{ linear_workspace_id: 'org-a', status: 'active' }], LastEvaluatedKey: { k: 1 } }, + { Items: [{ linear_workspace_id: 'org-b', status: 'active' }] }, + ]); + await expect(resolveSoleActiveLinearWorkspace(ddb, 'registry')).resolves.toBeUndefined(); + }); + + test('returns undefined when no registry table is configured', async () => { + await expect(resolveSoleActiveLinearWorkspace(scanning([]), undefined)).resolves.toBeUndefined(); + }); +}); diff --git a/cdk/test/handlers/shared/linear-verify.test.ts b/cdk/test/handlers/shared/linear-verify.test.ts index c708cd5d4..59f110205 100644 --- a/cdk/test/handlers/shared/linear-verify.test.ts +++ b/cdk/test/handlers/shared/linear-verify.test.ts @@ -27,11 +27,16 @@ jest.mock('@aws-sdk/client-secrets-manager', () => ({ jest.mock('@aws-sdk/client-dynamodb', () => ({ DynamoDBClient: jest.fn(() => ({})), })); +const ddbSend = jest.fn(); jest.mock('@aws-sdk/lib-dynamodb', () => ({ - DynamoDBDocumentClient: { from: jest.fn(() => ({ send: jest.fn() })) }, + DynamoDBDocumentClient: { from: jest.fn(() => ({ send: ddbSend })) }, + GetCommand: jest.fn((input: unknown) => ({ _type: 'Get', input })), + ScanCommand: jest.fn((input: unknown) => ({ _type: 'Scan', input })), })); import { + _resetActiveWorkspaceCountCache, + countActiveLinearWorkspaces, getLinearSecret, invalidateLinearSecretCache, isWebhookTimestampFresh, @@ -187,3 +192,49 @@ describe('isWebhookTimestampFresh', () => { expect(isWebhookTimestampFresh(Infinity)).toBe(false); }); }); + +describe('countActiveLinearWorkspaces — one tenant versus more than one', () => { + beforeEach(() => { + ddbSend.mockReset(); + _resetActiveWorkspaceCountCache(); + }); + + test('counts only active rows', async () => { + ddbSend.mockResolvedValue({ + Items: [ + { linear_workspace_id: 'a', status: 'active' }, + { linear_workspace_id: 'b', status: 'revoked' }, + ], + }); + await expect(countActiveLinearWorkspaces('registry')).resolves.toBe(1); + }); + + test('reports more than one so callers can refuse a tenant-less secret', async () => { + ddbSend.mockResolvedValue({ + Items: [ + { linear_workspace_id: 'a', status: 'active' }, + { linear_workspace_id: 'b', status: 'active' }, + ], + }); + expect(await countActiveLinearWorkspaces('registry')).toBeGreaterThan(1); + }); + + test('caches, so the Scan does not run on every delivery', async () => { + ddbSend.mockResolvedValue({ Items: [{ linear_workspace_id: 'a', status: 'active' }] }); + await countActiveLinearWorkspaces('registry'); + await countActiveLinearWorkspaces('registry'); + expect(ddbSend).toHaveBeenCalledTimes(1); + }); + + test('answers 1 when the registry cannot be read, rather than rejecting every delivery', async () => { + // Permissive on purpose. The callers are hardening a signature that already + // verified; a DynamoDB throttle must not turn into a stack-wide outage. + ddbSend.mockRejectedValue(new Error('ProvisionedThroughputExceededException')); + await expect(countActiveLinearWorkspaces('registry')).resolves.toBe(1); + }); + + test('answers 1 when no registry table is configured', async () => { + await expect(countActiveLinearWorkspaces(undefined)).resolves.toBe(1); + expect(ddbSend).not.toHaveBeenCalled(); + }); +}); diff --git a/cdk/test/integration/linear-tenant-binding-e2e.test.ts b/cdk/test/integration/linear-tenant-binding-e2e.test.ts new file mode 100644 index 000000000..4c19e3c37 --- /dev/null +++ b/cdk/test/integration/linear-tenant-binding-e2e.test.ts @@ -0,0 +1,730 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +/** + * Linear workspace binding, end to end. + * + * A delivery runs through the REAL receiver, the receiver's processor invoke is + * handed to the REAL processor, and the processor runs the REAL token resolver, + * user lookup, orchestration store and `createTaskCore`. Every table and secret is + * one stateful fake shared by both handlers, so the receiver's workspace count and + * the processor's binding read the same registry — which is what the per-handler + * unit tests cannot show. + * + * Each case asserts the outcome that matters: whether a task record was persisted, + * and for which repository, platform user and workspace. Only the modules that call + * Linear's API are mocked; network access is otherwise refused. + * + * Tenants: + * A — owns its signing secret (recorded). + * B — owns its signing secret (recorded). + * C — onboarded before per-workspace secrets: verifies via the stack-wide secret. + * D — holds a copy of A's secret, ownership not recorded. + */ + +import * as crypto from 'crypto'; +import type { APIGatewayProxyEvent } from 'aws-lambda'; + +// ── Stateful fakes ────────────────────────────────────────────────────── +type Item = Record; + +const TABLE = { + registry: 'LinearWorkspaceRegistry', + projects: 'LinearProjects', + users: 'LinearUsers', + dedup: 'LinearDedup', + tasks: 'TaskTable', + events: 'TaskEventsTable', + orch: 'OrchestrationTable', +} as const; + +const KEY_SCHEMA: Record = { + [TABLE.registry]: ['linear_workspace_id'], + [TABLE.projects]: ['linear_project_id'], + [TABLE.users]: ['linear_identity'], + [TABLE.dedup]: ['dedup_key'], + [TABLE.tasks]: ['task_id'], + [TABLE.events]: ['task_id', 'event_id'], + [TABLE.orch]: ['orchestration_id', 'sub_issue_id'], +}; + +const mockTables = new Map>(); +const mockSecrets = new Map(); +const mockInvokes: Array<{ FunctionName: string; payload: Record }> = []; +const mockLogs: Array<{ level: string; message: string; ctx?: Record }> = []; + +function table(name: string): Map { + if (!mockTables.has(name)) mockTables.set(name, new Map()); + return mockTables.get(name)!; +} +function keyFor(tableName: string, item: Item): string { + const schema = KEY_SCHEMA[tableName]; + if (!schema) throw new Error(`fake DDB: no key schema for table ${tableName}`); + return schema.map((k) => String(item[k])).join('\u0000'); +} +function conditionalFail(): never { + const e = new Error('The conditional request failed'); + e.name = 'ConditionalCheckFailedException'; + throw e; +} +function resolveName(token: string, names?: Record): string { + return token.startsWith('#') ? (names?.[token] ?? token) : token; +} +/** `attribute_not_exists(x)` / `attribute_exists(x)` — the only conditions these paths rely on. */ +function checkCondition(expr: string | undefined, existing: Item | undefined, names?: Record): void { + if (!expr) return; + const notExists = /attribute_not_exists\(([#\w]+)\)/.exec(expr); + if (notExists && existing && existing[resolveName(notExists[1], names)] !== undefined) conditionalFail(); + const exists = /(?): (i: Item) => boolean { + const clauses = expr.split(/\s+AND\s+/i).map((c) => /([#\w]+)\s*=\s*(:\w+)/.exec(c)).filter(Boolean) as RegExpExecArray[]; + return (i) => clauses.every((m) => i[resolveName(m[1], names)] === values[m[2]]); +} + +const mockDdbSend = jest.fn(async (cmd: { _type: string; input: Item }) => { + const input = cmd.input; + const tn = input.TableName as string; + const names = input.ExpressionAttributeNames as Record | undefined; + const values = (input.ExpressionAttributeValues ?? {}) as Item; + const t = table(tn); + switch (cmd._type) { + case 'Get': + return { Item: t.get(keyFor(tn, input.Key as Item)) }; + case 'Put': { + const item = input.Item as Item; + checkCondition(input.ConditionExpression as string | undefined, t.get(keyFor(tn, item)), names); + t.set(keyFor(tn, item), { ...item }); + return {}; + } + case 'Delete': + t.delete(keyFor(tn, input.Key as Item)); + return {}; + case 'Scan': + return { Items: [...t.values()].map((i) => ({ ...i })) }; + case 'Query': { + let items = [...t.values()].filter(keyMatcher(input.KeyConditionExpression as string, values, names)); + if (input.IndexName === 'LinearIssueIndex' || input.ScanIndexForward === false) { + items = items.sort((a, b) => String(b.created_at ?? '').localeCompare(String(a.created_at ?? ''))); + } + if (typeof input.Limit === 'number') items = items.slice(0, input.Limit); + return { Items: items.map((i) => ({ ...i })) }; + } + case 'Update': { + const k = keyFor(tn, input.Key as Item); + const existing = t.get(k); + checkCondition(input.ConditionExpression as string | undefined, existing, names); + const next: Item = { ...(existing ?? (input.Key as Item)) }; + const set = /SET\s+(.+?)(?:\s+(?:REMOVE|ADD|DELETE)\s|$)/i.exec(String(input.UpdateExpression ?? '')); + for (const part of set ? set[1].split(',') : []) { + const m = /^\s*([#\w.]+)\s*=\s*(:\w+)\s*$/.exec(part); + if (m) next[resolveName(m[1], names)] = values[m[2]]; + } + t.set(k, next); + return {}; + } + case 'BatchWrite': + for (const [tbl, reqs] of Object.entries(input.RequestItems as Record>)) { + for (const r of reqs) if (r.PutRequest) table(tbl).set(keyFor(tbl, r.PutRequest.Item), r.PutRequest.Item); + } + return {}; + default: + throw new Error(`fake DDB: unhandled ${cmd._type}`); + } +}); + +jest.mock('@aws-sdk/client-dynamodb', () => ({ DynamoDBClient: jest.fn(() => ({})) })); +jest.mock('@aws-sdk/lib-dynamodb', () => { + const cmd = (t: string) => jest.fn((input: unknown) => ({ _type: t, input })); + return { + DynamoDBDocumentClient: { from: jest.fn(() => ({ send: (c: never) => mockDdbSend(c) })) }, + GetCommand: cmd('Get'), + PutCommand: cmd('Put'), + DeleteCommand: cmd('Delete'), + ScanCommand: cmd('Scan'), + QueryCommand: cmd('Query'), + UpdateCommand: cmd('Update'), + BatchWriteCommand: cmd('BatchWrite'), + }; +}); + +jest.mock('@aws-sdk/client-secrets-manager', () => ({ + SecretsManagerClient: jest.fn(() => ({ + send: async (c: { _type: string; input: { SecretId: string; SecretString?: string } }) => { + if (c._type === 'Put') { + mockSecrets.set(c.input.SecretId, c.input.SecretString!); + return {}; + } + const v = mockSecrets.get(c.input.SecretId); + if (v === undefined) { + const e = new Error('Secrets Manager can\'t find the specified secret.'); + e.name = 'ResourceNotFoundException'; + throw e; + } + return { SecretString: v }; + }, + })), + GetSecretValueCommand: jest.fn((input: unknown) => ({ _type: 'Get', input })), + PutSecretValueCommand: jest.fn((input: unknown) => ({ _type: 'Put', input })), +})); + +jest.mock('@aws-sdk/client-lambda', () => ({ + LambdaClient: jest.fn(() => ({ + send: async (c: { input: { FunctionName: string; Payload: Uint8Array } }) => { + mockInvokes.push({ + FunctionName: c.input.FunctionName, + payload: JSON.parse(new TextDecoder().decode(c.input.Payload)) as Record, + }); + return { StatusCode: 202 }; + }, + })), + InvokeCommand: jest.fn((input: unknown) => ({ input })), +})); + +jest.mock('./../../src/handlers/shared/logger', () => { + const rec = (level: string) => (message: string, ctx?: Record) => { + mockLogs.push({ level, message, ctx }); + }; + return { logger: { debug: rec('debug'), info: rec('info'), warn: rec('warn'), error: rec('error') } }; +}); + +// ── Linear API boundary ──────────────────────────────────────────────── +const mockFeedback = { + reportIssueFailure: jest.fn(async () => true), + swapIssueReaction: jest.fn(async () => true), + swapCommentReaction: jest.fn(async () => true), + transitionIssueState: jest.fn(async () => true), + upsertStatusComment: jest.fn(async () => 'status-comment-1'), + reactToComment: jest.fn(async () => true), + replyToComment: jest.fn(async () => true), + upsertThreadedReply: jest.fn(async () => 'reply-1'), + fetchRecentComments: jest.fn(async () => []), + postIssueComment: jest.fn(async () => true), +}; +jest.mock('../../src/handlers/shared/linear-feedback', () => ({ + ...mockFeedback, + EMOJI_STARTED: 'eyes', + EMOJI_SUCCESS: 'white_check_mark', + EMOJI_FAILURE: 'x', + EMOJI_NEEDS_INPUT: 'question', +})); + +const mockParentOf = new Map(); +jest.mock('../../src/handlers/shared/linear-subissue-fetch', () => { + const lookup = jest.requireActual('../../src/handlers/shared/lookup-result'); + return { + fetchSubIssueGraph: jest.fn(async () => ({ kind: 'no_children' })), + fetchIssueParentId: jest.fn(async (_token: string, issueId: string) => { + const parent = mockParentOf.get(issueId); + return parent ? lookup.lookupFound(parent) : lookup.LOOKUP_ABSENT; + }), + }; +}); + +jest.mock('../../src/handlers/shared/linear-issue-context-probe', () => ({ + probeLinearIssueContext: jest.fn(async () => ({ + attachmentTitles: [], + attachments: [], + projectName: null, + projectHasDocuments: false, + projectDocuments: [], + ok: true, + projectDocumentCount: 0, + })), + renderIssueContextHint: jest.fn(() => ''), +})); + +// ── Environment (read at module load) ─────────────────────────────────── +const STACK_WIDE_ARN = 'arn:aws:secretsmanager:us-east-1:000000000000:secret:bgagent/linear/webhook-TEST'; +process.env.LINEAR_WEBHOOK_SECRET_ARN = STACK_WIDE_ARN; +process.env.LINEAR_WEBHOOK_DEDUP_TABLE_NAME = TABLE.dedup; +process.env.LINEAR_WEBHOOK_PROCESSOR_FUNCTION_NAME = 'linear-processor'; +process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME = TABLE.registry; +process.env.LINEAR_PROJECT_MAPPING_TABLE_NAME = TABLE.projects; +process.env.LINEAR_USER_MAPPING_TABLE_NAME = TABLE.users; +process.env.TASK_TABLE_NAME = TABLE.tasks; +process.env.TASK_EVENTS_TABLE_NAME = TABLE.events; +process.env.ORCHESTRATION_TABLE_NAME = TABLE.orch; + +import { handler as receiver } from '../../src/handlers/linear-webhook'; +import { handler as processor } from '../../src/handlers/linear-webhook-processor'; +import { _resetCachesForTesting } from '../../src/handlers/shared/linear-oauth-resolver'; +import * as linearVerify from '../../src/handlers/shared/linear-verify'; +import { deriveOrchestrationId } from '../../src/handlers/shared/orchestration-store'; + +// Optional so this file also runs against code that predates the count cache, which +// is how it was checked to fail on the unfixed handlers. +const _resetActiveWorkspaceCountCache = (): void => { + (linearVerify as { _resetActiveWorkspaceCountCache?: () => void })._resetActiveWorkspaceCountCache?.(); +}; +const invalidateLinearSecretCache = linearVerify.invalidateLinearSecretCache; + +// ── Tenants ───────────────────────────────────────────────────────────── +// UUID-shaped ids whose last group is not twelve digits, so nothing here reads as +// an account number to the repo's scanners. +const STACK_WIDE_SECRET = 'lin_wh_stackwide_0000000000000000'; +interface Tenant { id: string; slug: string; secret?: string; owned: boolean; repo: string; project: string; actor: string; platformUser: string } +const T: Record<'A' | 'B' | 'C' | 'D', Tenant> = { + A: { id: 'aaaaaaaa-0000-4000-8000-aaaaaaaaaaaa', slug: 'ws-a', secret: 'lin_wh_a_aaaaaaaaaaaaaaaaaaaaaaaa', owned: true, repo: 'org-a/repo-a', project: 'proj-a', actor: 'user-a', platformUser: 'platform-a' }, + B: { id: 'bbbbbbbb-0000-4000-8000-bbbbbbbbbbbb', slug: 'ws-b', secret: 'lin_wh_b_bbbbbbbbbbbbbbbbbbbbbbbb', owned: true, repo: 'org-b/repo-b', project: 'proj-b', actor: 'user-b', platformUser: 'platform-b' }, + C: { id: 'cccccccc-0000-4000-8000-cccccccccccc', slug: 'ws-c', owned: false, repo: 'org-c/repo-c', project: 'proj-c', actor: 'user-c', platformUser: 'platform-c' }, + D: { id: 'dddddddd-0000-4000-8000-dddddddddddd', slug: 'ws-d', secret: 'lin_wh_a_aaaaaaaaaaaaaaaaaaaaaaaa', owned: false, repo: 'org-d/repo-d', project: 'proj-d', actor: 'user-d', platformUser: 'platform-d' }, +}; +const UNREGISTERED = 'eeeeeeee-0000-4000-8000-eeeeeeeeeeee'; + +function oauthArn(t: Tenant): string { + return `arn:aws:secretsmanager:us-east-1:000000000000:secret:bgagent-linear-oauth-${t.slug}`; +} + +function onboard(t: Tenant, opts: { status?: string; withProject?: boolean; projectOwner?: string | null } = {}): void { + const now = new Date().toISOString(); + mockSecrets.set(oauthArn(t), JSON.stringify({ + access_token: `lin_oauth_${t.slug}`, + refresh_token: `lin_refresh_${t.slug}`, + expires_at: '2099-01-01T00:00:00.000Z', + scope: 'read write', + client_id: 'cid', + client_secret: 'csec', + workspace_id: t.id, + workspace_slug: t.slug, + installed_at: now, + updated_at: now, + installed_by_platform_user_id: t.platformUser, + ...(t.secret && { webhook_signing_secret: t.secret }), + })); + table(TABLE.registry).set(t.id, { + linear_workspace_id: t.id, + workspace_slug: t.slug, + oauth_secret_arn: oauthArn(t), + status: opts.status ?? 'active', + installed_at: now, + ...(t.owned && { webhook_secret_owned: true }), + }); + table(TABLE.users).set(`${t.id}#${t.actor}`, { + linear_identity: `${t.id}#${t.actor}`, platform_user_id: t.platformUser, status: 'active', + }); + if (opts.withProject !== false) mapProject(t.project, t.repo, opts.projectOwner === undefined ? t.id : opts.projectOwner); +} + +function mapProject(projectId: string, repo: string, ownerWorkspaceId: string | null): void { + table(TABLE.projects).set(projectId, { + linear_project_id: projectId, + repo, + status: 'active', + label_filter: 'bgagent', + ...(ownerWorkspaceId && { linear_workspace_id: ownerWorkspaceId }), + }); +} + +/** An orchestration owned by `owner`: an epic with one started child that has a PR. */ +function seedEpic(owner: Tenant, epicId: string, childId: string, childIdentifier: string): void { + const orchestrationId = deriveOrchestrationId(epicId); + const base = { + orchestration_id: orchestrationId, + parent_issue_ref: epicId, + credentials_ref: owner.id, + repo: owner.repo, + }; + const t = table(TABLE.orch); + t.set(`${orchestrationId}\u0000#meta`, { ...base, sub_issue_id: '#meta', child_count: 1, platform_user_id: owner.platformUser }); + t.set(`${orchestrationId}\u0000${childId}`, { + ...base, + sub_issue_id: childId, + depends_on: [], + child_status: 'succeeded', + linear_identifier: childIdentifier, + title: 'Child work', + child_task_id: `task-${childId}`, + }); + table(TABLE.tasks).set(`task-${childId}`, { task_id: `task-${childId}`, repo: owner.repo, user_id: owner.platformUser, pr_number: 41, status: 'COMPLETED' }); + mockParentOf.set(childId, epicId); +} + +/** A plain (non-orchestration) issue that already has a task with a PR, owned by `owner`. */ +function seedStandaloneTask(owner: Tenant, issueId: string): void { + table(TABLE.tasks).set(`task-${issueId}`, { + task_id: `task-${issueId}`, + linear_issue_id: issueId, + created_at: '2026-10-01T00:00:00.000Z', + repo: owner.repo, + user_id: owner.platformUser, + pr_number: 52, + status: 'COMPLETED', + channel_metadata: { linear_workspace_id: owner.id }, + }); +} + +// ── Deliveries ────────────────────────────────────────────────────────── +let seq = 0; +function issueEvent(org: string | undefined, projectId: string, actor: string): string { + seq += 1; + return JSON.stringify({ + action: 'create', + type: 'Issue', + ...(org !== undefined && { organizationId: org }), + webhookTimestamp: Date.now(), + actor: { id: actor, type: 'user' }, + data: { + id: `issue-${seq}`, + identifier: `ISS-${seq}`, + title: `Issue ${seq}`, + description: 'Do the thing.', + projectId, + teamId: 'team-1', + labels: [{ id: 'lbl', name: 'bgagent' }], + }, + }); +} +function commentEvent(org: string, issueId: string, actor: string, text: string): string { + seq += 1; + return JSON.stringify({ + action: 'create', + type: 'Comment', + organizationId: org, + webhookTimestamp: Date.now(), + actor: { id: actor, type: 'user' }, + data: { id: `comment-${seq}`, body: text, issueId }, + }); +} + +interface Outcome { + status: number; + error?: string; + /** What the receiver told the processor, when it invoked it. */ + forwarded?: Record; + /** Tasks persisted by this delivery. */ + tasks: Item[]; + /** Warn/error log messages emitted while handling it. */ + warnings: string[]; +} + +async function deliver(raw: string, signingKey: string, tamper?: (raw: string) => string): Promise { + const before = new Set(table(TABLE.tasks).keys()); + const logStart = mockLogs.length; + const invokeStart = mockInvokes.length; + const signature = crypto.createHmac('sha256', signingKey).update(raw).digest('hex'); + const res = await receiver({ + body: tamper ? tamper(raw) : raw, + headers: { 'linear-signature': signature }, + httpMethod: 'POST', + } as unknown as APIGatewayProxyEvent); + + const invoked = mockInvokes.slice(invokeStart).find((i) => i.FunctionName === 'linear-processor'); + if (invoked) await processor(invoked.payload as never); + + const tasks = [...table(TABLE.tasks).entries()].filter(([k]) => !before.has(k)).map(([, v]) => v); + return { + status: res.statusCode, + error: res.statusCode >= 400 ? (JSON.parse(res.body) as { error?: string }).error : undefined, + forwarded: invoked?.payload, + tasks, + warnings: mockLogs.slice(logStart).filter((l) => l.level === 'warn' || l.level === 'error').map((l) => l.message), + }; +} + +function expectNoTask(o: Outcome): void { + expect(o.tasks).toEqual([]); +} +function expectOneTask(o: Outcome, t: Tenant): Item { + expect(o.status).toBe(200); + expect(o.tasks).toHaveLength(1); + const task = o.tasks[0]; + expect(task.repo).toBe(t.repo); + expect(task.user_id).toBe(t.platformUser); + expect((task.channel_metadata as Record).linear_workspace_id).toBe(t.id); + return task; +} +/** No Linear write of any kind — a silent drop tells the sender nothing. */ +function expectNoLinearWrites(): void { + for (const fn of Object.values(mockFeedback)) { + if (fn === mockFeedback.fetchRecentComments) continue; + expect(fn).not.toHaveBeenCalled(); + } +} + +beforeEach(() => { + mockTables.clear(); + mockSecrets.clear(); + mockInvokes.length = 0; + mockLogs.length = 0; + mockParentOf.clear(); + for (const fn of Object.values(mockFeedback)) fn.mockClear(); + invalidateLinearSecretCache(); + _resetCachesForTesting(); + _resetActiveWorkspaceCountCache(); + mockSecrets.set(STACK_WIDE_ARN, STACK_WIDE_SECRET); + // Nothing in these flows may reach the network; a stray call fails loudly. + global.fetch = jest.fn(async () => { throw new Error('unexpected network call in e2e test'); }) as never; +}); + +// ═══════════════════════════════════════════════════════════════════════ +describe('several workspaces: the signature must attest the workspace', () => { + beforeEach(() => { + onboard(T.A); + onboard(T.B); + onboard(T.C); + onboard(T.D); + }); + + test('control: a workspace signing its own delivery for its own project gets a task', async () => { + const o = await deliver(issueEvent(T.A.id, T.A.project, T.A.actor), T.A.secret!); + expectOneTask(o, T.A); + expect(o.forwarded?.verified_via_stack_wide).toBe(false); + }); + + test('control: the same holds for the other owned workspace', async () => { + expectOneTask(await deliver(issueEvent(T.B.id, T.B.project, T.B.actor), T.B.secret!), T.B); + }); + + test('the stack-wide secret naming a workspace with no secret of its own is refused, even with that workspace\'s project and user', async () => { + const o = await deliver(issueEvent(T.C.id, T.C.project, T.C.actor), STACK_WIDE_SECRET); + expectNoTask(o); + expect(o.status).toBe(401); + expect(o.error).toMatch(/Per-workspace signing secret required/); + expect(o.forwarded).toBeUndefined(); + }); + + test('the stack-wide secret naming another workspace\'s project and user is refused', async () => { + const o = await deliver(issueEvent(T.C.id, T.B.project, T.B.actor), STACK_WIDE_SECRET); + expectNoTask(o); + expect(o.status).toBe(401); + }); + + test('the stack-wide secret naming a workspace that has its own secret is a mismatch', async () => { + const o = await deliver(issueEvent(T.B.id, T.B.project, T.B.actor), STACK_WIDE_SECRET); + expectNoTask(o); + expect(o.status).toBe(401); + expect(o.error).toMatch(/Invalid signature/); + }); + + test('the stack-wide secret naming an unregistered workspace is refused', async () => { + const o = await deliver(issueEvent(UNREGISTERED, T.B.project, T.B.actor), STACK_WIDE_SECRET); + expectNoTask(o); + expect(o.status).toBe(401); + }); + + test('the stack-wide secret with no organizationId at all is refused', async () => { + const o = await deliver(issueEvent(undefined, T.B.project, T.B.actor), STACK_WIDE_SECRET); + expectNoTask(o); + expect(o.status).toBe(401); + }); + + test('one workspace\'s secret cannot sign for another workspace', async () => { + const o = await deliver(issueEvent(T.B.id, T.B.project, T.B.actor), T.A.secret!); + expectNoTask(o); + expect(o.status).toBe(401); + }); + + test('a secret held by two workspaces attests neither, whichever one is named', async () => { + // D holds a copy of A's secret. Signing with it and naming D must not pass, + // because the signer could equally be A. + const asD = await deliver(issueEvent(T.D.id, T.D.project, T.D.actor), T.D.secret!); + expectNoTask(asD); + expect(asD.status).toBe(401); + expect(asD.error).toMatch(/not its own/); + }); + + test('a body altered after signing is refused', async () => { + const o = await deliver( + issueEvent(T.A.id, T.A.project, T.A.actor), + T.A.secret!, + (raw) => raw.replace(T.A.project, T.B.project), + ); + expectNoTask(o); + expect(o.status).toBe(401); + }); + + test('a correctly signed delivery naming another workspace\'s project creates nothing', async () => { + const o = await deliver(issueEvent(T.A.id, T.B.project, T.A.actor), T.A.secret!); + expectNoTask(o); + expect(o.status).toBe(200); + expect(o.warnings).toContain('Linear project is mapped to a different workspace than this webhook — dropping'); + expectNoLinearWrites(); + }); + + test('...and naming the other workspace\'s user as the actor changes nothing', async () => { + // User links are keyed by workspace, so B's user does not exist inside A. + const o = await deliver(issueEvent(T.A.id, T.B.project, T.B.actor), T.A.secret!); + expectNoTask(o); + }); + + test('a correctly signed delivery with an actor from another workspace does not run as that user', async () => { + const o = await deliver(issueEvent(T.A.id, T.A.project, T.B.actor), T.A.secret!); + expectNoTask(o); + }); + + test('a project mapped before owners were recorded is still served (reported by platform doctor)', async () => { + // Pins the documented back-compat behaviour so a change to it is deliberate. + mapProject('proj-legacy', 'org-legacy/repo', null); + const o = await deliver(issueEvent(T.A.id, 'proj-legacy', T.A.actor), T.A.secret!); + expect(o.tasks).toHaveLength(1); + expect(o.warnings).toContain('Linear project mapping records no owning workspace — cannot verify the tenant'); + }); + + test('a revoked workspace is refused even with its own secret', async () => { + table(TABLE.registry).get(T.B.id)!.status = 'revoked'; + _resetCachesForTesting(); + const o = await deliver(issueEvent(T.B.id, T.B.project, T.B.actor), T.B.secret!); + expectNoTask(o); + expect(o.status).toBe(401); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════ +describe('one workspace: the stack-wide secret binds to it, never to the body', () => { + beforeEach(() => { + // A single install that predates per-workspace secrets: C has no secret of its own. + onboard(T.C); + }); + + test('control: the stack-wide fallback still works for the only workspace', async () => { + const o = await deliver(issueEvent(T.C.id, T.C.project, T.C.actor), STACK_WIDE_SECRET); + expectOneTask(o, T.C); + expect(o.forwarded?.verified_via_stack_wide).toBe(true); + }); + + test('a claimed organizationId is replaced by the only workspace there is', async () => { + const o = await deliver(issueEvent(UNREGISTERED, T.C.project, T.C.actor), STACK_WIDE_SECRET); + const task = expectOneTask(o, T.C); + expect((task.channel_metadata as Record).linear_workspace_id).not.toBe(UNREGISTERED); + expect(o.warnings).toContain('Ignoring body organizationId on a stack-wide-verified delivery; binding to the sole active workspace'); + }); + + test('a lingering mapping from a workspace that is no longer active is not served', async () => { + onboard(T.B, { status: 'revoked' }); + _resetActiveWorkspaceCountCache(); + const o = await deliver(issueEvent(UNREGISTERED, T.B.project, T.C.actor), STACK_WIDE_SECRET); + expectNoTask(o); + expect(o.warnings).toContain('Linear project is mapped to a different workspace than this webhook — dropping'); + }); + + test('a revoked workspace cannot ride the stack-wide fallback', async () => { + onboard(T.B, { status: 'revoked' }); + const o = await deliver(issueEvent(T.B.id, T.B.project, T.B.actor), STACK_WIDE_SECRET); + expectNoTask(o); + expect(o.status).toBe(401); + }); + + test('a second workspace arriving inside the receiver\'s cache window is caught by the processor', async () => { + // Warm the receiver's count cache at 1. + await deliver(issueEvent(T.C.id, T.C.project, T.C.actor), STACK_WIDE_SECRET); + // A second workspace is onboarded; the receiver still believes there is one. + onboard(T.A); + const o = await deliver(issueEvent(T.A.id, T.A.project, T.A.actor).replace(T.A.id, UNREGISTERED), STACK_WIDE_SECRET); + expectNoTask(o); + // The receiver lets it through on its stale count, flagged as stack-wide… + expect(o.status).toBe(200); + expect(o.forwarded?.verified_via_stack_wide).toBe(true); + // …and the processor, reading the registry itself, refuses to pick a tenant. + expect(o.warnings).toContain('Dropping stack-wide-verified Linear delivery: cannot determine the sending workspace'); + }); + + test('once the cache expires the receiver refuses the fallback outright', async () => { + onboard(T.A); + _resetActiveWorkspaceCountCache(); + const o = await deliver(issueEvent(T.C.id, T.C.project, T.C.actor), STACK_WIDE_SECRET); + expectNoTask(o); + expect(o.status).toBe(401); + }); + + test('an unrecorded secret provenance is fine while there is only one workspace', async () => { + const solo: Tenant = { ...T.D, secret: 'lin_wh_d_dddddddddddddddddddddddd' }; + mockTables.clear(); + onboard(solo); + const o = await deliver(issueEvent(solo.id, solo.project, solo.actor), solo.secret!); + expectOneTask(o, solo); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════ +describe('@bgagent comments act only on the sending workspace\'s work', () => { + beforeEach(() => { + onboard(T.A); + onboard(T.B); + seedEpic(T.B, 'epic-b', 'child-b', 'BBB-1'); + seedStandaloneTask(T.B, 'plain-b'); + }); + + test('control: a comment on its own epic iterates that epic\'s child PR', async () => { + const o = await deliver(commentEvent(T.B.id, 'epic-b', T.B.actor, '@bgagent BBB-1: tighten the timeout'), T.B.secret!); + const task = expectOneTask(o, T.B); + expect(task.pr_number).toBe(41); + }); + + test('control: a comment on its own sub-issue iterates it', async () => { + const o = await deliver(commentEvent(T.B.id, 'child-b', T.B.actor, '@bgagent tighten the timeout'), T.B.secret!); + expect(expectOneTask(o, T.B).pr_number).toBe(41); + }); + + test('control: a comment on its own plain issue iterates its PR', async () => { + const o = await deliver(commentEvent(T.B.id, 'plain-b', T.B.actor, '@bgagent tighten the timeout'), T.B.secret!); + expect(expectOneTask(o, T.B).pr_number).toBe(52); + }); + + test('another workspace\'s epic: nothing is created and nothing is posted', async () => { + const o = await deliver(commentEvent(T.A.id, 'epic-b', T.A.actor, '@bgagent BBB-1: tighten the timeout'), T.A.secret!); + expectNoTask(o); + expect(o.status).toBe(200); + expect(o.warnings).toContain('Comment trigger: commented issue belongs to a different workspace than this webhook — dropping'); + expectNoLinearWrites(); + }); + + test('another workspace\'s sub-issue: nothing is created', async () => { + const o = await deliver(commentEvent(T.A.id, 'child-b', T.A.actor, '@bgagent tighten the timeout'), T.A.secret!); + expectNoTask(o); + expectNoLinearWrites(); + }); + + test('another workspace\'s plain issue: nothing is created', async () => { + const o = await deliver(commentEvent(T.A.id, 'plain-b', T.A.actor, '@bgagent tighten the timeout'), T.A.secret!); + expectNoTask(o); + expectNoLinearWrites(); + }); + + test('a "retry" on another workspace\'s epic does not re-run its work', async () => { + table(TABLE.orch).get(`${deriveOrchestrationId('epic-b')}\u0000child-b`)!.child_status = 'failed'; + const o = await deliver(commentEvent(T.A.id, 'epic-b', T.A.actor, '@bgagent retry'), T.A.secret!); + expectNoTask(o); + expectNoLinearWrites(); + }); + + test('a comment signed with the stack-wide secret on a multi-workspace stack is refused', async () => { + const o = await deliver(commentEvent(T.B.id, 'epic-b', T.B.actor, '@bgagent BBB-1: x'), STACK_WIDE_SECRET); + expectNoTask(o); + expect(o.status).toBe(401); + }); + + test('a task with no recorded workspace is not acted on', async () => { + delete table(TABLE.tasks).get('task-plain-b')!.channel_metadata; + const o = await deliver(commentEvent(T.B.id, 'plain-b', T.B.actor, '@bgagent tighten the timeout'), T.B.secret!); + expectNoTask(o); + }); +}); + +describe('@bgagent comments on a single-workspace stack', () => { + test('a stack-wide comment claiming another organization is bound to the only workspace', async () => { + onboard(T.C); + seedEpic(T.C, 'epic-c', 'child-c', 'CCC-1'); + // The body claims B; the only workspace is C, and C's epic is what it reaches. + const o = await deliver(commentEvent(T.B.id, 'epic-c', T.C.actor, '@bgagent CCC-1: x'), STACK_WIDE_SECRET); + expectOneTask(o, T.C); + }); +}); diff --git a/cli/src/commands/linear.ts b/cli/src/commands/linear.ts index eef58480d..ab9d80cf9 100644 --- a/cli/src/commands/linear.ts +++ b/cli/src/commands/linear.ts @@ -28,7 +28,7 @@ import { ResourceExistsException, SecretsManagerClient, } from '@aws-sdk/client-secrets-manager'; -import { DynamoDBDocumentClient, PutCommand, ScanCommand } from '@aws-sdk/lib-dynamodb'; +import { DynamoDBDocumentClient, PutCommand, ScanCommand, UpdateCommand } from '@aws-sdk/lib-dynamodb'; import { Command } from 'commander'; import { ApiClient } from '../api-client'; import { loadConfig, loadCredentials } from '../config'; @@ -615,6 +615,243 @@ export async function findWorkspaceRowBySlug( return (result.Items ?? []).find((item) => item.workspace_slug === slug); } +/** + * Every `active` row in the workspace registry. + * + * Same unbounded-Scan reasoning as {@link findWorkspaceRowBySlug}: the registry holds + * one small row per install. Rows whose `status` is anything other than `active` are + * dropped here rather than by the caller, matching the runtime resolver's fail-closed + * reading of that column (a half-written row is not an install). Exported for tests. + */ +export async function listActiveWorkspaceRows( + ddb: DynamoDBDocumentClient, + registryTableName: string, +): Promise>> { + const rows: Array> = []; + let lastKey: Record | undefined; + do { + const page = await ddb.send(new ScanCommand({ + TableName: registryTableName, + ExclusiveStartKey: lastKey, + })); + for (const item of page.Items ?? []) { + if (item.status === 'active') rows.push(item); + } + lastKey = page.LastEvaluatedKey; + } while (lastKey); + return rows; +} + +/** + * Slugs of every onboarded workspace, registry first. + * + * The registry is authoritative when present because it carries `status`, so a revoked + * install is not offered as a candidate. Installs predating the registry have only the + * `bgagent-linear-oauth-*` secrets, so that prefix listing is the fallback — the same + * two-source order `list-projects` uses. + */ +export async function listOnboardedWorkspaceSlugs(args: { + readonly sm: SecretsManagerClient; + readonly ddb?: DynamoDBDocumentClient; + readonly registryTableName?: string; +}): Promise { + if (args.ddb && args.registryTableName) { + const rows = await listActiveWorkspaceRows(args.ddb, args.registryTableName); + const slugs = rows + .map((r) => r.workspace_slug as string | undefined) + .filter((s): s is string => Boolean(s)); + if (slugs.length > 0) return slugs; + } + + // ListSecretsCommand caps at 100 per page; paginate so a deployment with more + // matching secrets than that does not silently miss installs after page one. + const collected: string[] = []; + let nextToken: string | undefined; + do { + const listed = await args.sm.send(new ListSecretsCommand({ + Filters: [{ Key: 'name', Values: [LINEAR_OAUTH_SECRET_PREFIX] }], + MaxResults: 100, + NextToken: nextToken, + })); + for (const s of listed.SecretList ?? []) { + const name = s.Name ?? ''; + if (name.startsWith(LINEAR_OAUTH_SECRET_PREFIX)) { + collected.push(name.slice(LINEAR_OAUTH_SECRET_PREFIX.length)); + } + } + nextToken = listed.NextToken; + } while (nextToken); + return collected; +} + +/** + * What can be established about where a workspace's signing secret came from. + * + * - `own` — the secret is provably NOT a copy of another workspace's. + * - `inherited` — it equals the stack-wide value, so ownership cannot be established. + * - `absent` — the workspace has no per-workspace secret at all. + */ +export type SecretProvenance = 'own' | 'inherited' | 'absent'; + +/** + * Decide whether a workspace's signing secret is provably its own. + * + * Sound in ONE direction only, and the asymmetry is the whole point. `mirror-stackwide` + * can copy nothing except the stack-wide value, so a secret that DIFFERS from it was + * never mirrored and is therefore owned — a conclusion that cannot be wrong. The + * converse is not available: a secret that EQUALS the stack-wide value is equally + * consistent with a healthy single-workspace install, whose first `setup` stamps the + * same real secret into both slots. That is why `inherited` withholds a verdict rather + * than asserting "not owned", and why only the `own` case is written to the registry. + * + * A stack with no stack-wide secret at all cannot have mirrored anything, so any stored + * per-workspace secret there is owned by the same argument. + * + * @param workspaceSecret - the workspace's stored `webhook_signing_secret`. + * @param stackWideSecret - the stack-wide back-compat secret, if it is set. + */ +export function classifyWebhookSecretProvenance( + workspaceSecret: string | undefined, + stackWideSecret: string | undefined, +): SecretProvenance { + if (!workspaceSecret) return 'absent'; + if (!stackWideSecret) return 'own'; + return workspaceSecret === stackWideSecret ? 'inherited' : 'own'; +} + +/** Outcome of resolving a usable Linear access token for one workspace. */ +export type WorkspaceTokenResult = + | { readonly kind: 'token'; readonly accessToken: string } + | { readonly kind: 'unavailable'; readonly reason: string }; + +/** + * Resolve a usable Linear access token for one workspace, vault first. + * + * Vault-before-Secrets-Manager mirrors the runtime resolver, and the order matters for + * more than preference: a vault-managed workspace holds no usable Secrets Manager + * token, so reading the bundle first reports a bare 401 on precisely the workspaces + * that are healthy. + * + * Returns a reason rather than throwing because every caller iterates workspaces and + * must keep going when one is unreachable — a single unreadable install should narrow + * the answer, not abort the command. + */ +export async function resolveWorkspaceAccessToken(args: { + readonly slug: string; + readonly sm: SecretsManagerClient; + readonly ddb?: DynamoDBDocumentClient; + readonly registryTableName?: string; + readonly region: string; + readonly vaultWorkloadName: string; +}): Promise { + const { slug, sm, ddb, registryTableName, vaultWorkloadName } = args; + + if (ddb && registryTableName) { + const row = await findWorkspaceRowBySlug(ddb, registryTableName, slug).catch(() => undefined); + const providerName = row?.provider_name as string | undefined; + if (providerName) { + const workspaceId = row?.linear_workspace_id as string | undefined; + const recorded = row?.vault_user_id as string | undefined; + const userId = recorded + ?? (workspaceId ? linearVaultUserId(workspaceId) : linearVaultUserIdForSlug(slug)); + const minted = await mintLinearTokenFromVault({ + region: args.region, + workloadName: vaultWorkloadName, + providerName, + userId, + }); + if (minted.kind === 'token') return { kind: 'token', accessToken: minted.accessToken }; + } + } + + try { + const resp = await sm.send(new GetSecretValueCommand({ SecretId: linearOauthSecretName(slug) })); + const stored = JSON.parse(resp.SecretString ?? '{}') as { access_token?: string }; + if (!stored.access_token) { + return { kind: 'unavailable', reason: `secret ${linearOauthSecretName(slug)} is missing access_token` }; + } + return { kind: 'token', accessToken: stored.access_token }; + } catch (err) { + return { + kind: 'unavailable', + reason: `failed to read ${linearOauthSecretName(slug)}: ${err instanceof Error ? err.message : String(err)}`, + }; + } +} + +/** Which workspace a project belongs to, as Linear itself reports it. */ +export type ProjectOwnerResult = + | { readonly kind: 'found'; readonly slug: string; readonly workspaceId: string } + | { readonly kind: 'not-found'; readonly searched: readonly string[]; readonly errors: readonly string[] }; + +/** + * Ask Linear which onboarded workspace owns `projectId`. + * + * Resolved from the provider rather than taken as an operator flag on purpose. The + * owning workspace is the value the webhook path will later check a delivery against, + * so a typo'd flag would durably write a mapping that points one tenant's project at + * another tenant's repository — the exact state the check exists to make unreachable. + * Linear answering "this project is visible to this workspace's token" is the only + * authority on the question that does not depend on the operator being careful. + * + * `organization.id` comes from the same authenticated response as the project rather + * than from the registry row, so the recorded id is the workspace Linear says owns the + * project, not the workspace we assumed we were asking. + */ +export async function findProjectOwnerWorkspace(args: { + readonly projectId: string; + readonly slugs: readonly string[]; + readonly sm: SecretsManagerClient; + readonly ddb?: DynamoDBDocumentClient; + readonly registryTableName?: string; + readonly region: string; + readonly vaultWorkloadName: string; + readonly fetchImpl?: typeof fetch; +}): Promise { + const doFetch = args.fetchImpl ?? fetch; + const errors: string[] = []; + + for (const slug of args.slugs) { + const token = await resolveWorkspaceAccessToken({ ...args, slug }); + if (token.kind !== 'token') { + errors.push(`${slug}: ${token.reason}`); + continue; + } + + try { + const res = await doFetch('https://api.linear.app/graphql', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${token.accessToken}`, + }, + body: JSON.stringify({ + query: 'query($id: String!) { organization { id } project(id: $id) { id } }', + variables: { id: args.projectId }, + }), + }); + if (!res.ok) { + errors.push(`${slug}: Linear API returned ${res.status}`); + continue; + } + const body = await res.json() as { + data?: { organization?: { id?: string }; project?: { id?: string } | null }; + }; + // A workspace whose token cannot see the project answers `project: null` with a + // 200 — that is the "not this workspace" signal, not an error worth reporting. + const foundId = body.data?.project?.id; + const orgId = body.data?.organization?.id; + if (foundId === args.projectId && orgId) { + return { kind: 'found', slug, workspaceId: orgId }; + } + } catch (err) { + errors.push(`${slug}: ${err instanceof Error ? err.message : String(err)}`); + } + } + + return { kind: 'not-found', searched: args.slugs, errors }; +} + export function makeLinearCommand(): Command { const linear = new Command('linear') .description('Manage Linear integration'); @@ -1396,6 +1633,26 @@ export function makeLinearCommand(): Command { ); } const teamKeys = teamKeysResult.ok ? teamKeysResult.keys : []; + + // Provenance of the signing secret, recorded so verification can tell a + // workspace's OWN secret from a copy of another workspace's. Derived from where + // the value came from, never from comparing it to the stack-wide copy: a healthy + // single-workspace install holds a secret equal to that copy, because the first + // install stamps the same real secret into both slots. + // + // Either input proves ownership. `suppliedWebhookSecret` was read off the app + // being onboarded moments ago, and `holdsOwnWebhookSecret` means a stored + // `lin_wh_…` that differs from the stack-wide value. Anything else either threw + // above (nothing supplied, nothing legitimate stored) or is inherited, and + // inherited is precisely what must not be recorded as owned. + // + // Today this cannot evaluate false — the up-front prompt refuses the one case + // that would produce it — so it reads as a constant and no test can distinguish + // it from `true`. It is written as a derivation rather than a literal because it + // becomes load-bearing the moment that prompt is relaxed, and the failure would + // otherwise be silent: a workspace recorded as owning a secret it inherited. + const webhookSecretOwned = Boolean(suppliedWebhookSecret) || holdsOwnWebhookSecret; + await ddb.send(new PutCommand({ TableName: workspaceRegistryTable!, Item: { @@ -1406,6 +1663,10 @@ export function makeLinearCommand(): Command { installed_at: now, updated_at: now, status: 'active', + // Written only when true. An explicit `false` would be indistinguishable + // from the absent field on rows predating this, and both mean the same + // thing to the reader — not proven. + ...(webhookSecretOwned ? { webhook_secret_owned: true } : {}), ...(teamKeys.length > 0 ? { team_keys: teamKeys } : {}), // Vault substrate: the provider to mint from, and the subject the grant // is bound to. vault_user_id is stored rather than derived because it is @@ -1492,10 +1753,19 @@ export function makeLinearCommand(): Command { console.log(' ✓ Preserving this workspace\'s existing webhook signing secret (re-run — not overwriting)'); webhookSigningSecret = secretAction.secret; } else if (secretAction.kind === 'mirror-stackwide') { - // No per-workspace secret yet, but the stack-wide one is set. Safe to - // mirror ONLY when this is the first/only workspace — for a genuinely - // new ADDITIONAL workspace the stack-wide secret is the wrong one, so - // warn that the operator should verify (or run `update-webhook-secret`). + // Mirroring is only ever correct for the first/only workspace: the stack-wide + // value IS that workspace's own secret. For an additional workspace it copies a + // DIFFERENT tenant's secret into this one's bundle, after which both verify + // under the same key and either can sign an event whose routing values the + // other's are read from. + // + // Unreachable from here today: the up-front prompt above collects this + // workspace's own secret and throws when neither a supplied nor a legitimately + // stored one exists, which are the only ways into this branch. Left as-is + // rather than hardened, because the value it would produce is no longer + // trusted on its own — the receiver rejects a secret that is not recorded as + // the workspace's own once a second workspace is active. If that prompt is + // ever relaxed, this branch needs to refuse rather than warn. console.log(' ✓ No per-workspace secret yet; mirroring the stack-wide signing secret'); console.log(' (if this is an ADDITIONAL workspace, its Linear webhook secret differs —'); console.log(` run \`bgagent linear update-webhook-secret ${slug}\` with this workspace's secret.)`); @@ -1922,6 +2192,7 @@ export function makeLinearCommand(): Command { .description('Update the per-workspace webhook signing secret without re-running OAuth') .argument('', 'Linear workspace urlKey (e.g. "acme" from linear.app/acme/...)') .option('--region ', 'AWS region (defaults to configured region)') + .option('--stack-name ', 'CloudFormation stack name', 'backgroundagent-dev') .action(async (slug: string, opts) => { // Use case: rotation, recovery from misconfig, or first-time // configuration after Linear regenerated the signing secret. @@ -2002,6 +2273,54 @@ export function makeLinearCommand(): Command { }; await upsertOauthSecret(sm, secretName, merged, slug); + // Record that this workspace now holds a secret of its own. The operator just + // read it off this workspace's Linear app, which is the only evidence of + // ownership that exists — and without recording it, verification cannot tell + // this repaired workspace from one still carrying an inherited copy, so the + // repair would appear to do nothing. + // + // A hard failure rather than a warning: the secret write above already + // succeeded, so a silent miss here leaves the operator believing the fix landed + // when the state that verification reads is unchanged. + const registryTableName = await getStackOutput(region, opts.stackName, 'LinearWorkspaceRegistryTableName'); + if (!registryTableName) { + // Refuse rather than skip. Skipping is the failure this whole block exists to + // avoid: the secret write above already succeeded, so an operator who defaulted + // or mistyped --stack-name would see "Updated" and keep 401ing, with nothing + // in the output pointing at the reason. + throw new CliError( + 'Updated the signing secret, but could not find LinearWorkspaceRegistryTableName in ' + + `stack '${opts.stackName}'.\n` + + ' The secret is only half the repair — verification also reads a registry flag\n' + + ' recording that this workspace owns it, and that flag was not written.\n' + + ' Re-run with --stack-name (the secret you entered is preserved).', + ); + } + try { + await makeDocClient({ region }).send(new UpdateCommand({ + TableName: registryTableName, + Key: { linear_workspace_id: stored.workspace_id }, + UpdateExpression: 'SET webhook_secret_owned = :t, updated_at = :u', + ConditionExpression: 'attribute_exists(linear_workspace_id)', + ExpressionAttributeValues: { ':t': true, ':u': new Date().toISOString() }, + })); + console.log(' ✓ Recorded that this workspace owns its signing secret'); + } catch (err) { + if ((err as { name?: string })?.name === 'ConditionalCheckFailedException') { + throw new CliError( + `Updated the signing secret, but workspace '${slug}' has no registry row ` + + `(${stored.workspace_id}).\n Run \`bgagent linear setup ${slug}\` so the workspace ` + + 'is registered; the secret you just entered will be preserved.', + ); + } + throw new CliError( + 'Updated the signing secret, but could not record its provenance: ' + + `${err instanceof Error ? err.message : String(err)}\n` + + ' Re-run this command once the registry table is writable, otherwise this\n' + + ' workspace is still treated as carrying an inherited secret.', + ); + } + console.log(); console.log(`✅ Updated webhook signing secret for '${slug}'.`); console.log(); @@ -2279,6 +2598,12 @@ export function makeLinearCommand(): Command { .requiredOption('--repo ', 'GitHub repository the mapped project should route tasks to') .option('--label