feat(github-app): P4.2 — App webhook → push-to-deploy + security hardening#225
Merged
Merged
Conversation
…ening POST /webhooks/github: one App-level endpoint for ALL installations (vs the manual per-repo /webhooks/github/:webhook_id). Verifies the App HMAC, dedupes on X-GitHub-Delivery (Redis), and dispatches: `push` → match repo+branch to a connection linked to that installation+team → enqueue the same rate-limited redeploy the manual path uses; `installation` deleted/suspend/unsuspend → sync github_installations. Gated by GITHUB_APP_ENABLED (default OFF). Token-into-clone wiring is P4.2b. Built with parallel agents: internal/github/webhook.go (HMAC verify + push/installation parsers, 100%), internal/models/github_push_match.go (repo/branch + installation lookups, 100%), infra observability (separate repo). Security review fixes (adversarial pass): - HIGH-1: config.Load now fails closed — GITHUB_APP_ENABLED=true with an empty GITHUB_APP_WEBHOOK_SECRET (<16 chars) / missing key / missing id panics, instead of serving an HMAC verifiable with a public empty key. - HIGH-2: UpsertGitHubInstallation is WHERE-guarded so a callback can't rebind an installation owned by another team → ErrGitHubInstallationTeamConflict → 409 install_conflict (anti-hijack). Residual first-writer race + worker-dispatch guard (MED-2) documented as pre-flag-flip prerequisites in the runbook. - MED-1: webhook event metric label canonicalized to a bounded set before the HMAC check (pre-auth Prometheus cardinality DoS). - LOW-2: push `after` validated as 40-hex; response reports successfully-enqueued count, not matched (matched/enqueued split). Metrics instant_github_webhook_received_total / _pushdeploy_total (+ infra alerts/dashboard/catalog in the companion infra change, rule 25). New routes whitelisted in the openapi route-coverage gate (GitHub-facing, flag-gated; contract sync is P4.3). Coverage: 100% on the new github/models packages + the webhook handler arms (white-box + sqlmock + DB/Redis-gated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Close the patch-coverage gaps CI flagged on the P4.2 review fixes: - config.go 492/495 — the private-key-missing + app-id-missing fail-closed panic arms (HIGH-1), via two more mustPanic cases. - github_app_webhook.go 132-141/201-207 — the installation delete/suspend/ unsuspend slog.Warn arms, the generic enqueue-error arm, and the last-deploy-update warn arm, via sqlmock white-box tests (force each DB op to error; the real-DB integration path can't reach them). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
POST /webhooks/github: one App-level endpoint for ALL installations (vs the
manual per-repo /webhooks/github/:webhook_id). Verifies the App HMAC, dedupes on
X-GitHub-Delivery (Redis), and dispatches:
push→ match repo+branch to aconnection linked to that installation+team → enqueue the same rate-limited
redeploy the manual path uses;
installationdeleted/suspend/unsuspend → syncgithub_installations. Gated by GITHUB_APP_ENABLED (default OFF). Token-into-clone
wiring is P4.2b.
Built with parallel agents: internal/github/webhook.go (HMAC verify +
push/installation parsers, 100%), internal/models/github_push_match.go
(repo/branch + installation lookups, 100%), infra observability (separate repo).
Security review fixes (adversarial pass):
GITHUB_APP_WEBHOOK_SECRET (<16 chars) / missing key / missing id panics,
instead of serving an HMAC verifiable with a public empty key.
installation owned by another team → ErrGitHubInstallationTeamConflict → 409
install_conflict (anti-hijack). Residual first-writer race + worker-dispatch
guard (MED-2) documented as pre-flag-flip prerequisites in the runbook.
HMAC check (pre-auth Prometheus cardinality DoS).
aftervalidated as 40-hex; response reports successfully-enqueuedcount, not matched (matched/enqueued split).
Metrics instant_github_webhook_received_total / _pushdeploy_total (+ infra
alerts/dashboard/catalog in the companion infra change, rule 25). New routes
whitelisted in the openapi route-coverage gate (GitHub-facing, flag-gated;
contract sync is P4.3). Coverage: 100% on the new github/models packages +
the webhook handler arms (white-box + sqlmock + DB/Redis-gated).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com