feat: Webhooks with Kronos - #365
Open
yuvrajjsingh0 wants to merge 1 commit into
Open
yuvrajjsingh0 wants to merge 1 commit into
yuvrajjsingh0 wants to merge 1 commit into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds a Kronos-backed webhooks subsystem to Airborne: new database tables and models, expanded server config/startup wiring, a Kronos client utility, signing/SSRF-protected dispatch and maintenance workers, HTTP API endpoints, dashboard management/delivery UI, and documentation. Separately, the Makefile's ChangesWebhooks Feature
Local Dev Env-file Tooling
Estimated code review effort: 4 (Complex) | ~75 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ReleaseHandler
participant EmitEvent
participant KronosClient
participant DispatchHandler
participant CustomerURL
ReleaseHandler->>EmitEvent: emit_event("release.conclude", delay)
EmitEvent->>EmitEvent: insert delivery row (scheduled)
EmitEvent->>KronosClient: submit_webhook_job
KronosClient->>DispatchHandler: POST /internal/webhooks/dispatch
DispatchHandler->>CustomerURL: signed HTTP POST
CustomerURL-->>DispatchHandler: response / error
DispatchHandler->>DispatchHandler: record_attempt, update delivery status
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
airborne_server/src/webhook/store.rs (1)
84-112: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider SQL-level JSONB filtering for
list_subscribed_webhooks.This function loads all enabled webhooks for an org/app into memory and filters by event in Rust. For typical workloads (tens of webhooks per app) this is fine, but at scale it becomes inefficient. A SQL-level JSONB containment check (e.g.,
events @> '["event.key"]'in PostgreSQL via Diesel) would push the filter to the database and avoid loading non-matching rows.🤖 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 `@airborne_server/src/webhook/store.rs` around lines 84 - 112, `list_subscribed_webhooks` is filtering `events` in Rust after loading all enabled rows, which is inefficient at scale. Move the event match into the Diesel query on `webhooks::table` using a PostgreSQL JSONB containment filter against `webhooks::events`, so only matching webhook rows are returned from the database. Keep the existing `org_id`, `app_id`, and `enabled` filters in the query and remove the in-memory `serde_json::from_value`/`into_iter().filter(...)` pass.
🤖 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 `@airborne_docs/docs/guides/webhooks.mdx`:
- Around line 73-79: The webhook signature verifier in the example can crash
when `parts["v1"]` is missing or malformed because `crypto.timingSafeEqual` is
called with buffers of different lengths. Update the verification logic around
the `expected`/`ok` check to first validate that the provided signature exists
and has the same byte length as the computed digest before calling
`timingSafeEqual`, so the function safely returns false instead of throwing.
In `@airborne_server/src/config.rs`:
- Around line 311-314: The `webhook_internal_secret` config currently falls back
to a hardcoded default in `Config::from_env` via `get_env`, which makes
`/internal/webhooks/dispatch` insecure when `KRONOS_ENABLED=true`. Update the
config loading logic so this secret is required in Kronos mode, or at minimum
detect the default `"airborne-internal-dev-secret"` and emit a clear startup
warning from the `config.rs` initialization path. Use the
`webhook_internal_secret` field and the surrounding `get_env` call to locate the
change.
In `@airborne_server/src/utils/kronos.rs`:
- Around line 54-65: The remote Kronos branch in kronos.rs is silently
defaulting `cfg.kronos_api_key` to `"dev-api-key"` inside the
`KronosHttpClient::new` setup, which should not happen when `cfg.kronos_url` is
present. Update the `if let Some(url) = &cfg.kronos_url` path to require
`kronos_api_key` explicitly and fail startup (or at minimum emit a clear
warning) instead of using the fallback, so the `KronosClient` is only created
with an intentional remote API key.
In `@airborne_server/src/webhook/dispatch.rs`:
- Around line 362-367: The webhook response handling in dispatch logic reads the
entire body with r.text().await before applying truncate, which can exhaust
memory on large responses. Update the response processing in the match arm for
Ok(r) to cap the body size while reading, such as by checking content_length()
first and/or streaming chunks up to the limit, and then pass only the bounded
text into truncate and logging.
- Around line 444-452: The truncate helper can panic when MAX_RESPONSE_BODY
lands in the middle of a multi-byte UTF-8 character. Update truncate in
dispatch.rs to avoid direct byte slicing of s[..MAX_RESPONSE_BODY]; instead,
snap the cutoff to a valid char boundary before building the truncated String
and appending the marker. Keep the behavior the same for short bodies, and
ensure the change is applied in the truncate function used by the webhook
delivery/test response path.
---
Nitpick comments:
In `@airborne_server/src/webhook/store.rs`:
- Around line 84-112: `list_subscribed_webhooks` is filtering `events` in Rust
after loading all enabled rows, which is inefficient at scale. Move the event
match into the Diesel query on `webhooks::table` using a PostgreSQL JSONB
containment filter against `webhooks::events`, so only matching webhook rows are
returned from the database. Keep the existing `org_id`, `app_id`, and `enabled`
filters in the query and remove the in-memory
`serde_json::from_value`/`into_iter().filter(...)` pass.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b2ef9dc2-ce22-4a87-b3b5-18ec783590b2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
Makefileairborne_authz_macros/src/lib.rsairborne_dashboard/app/dashboard/[orgId]/[appId]/webhooks/[webhookId]/deliveries/page.tsxairborne_dashboard/app/dashboard/[orgId]/[appId]/webhooks/page.tsxairborne_dashboard/components/shared-layout.tsxairborne_dashboard/next.config.mjsairborne_docs/docs/guides/webhooks.mdxairborne_docs/docs/server/configuration.mdairborne_docs/docs/server/running-locally.mdairborne_docs/sidebars.tsairborne_server/.env.exampleairborne_server/Cargo.tomlairborne_server/migrations/2026-07-08-120000_add_webhooks/down.sqlairborne_server/migrations/2026-07-08-120000_add_webhooks/up.sqlairborne_server/src/config.rsairborne_server/src/main.rsairborne_server/src/release.rsairborne_server/src/types.rsairborne_server/src/utils.rsairborne_server/src/utils/db/models.rsairborne_server/src/utils/db/schema.rsairborne_server/src/utils/kronos.rsairborne_server/src/webhook.rsairborne_server/src/webhook/dispatch.rsairborne_server/src/webhook/emit.rsairborne_server/src/webhook/maintenance.rsairborne_server/src/webhook/mw.rsairborne_server/src/webhook/sign.rsairborne_server/src/webhook/store.rsairborne_server/src/webhook/types.rs
yuvrajjsingh0
force-pushed
the
webhooks-with-kronos
branch
4 times, most recently
from
July 16, 2026 11:25
a349f07 to
158939f
Compare
yuvrajjsingh0
force-pushed
the
webhooks-with-kronos
branch
from
August 17, 2026 07:43
aa72dcf to
d64e810
Compare
yuvrajjsingh0
force-pushed
the
webhooks-with-kronos
branch
from
August 17, 2026 07:43
d64e810 to
56dbe67
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Screenshots:
