Skip to content

fix(webhooks): route webhook endpoints through tenant-aware engine methods#1388

Merged
nicoloboschi merged 1 commit intomainfrom
fix/webhook-schema-context
May 4, 2026
Merged

fix(webhooks): route webhook endpoints through tenant-aware engine methods#1388
nicoloboschi merged 1 commit intomainfrom
fix/webhook-schema-context

Conversation

@cdbartholomew
Copy link
Copy Markdown
Contributor

Summary

The HTTP webhook endpoints (create_webhook, list_webhooks, update_webhook, delete_webhook, list_webhook_deliveries) in hindsight-api-slim/hindsight_api/api/http.py resolved fq_table("webhooks") before any tenant authentication, doing raw pool.fetchrow / pool.fetch calls directly. fq_table reads the target schema from async-local context (get_current_schema()), which is set per-request by _authenticate_tenant. Skipping that step caused these endpoints to fall through to the default schema regardless of any per-request schema resolution.

Under any deployment that uses async-local context to set a per-request target schema (multi-tenant routing), this meant webhook rows were silently written to and read from the default schema while every other operation on the same bank — retain, consolidate, mental-models, etc. — correctly resolved to the per-target schema. The fire path then never matched the webhooks (it looks them up in the bank's resolved schema) and no webhook_delivery async operations were ever enqueued. The failure was completely silent: no errors, the webhook lists/creates/updates fine via the API, but no events ever fire.

Fix

Move the SQL into MemoryEngine methods (create_webhook, list_webhooks, update_webhook, delete_webhook, list_webhook_deliveries) that call _authenticate_tenant(request_context) first — matching the pattern already used by retain, consolidate, mental-models, etc. The HTTP handlers now delegate to those engine methods. fq_table("webhooks") resolves through the same schema contextvar as the rest of the bank's data.

The fire path itself is unchanged — _webhook_manager.fire_event_with_conn already takes its own schema= parameter.

Test plan

  • Six new schema-isolation tests in tests/test_webhooks.py covering create / list / update / delete / list-deliveries — each provisions a non-default schema via run_migrations and a swapped _tenant_extension, then asserts the relevant SQL targets the resolved schema and not the default.
  • All 31 pre-existing tests/test_webhooks.py cases still pass (37/37 total).
  • tests/test_audit_log.py (17/17) and tests/test_schema_isolation.py (4/4) clean.
  • One pre-existing failure in test_extensions.py::test_reflect_validation reproduces identically on main — unrelated to this change.

Notes

  • No public API contract changes (request/response shapes, status codes, paths all identical).
  • No new database migrations.
  • Other endpoints in http.py that follow a similar shape (_get_backend() then fq_table(...) without an engine method indirection) were spot-checked but not exhaustively audited — worth a follow-up grep job for from hindsight_api.engine.memory_engine import fq_table inside http.py to confirm webhooks were the only case where the schema context was missed before SQL was emitted.

…thods

Webhook create/list/get/update/delete and list-deliveries endpoints in
the HTTP layer were calling pool.fetchrow/pool.fetch directly with
fq_table("webhooks"), bypassing the async-local schema context that
fq_table reads via get_current_schema(). Under deployments that set a
per-request target schema (multi-tenant routing), this caused webhooks
to be written to and read from the default schema while every other
operation on the same bank correctly resolved to the per-target
schema. Webhooks would land in the wrong schema; the fire path
(which uses the bank's resolved schema) would not see them and never
enqueued webhook_delivery operations -- silent failure, no errors.

Move the SQL into MemoryEngine methods that call _authenticate_tenant
first (matching the pattern used by retain/consolidate/mental-models),
so fq_table sees the same schema as the rest of the bank's data.

Add schema-isolation tests covering create/list/get/update/delete and
deliveries.
@cdbartholomew cdbartholomew requested a review from nicoloboschi May 2, 2026 14:49
@nicoloboschi nicoloboschi merged commit b9069c2 into main May 4, 2026
62 of 64 checks passed
liling pushed a commit to liling/hindsight that referenced this pull request May 5, 2026
…thods (vectorize-io#1388)

Webhook create/list/get/update/delete and list-deliveries endpoints in
the HTTP layer were calling pool.fetchrow/pool.fetch directly with
fq_table("webhooks"), bypassing the async-local schema context that
fq_table reads via get_current_schema(). Under deployments that set a
per-request target schema (multi-tenant routing), this caused webhooks
to be written to and read from the default schema while every other
operation on the same bank correctly resolved to the per-target
schema. Webhooks would land in the wrong schema; the fire path
(which uses the bank's resolved schema) would not see them and never
enqueued webhook_delivery operations -- silent failure, no errors.

Move the SQL into MemoryEngine methods that call _authenticate_tenant
first (matching the pattern used by retain/consolidate/mental-models),
so fq_table sees the same schema as the rest of the bank's data.

Add schema-isolation tests covering create/list/get/update/delete and
deliveries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants