Skip to content

bug(infra): webhook processors silently discard admitted events — no DLQ + transient DDB failure misread as "tenant not onboarded" #734

Description

@scottschreckengaust

Problem

The Jira, Linear, and Slack webhook processors are invoked with InvocationType: 'Event' (async) and have no DLQ and no onFailure destination. Combined with a resolver that degrades a transient DynamoDB error into "tenant not in registry", a fully-admitted, correctly-signed, label-triggered task can be silently and permanently discarded with a single WARN line and no operator signal.

This is the same class of gap that #284 fixed for the GitHub webhook processor. That work landed a DLQ + alarm there; the three chat/tracker integrations were never given the same treatment.

The two halves

1. A transient failure is misclassified as a permanent "not onboarded" condition.

cdk/src/handlers/shared/jira-oauth-resolver.ts:376-382 swallows every DynamoDB error to null:

} catch (err) {
  logger.error('Failed to fetch Jira workspace registry row', { ... });
  return null; // nosemgrep: ts-silent-success-masking -- transient DDB throttle degrades to "tenant not in registry"; the verify path uses getRegistryRowStrict which rethrows
}

The nosemgrep justification is accurate about the verify path (which correctly uses getRegistryRowStrict), but the processor path uses the lenient variant. That null becomes:

cdk/src/handlers/jira-webhook-processor.ts:428

logger.warn('Jira tenant not resolvable from registry — dropping event', { ... });

So ProvisionedThroughputExceededException, a timeout, or an IAM denial on workspaceRegistryTable is indistinguishable from "this tenant was never onboarded" — and the latter is a legitimate reason to drop.

2. Nothing catches the dropped event.

InvocationType: 'Event' (cdk/src/handlers/jira-webhook.ts:244-249) means AWS retries twice with backoff and then discards the payload. Because the handler returns rather than throwing, it exits 200 — so even those two retries never fire. Verified on main @ 46263c09:

construct deadLetterQueue / onFailure
fanout-consumer.ts ✅ (7 refs)
github-screenshot-integration.ts ✅ (3 refs)
jira-integration.ts 0
linear-integration.ts 0
slack-integration.ts 0

The two constructs that already have DLQs establish the house pattern; the three webhook processors are the outliers.

Why this matters for the least-privilege effort (RFC #120 / ADR-002)

This is the failure mode that makes an IAM regression invisible, and it is why I am filing it out of the least-privilege work rather than as an unrelated ops nit.

cdk/src/constructs/jira-integration.ts:281-283 grants the processor grantReadData on the project-mapping, user-mapping, and workspace-registry tables. If a future least-privilege tightening scopes any of those too narrowly — a missing table ARN, a condition key that excludes an index, an over-narrow resource pattern — the resulting AccessDeniedException lands in the catch above and is reported as "tenant not in registry". The deploy succeeds, CloudFormation is green, cdk-nag is clean, and tasks simply stop being created. There is no alarm, no DLQ, and no error metric: the only evidence is a WARN whose text actively points the reader away from the real cause.

That inverts the guarantee the bootstrap work is meant to provide. ADR-002 and the resource-action-map exist to make permission gaps fail loudly at deploy or synth time. An over-tight runtime grant on this path instead fails silently at event time, days later, as "no tasks are being created" — precisely the class of defect #350 and the resource-action-map CRUD depth (#124) were built to eliminate. Until the drop is loud, the least-privilege ratchet can tighten a grant and quietly break the control plane.

Related: the read phase entries added to cdk/src/bootstrap/resource-action-map.ts in #124 cover the CloudFormation execution role, not the Lambda execution roles at issue here. That is a separate surface and worth stating explicitly so a reader does not assume the map already protects it.

How to detect it (today, without code changes)

Is it happening right now? The drop is only observable in logs:

# Any silent drop in the last 24h (Jira; substitute linear-/slack- as needed)
aws logs filter-log-events \
  --log-group-name /aws/lambda/<stack>-JiraWebhookProcessorFn \
  --start-time $(( ($(date +%s) - 86400) * 1000 )) \
  --filter-pattern '"dropping event"'

# Distinguish a REAL infra failure from a genuine not-onboarded tenant:
# an ERROR from the resolver immediately preceding the WARN proves the former.
aws logs filter-log-events \
  --log-group-name /aws/lambda/<stack>-JiraWebhookProcessorFn \
  --filter-pattern '"Failed to fetch Jira workspace registry row"'

The second query is the tell: Failed to fetch … + dropping event in the same invocation is always a bug, never a legitimate drop. Zero hits of the first with a live tenant is the healthy state.

Reproduce it locally — assert the current (wrong) behavior, then use the same test to prove the fix:

// cdk/test/handlers/jira-webhook-processor.test.ts
it('does not silently discard an admitted event when the registry read fails', async () => {
  ddbMock.on(GetCommand).rejects(
    Object.assign(new Error('rate exceeded'),
      { name: 'ProvisionedThroughputExceededException' }),
  );
  // Today: resolves undefined, createTaskCore never called, event lost.
  // Desired: rejects, so the async retry + DLQ can catch it.
  await expect(handler(eventWith(triggeredIssue()))).rejects.toThrow(/registry/i);
});

How to resolve it

Three parts; (1) and (2) are load-bearing together — a DLQ with a handler that never throws catches nothing.

1. Make the transient case loud in the processor. Use the existing getRegistryRowStrict (jira-oauth-resolver.ts:347) on the processor path, or classify the error and rethrow when it is retryable, keeping the silent return only for a genuine !Item. The lenient variant is right for best-effort hydration; it is wrong for an admission decision.

2. Add a DLQ + onFailure to the three processors, following the pattern already proven in fanout-consumer.ts and github-screenshot-integration.ts. Note the datapointsToAlarm: 1 detail from #284/#674: evaluationPeriods: 2 without it silently requires two consecutive breaching periods, which is 1-of-2 vs 2-of-2 — easy to get wrong.

3. Alarm on DLQ depth, mirroring #284's WebhookProcessorErrorAlarm, so a dropped event pages rather than waiting for a user to notice missing tasks.

Bootstrap impact: adding AWS::SQS::Queue to these constructs requires the ADR-002 checklist — cdk/src/bootstrap/policies/application.ts already grants sqs:CreateQueue/DeleteQueue/SetQueueAttributes and, as of #124, sqs:AddPermission/RemovePermission for AWS::SQS::QueuePolicy; resource-action-map.ts already maps both types. So the bootstrap side should need no change — but cdk/test/bootstrap/synth-coverage.test.ts must be re-run to confirm, since it validates every synthesized type against the granted set.

Acceptance criteria

  • A transient DynamoDB failure on the registry read causes the Jira/Linear/Slack processor to throw, not return, so the async retry fires.
  • Each of the three processors has a DLQ (or onFailure destination) and an alarm on its depth.
  • A test proves an admitted event is not lost when the registry read fails (see repro above).
  • The WARN text distinguishes "tenant not onboarded" from "registry unreadable" so logs are diagnosable.
  • mise //cdk:test -- test/bootstrap/synth-coverage passes with the new queues.
  • Runbook note in the Jira/Linear/Slack troubleshooting guides on inspecting the DLQ.

Provenance

Surfaced by the silent-failure-hunter agent while reviewing #710. Not caused by #710 — verified the identical dropping event path and the absent DLQ on origin/main @ 46263c09, so I explicitly excluded it from that review rather than blocking the PR on pre-existing debt.

Related: #284 (the same fix for the GitHub processor — the precedent to copy), #674 (its datapointsToAlarm 1-of-2 detail), #120 / ADR-002 / #350 (the least-privilege effort this protects), #124 (resource-action-map CRUD depth — CFN execution role, a different surface from the Lambda roles here), #709 / #710 (the review that surfaced it).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingci-cdBuild pipeline, deploy.yml, CI perf/caching, GitHub Actions workflowssecurityCedar/HITL, IAM least-privilege, secrets, PII/DLP, guardrails, supply-chain/CVE

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions