From 1aea110397c14cc49979c9fc45a1b9c2b235765f Mon Sep 17 00:00:00 2001 From: David Shoen Date: Sun, 12 Jul 2026 13:49:40 +0300 Subject: [PATCH 1/5] feat: add ENFORCE_OPERATIONAL_ROUTE_AUTH rollout toggle (PER-15243) The auth-hardening PRs (#317/#320/#321) made the update-trigger routes (/policy-updater/trigger, /data-updater/trigger, /update_policy, /update_policy_data) and /kong unconditionally require the PDP token, with no way to relax it during a coordinated fleet upgrade. Add a single flag, ENFORCE_OPERATIONAL_ROUTE_AUTH (PDP_ prefixed env var, also overridable from the cloud control plane), defaulting to FALSE for a safe rollout: those five routes accept unauthenticated requests and only LOG the ones that would be rejected, so callers that don't yet send the token (e.g. the Kong OPA plugin, older SDK automations) keep working while the logs surface them. Flip to true per-fleet once every caller sends the token. Every other PDP route stays strictly enforced regardless. - enforce_pdp_token_operational: conditional gate reading the flag per request; delegates to enforce_pdp_token when on, warn-and-allows when off. Distinct named callable so the fail-closed route audit still recognises it as a gate. - /kong split onto its own router in the same closure so it can be mounted under the conditional gate while the rest of the enforcer router keeps the strict gate. - Startup warning while the flag is off (the rollout default) so a fleet in the permissive phase stays visible. - The route audit stays green but its guarantee is now structural (a gate is attached), not that the gate rejects at runtime. - Enforce-by-default tests from #317/#320/#321 now opt into enforcement; new tests assert the permissive default. 140 pass, ruff clean. Co-Authored-By: Claude Fable 5 --- horizon/authentication.py | 34 ++++++++++- horizon/config.py | 12 ++++ horizon/enforcer/api.py | 10 ++- horizon/pdp.py | 56 +++++++++++++---- horizon/tests/test_authentication.py | 71 ++++++++++++++++++++++ horizon/tests/test_enforcer_api.py | 23 +++++++ horizon/tests/test_legacy_update_routes.py | 5 ++ horizon/tests/test_opal_trigger_auth.py | 59 +++++++++++++++++- horizon/tests/test_route_auth_audit.py | 13 +++- 9 files changed, 265 insertions(+), 18 deletions(-) diff --git a/horizon/authentication.py b/horizon/authentication.py index 342bb4e3..d3086fa4 100644 --- a/horizon/authentication.py +++ b/horizon/authentication.py @@ -1,8 +1,9 @@ import hmac from typing import Annotated -from fastapi import Depends, HTTPException, status +from fastapi import Depends, HTTPException, Request, status from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer +from opal_client.logger import logger from horizon.config import MOCK_API_KEY, sidecar_config from horizon.startup.api_keys import get_env_api_key @@ -65,6 +66,37 @@ def enforce_pdp_token(credentials: PdpCredentials = None): raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail="Invalid PDP token") +def enforce_pdp_token_operational(request: Request, credentials: PdpCredentials = None): + """PDP-token gate for the operational routes hardened by PER-15244/PER-15245, with a rollout default. + + Governed by ``ENFORCE_OPERATIONAL_ROUTE_AUTH``. When it is true this is exactly ``enforce_pdp_token``. + When it is false - the default, for a safe fleet rollout - a request that would be rejected is allowed + through but logged, so callers that don't yet send the PDP token keep working while the logs surface + them before enforcement is switched on. + + The flag is read per-request (never captured at import) so a cloud control-plane override takes + effect and tests can toggle it. Kept as a distinct, named module-level function because the + fail-closed route audit recognises auth gates by callable name - a bare ``enforce_pdp_token`` here + could not carry the conditional behaviour, and an inline lambda would be invisible to the audit. + """ + if sidecar_config.ENFORCE_OPERATIONAL_ROUTE_AUTH: + enforce_pdp_token(credentials) + return + # Permissive rollout default: reuse enforce_pdp_token's exact reject logic, but downgrade a + # rejection to warn-and-allow so no caller breaks while every would-be rejection is still flagged. + try: + enforce_pdp_token(credentials) + except HTTPException as exc: + logger.warning( + "ENFORCE_OPERATIONAL_ROUTE_AUTH is off: allowing {method} {path} unauthenticated - it would " + "otherwise be rejected ({detail}). Set ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP " + "token on this route.", + method=request.method, + path=request.url.path, + detail=exc.detail, + ) + + def enforce_pdp_control_key(credentials: PdpCredentials = None): if sidecar_config.CONTAINER_CONTROL_KEY == MOCK_API_KEY: raise HTTPException( diff --git a/horizon/config.py b/horizon/config.py index 87fb42dc..651774e0 100644 --- a/horizon/config.py +++ b/horizon/config.py @@ -262,6 +262,18 @@ def parse_plugins(value: Any) -> dict[str, dict[str, int | bool | str]]: # enables debug ouptut for the Kong integration endpoint KONG_INTEGRATION_DEBUG = confi.bool("KONG_INTEGRATION_DEBUG", False) + ENFORCE_OPERATIONAL_ROUTE_AUTH = confi.bool( + "ENFORCE_OPERATIONAL_ROUTE_AUTH", + False, + description="When true, enforce the PDP token on the operational routes hardened by the auth-hardening " + "rollout: the update-trigger routes (/policy-updater/trigger, /data-updater/trigger, /update_policy, " + "/update_policy_data) and the /kong decision endpoint. Defaults to FALSE for a safe fleet rollout: those " + "routes accept unauthenticated requests and only LOG the ones that would be rejected, so callers that do not " + "yet send the token keep working while you watch the logs. Flip to true (per-fleet via the cloud control " + "plane, or PDP_ENFORCE_OPERATIONAL_ROUTE_AUTH) once every caller sends the token. Every other PDP route is " + "always enforced regardless of this flag.", + ) + LOCAL_FACTS_WAIT_TIMEOUT = confi.float( "LOCAL_FACTS_WAIT_TIMEOUT", 10, diff --git a/horizon/enforcer/api.py b/horizon/enforcer/api.py index 80193b74..a8759c60 100644 --- a/horizon/enforcer/api.py +++ b/horizon/enforcer/api.py @@ -277,6 +277,12 @@ async def health(): def init_enforcer_api_router(policy_store: BasePolicyStoreClient = None): # noqa: C901 policy_store = policy_store or DEFAULT_POLICY_STORE_GETTER() router = APIRouter() + # /kong lives on its own router so it can be mounted under the ENFORCE_OPERATIONAL_ROUTE_AUTH-aware + # gate (enforce_pdp_token_operational) while every other enforcer route keeps the plain + # enforce_pdp_token gate. Same closure as the rest of the enforcer routes: /kong's handler closes + # over kong_routes_table and the nested _is_allowed helper, so a router split here avoids a much + # larger refactor. Mounted separately in PermitPDP._configure_api_routes. + kong_router = APIRouter() if sidecar_config.KONG_INTEGRATION: with Path(KONG_ROUTES_TABLE_FILE).open() as f: kong_routes_table_raw = json.load(f) @@ -543,7 +549,7 @@ async def is_allowed_nginx( ) return {"allow": False, "result": False} - @router.post( + @kong_router.post( "/kong", response_model=KongAuthorizationResult, status_code=status.HTTP_200_OK, @@ -620,7 +626,7 @@ async def is_allowed_kong(request: Request, query: KongAuthorizationQuery): ) return {"allow": False, "result": False} - return router + return router, kong_router def _extract_regex_attributes(pattern: str, url: str) -> dict: diff --git a/horizon/pdp.py b/horizon/pdp.py index 24a2b7a5..01f7b2d6 100644 --- a/horizon/pdp.py +++ b/horizon/pdp.py @@ -26,7 +26,7 @@ from opal_common.logging_utils.formatter import Formatter from scalar_fastapi import get_scalar_api_reference -from horizon.authentication import enforce_pdp_token +from horizon.authentication import enforce_pdp_token, enforce_pdp_token_operational from horizon.config import MOCK_API_KEY, sidecar_config from horizon.connectivity.api import init_connectivity_router from horizon.enforcer.api import init_enforcer_api_router, init_enforcer_health_router, stats_manager @@ -108,28 +108,32 @@ def apply_config(overrides_dict: dict, config_object: Confi): def _gate_opal_trigger_routes(app: FastAPI) -> None: - """Inject ``Depends(enforce_pdp_token)`` into the OPAL-mounted trigger routes. + """Inject ``Depends(enforce_pdp_token_operational)`` into the OPAL-mounted trigger routes. OpalClient mounts ``POST /policy-updater/trigger`` and ``POST /data-updater/trigger`` on the app before ``PermitPDP`` gains control (opal_client.client._configure_api_routes), so the include_router-level dependencies used for every PDP-owned router cannot reach - them. We inject the standard PDP-token dependency into the already-mounted route objects - instead, mirroring what FastAPI itself does at ``APIRoute.__init__`` (fastapi/routing.py): - insert a parameterless sub-dependant at the head of ``route.dependant.dependencies``. + them. We inject the PDP-token dependency into the already-mounted route objects instead, + mirroring what FastAPI itself does at ``APIRoute.__init__`` (fastapi/routing.py): insert a + parameterless sub-dependant at the head of ``route.dependant.dependencies``. + + The gate is the operational wrapper, so ``ENFORCE_OPERATIONAL_ROUTE_AUTH`` governs these routes + like the sibling operational routes: unauthenticated-but-logged by default, enforced when set. The Dependant is mutated IN PLACE - the route's request handler closes over that exact object, so the check is enforced on every request; do NOT reassign ``route.dependant``. - ``enforce_pdp_token`` only reads a header, so the route's body field needs no rebuild. + ``enforce_pdp_token_operational`` only reads the header and request, so the route's body field + needs no rebuild. Fails loud if a target route is missing (e.g. an OPAL upgrade renamed it): a silently - skipped injection would leave an update-trigger endpoint unauthenticated. + skipped injection would leave an update-trigger endpoint ungated even when enforcement is on. """ gated: set[str] = set() for route in app.routes: if isinstance(route, APIRoute) and route.path in OPAL_TRIGGER_ROUTE_PATHS: route.dependant.dependencies.insert( 0, - get_parameterless_sub_dependant(depends=Depends(enforce_pdp_token), path=route.path_format), + get_parameterless_sub_dependant(depends=Depends(enforce_pdp_token_operational), path=route.path_format), ) gated.add(route.path) @@ -143,6 +147,23 @@ def _gate_opal_trigger_routes(app: FastAPI) -> None: raise SystemExit(GUNICORN_EXIT_APP) +def _warn_if_operational_route_auth_disabled() -> None: + """Warn when ``ENFORCE_OPERATIONAL_ROUTE_AUTH`` is off and the operational routes accept unauthenticated calls. + + This is the safe-rollout default (see SidecarConfig.ENFORCE_OPERATIONAL_ROUTE_AUTH): the token is not + enforced on the update-trigger routes or /kong, only logged. Surfaced at startup as well as per-request + so a fleet still in the permissive rollout phase stays visible; flip the flag on once every caller sends + the token. + """ + if not sidecar_config.ENFORCE_OPERATIONAL_ROUTE_AUTH: + logger.warning( + "ENFORCE_OPERATIONAL_ROUTE_AUTH is OFF: the update-trigger routes (/policy-updater/trigger, " + "/data-updater/trigger, /update_policy, /update_policy_data) and /kong accept UNAUTHENTICATED " + "requests - would-be rejections are only logged. This is the safe fleet-rollout default; set " + "ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP token on these routes." + ) + + def _warn_if_opal_verifier_disabled(opal_client: OpalClient) -> None: """Warn loudly when the OPAL-authenticated routes are effectively open. @@ -447,7 +468,7 @@ def _configure_api_routes(self, app: FastAPI): app.on_event("shutdown")(stats_manager.stop_tasks) enforcer_health_router = init_enforcer_health_router() - enforcer_router = init_enforcer_api_router(policy_store=self._opal.policy_store) + enforcer_router, kong_router = init_enforcer_api_router(policy_store=self._opal.policy_store) local_router = init_local_cache_api_router(policy_store=self._opal.policy_store) # Init system router system_router = init_system_api_router() @@ -460,6 +481,14 @@ def _configure_api_routes(self, app: FastAPI): tags=["Authorization API"], dependencies=[Depends(enforce_pdp_token)], ) + # /kong is gated with the operational wrapper so ENFORCE_OPERATIONAL_ROUTE_AUTH governs it + # during a fleet rollout (Kong's OPA plugin may not yet forward the PDP token) without + # touching the rest of the enforcer router. + app.include_router( + kong_router, + tags=["Authorization API"], + dependencies=[Depends(enforce_pdp_token_operational)], + ) app.include_router( local_router, @@ -498,11 +527,13 @@ def _configure_api_routes(self, app: FastAPI): ) # TODO: remove this when clients update sdk version (legacy routes) + # Gated with the operational wrapper: these are aliases of the OPAL trigger routes, so they + # follow the same ENFORCE_OPERATIONAL_ROUTE_AUTH rollout default. @app.post( "/update_policy", status_code=status.HTTP_200_OK, include_in_schema=False, - dependencies=[Depends(enforce_pdp_token)], + dependencies=[Depends(enforce_pdp_token_operational)], ) async def legacy_trigger_policy_update(): logger.info("triggered policy update from api (legacy route)") @@ -515,7 +546,7 @@ async def legacy_trigger_policy_update(): "/update_policy_data", status_code=status.HTTP_200_OK, include_in_schema=False, - dependencies=[Depends(enforce_pdp_token)], + dependencies=[Depends(enforce_pdp_token_operational)], ) async def legacy_trigger_data_update(): logger.info("triggered policy data update from api (legacy route)") @@ -534,6 +565,9 @@ async def legacy_trigger_data_update(): # High-signal warning if the OPAL-authenticated routes are left open by a disabled # verifier (must never happen in a managed PDP). _warn_if_opal_verifier_disabled(self._opal) + # High-signal warning while ENFORCE_OPERATIONAL_ROUTE_AUTH is off (the rollout default) and + # the update-trigger and /kong routes accept unauthenticated requests. + _warn_if_operational_route_auth_disabled() @property def app(self): diff --git a/horizon/tests/test_authentication.py b/horizon/tests/test_authentication.py index 1ba336af..95918683 100644 --- a/horizon/tests/test_authentication.py +++ b/horizon/tests/test_authentication.py @@ -7,6 +7,8 @@ public-route allowlist that the route-audit test relies on. """ +from types import SimpleNamespace + import horizon.authentication as auth import pytest from fastapi import HTTPException, status @@ -16,8 +18,10 @@ _token_matches, enforce_pdp_control_key, enforce_pdp_token, + enforce_pdp_token_operational, ) from horizon.config import MOCK_API_KEY, sidecar_config +from loguru import logger VALID_TOKEN = "s3cr3t-token" @@ -27,6 +31,11 @@ def _creds(token: str) -> HTTPAuthorizationCredentials: return HTTPAuthorizationCredentials(scheme="Bearer", credentials=token) +def _fake_request(path: str = "/policy-updater/trigger", method: str = "POST") -> SimpleNamespace: + """A stand-in for the Request; the compat wrapper only reads ``.method`` and ``.url.path``.""" + return SimpleNamespace(method=method, url=SimpleNamespace(path=path)) + + @pytest.mark.parametrize( ("credentials", "expected"), [ @@ -69,6 +78,68 @@ def test_wrong_token_is_401(self): assert exc.value.detail == "Invalid PDP token" +class TestEnforcePdpTokenOperational: + """The ENFORCE_OPERATIONAL_ROUTE_AUTH wrapper: enforce when on, warn-and-allow when off (default).""" + + WARN_SUBSTRING = "ENFORCE_OPERATIONAL_ROUTE_AUTH is off" + + @pytest.fixture(autouse=True) + def _patch_key(self, monkeypatch): + monkeypatch.setattr(auth, "get_env_api_key", lambda: VALID_TOKEN) + + @pytest.fixture + def enforce_on(self, monkeypatch): + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) + + @pytest.fixture + def enforce_off(self, monkeypatch): + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", False) + + @pytest.mark.usefixtures("enforce_on") + def test_enforce_on_missing_credentials_is_401(self): + # Flag on: identical to enforce_pdp_token. + with pytest.raises(HTTPException) as exc: + enforce_pdp_token_operational(_fake_request(), credentials=None) + assert exc.value.status_code == status.HTTP_401_UNAUTHORIZED + assert exc.value.detail == "Missing Authorization header" + + @pytest.mark.usefixtures("enforce_on") + def test_enforce_on_wrong_token_is_401(self): + with pytest.raises(HTTPException) as exc: + enforce_pdp_token_operational(_fake_request(), credentials=_creds("nope")) + assert exc.value.status_code == status.HTTP_401_UNAUTHORIZED + assert exc.value.detail == "Invalid PDP token" + + @pytest.mark.usefixtures("enforce_on") + def test_enforce_on_valid_token_passes(self): + assert enforce_pdp_token_operational(_fake_request(), credentials=_creds(VALID_TOKEN)) is None + + @pytest.mark.usefixtures("enforce_off") + def test_enforce_off_missing_credentials_is_allowed_and_warns(self, capture_loguru): + # The rollout default: a request that would be rejected is let through, but logged. + assert enforce_pdp_token_operational(_fake_request(), credentials=None) is None + assert any(self.WARN_SUBSTRING in record for record in capture_loguru) + + @pytest.mark.usefixtures("enforce_off") + def test_enforce_off_wrong_token_is_allowed_and_warns(self, capture_loguru): + assert enforce_pdp_token_operational(_fake_request(), credentials=_creds("nope")) is None + assert any(self.WARN_SUBSTRING in record for record in capture_loguru) + + @pytest.mark.usefixtures("enforce_off") + def test_enforce_off_valid_token_passes_without_warning(self, capture_loguru): + # A caller that already sends the token is not flagged - only would-be rejections warn. + assert enforce_pdp_token_operational(_fake_request(), credentials=_creds(VALID_TOKEN)) is None + assert not any(self.WARN_SUBSTRING in record for record in capture_loguru) + + +@pytest.fixture +def capture_loguru(): + records: list[str] = [] + sink_id = logger.add(lambda message: records.append(str(message)), level="WARNING") + yield records + logger.remove(sink_id) + + class TestEnforcePdpControlKey: CONTROL_KEY = "control-key" diff --git a/horizon/tests/test_enforcer_api.py b/horizon/tests/test_enforcer_api.py index 84052175..96e90db5 100644 --- a/horizon/tests/test_enforcer_api.py +++ b/horizon/tests/test_enforcer_api.py @@ -81,7 +81,18 @@ async def pdp_api_client() -> TestClient: } +@pytest.fixture +def enforce_operational_auth(monkeypatch): + """Enable ENFORCE_OPERATIONAL_ROUTE_AUTH so the conditionally-gated route (/kong) also rejects. + + The 8 other endpoints in the sweep carry the plain enforce_pdp_token gate and reject regardless; + /kong defaults to permissive (rollout default), so the sweep turns enforcement on to cover it too. + """ + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) + + @pytest.mark.parametrize("endpoint", PROTECTED_ENFORCER_ENDPOINTS) +@pytest.mark.usefixtures("enforce_operational_auth") def test_enforcer_endpoint_missing_token_returns_401(endpoint): client = TestClient(sidecar._app) response = client.post(endpoint, json={}) @@ -90,6 +101,7 @@ def test_enforcer_endpoint_missing_token_returns_401(endpoint): @pytest.mark.parametrize("endpoint", PROTECTED_ENFORCER_ENDPOINTS) +@pytest.mark.usefixtures("enforce_operational_auth") def test_enforcer_endpoint_invalid_token_returns_401(endpoint): client = TestClient(sidecar._app) response = client.post(endpoint, headers={"authorization": "Bearer wrong_token"}, json={}) @@ -115,11 +127,22 @@ def test_kong_endpoint_valid_token_integration_disabled_returns_503(): assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE +def test_kong_endpoint_permissive_default_bypasses_auth(): + # Rollout default (ENFORCE_OPERATIONAL_ROUTE_AUTH off): a tokenless /kong is no longer 401 - the + # gate lets it through to the handler, which then 503s because KONG_INTEGRATION is off. The 503 + # (not 401) is the proof that the auth gate allowed it through. + client = TestClient(sidecar._app) + response = client.post("/kong", json=KONG_QUERY) + assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE + + def test_kong_endpoint_enabled_integration_allowed_flow(tmp_path, monkeypatch): routes_file = tmp_path / "kong_routes.json" routes_file.write_text('[["^/resource1/.*$", "resource1"]]') monkeypatch.setattr("horizon.enforcer.api.KONG_ROUTES_TABLE_FILE", str(routes_file)) monkeypatch.setattr(sidecar_config, "KONG_INTEGRATION", True) + # /kong defaults to permissive; enable enforcement so the tokenless call below is a clean 401. + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) class FakeStateHandler: async def seen_sdk(self, _sdk: str) -> None: diff --git a/horizon/tests/test_legacy_update_routes.py b/horizon/tests/test_legacy_update_routes.py index 8cbcdc68..190353f7 100644 --- a/horizon/tests/test_legacy_update_routes.py +++ b/horizon/tests/test_legacy_update_routes.py @@ -35,6 +35,9 @@ def test_update_policy_triggers_updater(pdp: MockPermitPDP, auth: dict[str, str] def test_update_policy_rejects_unauthenticated(pdp: MockPermitPDP, monkeypatch): + # Rejection on these routes is now opt-in (ENFORCE_OPERATIONAL_ROUTE_AUTH defaults off for a + # safe fleet rollout); enable enforcement to assert the reject behaviour. + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) trigger = AsyncMock() monkeypatch.setattr(pdp._opal.policy_updater, "trigger_update_policy", trigger) client = TestClient(pdp._app) @@ -74,6 +77,8 @@ def test_update_policy_data_returns_503_when_updater_disabled(pdp: MockPermitPDP def test_update_policy_data_rejects_unauthenticated(pdp: MockPermitPDP, monkeypatch): + # Rejection is opt-in now (see test_update_policy_rejects_unauthenticated); enable enforcement. + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) get_base = AsyncMock() monkeypatch.setattr(pdp._opal.data_updater, "get_base_policy_data", get_base) client = TestClient(pdp._app) diff --git a/horizon/tests/test_opal_trigger_auth.py b/horizon/tests/test_opal_trigger_auth.py index 9d0cad7c..97368cf4 100644 --- a/horizon/tests/test_opal_trigger_auth.py +++ b/horizon/tests/test_opal_trigger_auth.py @@ -16,13 +16,16 @@ from fastapi.testclient import TestClient from horizon.config import sidecar_config from horizon.enforcer.api import stats_manager -from horizon.pdp import PermitPDP, _warn_if_opal_verifier_disabled +from horizon.pdp import PermitPDP, _warn_if_opal_verifier_disabled, _warn_if_operational_route_auth_disabled from loguru import logger from opal_client.client import OpalClient from starlette import status VALID_TOKEN = "mock_api_key" TRIGGER_ROUTES = ["/policy-updater/trigger", "/data-updater/trigger"] +# The legacy SDK aliases of the two trigger routes; gated with the same operational wrapper. +LEGACY_TRIGGER_ROUTES = ["/update_policy", "/update_policy_data"] +OPERATIONAL_UPDATE_ROUTES = TRIGGER_ROUTES + LEGACY_TRIGGER_ROUTES class MockPermitPDP(PermitPDP): @@ -49,7 +52,18 @@ def _auth(token: str) -> dict[str, str]: return {"Authorization": f"Bearer {token}"} +@pytest.fixture +def enforce_on(monkeypatch): + """Turn ENFORCE_OPERATIONAL_ROUTE_AUTH on (read per-request by the gate) so these routes reject.""" + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) + + +# The trigger-route enforcement tests below assert the reject behaviour, which is now opt-in +# (ENFORCE_OPERATIONAL_ROUTE_AUTH defaults to off for a safe fleet rollout), so they enable it. + + @pytest.mark.parametrize("path", TRIGGER_ROUTES) +@pytest.mark.usefixtures("enforce_on") def test_trigger_route_without_token_is_401(client: TestClient, path: str): # The actual vulnerability: OPAL-mounted trigger routes were callable unauthenticated. resp = client.post(path) @@ -59,12 +73,14 @@ def test_trigger_route_without_token_is_401(client: TestClient, path: str): @pytest.mark.parametrize("path", TRIGGER_ROUTES) def test_trigger_route_with_valid_token_is_not_blocked(client: TestClient, path: str): - # The dependency passes; the handler may 500 offline, but it must not be a 401. + # The dependency passes; the handler may 500 offline, but it must not be a 401. True in both + # enforcement modes, so left flag-agnostic. resp = client.post(path, headers=_auth(VALID_TOKEN)) assert resp.status_code != status.HTTP_401_UNAUTHORIZED @pytest.mark.parametrize("path", TRIGGER_ROUTES) +@pytest.mark.usefixtures("enforce_on") def test_trigger_route_with_wrong_token_is_401(client: TestClient, path: str): resp = client.post(path, headers=_auth("wrong-token")) assert resp.status_code == status.HTTP_401_UNAUTHORIZED @@ -73,6 +89,7 @@ def test_trigger_route_with_wrong_token_is_401(client: TestClient, path: str): @pytest.mark.parametrize("path", TRIGGER_ROUTES) @pytest.mark.parametrize("value", ["garbage", "Bearer", "Bearer ", "Bearer a b c"]) +@pytest.mark.usefixtures("enforce_on") def test_trigger_route_malformed_header_is_401_not_500(client: TestClient, path: str, value: str): # Regression for the unguarded split(" ") -> ValueError -> 500 footgun. resp = client.post(path, headers={"Authorization": value}) @@ -108,6 +125,44 @@ def test_exit_without_header_is_503_when_control_key_unset(client: TestClient): assert client.post("/_exit").status_code == status.HTTP_503_SERVICE_UNAVAILABLE +@pytest.mark.parametrize("path", OPERATIONAL_UPDATE_ROUTES) +@pytest.mark.usefixtures("enforce_on") +def test_update_route_without_token_is_401_when_enforced(client: TestClient, path: str): + # With enforcement on, the update-trigger + legacy routes reject a tokenless call. + assert client.post(path).status_code == status.HTTP_401_UNAUTHORIZED + + +@pytest.mark.parametrize("path", OPERATIONAL_UPDATE_ROUTES) +def test_update_route_without_token_is_allowed_by_default(client: TestClient, path: str): + # Rollout default (enforcement off, no flag set here): a tokenless call is not blocked. It may + # 500 on real offline I/O once it reaches the handler, but it must not be the auth 401 - which + # proves the gate allowed it through. + assert client.post(path).status_code != status.HTTP_401_UNAUTHORIZED + + +@pytest.mark.parametrize("path", OPERATIONAL_UPDATE_ROUTES) +def test_update_route_wrong_token_is_allowed_by_default(client: TestClient, path: str): + assert client.post(path, headers=_auth("wrong-token")).status_code != status.HTTP_401_UNAUTHORIZED + + +def test_default_permissive_logs_would_be_rejection(client: TestClient, capture_loguru): + # The escape hatch is observable: each would-be rejection is logged so lagging callers surface. + client.post("/policy-updater/trigger") + assert any("ENFORCE_OPERATIONAL_ROUTE_AUTH is off" in record for record in capture_loguru) + + +def test_warn_if_operational_route_auth_disabled_fires_when_off(monkeypatch, capture_loguru): + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", False) + _warn_if_operational_route_auth_disabled() + assert any("ENFORCE_OPERATIONAL_ROUTE_AUTH is OFF" in record for record in capture_loguru) + + +def test_warn_if_operational_route_auth_disabled_silent_when_on(monkeypatch, capture_loguru): + monkeypatch.setattr(sidecar_config, "ENFORCE_OPERATIONAL_ROUTE_AUTH", True) + _warn_if_operational_route_auth_disabled() + assert not any("ENFORCE_OPERATIONAL_ROUTE_AUTH is OFF" in record for record in capture_loguru) + + def test_warn_if_opal_verifier_disabled_fires(capture_loguru): # The real MockPermitPDP OpalClient runs with no public key -> verifier disabled. _warn_if_opal_verifier_disabled(_sidecar._opal) diff --git a/horizon/tests/test_route_auth_audit.py b/horizon/tests/test_route_auth_audit.py index 0291f5a4..820edd18 100644 --- a/horizon/tests/test_route_auth_audit.py +++ b/horizon/tests/test_route_auth_audit.py @@ -35,6 +35,10 @@ AUTH_GATE_CALLABLES: frozenset[str] = frozenset( { "enforce_pdp_token", + # ENFORCE_OPERATIONAL_ROUTE_AUTH rollout wrapper on the update-trigger and /kong routes. This + # audit is structural: it asserts a gate is attached, not that the gate rejects at runtime + # (that gate warns-and-allows while enforcement is off, the rollout default). + "enforce_pdp_token_operational", "enforce_pdp_control_key", "JWTAuthenticator", "require_listener_token", @@ -102,10 +106,15 @@ def test_no_route_is_unprotected(): @pytest.mark.parametrize("path", sorted(OPAL_TRIGGER_ROUTE_PATHS)) def test_opal_trigger_route_is_pdp_gated(path: str): - """Regression guard for the actual fix: the OPAL-mounted trigger routes require the PDP token.""" + """Regression guard for the actual fix: the OPAL-mounted trigger routes carry the PDP-token gate. + + They are gated with the operational wrapper (enforce_pdp_token_operational) so + ENFORCE_OPERATIONAL_ROUTE_AUTH governs them during a rollout; with the flag on the wrapper enforces + the token identically to enforce_pdp_token. This asserts the gate is attached, not its runtime mode. + """ by_path = {route.path: route for route in _sidecar._app.routes if isinstance(route, APIRoute)} assert path in by_path, f"{path} is no longer mounted (OPAL rename?) - _gate_opal_trigger_routes must be updated" - assert "enforce_pdp_token" in _route_auth_gates(by_path[path]) + assert "enforce_pdp_token_operational" in _route_auth_gates(by_path[path]) def test_audit_detects_a_bare_ungated_route(): From 6b9bc7251c0d6fe21c813cc171c91186c0991039 Mon Sep 17 00:00:00 2001 From: David Shoen Date: Sun, 12 Jul 2026 14:39:06 +0300 Subject: [PATCH 2/5] refactor: consolidate enforce_pdp_token_operational into one try/except (PER-15243) Read ENFORCE_OPERATIONAL_ROUTE_AUTH once and call enforce_pdp_token a single time: honour a rejection by re-raising when the flag is on, downgrade it to warn-and-allow when off. Behaviour is unchanged from the two-branch version. Co-Authored-By: Claude Opus 4.8 (1M context) --- horizon/authentication.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/horizon/authentication.py b/horizon/authentication.py index d3086fa4..900e3ce1 100644 --- a/horizon/authentication.py +++ b/horizon/authentication.py @@ -79,14 +79,14 @@ def enforce_pdp_token_operational(request: Request, credentials: PdpCredentials fail-closed route audit recognises auth gates by callable name - a bare ``enforce_pdp_token`` here could not carry the conditional behaviour, and an inline lambda would be invisible to the audit. """ - if sidecar_config.ENFORCE_OPERATIONAL_ROUTE_AUTH: - enforce_pdp_token(credentials) - return - # Permissive rollout default: reuse enforce_pdp_token's exact reject logic, but downgrade a - # rejection to warn-and-allow so no caller breaks while every would-be rejection is still flagged. + # Reuse enforce_pdp_token's exact reject logic in one place. When the flag is on, a rejection is + # honoured (re-raised). When it is off - the rollout default - the rejection is downgraded to + # warn-and-allow so no caller breaks while every would-be rejection is still flagged. try: enforce_pdp_token(credentials) except HTTPException as exc: + if sidecar_config.ENFORCE_OPERATIONAL_ROUTE_AUTH: + raise logger.warning( "ENFORCE_OPERATIONAL_ROUTE_AUTH is off: allowing {method} {path} unauthenticated - it would " "otherwise be rejected ({detail}). Set ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP " From b9589486dcb6d155b50790544bfe647d09705768 Mon Sep 17 00:00:00 2001 From: David Shoen Date: Sun, 12 Jul 2026 16:50:52 +0300 Subject: [PATCH 3/5] fix: coalesce operational-route warn-and-allow logging to avoid /kong flood (PER-15243) While ENFORCE_OPERATIONAL_ROUTE_AUTH is off (the rollout default), enforce_pdp_token_operational logged one WARNING per would-be-rejected request. /kong is a per-decision endpoint (potentially high QPS), so during the shadow-mode window that floods the synchronous stdout sink and adds a blocking write to the request path. Rewrite the warn-and-allow log to a per-route coalesced counter: at most one line per route per 60s, carrying the number of tokenless requests allowed in that window. Every request is still counted; only the logging is rate-limited. The counter is guarded by a threading.Lock because the gate runs in FastAPI's threadpool. That per-route count also doubles as the rollout signal - watch it fall to zero per route, then flip the flag on. - authentication.py: add _operational_warn_should_emit throttle + reset_operational_warn_throttle. - conftest.py: autouse fixture resets throttle state per test so asserted warnings aren't suppressed. - test_authentication.py: add anti-flood (burst -> one line) and coalesced-count tests. Co-Authored-By: Claude Fable 5 --- horizon/authentication.py | 66 ++++++++++++++++++++++++---- horizon/tests/conftest.py | 14 ++++++ horizon/tests/test_authentication.py | 23 ++++++++++ 3 files changed, 94 insertions(+), 9 deletions(-) diff --git a/horizon/authentication.py b/horizon/authentication.py index 900e3ce1..d5b71fd9 100644 --- a/horizon/authentication.py +++ b/horizon/authentication.py @@ -1,4 +1,6 @@ import hmac +import threading +import time from typing import Annotated from fastapi import Depends, HTTPException, Request, status @@ -66,6 +68,47 @@ def enforce_pdp_token(credentials: PdpCredentials = None): raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail="Invalid PDP token") +# Rate-limit for the warn-and-allow path of enforce_pdp_token_operational. While +# ENFORCE_OPERATIONAL_ROUTE_AUTH is off, every unauthenticated request to a governed route would +# otherwise emit one WARNING - which on the high-QPS /kong decision endpoint floods the synchronous +# stdout log sink (and adds a blocking write to the request path). Instead we coalesce to one line +# per route per interval that reports how many would-be rejections were allowed; that count is also +# the rollout signal - watch it fall to zero, then flip the flag on. The gate runs in a threadpool +# (it is a sync dependency), so the shared counters are guarded by a plain threading.Lock. +_OPERATIONAL_WARN_INTERVAL_SECONDS = 60.0 +_operational_warn_lock = threading.Lock() +# path -> (would-be rejections accumulated since the last emitted line, monotonic time of that emit) +_operational_warn_state: dict[str, tuple[int, float]] = {} + + +def reset_operational_warn_throttle() -> None: + """Clear the throttle state. For tests, whose asserted warnings must not be suppressed by a prior test.""" + with _operational_warn_lock: + _operational_warn_state.clear() + + +def _operational_warn_should_emit(path: str) -> tuple[bool, int]: + """Decide whether to emit the warn-and-allow line for ``path`` now, coalescing bursts. + + Returns ``(emit_now, count_since_last_emit)``. Every call counts as one would-be rejection; a + line is emitted on the first hit for a path and then at most once per + ``_OPERATIONAL_WARN_INTERVAL_SECONDS``, carrying the number of rejections coalesced into it. + """ + now = time.monotonic() + with _operational_warn_lock: + state = _operational_warn_state.get(path) + if state is None: + _operational_warn_state[path] = (0, now) + return True, 1 + pending, last_emit = state + pending += 1 + if now - last_emit >= _OPERATIONAL_WARN_INTERVAL_SECONDS: + _operational_warn_state[path] = (0, now) + return True, pending + _operational_warn_state[path] = (pending, last_emit) + return False, pending + + def enforce_pdp_token_operational(request: Request, credentials: PdpCredentials = None): """PDP-token gate for the operational routes hardened by PER-15244/PER-15245, with a rollout default. @@ -81,20 +124,25 @@ def enforce_pdp_token_operational(request: Request, credentials: PdpCredentials """ # Reuse enforce_pdp_token's exact reject logic in one place. When the flag is on, a rejection is # honoured (re-raised). When it is off - the rollout default - the rejection is downgraded to - # warn-and-allow so no caller breaks while every would-be rejection is still flagged. + # warn-and-allow so no caller breaks; every would-be rejection is still counted, and surfaced in a + # per-route coalesced warning (see _operational_warn_should_emit) rather than one line per request. try: enforce_pdp_token(credentials) except HTTPException as exc: if sidecar_config.ENFORCE_OPERATIONAL_ROUTE_AUTH: raise - logger.warning( - "ENFORCE_OPERATIONAL_ROUTE_AUTH is off: allowing {method} {path} unauthenticated - it would " - "otherwise be rejected ({detail}). Set ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP " - "token on this route.", - method=request.method, - path=request.url.path, - detail=exc.detail, - ) + emit, coalesced = _operational_warn_should_emit(request.url.path) + if emit: + logger.warning( + "ENFORCE_OPERATIONAL_ROUTE_AUTH is off: allowed {count} unauthenticated request(s) to " + "{method} {path} in the last ~{interval}s that would otherwise be rejected (e.g. {detail}). " + "Set ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP token on this route.", + count=coalesced, + method=request.method, + path=request.url.path, + interval=int(_OPERATIONAL_WARN_INTERVAL_SECONDS), + detail=exc.detail, + ) def enforce_pdp_control_key(credentials: PdpCredentials = None): diff --git a/horizon/tests/conftest.py b/horizon/tests/conftest.py index e44935e7..2f1ea1d4 100644 --- a/horizon/tests/conftest.py +++ b/horizon/tests/conftest.py @@ -22,8 +22,22 @@ from unittest.mock import Mock import aioresponses.core as _aioresponses_core +import horizon.authentication as _authentication +import pytest from aiohttp.client_reqrep import ClientResponse as _ClientResponse + +@pytest.fixture(autouse=True) +def _reset_operational_warn_throttle(): + """Give each test a clean warn-and-allow throttle window. + + enforce_pdp_token_operational coalesces its "unauthenticated but allowed" warning to one line per + route per interval, and that state is process-global. Without this reset, a test that asserts the + warning could be silently suppressed by an earlier test that already logged for the same path. + """ + _authentication.reset_operational_warn_throttle() + yield + if "stream_writer" in inspect.signature(_ClientResponse.__init__).parameters: class _StreamWriterCompatClientResponse(_ClientResponse): diff --git a/horizon/tests/test_authentication.py b/horizon/tests/test_authentication.py index 95918683..77798e19 100644 --- a/horizon/tests/test_authentication.py +++ b/horizon/tests/test_authentication.py @@ -7,6 +7,7 @@ public-route allowlist that the route-audit test relies on. """ +import time from types import SimpleNamespace import horizon.authentication as auth @@ -131,6 +132,28 @@ def test_enforce_off_valid_token_passes_without_warning(self, capture_loguru): assert enforce_pdp_token_operational(_fake_request(), credentials=_creds(VALID_TOKEN)) is None assert not any(self.WARN_SUBSTRING in record for record in capture_loguru) + @pytest.mark.usefixtures("enforce_off") + def test_enforce_off_coalesces_repeated_warnings(self, capture_loguru): + # The warn-and-allow path must not log once per request (that floods the hot /kong endpoint): + # a burst of would-be rejections on one route collapses to a single warning line. + for _ in range(50): + assert enforce_pdp_token_operational(_fake_request(path="/kong"), credentials=None) is None + assert sum(self.WARN_SUBSTRING in record for record in capture_loguru) == 1 + + +def test_operational_warn_throttle_coalesces_count(): + # First hit for a path emits immediately (count 1); further hits within the interval are counted + # but suppressed; once the interval elapses the next hit emits carrying the coalesced total. + auth.reset_operational_warn_throttle() + assert auth._operational_warn_should_emit("/kong") == (True, 1) + for _ in range(4): + emit, _count = auth._operational_warn_should_emit("/kong") + assert emit is False + # Simulate the interval having elapsed without waiting on the wall clock. + pending, _last = auth._operational_warn_state["/kong"] + auth._operational_warn_state["/kong"] = (pending, time.monotonic() - (auth._OPERATIONAL_WARN_INTERVAL_SECONDS + 1)) + assert auth._operational_warn_should_emit("/kong") == (True, 5) + @pytest.fixture def capture_loguru(): From 88679bc283ba21534085add1c3e3ce1bae658abb Mon Sep 17 00:00:00 2001 From: David Shoen Date: Sun, 12 Jul 2026 16:53:40 +0300 Subject: [PATCH 4/5] =?UTF-8?q?fix:=20address=20Copilot=20review=20?= =?UTF-8?q?=E2=80=94=20decouple=20logger,=20clarify=20warn-and-allow=20wor?= =?UTF-8?q?ding=20(PER-15243)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - authentication.py: import loguru's logger directly instead of opal_client.logger, matching the rest of horizon (e.g. pdp.py) and avoiding a dependency on opal_client internals for the rollout warnings and test log capture. Same singleton, so behaviour is unchanged. - authentication.py + pdp.py: the warn-and-allow branch also fires for an *invalid* token, not only a missing one, so reword "unauthenticated" -> "without a valid PDP token (missing or invalid)" in both the per-request and the startup warning for accurate log search/alerting. Co-Authored-By: Claude Fable 5 --- horizon/authentication.py | 9 +++++---- horizon/pdp.py | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/horizon/authentication.py b/horizon/authentication.py index d5b71fd9..5ad853b2 100644 --- a/horizon/authentication.py +++ b/horizon/authentication.py @@ -5,7 +5,7 @@ from fastapi import Depends, HTTPException, Request, status from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer -from opal_client.logger import logger +from loguru import logger from horizon.config import MOCK_API_KEY, sidecar_config from horizon.startup.api_keys import get_env_api_key @@ -134,9 +134,10 @@ def enforce_pdp_token_operational(request: Request, credentials: PdpCredentials emit, coalesced = _operational_warn_should_emit(request.url.path) if emit: logger.warning( - "ENFORCE_OPERATIONAL_ROUTE_AUTH is off: allowed {count} unauthenticated request(s) to " - "{method} {path} in the last ~{interval}s that would otherwise be rejected (e.g. {detail}). " - "Set ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP token on this route.", + "ENFORCE_OPERATIONAL_ROUTE_AUTH is off: allowed {count} request(s) without a valid PDP token " + "(missing or invalid) to {method} {path} in the last ~{interval}s that would otherwise be " + "rejected (e.g. {detail}). Set ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP token " + "on this route.", count=coalesced, method=request.method, path=request.url.path, diff --git a/horizon/pdp.py b/horizon/pdp.py index 01f7b2d6..c7225ab4 100644 --- a/horizon/pdp.py +++ b/horizon/pdp.py @@ -158,9 +158,9 @@ def _warn_if_operational_route_auth_disabled() -> None: if not sidecar_config.ENFORCE_OPERATIONAL_ROUTE_AUTH: logger.warning( "ENFORCE_OPERATIONAL_ROUTE_AUTH is OFF: the update-trigger routes (/policy-updater/trigger, " - "/data-updater/trigger, /update_policy, /update_policy_data) and /kong accept UNAUTHENTICATED " - "requests - would-be rejections are only logged. This is the safe fleet-rollout default; set " - "ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP token on these routes." + "/data-updater/trigger, /update_policy, /update_policy_data) and /kong accept requests WITHOUT " + "a valid PDP token (missing or invalid) - would-be rejections are only logged. This is the safe " + "fleet-rollout default; set ENFORCE_OPERATIONAL_ROUTE_AUTH=true to enforce the PDP token on these routes." ) From baee76498eb53f8d5f68eb2177f635e5df02307e Mon Sep 17 00:00:00 2001 From: David Shoen Date: Mon, 13 Jul 2026 11:25:54 +0300 Subject: [PATCH 5/5] style: satisfy ruff-format on conftest.py (PER-15243) The autouse throttle-reset fixture sits directly above a module-level `if`; ruff-format (the pre-commit hook, distinct from ruff check) requires two blank lines there. Whitespace only. Co-Authored-By: Claude Fable 5 --- horizon/tests/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/horizon/tests/conftest.py b/horizon/tests/conftest.py index 2f1ea1d4..6dc5e3e2 100644 --- a/horizon/tests/conftest.py +++ b/horizon/tests/conftest.py @@ -38,6 +38,7 @@ def _reset_operational_warn_throttle(): _authentication.reset_operational_warn_throttle() yield + if "stream_writer" in inspect.signature(_ClientResponse.__init__).parameters: class _StreamWriterCompatClientResponse(_ClientResponse):