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;
 }
 
 /**
@@ -689,6 +699,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.
@@ -720,6 +757,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. `bgagent linear backfill-project-workspaces` fills them in and
+    // `bgagent platform doctor` reports what is left, 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

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-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<number> {
+  if (!registryTableName) return 1;
+  const now = Date.now();
+  if (activeWorkspaceCountCache && activeWorkspaceCountCache.expiresAt > now) {
+    return activeWorkspaceCountCache.count;
+  }
+
+  try {
+    let count = 0;
+    let lastKey: Record<string, unknown> | 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';
 }