feat: add inbound webhook support (#909)#2182
Open
bvdwalt wants to merge 1 commit intogetarcaneapp:mainfrom
Open
feat: add inbound webhook support (#909)#2182bvdwalt wants to merge 1 commit intogetarcaneapp:mainfrom
bvdwalt wants to merge 1 commit intogetarcaneapp:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+188
to
+196
| } | ||
|
|
||
| actor := models.User{} | ||
| if currentUser, exists := humamw.GetCurrentUserFromContext(ctx); exists && currentUser != nil { | ||
| actor = *currentUser | ||
| } | ||
|
|
||
| wh, rawToken, err := h.webhookService.CreateWebhook( | ||
| ctx, |
Contributor
There was a problem hiding this comment.
err.Error() is returned verbatim for all non-404/403 errors, including 500-level failures. For domain errors (ErrWebhookNotFound, ErrWebhookDisabled, etc.) this is fine, but for wrapped operational errors (e.g., "container update failed: <docker daemon message>") internal implementation details are visible to anyone who can guess a valid token prefix.
Consider sanitising 500-level responses to a fixed message:
msg := err.Error()
if status == http.StatusInternalServerError {
msg = "internal server error"
}
c.JSON(status, gin.H{"success": false, "error": msg})Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/internal/huma/handlers/webhooks.go
Line: 188-196
Comment:
**Internal error details exposed to unauthenticated callers**
`err.Error()` is returned verbatim for all non-404/403 errors, including 500-level failures. For domain errors (`ErrWebhookNotFound`, `ErrWebhookDisabled`, etc.) this is fine, but for wrapped operational errors (e.g., `"container update failed: <docker daemon message>"`) internal implementation details are visible to anyone who can guess a valid token prefix.
Consider sanitising 500-level responses to a fixed message:
```go
msg := err.Error()
if status == http.StatusInternalServerError {
msg = "internal server error"
}
c.JSON(status, gin.H{"success": false, "error": msg})
```
How can I resolve this? If you propose a fix, please make it concise.
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.
Checklist
mainbranchWhat This PR Implements
Adds inbound webhook support, allowing external systems (CI/CD pipelines, GitHub Actions, etc.) to trigger updates in Arcane via an authenticated HTTP endpoint.
Fixes: #909
Changes Made
POST /api/webhooks/trigger/<token>- public, unauthenticated; token in the URL is the sole credentialarc_wh_prefix stored for indexed lookup (never stored in plaintext)container(single container update),project(compose project redeploy),updater(environment-wide image updater),gitops(GitOps sync)Testing Done
./scripts/development/dev.sh startcurl -X POST /api/webhooks/trigger/<token>and confirmed the container was updatedjust lint all)just test backendAI Tool Used (if applicable)
AI Tool: Claude Code Sonnet 4.6 (Anthropic)
Assistance Level: Significant
What AI helped with: Understanding repository, Full-stack implementation - backend service, handlers, migrations, frontend components, and tests
I reviewed and edited all AI-generated output: Yes
I ran all required tests and manually verified changes: Yes
Additional Context
The trigger endpoint intentionally bypasses session auth middleware — the token in the URL is the authentication mechanism, consistent with how most webhook systems work (GitHub, GitLab, etc.). Tokens are SHA-256 hashed at rest with a short prefix stored for indexed lookup, matching the existing API key security pattern.
Disclaimer Greptiles Reviews use AI, make sure to check over its work.
To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike
To have Greptile Re-Review the changes, mention
greptileai.Greptile Summary
This PR adds full-stack inbound webhook support, allowing external systems (CI/CD pipelines, GitHub Actions, etc.) to trigger container updates, project redeploys, environment-wide updater runs, or GitOps syncs via a single authenticated
POST /api/webhooks/trigger/:tokenendpoint. The implementation follows the existing API-key security pattern: 32-byte tokens are SHA-256 hashed at rest with only a short prefix stored for indexed lookup, the plain-text token is surfaced once at creation, and the trigger endpoint intentionally bypasses session middleware in favour of URL-token authentication.Key observations:
json:\"-\"prevents accidental hash serialisation, UNIQUE index ontoken_hash, prefix narrows the DB scan before a constant-time hash comparison.webhook_service.go(generateWebhookToken,hashWebhookToken,parseWebhookPrefix) are missing the requiredInternalsuffix per the repository naming convention.$effectblock inwebhook-form-sheet.svelteresets$statedirectly on dialog close, which the Svelte best-practices guide recommends moving into an event handler (handleOpenChange) instead.err.Error()verbatim for all error levels; 500-level responses can expose wrapped internal details (e.g. Docker daemon errors) to unauthenticated callers.Confidence Score: 5/5
Safe to merge; all findings are P2 style/convention issues with no impact on correctness or security of the core webhook mechanism.
The feature is well-implemented with solid test coverage, correct token security (SHA-256 at rest, prefix lookup, single-reveal), proper environment scoping throughout, and consistent patterns with the existing API-key implementation. All three flagged issues are P2: a naming convention violation, a Svelte reactive-state style guideline, and an informational error-message hygiene concern. None affect runtime correctness or introduce a current security defect.
backend/internal/services/webhook_service.go (naming convention), frontend/src/lib/components/sheets/webhook-form-sheet.svelte (Svelte $effect style), backend/internal/huma/handlers/webhooks.go (error message sanitisation)
Important Files Changed
Comments Outside Diff (2)
backend/internal/services/webhook_service.go, line 492-517 (link)Per the repository coding convention, all unexported (package-private) functions must end with the
Internalsuffix. Three functions introduced in this file violate that rule:generateWebhookToken→generateWebhookTokenInternalhashWebhookToken→hashWebhookTokenInternalparseWebhookPrefix→parseWebhookPrefixInternalBecause the test file lives in the same package (
package services), the rename would propagate cleanly towebhook_service_test.gowithout any export changes.Rule Used: What: All unexported functions must have the "Inte... (source)
Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
frontend/src/lib/components/sheets/webhook-form-sheet.svelte, line 1510-1518 (link)$stateupdated inside$effectThe effect mutates
selectedTargetType,selectedTargetId, andtargetOptionsdirectly, which is the anti-pattern described in the Svelte best-practices guide. For the reset-on-close branch, the idiomatic fix is to move the state reset into the existinghandleOpenChangecallback so it runs in direct response to the user action rather than as a reactive side-effect:Then the
$effectonly needs to callloadTargetOptions:$effect(() => { if (open) { loadTargetOptions(selectedTargetType); } });Rule Used: What: Avoid updating
$stateinside$effectblo... (source)Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: add inbound webhook support (#909)" | Re-trigger Greptile
(2/5) Greptile learns from your feedback when you react with thumbs up/down!
Context used:
$stateinside$effectblo... (source)