test(security): route-specific regression tests for #684 admin auth fix - #739
Merged
HongmingWang-Rabbit merged 1 commit intoApr 17, 2026
Merged
Conversation
…in routes The BE's tests (AdminTokenSet_*, FailOpen_*) validated the core AdminAuth contract on /admin/secrets. These table-driven additions pin the same contract on the three routes explicitly named in the #684 security report, each with three scenarios: workspace token rejected, correct ADMIN_TOKEN accepted, no bearer rejected. Routes covered: GET /admin/liveness GET /admin/github-installation-token GET /approvals/pending When ADMIN_TOKEN is set (tier 2), ValidateAnyToken is never called — the env-var comparison short-circuits before any DB lookup. The mock sets only HasAnyLiveTokenGlobal and nothing else; an extra DB expectation would itself be a test bug (calling it proves the middleware regressed to tier 3). All 18 TestAdminAuth_684* tests pass. Full go test ./... is green across all 15 platform packages. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5 tasks
Contributor
Author
There was a problem hiding this comment.
APPROVED — Correct approach, clean implementation.
Tests now match PR #729's ADMIN_TOKEN env-var implementation exactly:
t.Setenv("ADMIN_TOKEN", ...)— no DB mock approach ✅- Only
hasAnyLiveTokenGlobalQuerymocked —ValidateAnyTokennever called ✅ - The absence of a DB token-lookup expectation proves the env-var short-circuit fires; any stray DB call would hit sqlmock's 'unexpected query' error and fail the test — elegant self-checking design ✅
- Table-driven across all 3 vulnerable routes (#684 report: /admin/liveness, /admin/github-installation-token, /approvals/pending) ✅
rt := rtcapture for parallel-safe subtests ✅mock.ExpectationsWereMet()in every subtest ✅- 18/18
TestAdminAuth_684*(6 BE + 12 QA), fullgo test ./...green ✅
Ready to merge alongside PR #729. These two PRs together give complete application-layer coverage of the #684 fix.
HongmingWang-Rabbit
deleted the
test/issue-684-adminauth-bearer-scope-v2
branch
April 17, 2026 15:47
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 21, 2026
…rer-scope-v2 test(security): route-specific regression tests for #684 admin auth fix
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
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
What changed
3 new table-driven test functions in
wsauth_middleware_test.go, each covering/admin/liveness,/admin/github-installation-token, and/approvals/pending:TestAdminAuth_684_SpecificRoutes_WorkspaceTokenRejectedTestAdminAuth_684_SpecificRoutes_CorrectAdminTokenAcceptedTestAdminAuth_684_SpecificRoutes_NoBearer_Returns401Why this approach
PR #729's
AdminAuthimplementation usesADMIN_TOKENenv var +crypto/subtle.ConstantTimeCompare(tier 2) before falling back toValidateAnyToken(tier 3 deprecated). WhenADMIN_TOKENis set:HasAnyLiveTokenGlobalmakes a DB callValidateAnyTokenis never called — the mock sets no DB expectation for token validation, which itself proves the code path is correct (a stray DB call would hit sqlmock's "unexpected query" error and fail the test)Test output
All 18
TestAdminAuth_684*tests (6 existing from BE + 12 new) pass. Fullgo test ./...is green across all 15 platform packages.Test plan
go build ./...— clean (zero errors)TestAdminAuth_684*tests: 18/18 PASSgo test ./...across 15 packages: all greenwsauth_middleware_test.gochanged, +165 lines, no deletions🤖 Generated with Claude Code