Skip to content

feat(github-app): Phase 1 webhook receiver foundation (#246)#289

Draft
ChrisJr404 wants to merge 3 commits intoPwnKit-Labs:mainfrom
ChrisJr404:feat/github-app-webhook-receiver-246
Draft

feat(github-app): Phase 1 webhook receiver foundation (#246)#289
ChrisJr404 wants to merge 3 commits intoPwnKit-Labs:mainfrom
ChrisJr404:feat/github-app-webhook-receiver-246

Conversation

@ChrisJr404
Copy link
Copy Markdown

Draft — opening for architecture review before sinking another round of work into this. Scoped strictly to the Phase-1 foundation called out in #246 so the bones can land cleanly first; the actual scan/comment pipeline is staged as the immediate follow-up.

What's in

Webhook signature verification (`src/github_app/webhook.rs`)

  • HMAC-SHA256 over the raw request body using the configured webhook secret. Comparison goes through `hmac::Mac::verify_slice`, which does the constant-time check for us — no rolled-our-own subtle-time loop.
  • 10 unit tests pin the verification contract: known-good vector, modified body, wrong secret, missing prefix, empty digest, non-hex digest, short-length digest (rejecting 16-byte inputs that are structurally valid hex), trailing whitespace tolerance, and the kind-routing map. Test fixture is a deterministic `(secret, body, digest)` triple computed via Python's stdlib HMAC for reproducibility.
  • New `EventKind` enum maps `X-GitHub-Event` to the routed kinds (`Installation`, `PullRequest`, `Ping`, `Other`). Unknown values fall back to `Other` rather than erroring so the receiver stays forward-compatible with new GitHub event types.

Webhook server (`src/bin/foxguard_github_app.rs`)

  • axum-based HTTP server with `/healthz` and `/webhook` endpoints.
  • Verifies signatures, routes by event, returns `202 Accepted` for known kinds, `401` for verification failures (same status either way so we don't leak the failure mode).
  • Actual handler bodies are stubbed with clearly TODO-marked `tracing::info!` lines for each event kind — no scan execution, no GitHub API calls outbound yet, on purpose.
  • 1 MiB request-body cap layered ahead of the handler via `tower-http::limit::RequestBodyLimitLayer` so a hostile multi-GB delivery cannot exhaust memory.

Self-hosting (`Dockerfile.github-app`)

  • Multi-stage build that compiles the receiver with the `github-app` feature, drops to a non-root user, exposes `:8080`. Refuses to start without `FOXGUARD_WEBHOOK_SECRET` rather than silently accepting every delivery.

Build gating

New `github-app` Cargo feature flag. The optional dep closure (axum, tokio, hmac, hex, tower-http, tracing, tracing-subscriber) only enters the build when the feature is enabled, so the default `cargo build` and the `foxguard-mcp` binary stay untouched. Verified:

```
cargo build → clean
cargo build --features github-app --bin foxguard-github-app → clean
cargo test --lib → 413 existing tests still ok
cargo test --features github-app --lib → 10 new tests added, all green
```

What's NOT in

Deliberately deferred so the architecture above can land in isolation. Each is the next PR:

  1. JWT-based App→installation auth. Generate the App JWT from the `.pem` private key, exchange it for an installation token via `/app/installations/{id}/access_tokens`, cache the token until expiry. New dep: `jsonwebtoken`.
  2. `pull_request` handler. Clone the head ref into a sandboxed temp dir, run `foxguard scan` with a 60s timeout and a 1 GB repo cap, post the existing `--github-pr` output as a PR comment.
  3. `installation` handler. Persist install metadata (org, installation id, repos) so we know which orgs we're serving. SQLite is fine for Phase 1; data is small and ops is trivial to back up.
  4. Check Runs API. Inline annotations on the diff once the comment path is solid.
  5. Public webhook host. Per the issue's open question, I'd lean: publish the Docker image for self-hosters, run a single public instance under PwnKit-Labs for everyone else.

Open questions for you

  • The issue mentions "App owned by PwnKit-Labs org, or a separate app-specific org?" — that's a call for you to make. This PR doesn't take a position; it just gives you the binary that either deployment can run.
  • Tracing setup uses the `info,foxguard=debug` default filter. If you'd rather a different default, easy one-liner.
  • I picked `/webhook` and `/healthz` for the routes. If there's a convention you'd prefer (`/api/v1/webhook`, etc.), happy to swap.

Test plan

  • Default `cargo build` clean (no new deps in default closure)
  • `cargo build --features github-app` clean
  • `cargo test --lib` — 413 existing pass
  • `cargo test --features github-app --lib github_app::` — 10 new pass
  • Manual smoke test of the binary: `curl -H "X-Hub-Signature-256: sha256=…" -H "X-GitHub-Event: ping" --data '…'` returns 202 with valid signature, 401 with invalid
  • Manual smoke test of `Dockerfile.github-app` build
  • (For maintainer) — actually register a GitHub App, set the webhook URL, verify deliveries in the App's Recent Deliveries panel render as 202.

Lays the in-tree groundwork for the GitHub App distribution path
discussed in PwnKit-Labs#246, scoped strictly to the foundation so the
architecture can be reviewed before the scan/comment pipeline lands.

What's in:

  - src/github_app/webhook.rs — HMAC-SHA256 signature verification
    with constant-time comparison via hmac::Mac::verify_slice, plus
    the EventKind router enum that maps X-GitHub-Event values to the
    routed kinds (Installation, PullRequest, Ping, Other). 10 unit
    tests pin the verification contract: known-good vector,
    modified body, wrong secret, missing/empty/non-hex/short-length
    digest, trailing-whitespace tolerance, and the kind-routing map.

  - src/bin/foxguard_github_app.rs — axum HTTP server with /healthz
    and /webhook endpoints. Verifies signatures, routes by event,
    returns 202 for known kinds, 401 for verification failures, with
    actual handler bodies stubbed and clearly TODO-marked. 1 MiB
    request-body cap layered in front of the handler.

  - Dockerfile.github-app — multi-stage build that compiles the
    receiver with the github-app feature, drops to a non-root user,
    exposes :8080. Refuses to start without FOXGUARD_WEBHOOK_SECRET.

  - src/github_app/README.md — what's here, what's next, how to run.

Build is gated behind a new `github-app` feature flag so the
default `cargo build` remains untouched. Optional deps (axum,
tokio, hmac, hex, tower-http, tracing) only enter the dependency
closure when the feature is enabled, keeping the core scanner crate
lean for users who only want the CLI.

What's NOT here (deliberately deferred for follow-up review):

  - JWT-based App→installation auth (jsonwebtoken dep).
  - pull_request handler: clone, scan, comment with --github-pr.
  - installation handler + persistent install metadata.
  - Check Runs API for inline annotations.

Verified locally:

  cargo build                                                   → clean
  cargo build --features github-app --bin foxguard-github-app   → clean
  cargo test --lib                                              → 413 ok
  cargo test --features github-app --lib                        → +10 ok
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 86344ddc-a7e7-4c16-82b3-23c5c8f8c32d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Collaborator

@peaktwilight peaktwilight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read the diff end-to-end. Architecture is approved from my side — Phase-1 scope is right, the security model is right, the gating is clean. Specific things I checked:

What's right

  • verify_slice length-shortcut. hmac::Mac::verify_slice returns early non-constant-time on length mismatch. You sidestep that by rejecting any non-32-byte digest with MalformedHeader before calling verify_slice, so by the time we hit the constant-time compare the slice is always 32 bytes. Correct.
  • Same-status response on both failure modes. MalformedHeader and Mismatch both 401 — won't leak which check failed.
  • Body cap before handler. RequestBodyLimitLayer::new(1 MiB) as a tower layer means a hostile multi-GB delivery dies before allocating anything app-side, not after the handler tries to buffer it.
  • Feature gating. Default cargo build doesn't pull axum/tokio/etc; the foxguard-mcp binary closure is unaffected. Verified the optional dep block in Cargo.toml.
  • Docker: non-root user, refuses to start without FOXGUARD_WEBHOOK_SECRET rather than silently accepting every delivery.

Answers to your three open questions

  1. App ownership — own it under PwnKit-Labs. No benefit to a separate org until there's a second product; real cost in extra billing/secret surfaces. The brand is PwnKit-Labs/foxguard already.
  2. Tracing default info,foxguard=debug — keep it. Overridable via RUST_LOG, sane default.
  3. Routes /webhook + /healthz — keep simple. Layer /v2/webhook later if versioning is ever needed; premature abstraction otherwise.

Phase-2 follow-up items (none block this PR)

These are worth tracking as separate issues so the next handler-PR doesn't have to rediscover them:

  1. Delivery-ID dedup. GitHub retries on 5xx with the same X-GitHub-Delivery. Stub handlers don't care, but the moment the pull_request → scan handler lands, dedup is required or you double-scan / double-comment. SQLite or in-memory LRU both fine for Phase-1 scale.
  2. Body extractor must stay axum::body::Bytes — never Json<Payload>. The framework eats the body before HMAC verification can run otherwise. Worth a comment in webhook when handlers are added.
  3. Webhook secret zeroization is paranoid-level. For a long-running web service the process boundary is the trust boundary, so plain Vec<u8> is fine — but secrecy::Secret is a one-line upgrade if you ever ship multi-tenant.
  4. No timestamp/freshness check because GitHub doesn't sign one (unlike Stripe). Documenting this acceptance in the module-level doc would make it explicit for anyone auditing later.

Mark ready-for-review whenever you want this in. Phase-2 = clone + scan + comment lands as the next PR.

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