fix(security): gate /channels/discover behind AdminAuth (#250) - #252
Merged
Conversation
Closes #250 (MEDIUM). POST /channels/discover was on the open router and accepted an arbitrary Telegram bot token, turning it into: 1. A free bot-token validity oracle — attackers can enumerate/probe tokens at zero cost 2. A drive-by deleteWebhook side effect — every call invokes tgbotapi.DeleteWebhookConfig against the target bot, breaking legitimate webhook delivery 3. A rate-limit amplifier — getMe + deleteWebhook + getUpdates per call Fix: one-line addition of middleware.AdminAuth(db.DB) to the route, matching its actual intent (platform-operator admin helper, not a per-workspace route). Pattern mirrors /admin/liveness, /events, and /bundles/export from PR #167. No new test: AdminAuth behavior is covered by wsauth_middleware_test.go; this PR only wires it onto an additional route. The load-bearing code comment references #250 so future reviewers can't revert without an issue citation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
left a comment
HongmingWang-Rabbit
left a comment
Contributor
Author
There was a problem hiding this comment.
Post-merge code review — Dev Lead
Fix is correct and minimal. Moving /channels/discover inside middleware.AdminAuth(db.DB) is the exact right approach — consistent with all other channel CRUD routes (wsAuth group at line ~381). The comment explaining why AdminAuth is appropriate (platform-operator helper, not per-workspace route) is a useful addition for future readers.
No concerns. Closes #250 cleanly. ✅
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 21, 2026
fix(security): gate /channels/discover behind AdminAuth (#250)
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.
Closes #250 (MEDIUM).
Summary
`POST /channels/discover` was on the open router. Any unauthenticated caller could:
Fix
One-line: wrap the route with `middleware.AdminAuth(db.DB)`. Matches the pattern used on `/admin/liveness`, `/events`, `/bundles/export`, `/templates/import`, etc. from PRs #167/#190/#200.
Why no new test
`AdminAuth` behavior is covered in `platform/internal/middleware/wsauth_middleware_test.go`; this PR only adds an additional route to that already-tested middleware. A full router integration test would require mocking every dependency of `router.Setup` — excessive for a one-line wire-up.
A load-bearing code comment cites #250 so a future reviewer can't revert without an issue citation.
Test plan
🤖 Generated with Claude Code