Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,17 @@ behind the same wire format.
These routes bypass the provider entirely and are never sent to `/v1/authn`:

```
/agents/register /agents/forward /docs /api /openapi.json /redoc
/favicon.ico /health /healthcheck /healthz /readyz /ping /echo
/agents/register /agents/forward /slack /linear /favicon.ico
/health /healthcheck /healthz /readyz /ping /echo
```

(plus any sub-path under them, and all `OPTIONS` preflight requests).

The API documentation surfaces — `/openapi.json`, `/swagger`, `/api` (ReDoc)
and the MkDocs site at `/docs` — are **not** allowlisted. They are still
served, but the provider authenticates them like any other route, because the
schema enumerates every route, parameter and model the service exposes.

---

## Conventions
Expand Down
18 changes: 13 additions & 5 deletions agentex/src/api/middleware_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,24 @@

logger = make_logger(__name__)

# Routes (and their prefixes) that bypass authentication
# Routes (and their prefixes) that bypass authentication.
#
# The OpenAPI schema, the UIs that render it (/swagger, and /api, which is
# redoc_url) and the MkDocs site at /docs are deliberately NOT here. The schema
# is a complete description of every route, parameter and model this service
# exposes; served anonymously on an internet-facing deployment it hands an
# unauthenticated caller the whole attack surface for free, and the docs site
# describes the same setup in prose. They all stay mounted -- an authenticated
# session still reaches them -- they just no longer bypass the auth provider.
#
# Client and SDK codegen is unaffected: scripts/generate_openapi_spec.py builds
# the committed spec in-process from `fastapi_app.openapi()` and never fetches
# it over HTTP.
WHITELISTED_ROUTES: set[str] = {
"/agents/register",
"/agents/forward",
"/slack",
"/linear",
"/docs",
"/api",
"/openapi.json",
"/redoc",
"/favicon.ico",
"/health",
"/healthcheck",
Expand Down
43 changes: 38 additions & 5 deletions agentex/src/utils/otel_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
SERVICE_VERSION,
OTELResourceDetector,
Resource,
get_aggregated_resources,
ServiceInstanceIdResourceDetector,
)

from src.utils.logging import make_logger
Expand All @@ -88,8 +88,36 @@


def _detected_resource() -> Resource:
"""Resource attributes from OTEL_* env (operator-injected or local)."""
return get_aggregated_resources([OTELResourceDetector()])
"""Resource attributes from OTEL_* env (operator-injected or local).

`OTELResourceDetector().detect()`, not `get_aggregated_resources([...])`.
The aggregating helper merges the SDK's default resource, and since
opentelemetry-sdk 1.3x that default supplies a freshly generated random
UUID for `service.instance.id` when nothing else sets one. Aggregating
here would mean `_unique_instance_id` always sees an "existing" value and
returns `<random-uuid>.<pid>` -- a different identity on every process
start, with no trace of the service or pod it belongs to.

This function exists only to answer "what did the operator actually
configure", so it must report `service.instance.id` as absent when the
operator did not set one.
"""
return OTELResourceDetector().detect()
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved


def _sdk_generated_instance_id() -> str | None:
"""The random per-process id the SDK invents when nothing configures one.

The detector memoizes it, so repeated calls in one process return the same
value. Useful only as a last-resort uniqueness source -- it is opaque and
changes on every restart, so a configured identity always beats it.
"""
generated = (
ServiceInstanceIdResourceDetector()
.detect()
.attributes.get("service.instance.id")
)
return str(generated) if generated else None


def _unique_instance_id(resource: Resource) -> str:
Expand All @@ -105,8 +133,13 @@ def _unique_instance_id(resource: Resource) -> str:
or os.environ.get("OTEL_SERVICE_NAME")
or "unknown"
)
pod = resource.attributes.get("k8s.pod.name") or "unknown"
return f"{service}.{pod}.{pid}"
# Off Kubernetes there is no pod name, and pids repeat freely across
# containers and hosts -- two replicas would otherwise both report
# `<service>.unknown.<pid>` and silently merge their counter streams at a
# shared collector. The SDK's random id is a poor identity but a sound
# tiebreaker, so it stands in only when nothing better is configured.
pod = resource.attributes.get("k8s.pod.name") or _sdk_generated_instance_id()
return f"{service}.{pod or 'unknown'}.{pid}"


def _sync_instance_id_to_env(instance_id: str) -> None:
Expand Down
90 changes: 90 additions & 0 deletions agentex/tests/unit/api/test_auth_route_allowlist.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
"""The unauthenticated route allowlist is a security boundary, not a config list.

`is_whitelisted_route` decides which requests skip the auth provider entirely
(`AgentexAuthMiddleware.dispatch`). Anything it returns True for is served to
the internet with no principal, so each entry is asserted explicitly here
rather than inferred from the set.
"""

import pytest
from src.api.middleware_utils import WHITELISTED_ROUTES, is_whitelisted_route


@pytest.mark.unit
@pytest.mark.parametrize(
"path",
[
"/openapi.json",
"/swagger",
"/swagger/oauth2-redirect",
"/api",
"/api/",
"/redoc",
"/docs",
"/docs/index.html",
],
)
def test_api_schema_and_docs_require_authentication(path):
"""The schema enumerates every route, parameter and model this service
exposes; the docs describe its setup. Neither is served anonymously."""
assert not is_whitelisted_route(path)


@pytest.mark.unit
@pytest.mark.parametrize(
"path",
[
"/health",
"/healthz",
"/readyz",
"/ping",
"/echo",
"/favicon.ico",
"/agents/register",
"/agents/forward/name/some-agent/v1/messages",
],
)
def test_anonymous_routes_stay_anonymous(path):
"""Probes, the agent registration handshake and agent forwarding carry no
user principal by design and must not start demanding one."""
assert is_whitelisted_route(path)


@pytest.mark.unit
@pytest.mark.parametrize(
"path",
[
"/agents",
"/agents/some-agent-id",
"/agents/registered",
"/tasks",
"/messages",
"/spans",
"/health-metrics",
"/echoes/1",
],
)
def test_resource_routes_are_authenticated(path):
"""Entries match a whole path segment, so a route that merely shares a
prefix with an allowlisted one ("/agents/registered" vs
"/agents/register") is not born anonymous."""
assert not is_whitelisted_route(path)


@pytest.mark.unit
def test_allowlist_contents_are_pinned():
"""A new entry is a new piece of internet-facing anonymous surface. Adding
one here is the prompt to say why in review."""
assert WHITELISTED_ROUTES == {
"/agents/register",
"/agents/forward",
"/slack",
"/linear",
"/favicon.ico",
"/health",
"/healthcheck",
"/healthz",
"/readyz",
"/ping",
"/echo",
}
54 changes: 50 additions & 4 deletions agentex/tests/unit/utils/test_otel_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
from opentelemetry.sdk.metrics import MeterProvider
from opentelemetry.sdk.metrics.export import InMemoryMetricReader
from opentelemetry.sdk.resources import (
OTELResourceDetector,
Resource,
get_aggregated_resources,
)
from src.utils import cache_metrics, otel_metrics

Expand Down Expand Up @@ -176,7 +174,7 @@ def test_unique_instance_id_extends_operator_value(monkeypatch):
"k8s.pod.name=my-pod,service.instance.id=agentex.my-pod.agentex",
)
monkeypatch.setattr(otel_metrics.os, "getpid", lambda: 42)
base = get_aggregated_resources([OTELResourceDetector()])
base = otel_metrics._detected_resource()
assert otel_metrics._unique_instance_id(base) == "agentex.my-pod.agentex.42"


Expand All @@ -185,10 +183,58 @@ def test_unique_instance_id_builds_when_missing(monkeypatch):
monkeypatch.setenv("OTEL_SERVICE_NAME", "agentex")
monkeypatch.setenv("OTEL_RESOURCE_ATTRIBUTES", "k8s.pod.name=my-pod")
monkeypatch.setattr(otel_metrics.os, "getpid", lambda: 42)
base = get_aggregated_resources([OTELResourceDetector()])
base = otel_metrics._detected_resource()
assert otel_metrics._unique_instance_id(base) == "agentex.my-pod.42"


@pytest.mark.unit
def test_detected_resource_omits_the_sdk_generated_instance_id(monkeypatch):
"""`_detected_resource` reports what the operator configured, nothing else.

Since opentelemetry-sdk 1.3x the SDK's default resource invents a random
UUID for `service.instance.id`. If that leaks in here, `_unique_instance_id`
always takes its "existing value" branch and every process reports
`<random-uuid>.<pid>` -- unique, but opaque and different on every restart.
"""
monkeypatch.setenv("OTEL_SERVICE_NAME", "agentex")
monkeypatch.setenv("OTEL_RESOURCE_ATTRIBUTES", "k8s.pod.name=my-pod")

assert "service.instance.id" not in otel_metrics._detected_resource().attributes


@pytest.mark.unit
def test_unique_instance_id_without_a_pod_name_stays_unique(monkeypatch):
"""Off Kubernetes there is no pod name and pids repeat across containers.

Two replicas must not both report `agentex.unknown.42` -- at a shared
collector that merges their counter streams into one wrong series. The
SDK's random id is a poor identity but a sound tiebreaker, so it stands in
for the missing pod rather than the literal "unknown".
"""
monkeypatch.setenv("OTEL_SERVICE_NAME", "agentex")
monkeypatch.delenv("OTEL_RESOURCE_ATTRIBUTES", raising=False)
monkeypatch.setattr(otel_metrics.os, "getpid", lambda: 42)

instance_id = otel_metrics._unique_instance_id(otel_metrics._detected_resource())

assert instance_id == f"agentex.{otel_metrics._sdk_generated_instance_id()}.42"
assert instance_id != "agentex.unknown.42"


@pytest.mark.unit
def test_unique_instance_id_prefers_the_pod_over_the_sdk_id(monkeypatch):
"""The SDK id is a last resort: a real pod name is readable and stable
across restarts, so it wins whenever one is present."""
monkeypatch.setenv("OTEL_SERVICE_NAME", "agentex")
monkeypatch.setenv("OTEL_RESOURCE_ATTRIBUTES", "k8s.pod.name=my-pod")
monkeypatch.setattr(otel_metrics.os, "getpid", lambda: 42)

assert (
otel_metrics._unique_instance_id(otel_metrics._detected_resource())
== "agentex.my-pod.42"
)


@pytest.mark.unit
def test_sync_instance_id_to_env_updates_env(monkeypatch):
monkeypatch.setenv("OTEL_SERVICE_NAME", "agentex")
Expand Down
Loading