Skip to content

fix(api-server): require auth for /health/detailed - #33847

Closed
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/api-server-health-detailed-auth
Closed

fix(api-server): require auth for /health/detailed#33847
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/api-server-health-detailed-auth

Conversation

@Dusk1e

@Dusk1e Dusk1e commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

This change secures the detailed health endpoint by requiring bearer authentication on /health/detailed whenever API_SERVER_KEY is configured.

It keeps /health public for basic liveness checks, updates the API server implementation to enforce the existing auth contract, aligns the regression test coverage with the intended behavior, and documents the distinction between the public and authenticated health endpoints.

Testing

  • tests/gateway/test_api_server.py::TestHealthDetailedEndpoint::test_health_detailed_returns_ok — passed
  • tests/gateway/test_api_server.py::TestHealthDetailedEndpoint::test_health_detailed_no_runtime_status — passed
  • tests/gateway/test_api_server.py::TestHealthDetailedEndpoint::test_health_detailed_requires_auth_when_key_configured — passed
  • tests/gateway/test_api_server.py::TestModelsEndpoint::test_models_requires_auth — passed
  • tests/gateway/test_api_server.py::TestCapabilitiesEndpoint::test_capabilities_requires_auth_when_key_configured — passed
  • tests/gateway/test_api_server.py::TestEndpointAuth::test_models_requires_auth — passed
  • tests/gateway/test_api_server.py::TestEndpointAuth::test_health_does_not_require_auth — passed

Test summary: 7 passed, 149 deselected

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools labels May 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing with #22280 which also addresses the unauthenticated /health/detailed endpoint (among other security fixes). Same author (Dusk1e) as #33844.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused security fix. This exact endpoint-authentication guarantee is already implemented on current main.

  • gateway/platforms/api_server.py:1412-1414 authenticates /health/detailed before diagnostics are read.
  • tests/gateway/test_api_server.py:791-806 covers both unauthenticated 401 and authenticated 200 behavior.
  • The canonical fix landed through merged fix(api-server): require auth for /health/detailed and fail closed on weak keys #56260 as 2d8d08cae (v2026.7.1); its discussion explicitly identifies this PR as a competing implementation.

Automated hermes-sweeper review.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants