fix(plugin-webhooks): re-arm an encrypted-secret webhook when the CryptoProvider registers (#8022)

.changeset/webhook-cache-crypto-registration-ordering.md

@@ -0,0 +1,55 @@
+---
+"@objectstack/plugin-webhooks": patch
+"@objectstack/objectql": patch
+---
+
+fix(plugin-webhooks): a webhook holding an encrypted signing secret re-arms the moment the CryptoProvider registers, instead of ~60s later (#8022)
+
+For roughly **60 seconds after every server start**, a webhook whose
+`signing_secret` is encrypted (the population #7799 created) was **not
+subscribed**. A record change in that window produced no delivery **and no
+`sys_http_delivery` row at all** — no dead letter, no retry, no durable trace
+that anything was missed — while `GET /api/v1/data/sys_webhook/` kept reading
+`active: true`, so the webhook looked armed in Setup the whole time. It
+self-healed at the next periodic cache refresh, which is why it was invisible to
+anyone not watching that window.
+
+**The fail-closed behaviour is unchanged and is not the bug.** Dropping a
+subscription whose stored key cannot be recovered — rather than delivering it
+unsigned — is #7799's whole point and still holds: the signature is the
+receiver's only proof of origin, and a webhook that stops arriving gets
+investigated while one that keeps arriving unsigned teaches the receiver to
+accept unauthenticated traffic. What was wrong is that a fail-closed drop
+outlived its own cause.
+
+**The ordering.** It was never a race that sometimes went the other way. Plugins
+run inside `kernel:ready`, which `runtime.start()` completes; the host's
+composition root calls `engine.setCryptoProvider(...)` only *after*
+`runtime.start()` returns (`packages/cli/src/commands/serve.ts`,
+`packages/verify/src/harness.ts`). So `AutoEnqueuer`'s first subscription-cache
+build reliably preceded the capability it needs, dropped every secret-bearing
+row on what it could see, and nothing re-read until the periodic refresh.
+
+`ObjectQL` now reports the registration (`onCryptoProviderChange(listener)`,
+fired after the provider is in place), and the auto-enqueuer subscribes
+**before** its first build and rebuilds the cache when it fires. Re-arming is
+immediate and event-driven — no polling, and no shorter-but-still-present
+window. The re-arm deliberately does not join an in-flight refresh: the build
+most likely running at that moment is the pre-registration one, and joining it
+would report success having re-armed nothing.
+
+The channel is feature-detected, as `resolveSecretField` already was — this
+plugin takes no dependency on `@objectstack/objectql`. An engine without it keeps
+the previous behaviour, with the periodic refresh as the backstop.
+
+**The drop is also no longer quiet.** A subscription dropped for an unresolvable
+key now reports at `error` with the consequence and the fix stated in the
+message, and carries an ADR-0112 `code`/`status` pair (`INTERNAL_ERROR`/500) in
+its metadata — the same pair the seeder's refusal for the same cause already
+carried. Per AGENTS.md it is said **once** per outage per webhook rather than
+every refresh cycle, and a webhook that recovers and breaks again is loud again.
+
+Unaffected, and verified still true: the secret's bytes appear nowhere in
+`sys_webhook` or in a delivery row, deliveries carry `signature` and never the
+key (#7722), and a delivery whose key exists only as ciphertext after a restart
+still produces the byte-identical HMAC receivers already verify.

packages/objectql/src/engine.ts

@@ -1661,6 +1661,16 @@ export class ObjectQL implements IObjectQLEngine {
  // persists cleartext). Injected by the host via setCryptoProvider().
  private cryptoProvider?: ICryptoProvider;
 
+ // [#8022] Listeners notified when a crypto provider is (re)registered.
+ // Server-side consumers that dereference a secret at BOOT — the webhook
+ // auto-enqueuer's subscription cache is the one this was built for — run
+ // inside `kernel:ready`, which every host completes BEFORE its composition
+ // root injects a provider. Their first read therefore fails closed against a
+ // capability that is about to exist, and without a notification the only way
+ // back is to poll. The engine is the sole party that knows the moment it
+ // arrives, so the notification belongs here.
+ private readonly cryptoProviderListeners = new Set<() => void>();
+
  // [ADR-0105 D2 / #3623] Posture accessor for driver-scope widening under the
  // `group` posture. Injected by SecurityPlugin via setTenancyPostureProvider();
  // absent = equality scoping (fail toward isolation).
@@ -4613,10 +4623,51 @@ export class ObjectQL implements IObjectQLEngine {
  * Mirrors the Settings subsystem's ICryptoProvider wiring; the host (e.g.
  * `serve`) injects `LocalCryptoProvider` in dev and a KMS/Vault-backed
  * provider in production.
+ *
+ * Notifies {@link onCryptoProviderChange} listeners AFTER the provider is in
+ * place, so a listener that immediately re-reads a secret sees the new
+ * capability rather than the state that made it fail (#8022).
  */
  setCryptoProvider(provider: ICryptoProvider): void {
  this.cryptoProvider = provider;
  this.logger.info('CryptoProvider configured for secret fields');
+ // A listener is a re-arm, never part of this call's contract: one that
+ // throws must not fail the host's composition root, and must not stop the
+ // listeners behind it from re-arming.
+ for (const listener of [...this.cryptoProviderListeners]) {
+ try {
+ listener();
+ } catch (err) {
+ this.logger.warn('CryptoProvider registration listener failed', {
+ error: (err as Error)?.message ?? String(err),
+ });
+ }
+ }
+ }
+
+ /**
+ * [#8022] Observe crypto-provider registration.
+ *
+ * Exists for consumers that must dereference a `secret` field on a schedule
+ * they do not control — the boot path. `secret` reads are fail-closed by
+ * design (#7799), which is correct, but "no provider" at boot is a
+ * *transient* state on every host: `kernel:ready` runs plugins, and only
+ * after `runtime.start()` returns does the composition root call
+ * {@link setCryptoProvider}. A consumer whose cache was built in that gap is
+ * wrong until it rebuilds, and polling is the only alternative to being told.
+ *
+ * Fires on every registration, including a later replacement (a KMS provider
+ * swapped in over the dev one) — a listener that re-reads is correct in both
+ * cases, and a re-read is cheap next to signing with a key from the wrong
+ * provider.
+ *
+ * @returns an unsubscribe function; call it when the listener's owner stops.
+ */
+ onCryptoProviderChange(listener: () => void): () => void {
+ this.cryptoProviderListeners.add(listener);
+ return () => {
+ this.cryptoProviderListeners.delete(listener);
+ };
  }
 
  /**

packages/plugins/plugin-webhooks/src/auto-enqueuer.ts

@@ -3,7 +3,14 @@
 import type { IDataEngine, IRealtimeService, RealtimeEventPayload } from '@objectstack/spec/contracts';
 import type { WebhookTriggerType } from '@objectstack/spec/automation';
 import type { EnqueueHttpInput } from '@objectstack/service-messaging';
-import { WEBHOOK_SECRET_FIELD, readLegacySecret, resolveWebhookSecret } from './webhook-secret.js';
+import {
+ WEBHOOK_SECRET_FIELD,
+ WEBHOOK_SECRET_REFUSAL_CODE,
+ WEBHOOK_SECRET_REFUSAL_STATUS,
+ onCryptoProviderChange,
+ readLegacySecret,
+ resolveWebhookSecret,
+} from './webhook-secret.js';
 
 /**
  * The authored trigger vocabulary, taken from the spec rather than restated
@@ -116,6 +123,16 @@ export class AutoEnqueuer {
  private refreshTimer: ReturnType<typeof setInterval> | undefined;
  private running = false;
  private refreshing: Promise<void> | undefined;
+ /** [#8022] Detach for the engine's crypto-registration listener. */
+ private unbindCryptoListener: (() => void) | undefined;
+ /**
+ * [#8022] Webhook ids currently dropped for an unresolvable signing key.
+ * Held so the loud first report is said ONCE per outage (AGENTS.md
+ * "Degradation log levels": *say it once, at the first degradation*) and
+ * again if the same webhook breaks after recovering — not once per row per
+ * refresh, forever.
+ */
+ private readonly droppedForSecret = new Set<string>();
 
  constructor(
  private readonly engine: IDataEngine,
@@ -135,6 +152,17 @@ export class AutoEnqueuer {
  if (this.running) return;
  this.running = true;
 
+ // [#8022] Bound BEFORE the first build, not after: on every host the
+ // composition root wires the CryptoProvider after `runtime.start()`
+ // returns, i.e. after the `kernel:ready` handler that runs this method
+ // — so the registration we need to hear about can land at any point
+ // from here on, including while the await below is still in flight.
+ // Subscribing first makes that unmissable; subscribing after the
+ // refresh would reintroduce the same race in miniature.
+ this.unbindCryptoListener = onCryptoProviderChange(this.engine, () =>
+ this.rearmAfterCryptoRegistered(),
+ );
+
  await this.refresh();
 
  // Main subscription: every data event → match → enqueue.
@@ -167,9 +195,38 @@ export class AutoEnqueuer {
  if (this.subId) await this.realtime.unsubscribe(this.subId);
  if (this.subIdSelfHeal) await this.realtime.unsubscribe(this.subIdSelfHeal);
  if (this.refreshTimer) clearInterval(this.refreshTimer);
+ this.unbindCryptoListener?.();
  this.subId = undefined;
  this.subIdSelfHeal = undefined;
  this.refreshTimer = undefined;
+ this.unbindCryptoListener = undefined;
+ }
+
+ /**
+ * [#8022] The engine just gained a CryptoProvider — rebuild the cache so
+ * subscriptions dropped for an unresolvable signing key re-arm now, instead
+ * of at the next periodic refresh up to {@link refreshIntervalMs} away.
+ *
+ * It deliberately does NOT call {@link refresh} directly. `refresh()`
+ * coalesces onto an in-flight build, and the build most likely to be in
+ * flight right now is the one from `start()` — the very build whose rows
+ * were read while there was no provider. Joining it would return "refreshed"
+ * having re-armed nothing, which is this issue with an extra step. So: let
+ * whatever is running finish, then read again.
+ */
+ private rearmAfterCryptoRegistered(): void {
+ const inFlight = this.refreshing ?? Promise.resolve();
+ void inFlight
+ // A failed in-flight refresh already logged; it must not stop the
+ // re-arm, which is the whole point of this callback.
+ .catch(() => undefined)
+ .then(() => (this.running ? this.refresh() : undefined))
+ .catch((err) =>
+ this.logger.warn?.(
+ '[webhook-auto-enqueuer] re-arm after CryptoProvider registration failed',
+ err,
+ ),
+ );
  }
 
  /**
@@ -220,6 +277,17 @@ export class AutoEnqueuer {
  this.subscriptions.clear();
  for (const [k, v] of next) this.subscriptions.set(k, v);
 
+ // [#8022] Forget rows this refresh no longer sees — deleted, or
+ // deactivated. Otherwise the set grows for the life of the process, and
+ // a webhook turned off while broken and later turned back on still
+ // broken would have its first report suppressed as a repeat.
+ if (this.droppedForSecret.size > 0) {
+ const live = new Set(rows.map((r) => String(r?.id)));
+ for (const id of this.droppedForSecret) {
+ if (!live.has(id)) this.droppedForSecret.delete(id);
+ }
+ }
+
  this.logger.debug?.('[webhook-auto-enqueuer] cache refreshed', {
  objects: this.subscriptions.size,
  rows: rows.length,
@@ -258,15 +326,13 @@ export class AutoEnqueuer {
  const stored = await resolveWebhookSecret(this.engine, row, this.subscriptionsObject);
  if (stored) {
  sub.secret = stored;
+ // Recovered — a later break is a new outage and gets said loudly
+ // again rather than being swallowed as a repeat.
+ this.droppedForSecret.delete(sub.id);
  return true;
  }
  } catch (err) {
- this.logger.warn?.(
- `[webhook-auto-enqueuer] webhook '${sub.name}' holds an encrypted signing secret that ` +
- `could not be decrypted — the subscription is DROPPED rather than delivered unsigned ` +
- `(#7799). Deliveries resume once the sys_secret row and CryptoProvider are reachable.`,
- { id: sub.id, field: WEBHOOK_SECRET_FIELD, err: (err as Error)?.message ?? err },
- );
+ this.reportDrop(sub, err);
  return false;
  }
 
@@ -281,9 +347,72 @@ export class AutoEnqueuer {
  );
  sub.secret = legacy;
  }
+ this.droppedForSecret.delete(sub.id);
  return true;
  }
 
+ /**
+ * [#8022] Report a subscription dropped for an unresolvable signing key.
+ *
+ * ## Why `error`, and why only the first time
+ * AGENTS.md decides the level with one question: *after the degradation,
+ * does the system still look normal from the outside while something the
+ * system claims is happening is not?* Here the answer is yes, and it is the
+ * whole defect — `GET /api/v1/data/sys_webhook` keeps reading
+ * `active: true`, Setup keeps showing the webhook armed, and every matching
+ * record change is discarded with no delivery and no `sys_http_delivery`
+ * row to find afterwards. That is a durability degradation wearing a
+ * functional degradation's clothes, so it owes the two things an `error`
+ * owes: the consequence, concretely, and the fix.
+ *
+ * Said ONCE per outage per webhook, per the same section. The cache is
+ * rebuilt every {@link refreshIntervalMs}; an unfixed misconfiguration would
+ * otherwise print this line every 60s forever, which is how an `error`
+ * channel becomes unreadable — the failure mode that made the founding
+ * incident's `warn` invisible. Repeats drop to `debug`; a recovery clears
+ * the id, so a re-break is loud again.
+ *
+ * ADR-0112: `code` + `status` travel in the meta so a consumer branches on
+ * the pair, not on message text. Same pair the seeder's refusal carries for
+ * the same underlying cause.
+ */
+ private reportDrop(sub: CachedSubscription, err: unknown): void {
+ const meta = {
+ id: sub.id,
+ webhook: sub.name,
+ field: WEBHOOK_SECRET_FIELD,
+ code: WEBHOOK_SECRET_REFUSAL_CODE,
+ status: WEBHOOK_SECRET_REFUSAL_STATUS,
+ err: (err as Error)?.message ?? err,
+ };
+ if (this.droppedForSecret.has(sub.id)) {
+ this.logger.debug?.(
+ `[webhook-auto-enqueuer] webhook '${sub.name}' is still dropped for an unresolvable ` +
+ 'signing secret (#7799/#8022)',
+ meta,
+ );
+ return;
+ }
+ this.droppedForSecret.add(sub.id);
+ const message =
+ `[webhook-auto-enqueuer] webhook '${sub.name}' holds an encrypted signing secret that ` +
+ 'could not be decrypted — the subscription is DROPPED rather than delivered unsigned ' +
+ '(#7799), so every matching record change is discarded with NO delivery and NO ' +
+ 'sys_http_delivery row, while the row keeps reading active:true in Setup. Fix: register a ' +
+ 'CryptoProvider (engine.setCryptoProvider — LocalCryptoProvider in dev, KMS/Vault in ' +
+ 'production) with the same key the secret was written under, and make sure the sys_secret ' +
+ 'row is reachable; the subscription re-arms on registration (#8022) and at the next ' +
+ 'periodic refresh.';
+ // The logger surface is a subset of console/kernel logger — `error` is
+ // optional on it, so fall back rather than silently losing the report
+ // on a logger that only implements `warn`.
+ if (typeof this.logger.error === 'function') {
+ this.logger.error(message, err, meta);
+ } else {
+ this.logger.warn?.(message, meta);
+ }
+ }
+
  private parseRow(row: any): CachedSubscription | null {
  if (!row?.id || !row?.url) return null;
  // `triggers` is now authored as a multi-select (stored as an array), but