fix(linear): bind webhook routing values to the workspace that signed the delivery

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<void> {
     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<void> {
       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;
 
   // ``<base>:help`` — post a one-time explainer of what the trigger labels do
@@ -1840,6 +1909,33 @@ async function handleNearMissMention(payload: LinearCommentEvent): Promise<void>
  * 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<void> {
   // Orchestration must be enabled + a workspace token resolvable.
   if (!ORCHESTRATION_TABLE || !WORKSPACE_REGISTRY_TABLE) {
@@ -1919,6 +2015,7 @@ async function handleCommentTrigger(payload: LinearCommentEvent): Promise<void>
   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<void>
     ? 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

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<APIGatewayPr
           linear_workspace_id: payload.organizationId,
         });
         return jsonResponse(401, { error: 'Workspace not active' });
+      } else if (result === 'shared-secret') {
+        // The signature matched, but against a secret this workspace shares with
+        // another on the same stack — so it proves the sender knows SOME tenant's
+        // secret, not this one's. Fatal for the same reason `mismatch` is: falling
+        // through would re-admit it via the stack-wide path.
+        logger.warn('Linear webhook verified against a secret shared with another workspace — rejecting', {
+          linear_workspace_id: payload.organizationId,
+          remedy: 'bgagent linear update-webhook-secret <slug>',
+        });
+        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 <slug>',
+        });
+        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<APIGatewayPr
       await lambdaClient.send(new InvokeCommand({
         FunctionName: PROCESSOR_FUNCTION_NAME,
         InvocationType: 'Event',
-        Payload: new TextEncoder().encode(JSON.stringify({ raw_body: event.body })),
+        // The verified context travels with the body. Without it the processor cannot
+        // tell a delivery whose tenant the signature attested from one whose tenant is
+        // only claimed, and it routes from that claim.
+        Payload: new TextEncoder().encode(JSON.stringify({
+          raw_body: event.body,
+          verified_via_stack_wide: verifiedViaStackWide,
+        })),
       }));
     } catch (invokeErr) {
       logger.error('Failed to invoke Linear webhook processor', {

cdk/src/handlers/shared/linear-oauth-resolver.ts

@@ -23,7 +23,7 @@ import {
   PutSecretValueCommand,
   SecretsManagerClient,
 } from '@aws-sdk/client-secrets-manager';
-import { DynamoDBDocumentClient, GetCommand, UpdateCommand } from '@aws-sdk/lib-dynamodb';
+import { DynamoDBDocumentClient, GetCommand, ScanCommand, UpdateCommand } from '@aws-sdk/lib-dynamodb';
 import { announceRevocation, revocationAlertTopicArn } from './linear-revocation-alert';
 import {
   LINEAR_VAULT_SCOPES,
@@ -142,6 +142,22 @@ export interface RegistryRow {
    * stored; those fall back to the derived form.
    */
   readonly vault_user_id?: string;
+  /**
+   * Whether this workspace's `webhook_signing_secret` is provably its OWN, rather than
+   * a copy of another workspace's.
+   *
+   * Recorded by provenance — which `bgagent linear setup` branch produced the value —
+   * and NOT inferred by comparing the stored secret against the stack-wide one. Value
+   * equality cannot tell the two apart: a healthy single-workspace install also holds a
+   * secret equal to the stack-wide copy, because the first install stamps the same real
+   * secret into both slots. Rejecting on equality would therefore 401 exactly the
+   * deployments that are safe.
+   *
+   * Absent on rows written before this was recorded, which is why the reader treats
+   * absence as "not proven" rather than as `false`, and only acts on it where sharing
+   * can actually cross a tenant boundary — a stack with two or more active workspaces.
+   */
+  readonly webhook_secret_owned?: boolean;
   /**
    * Why `status` was flipped to `revoked`, as written by {@link markWorkspaceRevoked}.
    *
@@ -886,6 +902,10 @@ function parseRegistryRow(rawItem: unknown, linearWorkspaceId: string): Registry
     // Distinguishes a latch built on Linear's own refusal from one built on an
     // inference the vault path can re-test. See RegistryRow.revoked_reason.
     ...(typeof item.revoked_reason === 'string' && { revoked_reason: item.revoked_reason }),
+    // Only a literal `true` counts as proof of ownership. A missing field, or any
+    // other value, leaves it absent so the reader sees "not proven" — the safe
+    // reading for the rows this field was added for, which predate it entirely.
+    ...(item.webhook_secret_owned === true && { webhook_secret_owned: true }),
   };
   registryCache.set(linearWorkspaceId, { value: row, expiresAt: Date.now() + REGISTRY_CACHE_TTL_MS });
   return row;
@@ -1359,6 +1379,51 @@ async function tryRefreshOnce(
   return { kind: 'success', token: next };
 }
 
+/**
+ * The one active Linear workspace on this stack, or undefined when that is not a
+ * well-defined question.
+ *
+ * For binding a delivery that was verified by the stack-wide secret. That secret is
+ * bound to no workspace, so the body's `organizationId` is claimed rather than attested
+ * and must not select a tenant. When exactly one workspace is active there is only one
+ * tenant it could mean; with zero or several there is no answer, and returning undefined
+ * makes the caller drop the delivery rather than pick one.
+ *
+ * Mirrors `resolveSoleActiveJiraTenant`. Not cached: the callers reach it only on the
+ * back-compat path of a single-workspace install, which is rare enough that a Scan per
+ * delivery is cheaper than another cache to invalidate.
+ */
+export async function resolveSoleActiveLinearWorkspace(
+  ddbClient: DynamoDBDocumentClient,
+  registryTableName: string | undefined,
+): Promise<string | undefined> {
+  if (!registryTableName) return undefined;
+  const active: string[] = [];
+  let lastKey: Record<string, unknown> | 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();

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) {