Skip to content

backend: headlamp_test: Add negative test for unauthed callers#5657

Draft
vyncent-t wants to merge 1 commit into
kubernetes-sigs:mainfrom
vyncent-t:add-negative-test-for-backend-token
Draft

backend: headlamp_test: Add negative test for unauthed callers#5657
vyncent-t wants to merge 1 commit into
kubernetes-sigs:mainfrom
vyncent-t:add-negative-test-for-backend-token

Conversation

@vyncent-t
Copy link
Copy Markdown
Contributor

Summary

The backend's mutating endpoints (/cluster, /plugins/{name}) are gated by the X-HEADLAMP_BACKEND-TOKEN header, which must match the HEADLAMP_BACKEND_TOKEN env var set by the desktop app. Until now there was no negative test proving that gate actually rejects unauthenticated callers, a regression that removed checkHeadlampBackendToken from any of those handlers would have shipped green.

This PR adds two focused tests in backend/cmd/headlamp_test.go that pin the gate's behavior.

Changes

Added in backend/cmd/headlamp_test.go:

  • TestRestrictedEndpointsRequireToken — table-driven over the three currently-gated routes:

    • POST /cluster
    • DELETE /cluster/{name}
    • DELETE /plugins/{name}

    For each route it runs three subtests:

    • missing X-HEADLAMP_BACKEND-TOKEN header → expects 403 Forbidden
    • wrong token value → expects 403 Forbidden
    • correct token → expects status to NOT be 401/403 (we only pin the auth gate, not the response body)

    Each subtest builds a fresh handler with its own t.TempDir() so the "valid token" subtest, which actually mutates state, can't bleed into the others. Env var is set with t.Setenv so it's restored automatically.

  • TestRestrictedEndpointsRejectEmptyEnvToken — pins a second property of checkHeadlampBackendToken: when HEADLAMP_BACKEND_TOKEN is empty, every request must be rejected, including one with an empty header. Without this, clearing the env var would silently disable the gate.

Steps to Test

  1. cd backend && go test -count=1 -run 'TestRestrictedEndpoints' ./cmd/... — both tests should pass.
  2. To prove the tests catch regressions, temporarily comment out the checkHeadlampBackendToken call in addCluster, deleteCluster, or the DELETE /plugins/{name} handler in backend/cmd/headlamp.go and re-run — the corresponding subtest should now fail with a clear message.
  3. Full backend suite: npm run backend:test.

Notes for the Reviewer

  • Scope is intentionally tight. No production code is changed; this is purely test coverage.
  • PUT /cluster/{name} (renameCluster) is deliberately excluded from the table. While writing these tests I noticed renameCluster does not call checkHeadlampBackendToken, unlike its addCluster/deleteCluster siblings — so it's currently reachable without the backend token. Adding it to this table would assert behavior that does not exist and fail CI. The omission is documented in a comment on TestRestrictedEndpointsRequireToken. I'll send the handler fix + the corresponding row in the test table as a separate follow-up PR so the security fix and the regression test land together and are easy to review.
  • The expected status is 403 Forbidden (not 401) because that's what checkHeadlampBackendToken returns via http.Error. The "valid token" branch asserts != 403 and != 401, so it stays robust to handlers returning 200/201/400/404 depending on body.

@vyncent-t vyncent-t self-assigned this May 14, 2026
@vyncent-t vyncent-t added the backend Issues related to the backend label May 14, 2026
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 14, 2026
@k8s-ci-robot k8s-ci-robot requested review from kahirokunn and skoeva May 14, 2026 19:03
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 14, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vyncent-t
Once this PR has been reviewed and has the lgtm label, please assign yolossn for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vyncent-t vyncent-t requested review from illume and sniok and removed request for kahirokunn and skoeva May 14, 2026 19:03
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Issues related to the backend cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants