refactor(webhooks): make admin webhooks view-only, remove write actions#1754
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedAn error occurred during the review process. Please try again later. 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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0445ed10-2956-4c1c-b75a-211d48b2d3bf
📒 Files selected for processing (7)
pkg/server/config.gopkg/server/server.goweb/apps/admin/configs.dev.jsonweb/apps/admin/src/pages/webhooks/WebhooksPage.tsxweb/apps/admin/src/utils/constants.tsweb/sdk/admin/views/webhooks/webhooks/columns.tsxweb/sdk/admin/views/webhooks/webhooks/index.tsx
Coverage Report for CI Build 29399637566Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Warning No base build found for commit Coverage: 45.198%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
Shreyag02
changed the title
# Conflicts: # pkg/server/server.go # web/sdk/admin/views/webhooks/webhooks/delete/index.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/server/server.go (2)
307-308: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse structured logging fields.
Avoid string concatenation in log messages. Passing variables as structured fields makes logs easier to parse and query.
🛠️ Proposed fix
- logger.Info("Graceful shutdown of " + name + " complete") + logger.Info("Graceful shutdown complete", "server", name)
100-103: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
ReadHeaderTimeoutto prevent resource exhaustion.The HTTP servers are constructed without read timeouts. A slow or malicious client can hold connections open indefinitely by sending headers slowly (Slowloris attack). Set
ReadHeaderTimeoutto bound the wait time for headers. Avoid settingReadTimeoutas it applies to the entire request body and could prematurely close legitimate long-lived streaming connections like gRPC/ConnectRPC.
pkg/server/server.go#L100-L103: SetReadHeaderTimeout: 5 * time.Secondon the UI server.pkg/server/server.go#L223-L226: SetReadHeaderTimeout: 5 * time.Secondon the Connect server.Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0d27c68b-e1af-46ab-8031-781a872654e6
📒 Files selected for processing (3)
pkg/server/config.gopkg/server/server.goweb/apps/admin/src/routes.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/apps/admin/src/routes.tsx
- pkg/server/config.go
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/server/server.go (2)
307-308: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse structured logging fields.
Avoid string concatenation in log messages. Passing variables as structured fields makes logs easier to parse and query.
🛠️ Proposed fix
- logger.Info("Graceful shutdown of " + name + " complete") + logger.Info("Graceful shutdown complete", "server", name)
100-103: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
ReadHeaderTimeoutto prevent resource exhaustion.The HTTP servers are constructed without read timeouts. A slow or malicious client can hold connections open indefinitely by sending headers slowly (Slowloris attack). Set
ReadHeaderTimeoutto bound the wait time for headers. Avoid settingReadTimeoutas it applies to the entire request body and could prematurely close legitimate long-lived streaming connections like gRPC/ConnectRPC.
pkg/server/server.go#L100-L103: SetReadHeaderTimeout: 5 * time.Secondon the UI server.pkg/server/server.go#L223-L226: SetReadHeaderTimeout: 5 * time.Secondon the Connect server.Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0d27c68b-e1af-46ab-8031-781a872654e6
📒 Files selected for processing (3)
pkg/server/config.gopkg/server/server.goweb/apps/admin/src/routes.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/apps/admin/src/routes.tsx
- pkg/server/config.go
🛑 Comments failed to post (1)
pkg/server/server.go (1)
75-76: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix proxy path replacement and
Hostheader.Replacing all occurrences of
/frontier-connectin the path could accidentally mutate legitimate downstream paths (e.g., if a resource or organization is namedfrontier-connect). Usestrings.TrimPrefixto safely strip only the routing prefix.Additionally,
connectRemoteHostcontains thehttp://scheme, which will result in an invalid HTTPHostheader. Use the parsedconnectRemote.Hostinstead, which correctly provides thehost:portstring.🛠️ Proposed fix
- r.URL.Path = strings.Replace(r.URL.Path, "/frontier-connect", "", -1) - r.Host = connectRemoteHost + r.URL.Path = strings.TrimPrefix(r.URL.Path, "/frontier-connect") + r.Host = connectRemote.Host📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.r.URL.Path = strings.TrimPrefix(r.URL.Path, "/frontier-connect") r.Host = connectRemote.Host
Reverts the Go-side removal of WebhooksConfig/WebhooksConfigApiResponse so pkg/server matches main; view-only scope is handled in the web layer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3 participants
Summary
Makes the admin console Webhooks page permanently view-only. All write actions (create, update, delete) are removed from the admin UI entirely — no config flag, no gating. The page now shows a read-only list of webhooks.
Changes
create/,update/, anddelete/components and the delete mutation from the query hook.WebhooksViewto a read-only table; its only prop is nowicon.webhooks/createandwebhooks/:webhookIdroutes.enable_actions/enable_delete) from the admin app config types and dev config.Technical Details
web/sdk/admin/views/webhooks/webhooks/):WebhooksViewrenders only the Description / State / URL / Created-at columns.getColumns()takes no arguments;useWebhookQueriesexposes only the list query.WebhooksPage.tsx,routes.tsx): renders<WebhooksView />with just an icon; create/detail routes removed.constants.ts,configs.dev.json):WebhooksConfigand thewebhooksblock removed from the admin app.WebhooksConfigand the/configswebhooksresponse remain identical tomain; the view-only scope is enforced entirely in the web layer.createWebhook/updateWebhook/deleteWebhook) are unchanged — only the admin UI surface was removed.Test Plan
tsc+ Vite production build, SDKtsupbuild, and ESLint all clean.SQL Safety (if your PR touches
*_repository.goorgoqu.*)N/A