test(web4): fix TestSetTagsDashboardAPI against admin-gated /api/stats - #431
Merged
Conversation
…/api/stats The security batch moved the rich /api/stats payload behind requireAdminToken in rendezvous. zz_dashboard_test.go was updated for this (it passes ?admin_token=), but zz_tags_test.go was not, so TestSetTagsDashboardAPI now receives a 401 with the body "admin token not configured" and fails deterministically with `decode JSON: invalid character 'a' looking for beginning of value`. Configure an admin token on the test registry and pass it on the /api/stats request, matching the existing pattern in zz_dashboard_test.go. Also assert the 200 so a future auth regression fails with a clear status mismatch instead of a JSON decode error. Test-only change; no product code touched. Note: this did not show up in PR CI because requireRealNetwork skips the test on GitHub-hosted runners. It fails locally and on the self-hosted nightly runner (PILOT_REAL_NETWORK=1). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ed endpoint TestSetTagsDashboardNoHostname and TestSetTagsDashboardNoIPLeak assert that the response body does NOT contain a hostname, 127.0.0.1, real_addr or public_key. Since /api/stats moved behind requireAdminToken these tests receive a 401 body, which trivially satisfies every assertion — they pass while checking nothing. Point them at /api/public-stats, the curated anonymous payload that is actually the surface this privacy property is about, and assert 200 plus a non-empty body so they cannot silently go vacuous again. Test-only change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Problem
TestSetTagsDashboardAPIfails deterministically onmain:The security batch moved the rich
/api/statspayload behindrequireAdminTokenin rendezvous. With no token configured the endpoint returns 401 with bodyadmin token not configured— the'a'in the decode error.tests/zz_dashboard_test.gowas updated for this (it passes?admin_token=), buttests/zz_tags_test.gowas missed.Why PR CI didn't catch it
requireRealNetwork(t)skips only whenGITHUB_ACTIONS == "true" && PILOT_REAL_NETWORK != "1". So the test is skipped on GitHub-hosted runners but runs — and fails — locally and on the self-hosted nightly runner (PILOT_REAL_NETWORK=1). Left as-is, tonight's nightly goes red.Fix
Configure an admin token on the test registry and pass it on the
/api/statsrequest, matching the existing pattern inzz_dashboard_test.go. Also assert200so a future auth regression fails with a clear status mismatch rather than a JSON decode error.Test-only change — no product code touched.
Verification
go test -count=1 -run TestSetTags ./tests/— all 12 passgo test -count=1 ./...— full suite green (exit 0)Follow-up (not in this PR)
TestSetTagsDashboardNoHostnameandTestSetTagsDashboardNoIPLeakhit the same gated endpoint but still pass vacuously — they assert the response body does not contain a hostname/IP, which is trivially true of a 401 body. They are asserting nothing right now and should get the same treatment.🤖 Generated with Claude Code