Add webhook endpoint for gate certification intake#1
Add webhook endpoint for gate certification intake#1github-actions[bot] merged 5 commits intomasterfrom
Conversation
- POST /api/webhooks/gate receives certification results - CertificationCompleted Verbs event stores all payloads - GateWebhookRequest validates repository, sha, verdict - Tests use describe()/it() spec pattern - 100% coverage on new code
|
Warning Rate limit exceeded@jordanpartridge has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughAdds a webhook integration: request validation, controller endpoint, and event class for certification payloads; registers an API route and updates bootstrap routing; includes feature and unit test additions and a simplified welcome view replacement. Changes
Sequence DiagramsequenceDiagram
participant Client as Webhook Client
participant Router as API Router
participant Validator as GateWebhookRequest
participant Controller as WebhookController
participant Event as CertificationCompleted
participant ClientResp as HTTP Response
Client->>Router: POST /api/webhooks/gate
Router->>Validator: Validate payload
alt valid
Validator->>Controller: call gate(validatedData)
Controller->>Event: fire CertificationCompleted(validatedData)
Event-->>Controller: dispatched
Controller->>ClientResp: return {"status":"accepted"}
ClientResp-->>Client: 200 OK
else invalid
Validator-->>ClientResp: 422 Unprocessable Entity (errors)
ClientResp-->>Client: 422
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 1
🧹 Nitpick comments (1)
tests/Feature/WebhookGateTest.php (1)
9-75: Good coverage; consider tightening the persistence assertionThe suite nicely covers success, required-field validation, verdict validation, and that an event record exists in
verb_events. If you want extra safety later, you could extend the finalassertDatabaseHasto also check one or two key payload fields (e.g., repository or verdict) once you settle on the JSON column/paths, but it’s not strictly necessary for this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/Events/CertificationCompleted.php(1 hunks)app/Http/Controllers/WebhookController.php(1 hunks)app/Http/Requests/GateWebhookRequest.php(1 hunks)bootstrap/app.php(1 hunks)routes/api.php(1 hunks)tests/Feature/ExampleTest.php(1 hunks)tests/Feature/WebhookGateTest.php(1 hunks)tests/Unit/ExampleTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
routes/api.php (1)
app/Http/Controllers/WebhookController.php (1)
WebhookController(11-27)
app/Http/Controllers/WebhookController.php (3)
app/Events/CertificationCompleted.php (1)
CertificationCompleted(9-20)app/Http/Requests/GateWebhookRequest.php (1)
GateWebhookRequest(10-32)app/Http/Controllers/Controller.php (1)
Controller(5-8)
🔇 Additional comments (6)
app/Events/CertificationCompleted.php (1)
11-19: Typed event payload looks goodConstructor promotion with precise types for the webhook fields is clear and matches the request validation; no functional issues here.
routes/api.php (1)
5-9: Route wiring is straightforward and consistentThe POST route cleanly targets
WebhookController::gatewith a sensible name; nothing blocking here.tests/Feature/ExampleTest.php (1)
5-16: Describe/it structure and coverage look fineSplitting into explicit homepage and health-check cases makes intent clear; the assertions are appropriate.
tests/Unit/ExampleTest.php (1)
5-8: Unit example test is a clean Pest templateMinimal describe/it structure is fine as a baseline unit-test example.
bootstrap/app.php (1)
10-12: API routes correctly wired into the application bootstrapHooking
routes/api.phpintowithRoutingis necessary for the webhook endpoint to be reachable; this change looks correct.app/Http/Controllers/WebhookController.php (1)
13-26: Field mapping and response semantics look correctUsing named arguments from
$request->validated()keeps the mapping toCertificationCompletedexplicit and hard to mis-order, and returning a simple{'status': 'accepted'}JSON payload for a successful intake is reasonable for this webhook.
| public function authorize(): bool | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * @return array<string, mixed> | ||
| */ | ||
| public function rules(): array | ||
| { | ||
| return [ | ||
| 'repository' => ['required', 'string'], | ||
| 'sha' => ['required', 'string'], | ||
| 'verdict' => ['required', Rule::in(['approved', 'rejected', 'escalate'])], | ||
| 'reason' => ['nullable', 'string'], | ||
| 'checks' => ['nullable', 'array'], | ||
| 'triggered_by' => ['nullable', 'string'], | ||
| 'pr_number' => ['nullable', 'integer'], | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and check its actual content
find . -name "GateWebhookRequest.php" -type fRepository: synapse-sentinel/core
Length of output: 107
🏁 Script executed:
# Check if the file exists at the specified path and show its content
if [ -f "app/Http/Requests/GateWebhookRequest.php" ]; then
echo "=== File found. Content:";
cat -n "app/Http/Requests/GateWebhookRequest.php"
else
echo "File not found at app/Http/Requests/GateWebhookRequest.php"
# Search for it
find . -name "*.php" -path "*/Requests/*" | grep -i webhook || echo "No webhook request files found"
fiRepository: synapse-sentinel/core
Length of output: 1086
🏁 Script executed:
# Look for the route definition for this webhook endpoint
# Check common route files
echo "=== Checking routes/web.php:";
if [ -f "routes/web.php" ]; then rg -i "gatewebhook|webhook.*gate" routes/web.php -A 5; fi
echo ""
echo "=== Checking routes/api.php:";
if [ -f "routes/api.php" ]; then rg -i "gatewebhook|webhook.*gate" routes/api.php -A 5; fi
echo ""
echo "=== Checking all route files:";
find routes -name "*.php" -type f 2>/dev/null | xargs rg -i "gatewebhook|webhook.*gate" -A 5 2>/dev/null || echo "No matches found"Repository: synapse-sentinel/core
Length of output: 374
🏁 Script executed:
# Search for any signature validation or auth mechanisms related to webhooks
echo "=== Searching for signature/HMAC validation:";
rg -i "signature|hmac|X-Hub-Signature" --type php | head -20
echo ""
echo "=== Searching for webhook auth patterns:";
rg -i "webhook.*auth|webhook.*token|webhook.*secret" --type php | head -20Repository: synapse-sentinel/core
Length of output: 151
🏁 Script executed:
# Check for middleware that might be protecting the webhook route
echo "=== Looking for webhook-related middleware:";
find app/Http/Middleware -name "*.php" -type f 2>/dev/null | xargs rg -l "webhook" 2>/dev/null || echo "No webhook middleware found"
echo ""
echo "=== Searching for VerifyWebhook or similar patterns:";
rg "VerifyWebhook|verify.*webhook|webhook.*middleware" --type phpRepository: synapse-sentinel/core
Length of output: 221
🏁 Script executed:
# Search for any controller handling this request to see if there are additional checks
echo "=== Looking for controllers that might handle webhook requests:";
find app/Http/Controllers -name "*Webhook*.php" -o -name "*Gate*.php" | head -10Repository: synapse-sentinel/core
Length of output: 171
🏁 Script executed:
# Check the full route definition with middleware
echo "=== Full routes/api.php around webhook route:";
rg -B 5 -A 10 "webhooks/gate" routes/api.phpRepository: synapse-sentinel/core
Length of output: 314
🏁 Script executed:
# Examine WebhookController to see how the webhook is processed
echo "=== WebhookController content:";
wc -l app/Http/Controllers/WebhookController.phpRepository: synapse-sentinel/core
Length of output: 141
🏁 Script executed:
# Read the WebhookController
head -100 app/Http/Controllers/WebhookController.phpRepository: synapse-sentinel/core
Length of output: 859
🏁 Script executed:
# Look at the test file to understand expected security behavior
echo "=== WebhookGateTest content (first 150 lines):";
head -150 tests/Feature/WebhookGateTest.phpRepository: synapse-sentinel/core
Length of output: 2335
🏁 Script executed:
# Search for any signature validation or custom middleware that might be applied
echo "=== Searching for signature or verification in config or middleware:";
find app/Http/Middleware -name "*.php" -type f 2>/dev/null | xargs cat 2>/dev/nullRepository: synapse-sentinel/core
Length of output: 133
🏁 Script executed:
# Check if there's a bootstrap or service provider that might register middleware
echo "=== Checking for webhook verification in service providers:";
rg -i "webhook|signature|verify" app/Providers --type php -A 3Repository: synapse-sentinel/core
Length of output: 124
Add authentication to the webhook endpoint; currently any caller can submit certification verdicts
The authorize() method returns true unconditionally, the route has no middleware or auth guard, and no signature/HMAC validation exists anywhere in the codebase. This means any caller with network access can submit "approved" verdicts for any repository and commit, which is a critical security risk if this endpoint drives production gating decisions.
Implement one of:
- Signature verification (e.g., validate
X-Hub-SignatureHMAC against a shared secret in middleware before the route is reached). - API token / bearer token authentication via an auth guard.
- If network isolation is already in place (firewall restricting access to trusted infrastructure only), document this assumption explicitly in code or a security note.
🤖 Prompt for AI Agents
app/Http/Requests/GateWebhookRequest.php lines 12-30: authorize() currently
returns true allowing any caller to submit verdicts; add authentication by
verifying a shared secret signature or bearer token before the controller
handles the request. Implement middleware that reads the incoming request body
and the signature header (e.g., X-Hub-Signature / X-Signature) or Authorization
bearer token, compute an HMAC using a secret stored in env/config, compare it
using a timing-safe comparison, and abort with 401/403 on mismatch; register
that middleware on the webhook route (or wire it into route middleware group)
and keep authorize() conservative (return false or only true when the request
user/context is authenticated) or remove unconditional true so only
authenticated requests proceed; alternatively, if network isolation is
guaranteed, add a documented comment and a runtime check that ensures the
expected network constraint or env flag is set.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
resources/views/welcome.blade.php (1)
3-11: Minimal, clear welcome view looks goodStatic head/body markup is valid, matches the app name, and avoids unnecessary assets/logic. This is a reasonable simplification for a core/event-intake service landing page.
If you ever care about external visibility, you could optionally add a
<meta name="description">summarizing the service, but not required for this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
resources/views/welcome.blade.php(1 hunks)tests/Feature/WebhookGateTest.php(1 hunks)tests/Pest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Feature/WebhookGateTest.php
🔇 Additional comments (1)
tests/Pest.php (1)
14-16: LGTM! Enabling RefreshDatabase for Feature tests.Uncommenting the RefreshDatabase trait is the correct approach for the new webhook feature tests that interact with the database. This ensures test isolation and a clean database state for each test run.
1 participant
Summary
POST /api/webhooks/gateendpoint to receive certification results from gate workflowsCertificationCompletedeventTest Plan
All tests use
describe()/it()spec pattern per project conventions.Files Changed
app/Events/CertificationCompleted.php- Verbs event for certification resultsapp/Http/Controllers/WebhookController.php- Webhook handlerapp/Http/Requests/GateWebhookRequest.php- Validation rulesroutes/api.php- API route definitiontests/Feature/WebhookGateTest.php- Spec testsSummary by CodeRabbit
New Features
Tests
Chore
Style
✏️ Tip: You can customize this high-level summary in your review settings.