Repository navigation
fix(auth): stop serving the OpenAPI schema and docs anonymously - #480
Merged
Merged
Conversation
`/openapi.json`, `/api` (redoc_url) and the MkDocs site at `/docs` were in WHITELISTED_ROUTES, so AgentexAuthMiddleware skipped the auth provider for them. On an internet-facing deployment that means an unauthenticated `GET /openapi.json` returns a complete description of every route, parameter, request body and response model the service exposes -- the whole attack surface, enumerated, with no credential. `/swagger` was already authenticated, which made the gap inconsistent as well as open. Nothing is unmounted: the schema and both UIs are still served, they just go through the auth provider like any other route, so an authenticated session reaches them exactly as before. Client and SDK codegen is unaffected. `scripts/generate_openapi_spec.py` builds the committed spec in-process from `fastapi_app.openapi()`; it has never fetched it over HTTP. Adds tests over `is_whitelisted_route` covering the three groups that matter -- the schema and docs surfaces, the routes that are anonymous by design (probes, the agent registration handshake, agent forwarding), and routes that merely share a prefix with an allowlisted one -- plus a pin on the allowlist contents, so adding anonymous internet-facing surface has to be argued in review. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`_detected_resource()` used `get_aggregated_resources([OTELResourceDetector()])`, which merges the SDK's default resource. Since opentelemetry-sdk 1.3x that default generates a random UUID for `service.instance.id` when nothing else supplies one, and the dependency floor here is `opentelemetry-sdk>=1.28.0`, so an ordinary resolver bump turned it on. `_unique_instance_id` then always took its "existing value" branch, and both call sites -- the auto-instrumentation bootstrap and the standalone MeterProvider resource -- started publishing `<random-uuid>.<pid>`. Still unique per worker, which is what the function guarantees, but it drops the service and pod the series belongs to and changes on every process start, so metric identity no longer survives a restart. `_detected_resource` exists to answer "what did the operator configure", so it now uses the detector alone and reports `service.instance.id` as absent when the operator did not set one. An operator-supplied value, via OTEL_RESOURCE_ATTRIBUTES, still wins and still gets the pid suffix. The two `_unique_instance_id` tests rebuilt the aggregated resource by hand instead of calling `_detected_resource()`, which is why they drifted from the code path they name; `test_unique_instance_id_builds_when_missing` had been failing on main. They now go through the real function, and a new test pins the absence of the SDK-generated id directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review catch on the previous commit. Dropping the SDK's generated `service.instance.id` left `<service>.unknown.<pid>` as the fallback whenever neither an operator value nor `k8s.pod.name` is present -- off Kubernetes, in local runs, in plain containers. Pids repeat freely across containers and hosts, so two replicas could report the same instance id and merge their counter streams into one wrong series at a shared collector. That is the exact uniqueness guarantee the function exists to provide. The SDK's random id is still the wrong thing to *prefer* -- opaque, and new on every restart -- but it is a sound tiebreaker, so it now stands in for the missing pod name instead of the literal "unknown". A configured identity, and a real pod name, both still win. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
danielmillerp
approved these changes
Nov 5, 2026
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.
What
Removes
/openapi.json,/api(the ReDoc UI, i.e.redoc_url),/redocand the MkDocs site at/docsfromWHITELISTED_ROUTES, the set of pathsAgentexAuthMiddlewareserves without calling the auth provider.Nothing is unmounted. All of them are still served — they just authenticate now, like every other route. An authenticated session reaches them exactly as before.
Why
On an internet-facing deployment, an unauthenticated
GET /openapi.jsonreturns a complete description of every route, parameter, request body and response model this service exposes: the whole attack surface, enumerated, with no credential./swaggerwas already authenticated, so the gap was inconsistent as well as open.Blast radius
scripts/generate_openapi_spec.pybuilds the committed spec in-process fromfastapi_app.openapi(). It has never fetched it over HTTP.AGENTEX_AUTH_URLunset the middleware is disabled outright, sohttp://localhost:5003/swaggerand friends keep working as documented./docs(MkDocs) now requires auth. It is bundled with the service and describes its setup, so it belongs on the same side of the line — but it is one line to put back if a deployment needs it anonymous./health,/healthz,/readyz,/ping,/echo), the agent registration handshake and agent forwarding are untouched.Tests
tests/unit/api/test_auth_route_allowlist.pycoversis_whitelisted_routein three groups — the schema and docs surfaces, the routes that are anonymous by design, and routes that merely share a prefix with an allowlisted one — plus a pin on the allowlist contents, so adding anonymous internet-facing surface has to be argued in review rather than slipped into a set literal.docs/docs/development_guides/auth_provider_contract.mdis updated: it published the old list.🤖 Generated with Claude Code


Confidence Score: 5/5
The PR appears safe to merge; the previous instance-ID finding is addressed.Summary
The PR puts the API schema and documentation behind the existing authentication provider.
Reviews (3) · Last reviewed commit: "fix(otel): keep instance ids unique when..." · Reviewed by Greptile