test(admin_test_token): pin ADMIN_TOKEN IDOR-fix (#112) gate behavior - #2373
Merged
Merged
Conversation
The admin test-token endpoint has a critical security check at admin_test_token.go:64-72 — the IDOR fix from #112 that requires an explicit ADMIN_TOKEN bearer when the env var is set. Pre-fix, the route accepted ANY bearer that matched a live org token, allowing cross-org test-token minting (and therefore cross-org workspace authentication). The current code uses subtle.ConstantTimeCompare against ADMIN_TOKEN. Test coverage was zero. The existing tests exercised the ADMIN_TOKEN-unset path (local dev / CI) but never set ADMIN_TOKEN. A regression that: - removed the os.Getenv("ADMIN_TOKEN") check - inverted the comparison - replaced ConstantTimeCompare with bytes.Equal (timing leak) - re-introduced the AdminAuth fallback that allows org tokens would not fail any test, and the breakage would re-open the IDOR that #112 closed. Adds four tests covering the gate matrix: - ADMIN_TOKEN set + no Authorization header → 401 - ADMIN_TOKEN set + wrong Authorization → 401 - ADMIN_TOKEN set + correct Authorization → 200 - ADMIN_TOKEN unset + no Authorization → 200 (gate bypassed safely) The 4-row matrix pins the gate's full truth table: any regression in either dimension (gate enabled/disabled, header correct/wrong) trips exactly one test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
requested a review
from hongmingwang-moleculeai
as a code owner
April 30, 2026 09:59
HongmingWang-Rabbit
enabled auto-merge
April 30, 2026 09:59
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
Serialized merge by gitea-merge-queue after current-main, genuine approvals, and required CI checks were green.
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.
Summary
The admin test-token endpoint has a critical security check at `admin_test_token.go:64-72` — the IDOR fix from #112 that requires an explicit `ADMIN_TOKEN` bearer when the env var is set. Pre-fix, the route accepted ANY bearer that matched a live org token, allowing cross-org test-token minting (and therefore cross-org workspace authentication). The current code uses `subtle.ConstantTimeCompare` against `ADMIN_TOKEN`.
Test coverage was zero. Existing tests exercised the `ADMIN_TOKEN`-unset path (local dev / CI) but never set `ADMIN_TOKEN`. A regression that:
would not fail any test, and the breakage would re-open the IDOR that #112 closed.
Changes
Adds four tests covering the gate's full truth table:
Any regression in either dimension (gate enabled/disabled × header correct/wrong) trips exactly one test.
Test plan
🤖 Generated with Claude Code