fix(cli): linear setup must not clobber a workspace's webhook signing secret (#611)

cli/src/commands/linear.ts

@@ -43,6 +43,8 @@ import {
   generatePkce,
   LINEAR_OAUTH_SECRET_PREFIX,
   linearOauthSecretName,
+  readExistingWebhookSecret,
+  resolveWebhookSecretAction,
   StoredLinearOauthToken,
 } from '../linear-oauth';
 import { awaitOauthCallback, CALLBACK_URL } from '../oauth-callback-server';
@@ -601,6 +603,42 @@ export function makeLinearCommand(): Command {
         process.stdout.write('  → Storing OAuth token...');
         const sm = new SecretsManagerClient({ region });
         const now = new Date().toISOString();
+        // Preserve any EXISTING per-workspace webhook signing secret before the
+        // OAuth overwrite below. Re-running `setup` on an already-installed
+        // workspace must NOT clobber its working signing secret with the
+        // stack-wide fallback (which belongs to whichever workspace installed
+        // first) — that silently breaks signature verification (401 "Invalid
+        // signature") for every workspace after the first in a multi-workspace
+        // deployment. Rotation stays the job of `update-webhook-secret`.
+        // Fail CLOSED (#612 review B1): read this workspace's existing signing
+        // secret before the overwrite. Only ResourceNotFoundException is a clean
+        // first-install; any other SM error (AccessDenied, KMSAccessDenied,
+        // Throttling, network) — or a corrupt bundle — must surface, NOT default
+        // to undefined (which would mirror the stack-wide secret over a working
+        // per-workspace one → the #611 401 clobber, silently, behind a green
+        // "Setup complete"). Extracted to readExistingWebhookSecret + unit-tested.
+        let existingWebhookSecret: string | undefined;
+        try {
+          existingWebhookSecret = await readExistingWebhookSecret(
+            async () => {
+              const prior = await sm.send(new GetSecretValueCommand({ SecretId: linearOauthSecretName(slug) }));
+              return prior.SecretString;
+            },
+            (err) => (err as { name?: string }).name === 'ResourceNotFoundException',
+          );
+        } catch (err) {
+          const errorName = (err as { name?: string }).name;
+          const message = err instanceof Error ? err.message : String(err);
+          throw new CliError(
+            `Failed to read the existing Linear webhook secret for '${slug}' before re-write: `
+            + `${errorName ?? 'Error'}: ${message}. Refusing to proceed — continuing could clobber `
+            + 'this workspace\'s webhook signing secret with the stack-wide value (a 401 on every '
+            + 'delivery). Likely an IAM/KMS gap: confirm your CLI principal has '
+            + '`secretsmanager:GetSecretValue` (and kms:Decrypt if a CMK) on '
+            + `${linearOauthSecretName(slug)}, or run \`bgagent linear update-webhook-secret ${slug}\` `
+            + 'after fixing access.',
+          );
+        }
         const stored: StoredLinearOauthToken = {
           access_token: tokenResponse.access_token,
           refresh_token: tokenResponse.refresh_token ?? '',
@@ -615,6 +653,14 @@ export function makeLinearCommand(): Command {
           installed_at: now,
           updated_at: now,
           installed_by_platform_user_id: cognitoSub,
+          // Fold the preserved secret into the INITIAL bundle (#612 review N2):
+          // the OAuth-secret write below then lands the correct bundle in ONE
+          // PutSecretValue, and the preserve path skips the later re-write. This
+          // also closes a narrow window — if any fallible step between the two
+          // writes threw, the bundle was left persisted WITHOUT the secret (401
+          // until update-webhook-secret). Only set for a real `lin_wh_` value;
+          // mirror-stackwide / prompt cases add it in the mirror-back below.
+          ...(existingWebhookSecret ? { webhook_signing_secret: existingWebhookSecret } : {}),
         };
         if (!stored.refresh_token) {
           throw new CliError(
@@ -676,19 +722,40 @@ export function makeLinearCommand(): Command {
         // without re-onboarding. Multi-workspace installs need each
         // workspace to own its own per-workspace signing secret — only
         // the FIRST install can populate the stack-wide one usefully.
-        // If stack-wide is already populated, this is either a re-run
-        // of setup on the SAME workspace or the FIRST workspace of a
-        // future multi-workspace install. Either way the stored value
-        // is this workspace's signing secret — lift it into the
-        // per-workspace bundle without prompting (auto-migration to
-        // the new shape). Rotation is not setup's job: use
-        // `bgagent linear update-webhook-secret <slug>` to rotate the
-        // signing secret without re-running OAuth.
+        //
+        // resolveWebhookSecretAction picks one of three outcomes from
+        // (existingWebhookSecret, stackWideAlreadyConfigured):
+        //   • preserve      — this workspace ALREADY has its own `lin_wh_`
+        //                     secret (a re-run). Keep it; do NOT overwrite from
+        //                     the stack-wide fallback, which is a DIFFERENT
+        //                     workspace's secret once >1 is installed. This is
+        //                     the #611 clobber fix — the stack-wide value is NOT
+        //                     necessarily this workspace's.
+        //   • mirror-stackwide — no per-workspace secret yet but stack-wide is
+        //                     set: mirror it (correct for the first/only
+        //                     workspace; warn for an additional one).
+        //   • prompt        — nothing to go on: ask for the signing secret.
+        // Rotation is not setup's job: use
+        // `bgagent linear update-webhook-secret <slug>` to rotate.
         const stackWideAlreadyConfigured = await isWebhookSecretConfigured(sm, webhookSecretArn!);
         let webhookSigningSecret: string | undefined;
 
-        if (stackWideAlreadyConfigured) {
-          console.log('  ✓ Webhook signing secret already configured stack-wide (mirroring to per-workspace)');
+        const secretAction = resolveWebhookSecretAction(existingWebhookSecret, stackWideAlreadyConfigured);
+        if (secretAction.kind === 'preserve') {
+          // This workspace already had its own signing secret — keep it. Do NOT
+          // overwrite from the stack-wide fallback (a DIFFERENT workspace's
+          // secret once >1 is installed). Re-run case; rotation is
+          // `update-webhook-secret`'s job, not setup's.
+          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`).
+          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.)`);
           try {
             const value = await sm.send(new GetSecretValueCommand({ SecretId: webhookSecretArn! }));
             if (value.SecretString && value.SecretString.startsWith('lin_wh_')) {
@@ -725,9 +792,11 @@ export function makeLinearCommand(): Command {
           webhookSigningSecret = webhookSecret;
         }
 
-        // Mirror into the per-workspace OAuth secret so the receiver can
-        // look it up by orgId. Re-upsert with the merged payload.
-        if (webhookSigningSecret) {
+        // Mirror into the per-workspace OAuth secret so the receiver can look it
+        // up by orgId. In the PRESERVE case the secret is already in `stored`
+        // (folded above) and was written by the OAuth-secret upsert — no re-write
+        // needed. Only mirror-stackwide / prompt produce a NEW secret to persist.
+        if (webhookSigningSecret && webhookSigningSecret !== stored.webhook_signing_secret) {
           const merged: StoredLinearOauthToken = {
             ...stored,
             webhook_signing_secret: webhookSigningSecret,

cli/src/linear-oauth.ts

@@ -293,3 +293,88 @@ function isLinearTokenResponse(value: unknown): value is LinearTokenResponse {
 export function computeExpiresAt(expiresInSeconds: number, now: Date = new Date()): string {
   return new Date(now.getTime() + expiresInSeconds * 1000).toISOString();
 }
+
+/** What `bgagent linear setup` should do about the webhook signing secret. */
+export type WebhookSecretAction =
+  /** This workspace already has its own signing secret — keep it (re-run). */
+  | { readonly kind: 'preserve'; readonly secret: string }
+  /** No per-workspace secret; mirror the stack-wide one (safe for the first
+   *  workspace, ambiguous for an additional one — caller should warn). */
+  | { readonly kind: 'mirror-stackwide' }
+  /** No secret anywhere — prompt the operator for it (first install). */
+  | { readonly kind: 'prompt' };
+
+/**
+ * Decide which webhook signing secret `setup` should use, WITHOUT clobbering a
+ * working per-workspace secret.
+ *
+ * The bug this fixes: re-running `setup` on an already-installed workspace used
+ * to lift the *stack-wide* signing secret into the per-workspace bundle. That's
+ * correct only for the FIRST workspace — the stack-wide secret belongs to
+ * whichever workspace installed first, so for any additional workspace it
+ * overwrites the correct per-workspace secret with the wrong one, silently
+ * breaking signature verification (webhook 401 "Invalid signature").
+ *
+ * Rule: an existing valid per-workspace secret always wins (rotation is
+ * `update-webhook-secret`'s job); else mirror the stack-wide secret if present;
+ * else prompt. Pure — the caller supplies what it read from Secrets Manager.
+ *
+ * @param existingPerWorkspaceSecret the `webhook_signing_secret` on this
+ *        workspace's OAuth bundle BEFORE the setup rewrite (undefined if none).
+ * @param stackWideConfigured whether the stack-wide fallback secret is set.
+ */
+export function resolveWebhookSecretAction(
+  existingPerWorkspaceSecret: string | undefined,
+  stackWideConfigured: boolean,
+): WebhookSecretAction {
+  if (existingPerWorkspaceSecret?.startsWith('lin_wh_')) {
+    return { kind: 'preserve', secret: existingPerWorkspaceSecret };
+  }
+  if (stackWideConfigured) {
+    return { kind: 'mirror-stackwide' };
+  }
+  return { kind: 'prompt' };
+}
+
+/**
+ * Read this workspace's EXISTING `webhook_signing_secret` before `setup`
+ * overwrites the OAuth bundle — FAIL CLOSED (#612 review B1).
+ *
+ * `fetchSecretString` fetches the prior per-workspace OAuth `SecretString`
+ * (typically a `SecretsManagerClient.send(GetSecretValueCommand)` wrapper). The
+ * value fed to {@link resolveWebhookSecretAction} decides whether the existing
+ * secret is preserved or the stack-wide one is mirrored over it — so a wrong
+ * `undefined` here re-arms the #611 clobber. Therefore:
+ *
+ *   • secret missing (`ResourceNotFoundException`) → genuine first install,
+ *     return undefined (nothing to preserve).
+ *   • secret present with a valid `lin_wh_…` `webhook_signing_secret` → return it.
+ *   • secret present but no/malformed secret → return undefined (nothing valid
+ *     to preserve; the caller mirrors/prompts).
+ *   • ANY OTHER error (AccessDenied, KMSAccessDenied, Throttling, network, or a
+ *     corrupt-JSON bundle) → THROW. Swallowing it would leave undefined and
+ *     silently clobber a working secret behind a green "Setup complete".
+ *
+ * `isNotFound` classifies the fetch error; callers pass an
+ * `name === 'ResourceNotFoundException'` check. On a throw the caller re-raises
+ * a CliError with an IAM/KMS hint (mirroring `isWebhookSecretConfigured`).
+ */
+export async function readExistingWebhookSecret(
+  fetchSecretString: () => Promise<string | undefined>,
+  isNotFound: (err: unknown) => boolean,
+): Promise<string | undefined> {
+  let raw: string | undefined;
+  try {
+    raw = await fetchSecretString();
+  } catch (err) {
+    if (isNotFound(err)) return undefined; // genuine first install
+    throw err; // fail closed — caller wraps with an actionable CliError
+  }
+  if (!raw) return undefined;
+  // A corrupt-but-present bundle must surface (JSON.parse throws → propagates),
+  // NOT silently become "nothing to preserve" → mirror-stackwide.
+  const bundle = JSON.parse(raw) as Partial<StoredLinearOauthToken>;
+  return bundle.webhook_signing_secret?.startsWith('lin_wh_')
+    ? bundle.webhook_signing_secret
+    : undefined;
+}