aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

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

Open
#734 0 comments 0 reactions 0 assignees View on GitHub
bug ci-cd security
Dominant language
TypeScript
Stars
143
Forks
46
Avg merge
3d 9h
Merged PRs (30d)
20

Description

## 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`:

```ts
} 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`
```ts
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 `return`s 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:

```bash
# Any silent drop in the last 24h (Jira; substitute linear-/slack- as needed)
aws logs filter-log-events \
--log-group-name /aws/lambda/-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/-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:

```ts
// 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).

Contributor guide

Open the contributing guide

Research direction

Start with cdk/src/handlers/shared/jira-oauth-resolver.ts and the Jira processor test, then compare the DLQ and alarm patterns in fanout-consumer.ts and github-screenshot-integration.ts. Inspect jira-integration.ts, linear-integration.ts, slack-integration.ts, and the webhook entry points, and run the named processor and bootstrap synth-coverage tests. Done means transient registry failures throw, all three processors have failure destinations and depth alarms, logs distinguish unreadable registries, and the troubleshooting guides document DLQ inspection.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
backend-api-design, infrastructure, observability
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.