mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 12:16:54 +00:00
feat(webhooks): HTTP webhooks to trigger agents with HMAC auth + durable callbacks (#2)
* feat(webhooks): HTTP webhooks to trigger agents with HMAC auth and durable callbacks
Add multi-tenant HTTP webhook endpoints for agent triggering:
- /v1/webhooks/message: send messages to channels
- /v1/webhooks/llm: sync/async LLM prompts with HMAC-signed callbacks
- HMAC-256 + bearer token authentication
- Rate limiting and tenant isolation
- Durable callback worker with exponential backoff
- PG 000056 + SQLite schema v25 migrations
- Unit + integration tests, P0 tenant isolation invariants
- Channel media capability helpers for attachment routing
- Comprehensive webhook documentation and i18n strings
* fix(webhooks): address post-review findings (K1-K10)
Comprehensive post-merge fixes addressing 10 blocking code review issues
and 2 adversarial re-audit findings in webhook-agent-triggering feature:
K1: Fix auth middleware tenant context lookup sequencing — move
tenant context injection before authenticate() call to prevent
unscoped secret lookups.
K2: Canonicalize JSON payload format for jsonb compatibility across
PostgreSQL and SQLite — ensure consistent serialization without
whitespace variance to prevent hash mismatches.
K3: Add fail-closed JSON parsing in body hash extraction with explicit
error handling for malformed payloads before HMAC verification.
K4: Fix worker queue wedge by properly draining slot reservations
when delivery succeeds, preventing permanent slot occupancy.
K5: Implement lease-token optimistic concurrency control to prevent
duplicate webhook delivery under high concurrency or retry storms.
K6: Add AES-256-GCM encrypted secret storage at rest with fail-fast
skip-mount when GOCLAW_ENCRYPTION_KEY environment variable unset.
K7: Implement IP allowlist enforcement supporting both CIDR ranges
and exact IP matching with proper X-Forwarded-For parsing.
K8: Add HMAC replay nonce cache (5min expiry, non-blocking async flush)
to prevent request replay attacks on webhook handler.
K9: Fix invariant test schema selection — replace hardcoded assumption
with explicit schema name from config to support multi-schema testing.
K10: Consolidate rate limiters into single shared instance to prevent
per-endpoint limiter starvation and ensure fair rate limiting.
New database migrations:
- 000057: webhook_calls.lease_token for optimistic concurrency
- 000058: webhooks.encrypted_secret_key for AES-256-GCM encryption
New i18n keys: MsgWebhookIPDenied, MsgWebhookEncryptionUnavailable
(with English, Vietnamese, Chinese translations).
New modules:
- internal/http/webhooks_payload.go: JSON canonicalization + body hash
- internal/http/webhooks_nonce.go: Replay nonce cache implementation
- internal/http/webhooks_idempotency_test.go: Integration tests
Documentation updates:
- docs/webhooks.md: §13-14 security sections, encryption flow
- docs/00-architecture-overview.md: webhook subsystem security overview
- docs/codebase-summary.md: webhook security patterns
- docs/project-changelog.md: webhook fixes changelog
Test coverage: 53 webhook tests + 4 P0 invariant tests all passing.
No tenant isolation violations. All security gates enforced.
* docs(journals): webhook feature ship + fix cycle entries
* fix(webhooks): address Claude review findings
- webhooks_llm.go: remove misleading ptr() helper; use &completedAt
pattern for error-path audit rows (matches success path)
- webhooks_auth.go: wrap TouchLastUsed context in WithoutCancel so
background DB update isn't cancelled when HTTP response completes
- store GetByIDUnscoped (PG+SQLite): add NOT revoked / revoked = 0
filter for defense-in-depth parity with GetByHashUnscoped
- webhooks/sign.go: fix package doc — HMAC key is raw plaintext
secret bytes, not hex-decoded SHA-256
- webhooks_admin.go: check auth before encKey guard to avoid leaking
config state to unauthenticated callers
- webhooks_ratelimit.go: two-phase Load→LoadOrStore to avoid per-call
entry allocation on the hot path
* docs(webhooks): fix Sign() function doc to match actual key input
Function-level comment still referenced hex-decoded SecretHash after
the package-level doc was corrected. Align with actual caller usage
([]byte(rawSecret)).
* fix(webhooks): use WithoutCancel for worker execute DB updates
Terminal status writes in execute() ran through the worker main-loop
ctx, which is cancelled on graceful shutdown. If the outbound send
completed but the status update raced with shutdown, the row stayed
in 'running' and got re-delivered via reclaimStale. WithoutCancel
lets the DB write survive worker cancellation while preserving
propagated values (tenant ID, etc.).
* fix(webhooks): move tctx init before panic defer in worker execute
Panic recovery called updateRetry with raw ctx (no tenant ID), making
requireTenantID fail and the reset-to-retry DB write silently drop.
Row stayed 'running' until reclaimStale (~90s delay). Init tctx first
so defer closure captures tenant-scoped non-cancellable context.
* fix(webhooks): pass tenant-scoped tctx to invokeAgent in worker
execute() was passing the raw worker-loop ctx (no tenant ID) to
invokeAgent → router.Get → PGAgentStore.GetByID. GetByID reads
TenantIDFromContext which returned uuid.Nil, making every lookup
return 'agent not found'. Async LLM webhook calls silently failed
all retries. Pass tctx (already tenant-scoped + WithoutCancel) so
the router resolves the agent correctly.
* fix(tests): resolve integration test compile errors
- Remove duplicate contains() in mcp_grant_revoke_test.go (already
defined in tts_gemini_live_test.go)
- Update webhooks_admin_test.go RotateSecret call to match current
5-arg signature (newSecretHash, newPrefix, newEncryptedSecret)
* fix(webhooks): default nil scopes/ip_allowlist to empty slice in Create
PG columns are NOT NULL DEFAULT '{}'. Explicit NULL from pqStringArray(nil)
violated the constraint, breaking TestWebhookAdminCRUD/TenantIsolation.
Coerce nil slices to empty []string{} so the default applies at the DB layer.
* chore: trigger CI on digitopvn/goclaw fork
* ci: retrigger workflows
* fix(webhooks): renumber migrations to 000059-000061 for merge train
This commit is contained in:
1 parent
e589545ff5
commit
ddf8e1099f
59 files changed
+10920
-2
No files matched your search
@@ -119,6 +119,7 @@ Parity enforced by `ui/web/src/__tests__/i18n-tts-key-parity.test.ts` (vitest).
|
||||
|
||||
---
|
||||
|
||||
<<<<<<< HEAD
|
||||
## Image Generation
|
||||
|
||||
Native `image_generation` support in the Codex provider (`POST /codex/responses`) + passthrough in the OpenAI-compat path.
|
||||
@@ -137,6 +138,57 @@ Native `image_generation` support in the Codex provider (`POST /codex/responses`
|
||||
**Persistence:** `internal/agent/media.go persistAssistantImages()` writes final images to `{workspace}/media/{sha256}.{ext}`, returns `MediaRef` entries, clears inline `Images[]`. Idempotent on hash. Invoked from `pipeline.FinalizeStage` via `Deps.PersistAssistantImages` callback.
|
||||
|
||||
**Web UI:** Download filename resolver (`imageGenDownloadName`) in `ui/web/src/components/chat/media-gallery.tsx`. Image generation works automatically when the agent has the `create_image` tool — no user-facing toggle.
|
||||
=======
|
||||
## Webhook Subsystem
|
||||
|
||||
External systems invoke agents or send channel messages via webhooks without gateway tokens.
|
||||
|
||||
### Components
|
||||
|
||||
| Path | Purpose |
|
||||
|------|---------|
|
||||
| `internal/http/webhooks_admin.go` | CRUD handlers (create, list, get, patch, rotate, revoke) |
|
||||
| `internal/http/webhooks_auth.go` | Bearer + HMAC signature verification, IPAllowlist, tenant scope |
|
||||
| `internal/http/webhooks_nonce.go` | Per-process HMAC replay cache (320s TTL) |
|
||||
| `internal/http/webhooks_llm.go` | `POST /v1/webhooks/llm` endpoint (sync 30s / async) |
|
||||
| `internal/http/webhooks_message.go` | `POST /v1/webhooks/message` endpoint (channel delivery) |
|
||||
| `internal/http/webhooks_ratelimit.go` | Per-webhook + per-tenant rate limiting |
|
||||
| `internal/http/webhooks_idempotency.go` | `Idempotency-Key` header dedup cache (24h TTL) |
|
||||
| `internal/http/webhooks_media_fetch.go` | SSRF-guarded media URL fetch + MIME validation |
|
||||
| `internal/webhooks/worker.go` | Async callback poller + delivery goroutines |
|
||||
| `internal/webhooks/backoff.go` | Exponential retry schedule `[30s, 2m, 10m, 1h, 6h]` |
|
||||
| `internal/webhooks/sign.go` | HMAC-SHA256 signing for outbound callbacks |
|
||||
| `internal/webhooks/limiter.go` | Shared rate limiter for callback delivery |
|
||||
| `internal/store/webhook_store.go` | `WebhookStore` interface + `WebhookCallStore` |
|
||||
| `internal/store/pg/webhook_store.go` | PostgreSQL implementation (tenant-scoped) |
|
||||
| `internal/store/sqlitestore/webhook_store.go` | SQLite implementation (Lite edition) |
|
||||
| `migrations/` | PG migrations 000056–000058 (webhooks + lease token + encrypted secret) |
|
||||
|
||||
### Auth Flow
|
||||
|
||||
1. **Bearer auth**: Hash the token, lookup `secret_hash` globally (via `GetByHashUnscoped`) → return webhook + tenantID.
|
||||
2. **HMAC auth**: Parse `X-Webhook-Id` header, lookup webhook globally → verify signature timestamp + nonce.
|
||||
3. **Tenant inject**: Re-scope context with webhook's tenantID for all downstream calls.
|
||||
4. **IP allowlist**: If non-empty, check request source IP (CIDR or exact) against list. Empty = allow all.
|
||||
5. **Rate limit**: Check per-webhook + per-tenant buckets. Either rejects = 429.
|
||||
|
||||
### Idempotency & Lease Tokens
|
||||
|
||||
- **Inbound**: `Idempotency-Key` header dedup (24h cache). Same key + same body = cached response; same key + different body = 409 Conflict.
|
||||
- **Outbound**: Each `webhook_calls` row has `lease_token` (UUID). Worker claims row with CAS. On update, token proves ownership — prevents stale receivers from overwriting.
|
||||
|
||||
### Secret Encryption
|
||||
|
||||
Raw webhook secret encrypted at rest via AES-256-GCM using `GOCLAW_ENCRYPTION_KEY` (same as LLM provider keys).
|
||||
- Database: stores `encrypted_secret` column + `secret_hash` (for bearer lookups).
|
||||
- DB compromise does not leak HMAC material.
|
||||
- Clients receive plaintext secret once (create/rotate response) — must store securely.
|
||||
|
||||
### Audit Payload
|
||||
|
||||
All webhook calls logged with canonical `{"body_hash":"<sha256-hex>","meta":{...}}` shape in `webhook_calls.request_payload` (JSON).
|
||||
Used by idempotency checker to detect body mismatches on replay.
|
||||
>>>>>>> a83f4090 (fix(webhooks): address post-review findings (K1-K10))
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in new issue
Block a user