feat/one-api

Missing SEALING_KEY causes silent delivery loss — EventBridge never retries

packages/event-bus/src/handlers/webhook-dispatcher.lambda.ts:85

When SEALING_KEY is missing the handler logs one error and returns normally. EventBridge treats a normal return as success (no DLQ, no retryAttempts). Skipped endpoints increment neither delivered nor failed counters, and recordDeliveryOutcome is never called, so failureCount never increments and auto-disable never fires. Every webhook event is silently dropped with no alerting. Fix: throw when sealingKey is null and at least one active endpoint exists, so EventBridge routes to DLQ and retries.

SEALING_KEY_ID not injected by CDK — key rotation silently breaks all deliveries

infra/cdk/src/stacks/events/events.stack.ts:149

The Lambda reads process.env.SEALING_KEY_ID ?? 'k1' but CDK only injects SEALING_KEY. Works today because CURRENT_KEY_ID in sealing-key.ts is also 'k1'. During rotation: new key with keyId 'k2' → sealed secrets embed 'k2' → Lambda builds Map with key 'k1' → open() returns null for every endpoint → complete silent outage. Fix: inject SEALING_KEY_ID from CDK alongside SEALING_KEY.

Migration 0067 NOT NULL columns fail if 0066 didn't run on a non-empty table

packages/database/drizzle/0067_seal_webhook_secret.sql:1

Drizzle applies migrations sequentially, but on a branch environment forked after 0066 was committed, the journal marks 0066 as applied but never executes it. If that branch DB has any webhook_endpoints rows, 0067's ADD COLUMN ... NOT NULL fails. A safer form: add as nullable, delete remaining rows as a safety net, then SET NOT NULL.

Negative caching on transient Secrets Manager failure — recovery requires process restart

apps/platform/src/lib/secrets/sealing-key.ts:63

cached = null is permanent for the process lifetime. A transient SM network error at first request permanently disables webhook creation until the process restarts. Acceptable in Vercel serverless (cold start re-reads). Undocumented recovery path.

SEALING_KEY stored as plaintext in Lambda env via unsafeUnwrap()

infra/cdk/src/stacks/events/events.stack.ts:158

SecretValue.secretsManager().unsafeUnwrap() emits a CloudFormation dynamic reference resolved at deploy time, injecting the raw 32-byte AES key as a plaintext Lambda env var. Any IAM principal with lambda:GetFunctionConfiguration (typical developer/CI role) can retrieve the key — collapsing Secrets Manager's access boundary onto a much broader permission. Fix: read from Secrets Manager at Lambda cold-start via the SDK instead, so the access boundary stays on secretsmanager:GetSecretValue.

Migration 0066 unconditionally deletes all webhook endpoints — no guard if assertion is stale

packages/database/drizzle/0066_purge_cleartext_webhook_secrets.sql:1

DELETE FROM webhook_endpoints with no WHERE clause. The comment asserts production has zero endpoints (verified 2026-07-28), but nothing enforces this in the SQL. If the migration runs on a future state or a branch environment with real rows, it silently destroys all endpoint config and secrets with no recovery path. Safer: add a guard (RAISE EXCEPTION if COUNT(*) > 0) so the migration refuses to run if the assertion is wrong.

No key rotation path — single-entry keyring means rotation causes complete outage

packages/event-bus/src/handlers/webhook-dispatcher.lambda.ts:79

The open() API accepts ReadonlyMap<string, Buffer> specifically to support rotation (both old and new key loaded during re-sealing window). But the Lambda always builds a one-entry Map. There is no re-sealing utility. During a key rotation, all rows sealed under the old keyId return null from open() and every webhook is silently skipped for the entire window between new Lambda deploy and full DB re-sealing.

Plaintext secret in CreateWebhookEndpointResult traverses handler — accidental log risk

domains/core/src/webhook-endpoint/webhook-endpoint.shells.ts:119

The shell returns ok({ endpoint, secret }) where secret is the full plaintext signing key. It travels through the handler and response mapper. Any future response-level audit logging, Sentry breadcrumbs, or structured logging that captures handler return values will inadvertently capture it. Consider wrapping in a non-serializable value object or an explicit strip before the logging boundary.

FORMAT_VERSION strict equality is correct but fragile if a v0/legacy mode is ever added

domains/core/src/lib/sealed-secret.ts:92

open() rejects anything that isn't FORMAT_VERSION. Correct now. Design constraint: if a legacy format is ever added, the version check must stay a strict set-membership check — never a range or fallback — or version-confusion attacks become possible. Worth a comment to flag this invariant.

Uniform null return from open() is correct — no error oracle

domains/core/src/lib/sealed-secret.ts:71

open() returns null for all failure modes. An attacker submitting arbitrary sealed blobs cannot determine whether failure was keyId miss, AAD mismatch, or ciphertext tampering. Correct design.

GCM timing side-channel is not exploitable in this deployment model

domains/core/src/lib/sealed-secret.ts:52

Node's OpenSSL GCM tag comparison may not be constant-time, but sealed secrets are stored in the application's own DB — not attacker-supplied inputs. Exploiting a timing oracle would require prior DB write access, at which point there are more direct attack paths.

getBatuEnv() defined three times — acknowledged in code but not consolidated

apps/platform/src/lib/env/batu-env.ts:14

The new batu-env.ts exports getBatuEnv(), but the same function exists inline in credentials.handler.ts:58 and admin-organizations.handler.ts:285. The JSDoc even names them. The canonical-form 'explore before creating' rule requires extending existing flows. Here the canonical copy was created new while two callers were left with private copies. If getBatuEnv() is ever updated (new env name), the two old handlers silently diverge. Both handlers should have been updated in this commit.

Decision pre-builds event with aggregateId: '' — shell ignores it, FCIS layering smell

domains/core/src/webhook-endpoint/webhook-endpoint.decisions.ts:142

decideCreate() produces a WebhookEndpointCreatedEvent with aggregateId: '' (shell 'post-fills'). But the shell passes inserted.id directly to outboxQueries — it never uses decision.value.event.aggregateId. The event struct in the Decision is intentionally wrong. Canonical FCIS: decisions describe what to write. Compare decideDelete() which correctly sets aggregateId: existing.id. Clean fix: omit aggregateId from CreateWebhookEndpointDecision event type and have the shell supply it.

seal() throws plain Error on wrong-length key — escapes shell's Result boundary

domains/core/src/lib/sealed-secret.ts:50

A wrong-sized key throws new Error() which escapes the shell's Promise<Result<...>> as an unhandled rejection. Domain-patterns.md allows exceptions for programmer errors (this qualifies), but the shell has no try/catch around the synchronous seal() call. A misconfigured key would surface as an unhandled exception rather than a Result.err, which is harder to handle uniformly at the caller.

No tests for sealing-key.ts resolver — env var path, SM fallback, negative caching all uncovered

apps/platform/src/lib/secrets/sealing-key.ts

The module exports __resetSealingKeyCacheForTests() as a test seam, but no test file exists. Missing: (1) SEALING_KEY env var correct → resolves; (2) SEALING_KEY with trailing newline (the .trim() comment calls this out) → resolves; (3) wrong-length key → null; (4) negative result cached so SM is not re-called; (5) cache reset unblocks re-read. A regression in the .trim() path would silently misconfigure every webhook in prod.

No unit tests for webhook-dispatcher Lambda — missing-key and tampered-ciphertext paths untested

packages/event-bus/src/handlers/webhook-dispatcher.lambda.ts

No test file for the dispatcher. Two new failure modes are completely untested: (1) SEALING_KEY absent → sealingKey null → all endpoints silently skipped; (2) open() returns null for an endpoint (tampered secretSealed, wrong keyId) → endpoint skipped, counter not incremented. Both are undetectable without tests.

No handler unit test for webhooks.handler.ts — sealingKey-null 500 path untested

apps/platform/src/api/handlers/public-v1/webhooks.handler.ts

The IDOR integration test relaxes assertions for webhooksCreate to 'no crash body leak' (it's in INFRA_DEPENDENT). No test asserts the 500 response shape (status:'error', data.error:'internal_error'), nor that a valid key produces a 201. A regression removing the null-key guard would not be caught.

webhookSecretPrefix() is untested

domains/core/src/webhook-endpoint/webhook-endpoint.lib.ts

New function used in the shell to compute the DB-stored display prefix, but has no test in webhook-endpoint.lib.test.ts. Missing: correct character count (whsec_ + 6 = 12), behavior on short secret, alignment with the mapper's secret_preview output.

No shell integration test — seal→open round-trip across the DB layer untested

domains/core/src/webhook-endpoint/webhook-endpoint.shells.ts

No webhook-endpoint.shells.integration.test.ts. The key correctness property — seal() → DB insert → read back via queries → open() returns the original secret — is untested. A mapper regression (dropping secretSealed) would leave the dispatcher silently broken with no test catching it.

IDOR test no longer verifies status-code isolation for webhooksCreate when sealing key is available

apps/platform/src/__tests__/integration/public-v1-idor.test.ts

webhooksCreate is in INFRA_DEPENDENT (relaxed to 'no crash body leak'). There is no separate test that seeds a valid sealing key and verifies IDOR isolation for webhook creates. The security property is only tested when CI happens to have SEALING_KEY configured, which it likely does not.

Migration ordering not tested — 0066 must run before 0067

packages/database/drizzle/0067_seal_webhook_secret.sql

0067 adds NOT NULL columns relying on 0066 having deleted all rows first. No migration test or guard. Branch environments with the known Drizzle journal-mismatch pattern could silently fail.

Key map construction duplicated between sealing-key.ts and webhook-dispatcher

packages/event-bus/src/handlers/webhook-dispatcher.lambda.ts:79

sealing-key.ts builds { key, keyId } (SealingKey) while the Lambda re-implements base64 decode, length check, and Map construction from the same env vars with its own SEALING_KEY_ID env var. Two divergent code paths for the same operation. A shared sealingKeyMap() helper would eliminate drift.

open() split-on-colon safety deserves a comment — base64url excluding ':' is the non-obvious invariant

domains/core/src/lib/sealed-secret.ts:87

parts.length !== 5 is safe only because base64url never contains ':'. This non-obvious invariant makes the simple split() correct as a framing mechanism. A comment would prevent a future maintainer from 'simplifying' to a different delimiter without realizing the current approach is already sound.