Merge branch 'main' into feat/366-synth-reuse

Author: scottschreckengaust

cdk/src/constructs/jira-integration.ts

@@ -38,6 +38,19 @@ const DEFAULT_TASK_RETENTION_DAYS = 90;
 /** Webhook-processor Lambda timeout (seconds). */
 const WEBHOOK_PROCESSOR_TIMEOUT_SECONDS = 30;
 
+/**
+ * Marker key embedded in the auto-generated stack-wide webhook-secret
+ * placeholder. The CLI (`bgagent jira setup`) recognizes a secret carrying
+ * this key as "never configured" and seeds the operator's value over it.
+ *
+ * MUST stay in sync with `JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY` in
+ * `cli/src/commands/jira.ts`. Unlike Linear (whose real secrets always start
+ * with `lin_wh_`), Atlassian webhook signing secrets are operator-chosen bare
+ * strings with no fixed shape, so the *placeholder* — not the real value — is
+ * the thing we make recognizable. See #368.
+ */
+const JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY = 'abca_jira_webhook_placeholder';
+
 /**
  * Webhook-processor Lambda memory (MB). Matches the Linear processor — the
  * same attachment-screening path bundles pdf-parse + the URL resolver, and
@@ -151,9 +164,24 @@ export class JiraIntegration extends Construct {
     // created by the CLI at runtime — not here. This stack-wide secret is
     // a back-compat fallback for single-tenant installs predating per-
     // tenant signing.
+    //
+    // The initial value is an explicit JSON placeholder carrying
+    // `JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY`. Without `generateSecretString`,
+    // CDK seeds a BARE random string — which the CLI's placeholder heuristic
+    // mistook for an already-configured secret, so `setup` never seeded the
+    // operator's value and every admin-UI webhook delivery (whose payload has
+    // no `cloudId`, forcing stack-wide verification) failed HMAC with 401,
+    // silently (#368). Making the placeholder explicit lets the CLI reliably
+    // tell "never configured" from an operator-set value.
     this.webhookSecret = new secretsmanager.Secret(this, 'WebhookSecret', {
       description: 'Jira webhook signing secret — populate via `bgagent jira setup`',
       removalPolicy,
+      generateSecretString: {
+        // Yields `{"abca_jira_webhook_placeholder":true,"value":"<random>"}`:
+        // a JSON object (starts with `{`) with an explicit marker key.
+        secretStringTemplate: JSON.stringify({ [JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY]: true }),
+        generateStringKey: 'value',
+      },
     });
 
     // --- Shared Lambda configuration ---

cdk/test/constructs/jira-integration.test.ts

@@ -0,0 +1,75 @@
+/**
+ *  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.
+ */
+
+import { App, Stack } from 'aws-cdk-lib';
+import { Template, Match } from 'aws-cdk-lib/assertions';
+import * as apigw from 'aws-cdk-lib/aws-apigateway';
+import * as cognito from 'aws-cdk-lib/aws-cognito';
+import * as dynamodb from 'aws-cdk-lib/aws-dynamodb';
+import { JiraIntegration } from '../../src/constructs/jira-integration';
+
+describe('JiraIntegration construct', () => {
+  let template: Template;
+
+  beforeAll(() => {
+    const app = new App();
+    const stack = new Stack(app, 'TestStack');
+
+    const api = new apigw.RestApi(stack, 'TestApi');
+    const userPool = new cognito.UserPool(stack, 'TestUserPool');
+    const taskTable = new dynamodb.Table(stack, 'TaskTable', {
+      partitionKey: { name: 'task_id', type: dynamodb.AttributeType.STRING },
+    });
+    const taskEventsTable = new dynamodb.Table(stack, 'TaskEventsTable', {
+      partitionKey: { name: 'task_id', type: dynamodb.AttributeType.STRING },
+      sortKey: { name: 'event_id', type: dynamodb.AttributeType.STRING },
+    });
+
+    new JiraIntegration(stack, 'JiraIntegration', {
+      api,
+      userPool,
+      taskTable,
+      taskEventsTable,
+    });
+
+    template = Template.fromStack(stack);
+  });
+
+  test('creates one Secrets Manager secret (webhook signing) — OAuth tokens are CLI-created at runtime', () => {
+    template.resourceCountIs('AWS::SecretsManager::Secret', 1);
+    template.hasResourceProperties('AWS::SecretsManager::Secret', {
+      Description: Match.stringLikeRegexp('Jira webhook signing secret'),
+    });
+  });
+
+  // #368: the webhook secret MUST seed an explicit JSON placeholder so the CLI
+  // can distinguish "never configured" from an operator-set value. A bare
+  // generated string (CDK's default with no GenerateSecretString) caused
+  // `bgagent jira setup` to skip seeding, leaving every admin-UI webhook
+  // delivery to fail HMAC verification with 401.
+  test('webhook secret seeds a JSON placeholder carrying the explicit marker key (#368)', () => {
+    template.hasResourceProperties('AWS::SecretsManager::Secret', {
+      GenerateSecretString: Match.objectLike({
+        // secretStringTemplate is the JSON object carrying the marker key.
+        SecretStringTemplate: Match.stringLikeRegexp('abca_jira_webhook_placeholder'),
+        GenerateStringKey: 'value',
+      }),
+    });
+  });
+});

cli/src/commands/jira.ts

@@ -184,27 +184,42 @@ export async function upsertOauthSecret(
 }
 
 /**
- * Check whether the JiraWebhookSecret already holds a real signing secret
- * (vs CDK's autogenerated placeholder). Used to decide whether to prompt
- * for the webhook secret on subsequent setup runs.
+ * Marker key embedded in the CDK-generated stack-wide webhook-secret
+ * placeholder. A secret whose JSON carries this key has never been
+ * configured by an operator, so `setup` is free to seed the real value.
  *
- * Atlassian's generic-webhook signing secrets are operator-chosen — they
- * have no fixed prefix like Linear's `lin_wh_`. We treat the placeholder
- * as a JSON-encoded value (CDK's default for autogenerated secrets) and
- * everything else as a real value.
+ * MUST stay in sync with `JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY` in
+ * `cdk/src/constructs/jira-integration.ts`. See #368.
  */
-async function isWebhookSecretConfigured(
+export const JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY = 'abca_jira_webhook_placeholder';
+
+/**
+ * Check whether the JiraWebhookSecret already holds a real, operator-set
+ * signing secret (vs the CDK-generated placeholder). Used to decide whether
+ * to seed the stack-wide secret on a `setup` run.
+ *
+ * Atlassian's generic-webhook signing secrets are operator-chosen — they have
+ * no fixed prefix like Linear's `lin_wh_`, so we cannot positively recognize a
+ * *real* value by shape. Instead we recognize the *placeholder*: the CDK
+ * construct seeds an explicit JSON object carrying
+ * `JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY`. Anything that is not that placeholder
+ * is treated as an operator value.
+ *
+ * NOTE (#368 migration): stacks deployed before the explicit-placeholder fix
+ * seeded a *bare random string* placeholder, which is indistinguishable from
+ * an operator value and so is (conservatively) reported as configured. Such
+ * installs must redeploy the CDK stack — which regenerates the secret with the
+ * JSON placeholder — before `setup` will seed it.
+ */
+export async function isWebhookSecretConfigured(
   client: SecretsManagerClient,
   secretArn: string,
 ): Promise<boolean> {
   try {
     const result = await client.send(new GetSecretValueCommand({ SecretId: secretArn }));
     const value = result.SecretString;
     if (typeof value !== 'string' || value.length === 0) return false;
-    // CDK's auto-generated secret is a JSON object string starting with `{`
-    // — operator-set secrets (the Atlassian-side configured value) are bare
-    // strings. Anything that doesn't look like the placeholder JSON is real.
-    return !value.trim().startsWith('{');
+    return !isWebhookSecretPlaceholder(value);
   } catch (err) {
     const errorName = (err as { name?: string }).name;
     if (errorName === 'ResourceNotFoundException') {
@@ -219,6 +234,29 @@ async function isWebhookSecretConfigured(
   }
 }
 
+/**
+ * True when `value` is the CDK-generated placeholder — a JSON object carrying
+ * the {@link JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY} marker. A non-JSON value, or
+ * JSON without the marker, is an operator-set secret.
+ */
+function isWebhookSecretPlaceholder(value: string): boolean {
+  const trimmed = value.trim();
+  // Fast reject: real Atlassian signing secrets are bare strings.
+  if (!trimmed.startsWith('{')) return false;
+  try {
+    const parsed: unknown = JSON.parse(trimmed);
+    return (
+      typeof parsed === 'object'
+      && parsed !== null
+      && JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY in (parsed as Record<string, unknown>)
+    );
+  } catch {
+    // Starts with `{` but isn't valid JSON — not our placeholder. Treat as a
+    // (malformed) operator value rather than silently re-seeding over it.
+    return false; // nosemgrep: ts-silent-success-masking -- unparseable secret is conservatively treated as operator-set (not the placeholder), so setup never overwrites it
+  }
+}
+
 function promptSecret(label: string): Promise<string> {
   return new Promise((resolve) => {
     const rl = readline.createInterface({ input: process.stdin, output: process.stdout });