feat: add config_webhook_endpoints table, store CRUD methods, and secret rotation with Standard Webhooks signing - #5247
Conversation
|
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds webhook endpoint models with validation and encrypted fields, a configstore migration, ConfigStore and RDB persistence methods, secret rotation, and comprehensive model and repository tests. ChangesWebhook endpoint configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant RDBConfigStore
participant TableWebhookEndpoint
participant ConfigstoreDatabase
Caller->>RDBConfigStore: CreateWebhookEndpoint
RDBConfigStore->>TableWebhookEndpoint: Validate endpoint
RDBConfigStore->>ConfigstoreDatabase: Save encrypted endpoint
ConfigstoreDatabase-->>RDBConfigStore: Commit result
RDBConfigStore-->>Caller: Return creation result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "feat: webhook endpoint registry in confi..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
framework/configstore/tables/webhooks.go (1)
29-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not expose a mutable global event registry.
Any importing package can modify
WebhookEvents, changing validation globally or racing withIsValid. Keep the slice private and return a clone from an accessor.Proposed refactor
-// WebhookEvents lists every supported webhook event. -var WebhookEvents = []WebhookEvent{ +var webhookEvents = []WebhookEvent{ WebhookEventAsyncJobCompleted, WebhookEventAsyncJobFailed, } +func SupportedWebhookEvents() []WebhookEvent { + return slices.Clone(webhookEvents) +} + func (e WebhookEvent) IsValid() bool { - return slices.Contains(WebhookEvents, e) + return slices.Contains(webhookEvents, e) }As per coding guidelines, shared state must have clear ownership and remain race-safe.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@framework/configstore/tables/webhooks.go` around lines 29 - 38, Make the webhook event registry private instead of exposing the mutable WebhookEvents slice. Update IsValid to use the private registry, and add an accessor that returns a clone of the registry so callers cannot mutate shared state or introduce races.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/configstore/rdb.go`:
- Around line 7444-7459: Update the create flow around the plaintextSecret
capture and transaction in the webhook endpoint method to defer restoring
endpoint.Secret before entering the transaction. Ensure the deferred restoration
runs on both successful and failed creates, including transaction commit errors,
while preserving the existing transaction and explicit success-path behavior
without restoring it redundantly.
In `@framework/configstore/tables/webhooks.go`:
- Around line 153-172: Update the webhook validation method containing the
numeric-field and header checks to enforce explicit upper bounds for retries,
backoff durations, timeouts, payload size, and concurrent deliveries, rejecting
values above the defined maxima. Also limit both the number of custom headers
and their aggregate serialized size, including header names and values, while
preserving the existing nonnegative and protected-header validation.
- Around line 176-199: Update validateWebhookEndpointURL to accept separate
allowHTTP and allowPrivateNetwork policies: permit HTTP only when allowHTTP is
enabled, while keeping private and loopback address access controlled
exclusively by allowPrivateNetwork. Ensure loopback IPv4 and IPv6 URLs are
rejected by default even when using HTTPS, and update webhook configuration
defaults and validation tests to cover HTTPS-only behavior plus both loopback
forms.
---
Nitpick comments:
In `@framework/configstore/tables/webhooks.go`:
- Around line 29-38: Make the webhook event registry private instead of exposing
the mutable WebhookEvents slice. Update IsValid to use the private registry, and
add an accessor that returns a clone of the registry so callers cannot mutate
shared state or introduce races.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fe28403-922c-4ea9-a95e-dde1892f7d20
📒 Files selected for processing (9)
framework/configstore/migrations.goframework/configstore/rdb.goframework/configstore/rdb_test.goframework/configstore/store.goframework/configstore/tables/clientconfig_test.goframework/configstore/tables/encryption_test.goframework/configstore/tables/webhooks.goframework/configstore/tables/webhooks_test.gotransports/bifrost-http/lib/config_test.go
💤 Files with no reviewable changes (1)
- framework/configstore/tables/clientconfig_test.go
b098b0b to
5354465
Compare
5354465 to
fbcfb94
Compare
Merge activity
|
The base branch was changed.
fbcfb94 to
4b735cb
Compare
…ecret rotation with Standard Webhooks signing (#5247) Introduces the `config_webhook_endpoints` table and all associated store-layer logic needed to persist, retrieve, update, and rotate secrets for registered webhook endpoints. This is the foundational persistence layer for outbound webhook delivery. - Added `TableWebhookEndpoint` model (`framework/configstore/tables/webhooks.go`) defining the schema for webhook endpoints, including: - Supported event types (`async_job.completed`, `async_job.failed`) - Per-endpoint delivery tuning knobs (retries, backoff, timeouts, concurrency) - Custom delivery headers stored encrypted at rest - Signing secrets generated in Standard Webhooks format (`whsec_` + base64 of 32 random bytes), encrypted at rest and never included in JSON serialization - `BeforeSave`/`AfterFind` GORM hooks handling serialization of virtual fields (`Events`, `Headers`) and encryption/decryption of sensitive columns - `Validate()` enforcing name, URL (HTTPS required unless `allow_private_network` is set, no credentials or fragments, private/link-local ranges blocked), event list, and header name rules - Added `migrationAddWebhookEndpointsTable` database migration step - Added `ConfigStore` interface methods: `GetWebhookEndpoints`, `GetWebhookEndpointByID`, `GetWebhookEndpointByName`, `CreateWebhookEndpoint`, `UpdateWebhookEndpoint`, `DeleteWebhookEndpoint`, `RotateWebhookEndpointSecret` - Implemented all store methods on `RDBConfigStore`: - `CreateWebhookEndpoint` auto-generates an ID and signing secret when not supplied, enforces name uniqueness, and returns the plaintext secret on the struct for one-time display - `UpdateWebhookEndpoint` resets the consecutive-failure counter only on URL changes; re-enabling a disabled endpoint does not reset it - `RotateWebhookEndpointSecret` replaces the signing secret atomically; only the new secret is stored - Added stub implementations of all new interface methods to `MockConfigStore` in the HTTP transport test package - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ```sh go test ./framework/configstore/... ./framework/configstore/tables/... ./transports/bifrost-http/lib/... ``` Key test scenarios covered: - `TestWebhookEndpointCreate` — ID and `whsec_`-prefixed secret are generated server-side; distinct endpoints receive distinct secrets - `TestWebhookEndpointCreateDuplicateName` — returns `ErrAlreadyExists` - `TestWebhookEndpointUpdate` — non-URL changes preserve the failure counter; URL changes reset it; signing secret is never modified - `TestWebhookEndpointUpdateReenablePreservesFailureCounter` — re-enabling a disabled endpoint does not reset the failure counter - `TestWebhookEndpointRotateSecret` — new secret is stored immediately; old secret is gone - `TestTableWebhookEndpoint_EncryptDecrypt` — ciphertext is stored, plaintext is restored after read - `TestTableWebhookEndpoint_SecretsExcludedFromJSON` — secret material never appears in JSON output - `TestTableWebhookEndpoint_HeadersEncryptDecrypt` — custom headers are encrypted at rest; env references survive the round-trip as references - [x] Yes - [ ] No Any type implementing the `ConfigStore` interface must add the seven new webhook endpoint methods. The `MockConfigStore` in the HTTP transport package has been updated; other mocks in the codebase will need the same stub additions. - Signing secrets are generated using `crypto/rand` and formatted as `whsec_` + base64(32 bytes), following the Standard Webhooks specification. - Secrets and custom delivery headers are encrypted at rest using the existing `encrypt` package when encryption is enabled. - Secrets are excluded from JSON serialization (`json:"-"`) and are surfaced in plaintext only once — at creation or rotation time — via the in-memory struct field. - Webhook URLs are validated against `bifrost.ValidateExternalURL`, which rejects link-local and metadata address ranges regardless of the `allow_private_network` flag. HTTP is only permitted when `allow_private_network` is explicitly set. - Credentials and fragments are rejected from webhook URLs. - Standard Webhooks signing headers and protocol-level headers are protected and cannot be overridden by caller-supplied custom headers. - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
…ecret rotation with Standard Webhooks signing (#5247) Introduces the `config_webhook_endpoints` table and all associated store-layer logic needed to persist, retrieve, update, and rotate secrets for registered webhook endpoints. This is the foundational persistence layer for outbound webhook delivery. - Added `TableWebhookEndpoint` model (`framework/configstore/tables/webhooks.go`) defining the schema for webhook endpoints, including: - Supported event types (`async_job.completed`, `async_job.failed`) - Per-endpoint delivery tuning knobs (retries, backoff, timeouts, concurrency) - Custom delivery headers stored encrypted at rest - Signing secrets generated in Standard Webhooks format (`whsec_` + base64 of 32 random bytes), encrypted at rest and never included in JSON serialization - `BeforeSave`/`AfterFind` GORM hooks handling serialization of virtual fields (`Events`, `Headers`) and encryption/decryption of sensitive columns - `Validate()` enforcing name, URL (HTTPS required unless `allow_private_network` is set, no credentials or fragments, private/link-local ranges blocked), event list, and header name rules - Added `migrationAddWebhookEndpointsTable` database migration step - Added `ConfigStore` interface methods: `GetWebhookEndpoints`, `GetWebhookEndpointByID`, `GetWebhookEndpointByName`, `CreateWebhookEndpoint`, `UpdateWebhookEndpoint`, `DeleteWebhookEndpoint`, `RotateWebhookEndpointSecret` - Implemented all store methods on `RDBConfigStore`: - `CreateWebhookEndpoint` auto-generates an ID and signing secret when not supplied, enforces name uniqueness, and returns the plaintext secret on the struct for one-time display - `UpdateWebhookEndpoint` resets the consecutive-failure counter only on URL changes; re-enabling a disabled endpoint does not reset it - `RotateWebhookEndpointSecret` replaces the signing secret atomically; only the new secret is stored - Added stub implementations of all new interface methods to `MockConfigStore` in the HTTP transport test package - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ```sh go test ./framework/configstore/... ./framework/configstore/tables/... ./transports/bifrost-http/lib/... ``` Key test scenarios covered: - `TestWebhookEndpointCreate` — ID and `whsec_`-prefixed secret are generated server-side; distinct endpoints receive distinct secrets - `TestWebhookEndpointCreateDuplicateName` — returns `ErrAlreadyExists` - `TestWebhookEndpointUpdate` — non-URL changes preserve the failure counter; URL changes reset it; signing secret is never modified - `TestWebhookEndpointUpdateReenablePreservesFailureCounter` — re-enabling a disabled endpoint does not reset the failure counter - `TestWebhookEndpointRotateSecret` — new secret is stored immediately; old secret is gone - `TestTableWebhookEndpoint_EncryptDecrypt` — ciphertext is stored, plaintext is restored after read - `TestTableWebhookEndpoint_SecretsExcludedFromJSON` — secret material never appears in JSON output - `TestTableWebhookEndpoint_HeadersEncryptDecrypt` — custom headers are encrypted at rest; env references survive the round-trip as references - [x] Yes - [ ] No Any type implementing the `ConfigStore` interface must add the seven new webhook endpoint methods. The `MockConfigStore` in the HTTP transport package has been updated; other mocks in the codebase will need the same stub additions. - Signing secrets are generated using `crypto/rand` and formatted as `whsec_` + base64(32 bytes), following the Standard Webhooks specification. - Secrets and custom delivery headers are encrypted at rest using the existing `encrypt` package when encryption is enabled. - Secrets are excluded from JSON serialization (`json:"-"`) and are surfaced in plaintext only once — at creation or rotation time — via the in-memory struct field. - Webhook URLs are validated against `bifrost.ValidateExternalURL`, which rejects link-local and metadata address ranges regardless of the `allow_private_network` flag. HTTP is only permitted when `allow_private_network` is explicitly set. - Credentials and fragments are rejected from webhook URLs. - Standard Webhooks signing headers and protocol-level headers are protected and cannot be overridden by caller-supplied custom headers. - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
…ecret rotation with Standard Webhooks signing (maximhq#5247) Introduces the `config_webhook_endpoints` table and all associated store-layer logic needed to persist, retrieve, update, and rotate secrets for registered webhook endpoints. This is the foundational persistence layer for outbound webhook delivery. - Added `TableWebhookEndpoint` model (`framework/configstore/tables/webhooks.go`) defining the schema for webhook endpoints, including: - Supported event types (`async_job.completed`, `async_job.failed`) - Per-endpoint delivery tuning knobs (retries, backoff, timeouts, concurrency) - Custom delivery headers stored encrypted at rest - Signing secrets generated in Standard Webhooks format (`whsec_` + base64 of 32 random bytes), encrypted at rest and never included in JSON serialization - `BeforeSave`/`AfterFind` GORM hooks handling serialization of virtual fields (`Events`, `Headers`) and encryption/decryption of sensitive columns - `Validate()` enforcing name, URL (HTTPS required unless `allow_private_network` is set, no credentials or fragments, private/link-local ranges blocked), event list, and header name rules - Added `migrationAddWebhookEndpointsTable` database migration step - Added `ConfigStore` interface methods: `GetWebhookEndpoints`, `GetWebhookEndpointByID`, `GetWebhookEndpointByName`, `CreateWebhookEndpoint`, `UpdateWebhookEndpoint`, `DeleteWebhookEndpoint`, `RotateWebhookEndpointSecret` - Implemented all store methods on `RDBConfigStore`: - `CreateWebhookEndpoint` auto-generates an ID and signing secret when not supplied, enforces name uniqueness, and returns the plaintext secret on the struct for one-time display - `UpdateWebhookEndpoint` resets the consecutive-failure counter only on URL changes; re-enabling a disabled endpoint does not reset it - `RotateWebhookEndpointSecret` replaces the signing secret atomically; only the new secret is stored - Added stub implementations of all new interface methods to `MockConfigStore` in the HTTP transport test package - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ```sh go test ./framework/configstore/... ./framework/configstore/tables/... ./transports/bifrost-http/lib/... ``` Key test scenarios covered: - `TestWebhookEndpointCreate` — ID and `whsec_`-prefixed secret are generated server-side; distinct endpoints receive distinct secrets - `TestWebhookEndpointCreateDuplicateName` — returns `ErrAlreadyExists` - `TestWebhookEndpointUpdate` — non-URL changes preserve the failure counter; URL changes reset it; signing secret is never modified - `TestWebhookEndpointUpdateReenablePreservesFailureCounter` — re-enabling a disabled endpoint does not reset the failure counter - `TestWebhookEndpointRotateSecret` — new secret is stored immediately; old secret is gone - `TestTableWebhookEndpoint_EncryptDecrypt` — ciphertext is stored, plaintext is restored after read - `TestTableWebhookEndpoint_SecretsExcludedFromJSON` — secret material never appears in JSON output - `TestTableWebhookEndpoint_HeadersEncryptDecrypt` — custom headers are encrypted at rest; env references survive the round-trip as references - [x] Yes - [ ] No Any type implementing the `ConfigStore` interface must add the seven new webhook endpoint methods. The `MockConfigStore` in the HTTP transport package has been updated; other mocks in the codebase will need the same stub additions. - Signing secrets are generated using `crypto/rand` and formatted as `whsec_` + base64(32 bytes), following the Standard Webhooks specification. - Secrets and custom delivery headers are encrypted at rest using the existing `encrypt` package when encryption is enabled. - Secrets are excluded from JSON serialization (`json:"-"`) and are surfaced in plaintext only once — at creation or rotation time — via the in-memory struct field. - Webhook URLs are validated against `bifrost.ValidateExternalURL`, which rejects link-local and metadata address ranges regardless of the `allow_private_network` flag. HTTP is only permitted when `allow_private_network` is explicitly set. - Credentials and fragments are rejected from webhook URLs. - Standard Webhooks signing headers and protocol-level headers are protected and cannot be overridden by caller-supplied custom headers. - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable

Summary
Introduces the
config_webhook_endpointstable and all associated store-layer logic needed to persist, retrieve, update, and rotate secrets for registered webhook endpoints. This is the foundational persistence layer for outbound webhook delivery.Changes
TableWebhookEndpointmodel (framework/configstore/tables/webhooks.go) defining the schema for webhook endpoints, including:async_job.completed,async_job.failed)whsec_+ base64 of 32 random bytes), encrypted at rest and never included in JSON serializationBeforeSave/AfterFindGORM hooks handling serialization of virtual fields (Events,Headers) and encryption/decryption of sensitive columnsValidate()enforcing name, URL (HTTPS required unlessallow_private_networkis set, no credentials or fragments, private/link-local ranges blocked), event list, and header name rulesmigrationAddWebhookEndpointsTabledatabase migration stepConfigStoreinterface methods:GetWebhookEndpoints,GetWebhookEndpointByID,GetWebhookEndpointByName,CreateWebhookEndpoint,UpdateWebhookEndpoint,DeleteWebhookEndpoint,RotateWebhookEndpointSecretRDBConfigStore:CreateWebhookEndpointauto-generates an ID and signing secret when not supplied, enforces name uniqueness, and returns the plaintext secret on the struct for one-time displayUpdateWebhookEndpointresets the consecutive-failure counter only on URL changes; re-enabling a disabled endpoint does not reset itRotateWebhookEndpointSecretreplaces the signing secret atomically; only the new secret is storedMockConfigStorein the HTTP transport test packageType of change
Affected areas
How to test
go test ./framework/configstore/... ./framework/configstore/tables/... ./transports/bifrost-http/lib/...Key test scenarios covered:
TestWebhookEndpointCreate— ID andwhsec_-prefixed secret are generated server-side; distinct endpoints receive distinct secretsTestWebhookEndpointCreateDuplicateName— returnsErrAlreadyExistsTestWebhookEndpointUpdate— non-URL changes preserve the failure counter; URL changes reset it; signing secret is never modifiedTestWebhookEndpointUpdateReenablePreservesFailureCounter— re-enabling a disabled endpoint does not reset the failure counterTestWebhookEndpointRotateSecret— new secret is stored immediately; old secret is goneTestTableWebhookEndpoint_EncryptDecrypt— ciphertext is stored, plaintext is restored after readTestTableWebhookEndpoint_SecretsExcludedFromJSON— secret material never appears in JSON outputTestTableWebhookEndpoint_HeadersEncryptDecrypt— custom headers are encrypted at rest; env references survive the round-trip as referencesBreaking changes
Any type implementing the
ConfigStoreinterface must add the seven new webhook endpoint methods. TheMockConfigStorein the HTTP transport package has been updated; other mocks in the codebase will need the same stub additions.Security considerations
crypto/randand formatted aswhsec_+ base64(32 bytes), following the Standard Webhooks specification.encryptpackage when encryption is enabled.json:"-") and are surfaced in plaintext only once — at creation or rotation time — via the in-memory struct field.bifrost.ValidateExternalURL, which rejects link-local and metadata address ranges regardless of theallow_private_networkflag. HTTP is only permitted whenallow_private_networkis explicitly set.Checklist
docs/contributing/README.mdand followed the guidelines