feat: default-deny authentication middleware (PER-15245)

[dshoen619]

What & why

Part of the PDP Unauthenticated Operational Endpoints — Authorization Hardening project. Implements PER-15245 — the load-bearing structural fix.

The PDP uses an allowlist-of-protected model: every route has to individually carry Depends(enforce_pdp_token). Two problems:

1. The OPAL trigger routers (POST /policy-updater/trigger, POST /data-updater/trigger) are mounted by OpalClient before the PDP wrapper gets control, so an include_router-level dependency can never reach them — they were reachable unauthenticated.
2. Any new route that forgets the dependency fails open, silently.

This PR closes (1) directly, moves the enforcer routes to router-level gating, hardens the token check, and closes (2) with a CI-enforced fail-closed audit so the allowlist becomes a real boundary rather than a convention everyone has to remember.

Note: the original approach here was a global ASGI default-deny middleware; that was replaced with the FastAPI-native design below (include_router dependencies + targeted injection + a route audit), which reuses FastAPI's own security machinery and OpenAPI integration instead of hand-rolling a parallel one.

Design

1. Token dependencies (horizon/authentication.py) now build on FastAPI's HTTPBearer(auto_error=False):

• Header parsing / OpenAPI "Authorize" support is delegated to HTTPBearer; auto_error=False lets us return the PDP's documented 401 (not HTTPBearer's default 403) and lets enforce_pdp_control_key's "control API disabled" 503 take precedence over the header check.
• Missing / malformed / non-bearer header → 401, never a 500 (replaces the old unguarded authorization.split(" ")).
• Token compared with hmac.compare_digest on utf-8 bytes (constant-time; safe for non-ASCII).

2. Router-level gating (horizon/pdp.py, horizon/enforcer/api.py):

• /health is split into its own router mounted without auth (k8s/LB liveness probes can't attach the PDP token).
• The enforcer router is now included with dependencies=[Depends(enforce_pdp_token)] at the include_router level, so auth runs before per-route deps like notify_seen_sdk — every enforcer/facts/local/proxy/system-owned route is gated. /healthchecks/opa/* correctly stays gated (no /health prefix trap).

3. OPAL-mounted trigger routes (_gate_opal_trigger_routes): the two trigger routes are mounted by OpalClient before the PDP takes over, so we inject Depends(enforce_pdp_token) into the already-mounted route objects directly (mirroring what FastAPI does at APIRoute.__init__). Fails loud (SystemExit) if a target route is missing — e.g. an OPAL upgrade renamed it — rather than silently shipping an unauthenticated trigger endpoint.

4. OPAL-native (tier-2) routes: /policy-store, /callbacks, /opal-server are authenticated by OPAL's own JWT verifier. _warn_if_opal_verifier_disabled logs a high-signal warning when OPAL_AUTH_PUBLIC_KEY is unset (verifier disabled → those routes effectively open). Expected in local/dev; a managed PDP ships a baked-in public key so the verifier is enabled by default.

5. Fail-closed route audit (horizon/tests/test_route_auth_audit.py) — the structural guarantee. Builds the real app and asserts every mounted route is either explicitly public (in PUBLIC_ROUTE_PATHS, the single source of truth) or carries a recognised auth gate (enforce_pdp_token / enforce_pdp_control_key, or OPAL's JWTAuthenticator / require_listener_token). Gates are matched by dependency presence, not path, so an OPAL rename can't slip past and an unrecognised mechanism also fails the audit. A new router, a bare @app.get, or a mounted sub-app added without a dependency now fails CI instead of shipping open.

Wired in at the end of PermitPDP._configure_api_routes so both production and the test app pick it up.

Tests

• test_route_auth_audit.py — the fail-closed guarantee: no non-public route is ungated; a negative test proves the audit isn't vacuous; a mounted sub-app is flagged; regression guard that both OPAL trigger routes require the PDP token.
• test_opal_trigger_auth.py — integration against the real app: unauth trigger routes → 401; wrong token → 401; malformed header → 401 not 500; /health public; /healthchecks/opa/* stays gated; /version gated; /_exit → 503 when control key unset; verifier-disabled warning fires/stays silent appropriately.
• test_authentication.py — unit tests for the constant-time compare (incl. non-ASCII), the 401/503 contract, and the public-route allowlist.
• test_enforcer_api.py — every protected enforcer endpoint returns 401 without/with a bad token; /health public; /kong + /nginx_allowed + /authorized_users happy-path with a valid token.

✅ 112 tests pass (106 existing/updated + 6 new audit tests). ruff check + ruff format clean.

Notes

• The /policy-store/config secret-leak and warn-vs-fail-closed behaviour of the OPAL verifier (when OPAL_AUTH_PUBLIC_KEY is cleared by an override) are pre-existing OPAL behaviours; hardening them to fail-closed for managed PDPs is a tracked follow-up.
• Branch is based on main. Overlap with PER-15244 PR fix: gate /kong, split /health public, enforce PDP token at router level (PER-15244) #317 is limited to pdp.py:_configure_api_routes — trivial rebase if that merges first.
• Also bundled: image CVE cleanup (sqlite-libs removal, cryptography/starlette pins + VEX waivers) and the pdp-tester CI migration from k3d to the Docker runtime — see PER-15358.

🤖 Generated with Claude Code

[linear-code]

PER-15245

[github-actions]

🔍 Vulnerabilities of permitio/pdp-v2:next

📦 Image Reference permitio/pdp-v2:next

digest	sha256:66dfe2b1b65621365bd8244bcd11a6b4509032eabea80d75b033610638349115
vulnerabilities
platform	linux/amd64
size	218 MB
packages	247

📦 Base Image python:3.10-alpine3.22

also known as	3.10.20-alpine3.22
digest	sha256:c8f94b3bb77e6ea9015ccd091b7f8aec1b1fcbca95159675235d9a93788797cd
vulnerabilities

starlette 0.50.0 (pypi) pkg:pypi/starlette@0.50.0 Allocation of Resources Without Limits or Throttling Affected range >=0.4.1 <1.3.1 Fixed version 1.3.1 CVSS Score 7.5 CVSS Vector CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H EPSS Score 0.275% EPSS Percentile 19th percentile Description Summary request.form() accepts max_fields and max_part_size to bound resource consumption while parsing form data. These limits are enforced for multipart/form-data, but silently ignored for application/x-www-form-urlencoded. An unauthenticated attacker can therefore send a urlencoded body with an arbitrarily large number of fields or an arbitrarily large field, even when the application configured limits it believed would apply. Details request.form() dispatches to a different parser depending on the Content-Type. For multipart/form-data the max_files, max_fields, and max_part_size limits are forwarded to the parser, but for application/x-www-form-urlencoded the parser is constructed without them. It has no max_fields or max_part_size parameter to receive them, and it appends every field with no count check and accumulates each field's name and value with no size check. The configured limits are therefore both unreachable and unenforced for url-encoded bodies. Because the url-encoded parser does its work synchronously between stream reads, the two attack shapes have different effects: Field count drives CPU and event-loop blocking. A body of ~1,000,000 fields (a sub-10MB payload such as f0=v&f1=v&...) blocks the worker's event loop for several seconds while parsing, during which the worker serves no other request. Field size drives memory. A single large field value (e.g. a 50MB value) is buffered in full to build the FormData, forcing memory allocation proportional to the request body. The equivalent multipart/form-data request is correctly rejected with 400 Too many fields / 400 Field exceeded maximum size. Impact This Denial of service (DoS) vulnerability affects all applications built with Starlette (or FastAPI) that call request.form() on application/x-www-form-urlencoded requests. A single request with a very large number of fields blocks the event loop for several seconds, and a single request with a very large field forces unbounded memory allocation; in either case, parallel requests can render the service unusable. A reverse proxy that enforces a request body size limit reduces but does not eliminate the exposure, since a sub-10MB body is already enough to block the event loop. Mitigation Upgrade to a patched version, which forwards max_fields and max_part_size to the url-encoded parser and enforces them while parsing, raising before the oversized field or excess fields are accumulated. The defaults match multipart/form-data (max_fields=1000, max_part_size=1MB) and can be customized via request.form(max_fields=..., max_part_size=...). Server-Side Request Forgery (SSRF) Affected range <1.1.0 Fixed version 1.1.0 CVSS Score 7.5 CVSS Vector CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N EPSS Score 0.368% EPSS Percentile 29th percentile Description Summary When serving static files on Windows, StaticFiles resolves the requested path with os.path.realpath. If a UNC path (such as \\attacker.com\share) reaches the resolver, realpath causes the process to open a connection to the remote host over SMB (port 445). This is a server-side request forgery (SSRF) that leaks the service account's NTLMv2 credentials to the attacker-controlled host, which can then be cracked offline or relayed to other hosts. Details StaticFiles.lookup_path() joins the requested path onto the served directory and calls os.path.realpath on the result before checking containment with os.path.commonpath. On Windows, a UNC path is absolute, so os.path.join discards the served directory and realpath resolves the bare UNC path, triggering the outbound SMB connection and NTLM authentication before the containment check rejects the path. The HTTP response is a benign 404, but the credential disclosure has already happened. POSIX systems are not affected. This only affects the default configuration (follow_symlink=False), which uses os.path.realpath. The follow_symlink=True branch uses os.path.abspath, which performs no I/O. Impact Applications running on Windows that serve files with StaticFiles (directly, or via a framework built on Starlette such as FastAPI) in the default configuration are affected. StaticFiles is typically unauthenticated, so any client can trigger the SMB connection and leak the service account's NTLMv2 hash. A secondary impact is discovering internal hosts reachable over SMB by timing responses for valid versus invalid addresses. Mitigation Applications not running on Windows are not affected. On Windows, serving static files through a dedicated web server (such as nginx or IIS) instead of StaticFiles avoids the issue. Blocking outbound SMB (port 445) from the application host prevents the credential disclosure even if a UNC path is resolved. Improper Validation of Unsafe Equivalence in Input Affected range <=1.0.0 Fixed version 1.0.1 CVSS Score 6.5 CVSS Vector CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:L/A:N EPSS Score 1.438% EPSS Percentile 70th percentile Description Summary In affected versions, the HTTP Host request header was not validated before being used to reconstruct request.url. Because the routing algorithm relies on the raw HTTP path while request.url is rebuilt from the Host header, a malformed header could make request.url.path differ from the path that was actually requested. Middleware and endpoints that apply security restrictions based on request.url (rather than the raw scope path) could therefore be bypassed. Details When a client requests http://example.com/foo, it sends: GET /foo HTTP/1.1 Host: example.com Affected versions reconstructed the URL by concatenating http://{host}{path} and re-parsing the result. The Host value is only valid as a uri-host [ ":" port ] per RFC 9112 §3.2, where uri-host follows the restricted host grammar of RFC 3986 §3.2.2. When it contains characters outside that grammar - notably /, ?, or # - those characters move the path/query/fragment boundaries during re-parsing, so the parsed request.url.path no longer matches the path the server actually received. For example: GET /foo HTTP/1.1 Host: example.com/abc?bar= reconstructs to http://example.com/abc?bar=/foo, whose parsed path is /abc - even though routing used the real path /foo. The router still dispatches to /foo and the endpoint executes, but any middleware or code that reads request.url.path sees /abc, so path-based authorization checks can be bypassed. Impact Any application running an affected version that relies on request.url (or request.url.path) for security-sensitive decisions is affected. The most common case is middleware that gates access to certain path prefixes based on request.url.path. Deployments fronted by a proxy or load balancer are mitigated only if that proxy rejects or normalizes the malformed Host header before forwarding and the application does not trust attacker-controlled host headers (e.g. X-Forwarded-Host) elsewhere. Mitigation Upgrade to a patched version, which validates the Host header against the grammar of RFC 9112 §3.2 / RFC 3986 §3.2.2 when constructing request.url and falls back to scope["server"] for malformed values. Use of Externally-Controlled Input to Select Classes or Code ('Unsafe Reflection') Affected range <1.1.0 Fixed version 1.1.0 CVSS Score 5.3 CVSS Vector CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:N EPSS Score 0.213% EPSS Percentile 12th percentile Description Summary When dispatching a request, HTTPEndpoint selects the handler by lowercasing the HTTP method and looking it up as an attribute with getattr, without restricting the lookup to a known set of HTTP verbs. When an HTTPEndpoint subclass is registered through Route(...) without an explicit methods= argument, the route does not constrain the method and every method reaches the endpoint. If a non-standard HTTP method whose lowercased name matches an attribute on the endpoint subclass reaches the endpoint, that attribute is invoked as if it were a request handler. An attacker can use this to reach methods that were never meant to be HTTP handlers, such as internal helpers, without the authorization checks applied by the intended public handler. Details HTTPEndpoint uses the client-supplied method name to resolve an instance attribute, without validating it against the set of HTTP verbs the endpoint supports. A method such as _DO_DELETE therefore resolves an attribute like _do_delete and invokes it. Non-standard methods are valid RFC 9110 token methods, so an endpoint must not treat the method name as a trusted attribute selector. Impact An application is affected when all of the following hold: It defines an HTTPEndpoint subclass and registers it via Route(...) without an explicit methods= argument. The subclass defines additional methods whose names match a non-standard HTTP-method token shape and that accept a single request argument and return a response. This also affects frameworks built on Starlette, like FastAPI. Mitigation Register HTTPEndpoint subclasses with an explicit methods= argument on the Route, listing only the HTTP verbs the endpoint supports. The route then rejects any other method with 405 Method Not Allowed before it reaches the endpoint, so non-standard methods cannot resolve an attribute. Improper Input Validation Affected range <1.3.0 Fixed version 1.3.0 CVSS Score 3.7 CVSS Vector CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:L/A:N EPSS Score 0.187% EPSS Percentile 8th percentile Description Summary In affected versions, the HTTP request path is not validated before being used to reconstruct request.url. Because request.url is rebuilt by concatenating {scheme}://{host}{path} and re-parsing the result, a path that does not begin with / (for example @<!-- -->google.com) moves the authority boundary during re-parsing, so request.url.hostname and request.url.netloc become attacker-controlled. Code that reads request.url.hostname (rather than the Host header or scope) can therefore be misled into trusting an attacker-supplied host. Details When a client requests a path that does not start with /: GET @<!-- -->google.com HTTP/1.1 Host: localhost affected versions reconstruct the URL as http://localhost@<!-- -->google.com. Per RFC 3986 §3.2.1, the substring before @ in the authority is userinfo, so re-parsing yields username = "localhost" and hostname = "google.com", with an empty path: request.url == "http://localhost@<!-- -->google.com" request.url.hostname == "google.com" request.url.path == "" The root cause is that the path is concatenated directly after the host without a separating /, and without validating that it begins with one. Only the Host header was validated when constructing request.url; the path was not. This requires an ASGI server that forwards a request-target lacking a leading / into scope["path"]. Impact Any application running an affected version that uses request.url, request.url.netloc, or request.url.hostname for a security-sensitive decision (host-based authorization, redirect/callback base, SSRF target, cache key, audit log) may be affected, when no fronting proxy or load balancer rejects the malformed request-target first. Note that this is less exploitable than GHSA-86qp-5c8j-p5mr: there, the poison is carried in the Host header, so the real path still routes to a valid endpoint while request.url.path lies. Here, the poison must be carried in the path itself, and that path (@<!-- -->google.com) does not match any registered route, so routing returns 404 and no endpoint handler runs. The exposure is limited to code that reads request.url before routing - notably middleware - or in 404/exception handlers. Mitigation Upgrade to a patched version, which prevents the request path from crossing into the URL authority. The request above instead yields http://localhost/@<!-- -->google.com with request.url.hostname == "localhost".

oras.land/oras-go/v2 2.6.1 (golang) pkg:golang/oras.land/oras-go/v2@2.6.1 Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') Affected range <=2.6.1 Fixed version Not Fixed CVSS Score 7.1 CVSS Vector CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:L/A:N Description Root cause The tar-extraction helper ensureLinkPath at content/file/utils.go:262-275 validates that a hardlink's target resolves inside the extract base, but then returns the original unresolved target string back to the caller: func ensureLinkPath(baseAbs, baseRel, link, target string) (string, error) { path := target if !filepath.IsAbs(target) { path = filepath.Join(filepath.Dir(link), target) // resolved FOR VALIDATION } if _, err := resolveRelToBase(baseAbs, baseRel, path); err != nil { return "", err } return target, nil // <-- returns the ORIGINAL target, not the validated path } The caller for TypeLink hardlinks then does: case tar.TypeLink: var target string if target, err = ensureLinkPath(dirPath, dirName, filePath, header.Linkname); err == nil { err = os.Link(target, filePath) } os.Link(oldname, newname) wraps the link(2) system call. From the link(2) man page: oldpath and newpath are interpreted relative to the current working directory of the calling process. So when target (i.e., header.Linkname) is a relative path, os.Link resolves it against the process's current working directory, not against filepath.Dir(link) as the validation assumed. Attack An attacker who controls an OCI-compliant registry (or any artifact source the victim consumes via oras pull) crafts a tarball layer with: A regular file: payload.tar.gz/README.txt. A hardlink entry: Typeflag=TypeLink, Name=payload.tar.gz/evil_cwd_link, Linkname="victim.secret" (relative). and marks the layer descriptor with io.deis.oras.content.unpack: "true" (a standard annotation that tells oras-go to auto-extract). When a victim runs oras pull (or any Go code using content.File), the extraction: Validates payload.tar.gz/evil_cwd_link — passes. Calls ensureLinkPath(dirPath, "payload.tar.gz", filePath, "victim.secret"): path = filepath.Join(filepath.Dir(filePath), "victim.secret") = <extract_base>/payload.tar.gz/victim.secret → inside base → validation passes. Returns target = "victim.secret" (NOT path). Calls os.Link("victim.secret", "<extract_base>/payload.tar.gz/evil_cwd_link"). link(2) resolves relative oldname="victim.secret" against process CWD → creates a hardlink inside the extract tree pointing to <invoker_CWD>/victim.secret. The resulting hardlink and the CWD file share an inode — reading one reads the other; writing to one writes to the other. Proof of Concept Tested on Ubuntu 24.04.4 LTS with oras CLI v1.3.0 (SHA-256 040e140304b7dbdd9b40dacd798e2303cea44ad84eeb210750afdf15f1dcf8b4, downloaded from https://github.com/oras-project/oras/releases/download/v1.3.0/oras_1.3.0_linux_amd64.tar.gz). Reproduction script (standalone, ~50 lines) attached. Summary of key steps: # 1. Place victim file in the future CWD. mkdir -p cwd-space extract echo "TOP SECRET FROM CWD" > cwd-space/victim.secret # 2. Craft malicious tarball with a TypeLink entry whose Linkname is RELATIVE. python3 -c ' import tarfile, io, os with tarfile.open("cwd-space/payload.tar.gz", "w:gz", format=tarfile.GNU_FORMAT) as t: info = tarfile.TarInfo(name="payload.tar.gz/README.txt") c = b"pulled from registry"; info.size = len(c); info.mode = 0o644 info.uid = os.getuid(); info.gid = os.getgid() t.addfile(info, io.BytesIO(c)) link = tarfile.TarInfo(name="payload.tar.gz/evil_cwd_link") link.type = tarfile.LNKTYPE link.linkname = "victim.secret" # RELATIVE link.mode = 0o644; link.uid = os.getuid(); link.gid = os.getgid() t.addfile(link) ' # 3. Push to OCI layout, patch in the unpack annotation, pull from cwd-space. (cd cwd-space && oras push --oci-layout ../layout:v1 \ payload.tar.gz:application/vnd.oci.image.layer.v1.tar+gzip) # ... patch layout/blobs/sha256/<manifest> to add # io.deis.oras.content.unpack: "true" on layers[0].annotations ... (cd cwd-space && oras pull --oci-layout ../layout:v1 --output ../extract) # 4. Observe inode sharing. stat -c '%i' extract/payload.tar.gz/evil_cwd_link # → 6554160 stat -c '%i' cwd-space/victim.secret # → 6554160 (SAME) cat extract/payload.tar.gz/evil_cwd_link # → "TOP SECRET FROM CWD" Observed output: evil_cwd_link (inside extract dir): inode=6554160 victim.secret (in invoker CWD): inode=6554160 *** ESCAPE CONFIRMED *** Reading through the extract-dir hardlink yields the CWD file contents: TOP SECRET FROM CWD A library-level regression test is also provided (poc_test.go) that drops into content/file/utils_test.go and runs via go test ./content/file/... -run TestPoC — output shows identical inode match for consumers of the library API. Impact Primary: arbitrary-CWD-file read primitive. An attacker-controlled OCI artifact, when pulled by a victim using the oras CLI or any Go program using oras-go/v2/content/file, can create a hardlink inside the victim's extract tree pointing to an arbitrary file in the victim's process CWD (that the invoker UID is permitted to read). Reading the extract-tree hardlink yields that file's contents verbatim. Secondary: inode-sharing tampering primitive. Any tool that later modifies the extract-tree hardlink (write, chmod, truncate, etc.) modifies the CWD file through the shared inode. This violates the "writes inside the extract dir are confined" invariant that downstream tooling (CI systems, container-image builders, artifact scanners) typically depends on. High-severity chains: CI pipelines where oras pull runs from a project workspace containing secrets/credentials (.env, .git/config, service-account tokens). The pulled artifact can hardlink those secrets into a location later archived/mounted/published. Container orchestration where the extract dir is bind-mounted into a lower-trust container while the pull-invoker's CWD is higher-trust. Hardlinks created in the extract tree expose invoker-CWD files across the trust boundary. Kubernetes operators / Flux source-controller using oras-go to fetch artifacts; their CWD is typically / or /root — very sensitive. Multi-tenant registry proxies that use oras-go to fetch and re-serve artifacts; each proxy process has a CWD with configuration, keys, or per-tenant state. Not affected: oras push (tarball creation side): tarDirectory in the same file explicitly skips hardlink generation (line 65 comment: "We don't support hard links and treat it as regular files"), so pushed content cannot trigger this on the server. Symlink extraction path (TypeSymlink): os.Symlink stores the target string verbatim and does not CWD-resolve at creation time. The current ensureLinkPath return-of-target is correct for symlinks (the existing validation correctly models the symlink-follow path). Attack-surface boundary (fs.protected_hardlinks) On Linux with fs.protected_hardlinks=1 (default on modern distros), link(2) additionally requires the linking user to have READ + WRITE permission on the source file (per may_linkat() in the kernel). Verified on Ubuntu 24.04: as non-root, ln /etc/passwd /tmp/x returns EPERM, and the same via the oras PoC path returns link passwd /tmp/.../evil_passwd: operation not permitted. So the attacker cannot use this bug to read arbitrary root-owned files (e.g., /etc/shadow) when the victim invokes oras pull as a regular user. The attack surface depends on the invocation context: Invocation context Reachable file classes oras pull run by a regular user Any file the user OWNS or has write access to in the process CWD: .env, .git/config, .aws/credentials, ~/.ssh/config, project-local secrets, CI workspace files. oras pull run as root (systemd without User=, container entrypoint default root, Kubernetes operator) Every file on the host filesystem. /etc/shadow, /root/.ssh/id_rsa, bind-mounted host paths, service private keys. The user-context attack surface alone is sufficient for supply-chain-grade impact: CI pipelines and developer machines routinely hold API keys, signing keys, and cloud credentials in user-owned files in the working directory. The root-context escalation makes the bug Critical in mainstream Kubernetes/GitOps tooling where oras-go is adopted for artifact distribution. Proposed fix Change ensureLinkPath to expose both the verbatim target (for symlinks) and the resolved absolute path (for hardlinks); have the TypeLink case use the resolved path. // Current behavior preserved for TypeSymlink. TypeLink switches to the resolved // path to avoid CWD-resolution mismatch at os.Link time. func ensureLinkPath(baseAbs, baseRel, link, target string) (symlinkTarget, hardlinkPath string, err error) { path := target if !filepath.IsAbs(target) { path = filepath.Join(filepath.Dir(link), target) } if _, err = resolveRelToBase(baseAbs, baseRel, path); err != nil { return "", "", err } return target, path, nil } case tar.TypeLink: var absTarget string if _, absTarget, err = ensureLinkPath(dirPath, dirName, filePath, header.Linkname); err == nil { err = os.Link(absTarget, filePath) } case tar.TypeSymlink: var symTarget string symTarget, _, err = ensureLinkPath(dirPath, dirName, filePath, header.Linkname) if err != nil { return err } if err = os.Symlink(symTarget, filePath); err != nil { ... } Regression test to add: Extend Test_extractTarDirectory_HardLink with a third sub-test that: Creates a sentinel file in the test's t.TempDir() (or an explicitly os.Chdir-entered directory) with a known name, e.g. sentinel.txt. Builds a tarball containing a TypeLink entry with Linkname: "sentinel.txt" (relative). Extracts. Asserts either extractTarDirectory returned an error, OR the resulting hardlink's inode does NOT match the sentinel's inode.

go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp 1.42.0 (golang) pkg:golang/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp@1.42.0 Memory Allocation with Excessive Size Value Affected range <1.43.0 Fixed version 1.43.0 CVSS Score 5.3 CVSS Vector CVSS:3.1/AV:A/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:H EPSS Score 0.190% EPSS Percentile 9th percentile Description overview: this report shows that the otlp HTTP exporters (traces/metrics/logs) read the full HTTP response body into an in-memory bytes.Buffer without a size cap. this is exploitable for memory exhaustion when the configured collector endpoint is attacker-controlled (or a network attacker can mitm the exporter connection). severity HIGH not claiming: this is a remote dos against every default deployment. claiming: if the exporter sends traces to an untrusted collector endpoint (or over a network segment where mitm is realistic), that endpoint can crash the process via a large response body. callsite (pinned): exporters/otlp/otlptrace/otlptracehttp/client.go:199 exporters/otlp/otlptrace/otlptracehttp/client.go:230 exporters/otlp/otlpmetric/otlpmetrichttp/client.go:170 exporters/otlp/otlpmetric/otlpmetrichttp/client.go:201 exporters/otlp/otlplog/otlploghttp/client.go:190 exporters/otlp/otlplog/otlploghttp/client.go:221 permalinks (pinned): https://github.com/open-telemetry/opentelemetry-go/blob/248da958375e4dfb4a1105645107be3ef04b1c59/exporters/otlp/otlptrace/otlptracehttp/client.go#L199 https://github.com/open-telemetry/opentelemetry-go/blob/248da958375e4dfb4a1105645107be3ef04b1c59/exporters/otlp/otlptrace/otlptracehttp/client.go#L230 https://github.com/open-telemetry/opentelemetry-go/blob/248da958375e4dfb4a1105645107be3ef04b1c59/exporters/otlp/otlpmetric/otlpmetrichttp/client.go#L170 https://github.com/open-telemetry/opentelemetry-go/blob/248da958375e4dfb4a1105645107be3ef04b1c59/exporters/otlp/otlpmetric/otlpmetrichttp/client.go#L201 https://github.com/open-telemetry/opentelemetry-go/blob/248da958375e4dfb4a1105645107be3ef04b1c59/exporters/otlp/otlplog/otlploghttp/client.go#L190 https://github.com/open-telemetry/opentelemetry-go/blob/248da958375e4dfb4a1105645107be3ef04b1c59/exporters/otlp/otlplog/otlploghttp/client.go#L221 root cause: each exporter client reads resp.Body using io.Copy(&respData, resp.Body) into a bytes.Buffer on both success and error paths, with no upper bound. impact: a malicious collector can force large transient heap allocations during export (peak memory scales with attacker-chosen response size) and can potentially crash the instrumented process (oom). affected component: go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp repro (local-only): unzip poc.zip -d poc cd poc make canonical resp_bytes=33554432 chunk_delay_ms=0 expected output contains: [CALLSITE_HIT]: otlptracehttp.UploadTraces::io.Copy(resp.Body) [PROOF_MARKER]: resp_bytes=33554432 peak_alloc_bytes=118050512 control (same env, patched target): unzip poc.zip -d poc cd poc make control resp_bytes=33554432 chunk_delay_ms=0 expected control output contains: [CALLSITE_HIT]: otlptracehttp.UploadTraces::io.Copy(resp.Body) [NC_MARKER]: resp_bytes=33554432 peak_alloc_bytes=512232 attachments: poc.zip (attached) PR_DESCRIPTION.md attack_scenario.md poc.zip Fixed in: open-telemetry/opentelemetry-go#8108

busybox 1.37.0-r20 (apk) pkg:apk/alpine/busybox@1.37.0-r20?os_name=alpine&os_version=3.22 Affected range <=1.37.0-r20 Fixed version Not Fixed EPSS Score 0.285% EPSS Percentile 20th percentile Description

util-linux 2.41-r9 (apk) pkg:apk/alpine/util-linux@2.41-r9?os_name=alpine&os_version=3.22 Affected range <=2.41-r9 Fixed version Not Fixed EPSS Score 0.118% EPSS Percentile 2nd percentile Description

sqlparse 0.5.0 (pypi) pkg:pypi/sqlparse@0.5.0 Allocation of Resources Without Limits or Throttling Affected range <=0.5.3 Fixed version 0.5.4 CVSS Score 6.9 CVSS Vector CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:L/VA:N/SC:N/SI:N/SA:N Description Summary The below gist hangs while attempting to format a long list of tuples. This was found while drafting a regression test for Dja ngo 5.2's composite primary key feature, which allows querying composite fields with tuples.

golang.org/x/crypto 0.53.0 (golang) pkg:golang/golang.org/x/crypto@0.53.0 Affected range >=0 Fixed version Not Fixed Description The golang.org/x/crypto/openpgp package is unsafe by design, has numerous known security issues, is not maintained, and should not be used. If you are required to interoperate with OpenPGP systems and need a maintained package, consider github.com/ProtonMail/go-crypto/openpgp which is a maintained fork that aims to be a drop-in replacement for this package.

[github-actions]

🔍 Vulnerabilities of permitio/pdp-v2:next

📦 Image Reference permitio/pdp-v2:next

digest	sha256:66dfe2b1b65621365bd8244bcd11a6b4509032eabea80d75b033610638349115
vulnerabilities
platform	linux/amd64
size	218 MB
packages	247

📦 Base Image python:3.10-alpine3.22

also known as	3.10.20-alpine3.22
digest	sha256:c8f94b3bb77e6ea9015ccd091b7f8aec1b1fcbca95159675235d9a93788797cd
vulnerabilities

[Copilot]

Pull request overview

Implements a default-deny, pure-ASGI authentication middleware to ensure all HTTP routes (including OPAL-mounted trigger routes) require a valid PDP token unless explicitly allowlisted or deferred, with an audit/enforce rollout switch via config.

Changes:

• Added DefaultDenyAuthMiddleware with tiered allowlisting, defensive Authorization parsing, and audit|enforce behavior.
• Wired the middleware into PermitPDP._configure_api_routes so it wraps the full app (including OPAL trigger routers).
• Added config support (PDP_AUTH_ENFORCEMENT) and a new integration-style test suite exercising both audit and enforce modes.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
horizon/middleware/default_deny.py	New default-deny ASGI middleware (allowlist + token validation + audit/enforce behavior).
horizon/pdp.py	Installs the middleware at the end of app route configuration so it wraps all routes.
horizon/config.py	Adds AuthEnforcement enum and AUTH_ENFORCEMENT sidecar config (prefixed as PDP_AUTH_ENFORCEMENT).
horizon/tests/test_default_deny_middleware.py	New tests validating middleware behavior against a real app build (including OPAL trigger routes).
horizon/middleware/init.py	Package init file (no functional logic).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dshoen619]

Addressed in 65a95db. The token check is now wrapped so any comparison/resolution error fails closed to a 401 (never a 500). SystemExit is a BaseException and is deliberately not caught — it signals a fatally-misconfigured PDP that should down the worker rather than be masked as a 401, which matches the existing enforce_pdp_token behavior. In practice the key is resolved and cached at startup (PermitPDP.__init__), so this path is not reachable during request handling.

[dshoen619]

Fixed in 65a95db. The middleware now bypasses only a genuine CORS preflight — OPTIONS carrying Access-Control-Request-Method (exactly what Starlette's CORSMiddleware treats as preflight). A bare/non-preflight OPTIONS to a protected route is now gated, so a future custom OPTIONS handler can't be reached unauthenticated. Covered by test_enforce_cors_preflight_bypassed + test_enforce_non_preflight_options_is_gated.

[dshoen619]

Fixed in 65a95db. This now asserts not _is_middleware_401(resp) (a helper checking 401 + WWW-Authenticate: Bearer + our detail) instead of == 200, so it proves the middleware deferred without coupling to OPAL's current no-verifier policy-store behavior.

[dshoen619]

Fixed in 65a95db — now asserts not _is_middleware_401(resp) instead of == 200. The real assertions (would-block logged with path/method/has_valid_pdp_token) are unchanged.

[dshoen619]

Fixed in 65a95db — status check relaxed to not _is_middleware_401(resp); the point of the test (no would-block log emitted when the token is valid) is unchanged.

[dshoen619]

Fixed in 65a95db — now not _is_middleware_401(resp). The key assertions (a would-block WAS logged, but the token value never appears in the log) are unchanged.

[dshoen619]

Fixed in 65a95db — now not _is_middleware_401(resp). The claim under test (a loguru-hostile User-Agent does not drop the audit line) is unchanged.

[zeevmoney]

Automated review — PER-15245 (default-deny authentication middleware) [load-bearing]

What this PR does: Adds a pure-ASGI DefaultDenyAuthMiddleware (horizon/middleware/default_deny.py), wired in PermitPDP._configure_api_routes via install_default_deny(app) as the last-added (outermost) middleware, so it sees every route — including the /policy-updater/trigger / /data-updater/trigger routers OPAL mounts before the PDP wrapper runs, which were never gated. It inverts the model to default-deny: every HTTP request needs a valid PDP bearer token unless its path is on an exact public allowlist (/health, /docs, /openapi.json, ...) or a defer-to-own-auth prefix (/policy-store, /callbacks, /opal-server, /_exit). A new PDP_AUTH_ENFORCEMENT config toggles audit (log + allow) vs enforce (401). +507/-0 across 5 files incl. 293 lines of tests.

Verdict: APPROVE. No Postable finding is HIGH or CRITICAL (severity rule: only MEDIUM/LOW postable -> approve). The middleware is well-engineered and the security-critical mechanics verify out:

• Wiring is correct: install_default_deny is called last in _configure_api_routes, and add_middleware prepends, so it runs outermost/first (before CORS and route handlers).
• Bypass-resistant allowlist: exact-match tier-1 (a startswith("/health") trap is explicitly avoided — /healthchecks/opa/ready stays gated) and boundary-aware tier-2 prefixes (/callbacksfoo does NOT match /callbacks). Trailing slash normalized. Token check is constant-time (hmac.compare_digest) and the token value is never logged.
• Fail-closed is proven: a synthetic brand-new route with no own auth returns 401 in enforce mode. Tests cover enforce/audit, missing/invalid/wrong-scheme/malformed tokens, public routes, trailing slash, the /health-prefix trap, tier-2 defer (/_exit reaches its own enforce_pdp_control_key), OPTIONS preflight, and loguru-injection-safe audit logging that never logs token material.
• #321's own tests pass (CI pytests: 66 passed; the 34 failures are the pre-existing aioresponses/aiohttp-3.14 breakage in other test files, not this PR).

Findings

Postable

#	Sev	File:Line	Category	Description
1	MEDIUM	horizon/config.py:325	Security posture	AUTH_ENFORCEMENT defaults to AUDIT (log + allow-through), so the "default-deny" control is inert on a fresh deploy until PDP_AUTH_ENFORCEMENT=enforce; the OPAL trigger routes it uniquely protects stay open by default. Intentional/documented but contradicts the "default-deny" framing — confirm the rollout end-state.
2	MEDIUM	horizon/middleware/default_deny.py:98	Robustness / doc-accuracy	Non-ASCII bearer token -> hmac.compare_digest TypeError -> raw 500 (middleware runs outside ExceptionMiddleware), contradicting the "never a 500" docstring; the malformed-header test only covers ASCII. Fails closed (not a bypass). Compare bytes / guard the check.

Informational

#	Sev	Ref	Category	Description
I1	INFO	default_deny.py:98 (Copilot)	Dedup	Copilot already flagged that get_env_api_key() can raise SystemExit on this line (unresolved) — a distinct path from finding 2; the same defensive-guard fix covers both. Not re-raised separately.
I2	LOW	default_deny.py:147 (Copilot)	Hardening	Copilot flagged the unconditional OPTIONS bypass. Low risk today (no sensitive OPTIONS handlers; FastAPI auto-handles preflight), but a future custom OPTIONS handler would be unauthenticated. Acknowledged; not re-raised.
I3	INFO	DEFER_AUTH_PREFIXES	Trust boundary	/policy-store, /callbacks, /opal-server rely on OPAL's own JWT auth, which the middleware cannot verify; /_exit is confirmed gated by enforce_pdp_control_key. A missing own-auth on any deferred OPAL route would be invisible to this middleware — worth periodic verification.
I4	LOW	PUBLIC_EXACT_PATHS	Info disclosure	/openapi.json, /docs, /redoc, /scalar are unauthenticated, so the full route/schema is readable by anyone. Deliberate (docs must load); consider gating docs in production for a hardened PDP.
I5	INFO	cross-PR (#317)	Interaction	In enforce mode this middleware + #317's router-level enforce_pdp_token double-gate the enforcer routes (defense-in-depth, fine). #321 allowlists /health, satisfying the constraint flagged on #317 (its public health router is not broken by the middleware). Token checks are consistent (both read get_env_api_key).
I6	INFO	pdp-tester / docker-scout / security-snyk (CI)	Pre-existing / infra	Red for the usual reasons (k3d breakage, residual base-image CVEs, snyk quota); none caused by this PR.

Blast radius: Global request path — every HTTP request now passes through this middleware. In enforce mode it newly gates the OPAL trigger routes; in audit mode behavior is unchanged except added WARN logs. No schema/data changes.

Isolation / scope: Well-isolated, all-additive (+507/-0), no unrelated churn — clean separation across a new middleware package, config, wiring, and tests.

[zeevmoney]

[MEDIUM] "Default-deny" middleware actually defaults to AUDIT (log + allow-through), not deny

Problem: AUTH_ENFORCEMENT defaults to AuthEnforcement.AUDIT, and in audit mode the middleware logs a would-block WARN and then lets the request through (default_deny.py lines 164-167). So a fresh PDP deploy does NOT reject unauthenticated requests — the "default-deny" control is inert until an operator sets PDP_AUTH_ENFORCEMENT=enforce. In particular the OPAL-mounted trigger routes (/policy-updater/trigger, /data-updater/trigger) — the routes this middleware uniquely protects, since #317's router-level dependency cannot reach them — remain callable unauthenticated by default. (The enforcer routes themselves stay gated unconditionally by their per-route / router-level enforce_pdp_token, so the exposure is limited to the OPAL-inherited routes, and it is not a regression since those were already open.)

Suggestion: Confirm audit-by-default is the intended rollout posture and that flipping the fleet to enforce is tracked as a follow-up (the title/PR describe this as "default-deny," which implies enforce). If a fresh deploy is expected to be protected out of the box, default to ENFORCE and make audit opt-in. Either way, make the intended end-state explicit so audit mode is not left on indefinitely.

[dshoen619]

Confirmed intentional — leaving the default as audit. The dedicated PDP fleet reports no HTTP telemetry to the main Datadog org and there is no WAF fallback, so audit mode is the instrument used to find any tokenless legitimate callers before enforcing (0 hits/30d per PER-15245). Audit-mode exposure is limited to the two OPAL-inherited trigger routes and is not a regression (they are open today); every previously-gated route keeps its own dependency regardless of mode. The enforce flip + removal of the audit branch is the tracked end-state under PER-15243, and the install-time "AUDIT MODE ACTIVE" banner keeps a stale-audit fleet greppable.

[zeevmoney]

[MEDIUM] Non-ASCII bearer token makes hmac.compare_digest raise TypeError -> 500 (a second path past the "never a 500" guarantee)

Problem: hmac.compare_digest(token.strip(), get_env_api_key()) raises TypeError when either argument is a str containing non-ASCII characters. HTTP header values are decoded latin-1, so Authorization: Bearer café (or any byte > 0x7F) yields a non-ASCII token, and the raised TypeError propagates out of this ASGI middleware — which runs outside Starlette's ExceptionMiddleware — as a raw 500. That contradicts the module docstring ("parses the Authorization header defensively so a malformed header yields 401, never a 500"), and test_enforce_malformed_header_is_401_not_500 only exercises ASCII inputs ("garbage", "Bearer", "Bearer ", "", "Bearer a b c"), so it doesn't catch this. It fails closed (deny), so it is not an auth bypass, but it is an unhandled exception on the front door. (This is a distinct path from the get_env_api_key() SystemExit concern already raised on this line.)

Suggestion: Compare bytes so any header value is safe, e.g. hmac.compare_digest(token.strip().encode("utf-8"), get_env_api_key().encode("utf-8")), and/or wrap the token check so any exception maps to "invalid token" (401). Add a non-ASCII case to test_enforce_malformed_header_is_401_not_500.

[dshoen619]

Fixed in 65a95db. The compare is now byte-based — hmac.compare_digest(token.strip().encode("utf-8"), get_env_api_key().encode("utf-8")) — and wrapped so any error fails closed to 401, so Authorization: Bearer café no longer raises TypeError → 500. httpx refuses to send non-ASCII header values, so the regression is covered at the unit level in test_has_valid_pdp_token (exercising the latin-1 scope uvicorn produces) rather than via the integration client.

[dshoen619]

Thanks @zeevmoney and the Copilot reviewer — addressed in 65a95db. Summary of how each finding landed:

Fixed

Non-ASCII bearer token → TypeError → 500 (Zeev MED, default_deny.py:98; also the Copilot dedup note)
The token is now compared as bytes (hmac.compare_digest(token.strip().encode("utf-8"), get_env_api_key().encode("utf-8"))) and the comparison is wrapped so any resolution/compare error fails closed to 401, never a 500. Authorization: Bearer café no longer crashes the front door. httpx won't send non-ASCII header values, so the regression is covered at the unit level in test_has_valid_pdp_token (exercising the latin-1 scope uvicorn actually produces).

get_env_api_key() can raise SystemExit (Copilot, default_deny.py:98)
The new guard maps unexpected comparison/resolution errors to an invalid token (401). SystemExit is a BaseException and is deliberately not caught — it signals a fatally-misconfigured PDP that should down the worker rather than be masked as a 401, and this matches the existing enforce_pdp_token behavior. In practice the key is resolved and cached at startup (PermitPDP.__init__), so this path isn't reachable during request handling.

OPTIONS bypass too broad (Copilot :147, Zeev I2)
Now bypasses only a genuine CORS preflight — an OPTIONS carrying Access-Control-Request-Method, which is exactly what Starlette's CORSMiddleware treats as preflight. A bare/non-preflight OPTIONS to a protected route is now gated, so a future custom OPTIONS handler can't be reached unauthenticated. New tests: test_enforce_cors_preflight_bypassed + test_enforce_non_preflight_options_is_gated.

Brittle == 200 test assertions (Copilot ×5 on the test file)
Tier-2 deferral and the audit-mode tests now assert "not the middleware's own 401" via a small _is_middleware_401(resp) helper (401 + WWW-Authenticate: Bearer + our detail), instead of coupling to whatever status OPAL's downstream handler returns. The audit tests' real assertions (would-block logged / not logged / token never logged / hostile-UA not dropped) are unchanged.

Confirmed, not changed

AUTH_ENFORCEMENT defaults to audit (Zeev MED #1, config.py:325)
This is intentional and a first-class requirement of the issue: the dedicated PDP fleet reports no HTTP telemetry to the main Datadog org and there is no WAF fallback, so audit mode is the instrument used to find any tokenless legitimate callers before enforcing (verified 0 hits/30d — see PER-15245 rollout section). Exposure in audit mode is limited to the two OPAL-inherited trigger routes and is not a regression (they're open today); every previously-gated route keeps its own dependency regardless of mode. The enforce flip + removal of the audit branch is the tracked end-state under PER-15243, and the install-time banner ("AUDIT MODE … Set PDP_AUTH_ENFORCEMENT=enforce …") keeps a stale-audit fleet greppable.

Acknowledged (informational)

• I3 — tier-2 defer trust boundary: the middleware can't verify OPAL's own JWT auth on /policy-store · /callbacks · /opal-server; the compensating control is the route auth-coverage CI guard in PER-15249 (matches auth deps by callable name, gates opal_client bumps).
• I4 — public /docs·/openapi.json·/redoc·/scalar: deliberate (docs must load); gating them in a hardened production profile can be a follow-up.

✅ 101 tests pass (52 for this middleware), ruff + format clean.

[zeevmoney]

Automated review — PR #321 (default-deny authentication middleware) — load-bearing

Reviewed with python-pro + fastapi-pro (security-focused); findings adversarially verified against OPAL 0.9.6 wiring and the reported production behavior.

Verdict: APPROVE (of the code). Highest confirmed severity is MEDIUM — no CRITICAL/HIGH correctness, auth-bypass, or crash defect. Independently confirmed:

• Middleware ordering is correct: OPAL registers CORSMiddleware first, so add_middleware makes DefaultDeny the outermost user middleware → it correctly bypasses only genuine CORS preflight and gates bare OPTIONS/HEAD/etc.
• No allowlist→protected-route bypass: .. traversal, percent-encoding, trailing-slash, case, and leading // all fail closed.
• add_middleware at construction time is safe (lazy stack build); the config enum coerces via AuthEnforcement(value) so an invalid value fails loud at startup (no silent enforce→audit downgrade); the token compare + defensive header parse never raise.
• In enforce mode the two trigger routes and a synthetic ungated route all return 401 (fail-closed) — the real fix for the reported CVE.

⚠️ Operational caveat (already tracked; restated for the human): this ships defaulting to PDP_AUTH_ENFORCEMENT=audit, which logs and allows requests through. So merging + deploying does NOT close the actively-exploited unauthenticated /policy-updater/trigger & /data-updater/trigger vuln until enforce is set on every fleet member. This is the intended, ticket-mandated rollout (no HTTP telemetry / no WAF → instrument first), with a loud startup WARN banner, and is captured in the existing open thread at config.py:325 and tracked under PER-15243. Not re-flagged inline — but do not merge this believing the CVE is closed on deploy.

MEDIUM/LOW inline notes below.

[zeevmoney]

[MEDIUM] Hot-path token check depends on an implicit warm-cache invariant; a cold cache degrades to on-loop blocking I/O or a per-request SystemExit

Problem: _has_valid_pdp_token runs on every non-allowlisted request and calls the sync get_env_api_key() directly on the event loop. get_env_api_key() only short-circuits when the module global _env_api_key is truthy; it is O(1) today only because PermitPDP.__init__ warms it at pdp.py:127 before serving. If that invariant breaks (e.g. an empty/falsy resolved key — ENVIRONMENT level returns sidecar_config.API_KEY verbatim with no non-empty check — or a future construction path that installs the middleware without warming), every request re-enters EnvApiKeyFetcher().get_env_api_key_by_level(), which for PROJECT/ORG levels does synchronous BlockingRequest network I/O with tenacity retry (up to 10 attempts, exponential backoff) on the event loop — freezing concurrent requests — and on failure raises SystemExit, killing the worker per request.

Suggestion: Don't couple the request hot-path to lazy cache warmth. Capture the resolved token at install time / from config, or assert the key is resolved when the middleware is installed so a cold cache fails loudly at startup rather than degrading per-request. Latent for a normally-started PDP, hence MEDIUM.

[dshoen619]

Fixed in 2a7c84b. install_default_deny now resolves get_env_api_key() once at install and fails loud (logger.critical + SystemExit(GUNICORN_EXIT_APP)) on an empty/unresolved key — so the per-request check is a guaranteed O(1) cache hit and can never degrade to on-loop blocking network I/O or a per-request SystemExit. This also adds the non-empty assertion nothing did before (pdp.py:127 warms but never checks; the ENVIRONMENT path returns PDP_API_KEY verbatim). Kept the per-request get_env_api_key() read so the token tests keep monkeypatching per-test. New test: test_install_fails_loud_on_empty_key. All 3 app-building test fixtures set a non-empty API_KEY before _configure_api_routes, so none break.

[zeevmoney]

[MEDIUM] Tier-2 DEFER_AUTH_PREFIXES assumes the OPAL verifier is enforcing, with no startup guard or test proving it

Problem: The middleware steps aside for /policy-store, /callbacks, /opal-server, trusting each to enforce its own auth. In OPAL those routes funnel through verify_logged_in, which returns {} (allows through) when verifier.enabled is False (OPAL_AUTH_PUBLIC_KEY unset). If the verifier were ever disabled, these routes would be fully unauthenticated — e.g. /opal-server/... connectivity control, /callbacks CRUD, or /policy-store/config (which can disclose POLICY_STORE_AUTH_TOKEN unless EXCLUDE_POLICY_STORE_SECRETS is set). The PR's own tier-2 test deliberately does not assert a downstream 401 (the mock runs with the verifier disabled), so nothing here proves the deferral is safe.

Production evidence (the reported vuln showed /policy-store/config returning 401 "access token was not provided") indicates the verifier IS enabled in prod — so this is not an active bypass and not a regression — but the middleware relies on it implicitly.

Suggestion: Add a startup assertion that fails closed if any tier-2 prefix is deferred while the OPAL verifier is not enforcing, and add a test that asserts a real 401 from a tier-2 route with the verifier enabled — so "defers to own auth" is verified rather than assumed.

[dshoen619]

Fixed in 2a7c84b. install_default_deny now emits a loud, greppable WARNING if app.state.opal_client.verifier is disabled while the tier-2 prefixes defer to it ("must never appear in a managed PDP") — a WARN rather than SystemExit, since dev/local PDPs legitimately run with the verifier off, and it is read via defensive getattr so test fixtures without opal_client stay silent. And per your suggestion, test_tier2_route_rejects_without_token_when_verifier_enabled builds a real enabled JWTVerifier (RSA keypair) and asserts /policy-store/config returns a real 401 that is not _is_middleware_401 — so deferral is now verified, not assumed. Also added tests for the warning firing/not-firing.

[zeevmoney]

[LOW] Comment claims the except guards non-ASCII tokens, but both operands are pre-encoded so compare_digest never raises TypeError

Problem: The comment says a non-ASCII token "would make hmac.compare_digest raise TypeError on str inputs," implying the except Exception -> False is what makes "Bearer café" safe. But the code .encode("utf-8")s both operands first, so compare_digest always receives bytes and never raises TypeError; "Bearer café" returns False via byte inequality, not via the handler. The test_has_valid_pdp_token café case therefore passes for a different reason than the comment states. A maintainer trusting the comment might later drop .encode() (comparing strs) assuming the except still fails closed on non-ASCII — reintroducing the TypeError path. The handler also swallows without logging, so a genuinely unexpected error class becomes a silent fail-closed with no diagnostic.

Suggestion: Correct the comment to state the safety comes from pre-encoding to bytes (not from except), and consider logging the exception type (never the token) inside the handler.

[dshoen619]

Fixed in 2a7c84b. Corrected the comment to state the safety comes from pre-encoding both operands to bytes (not the except), with an explicit "do NOT drop the .encode()" warning so a maintainer cannot reintroduce the TypeError path. The except is now documented as a last-resort net and logs the error class (never the token) so an unexpected failure is not silent.

[zeevmoney]

[LOW] _normalize_path docstring says "collapse a single trailing slash" but rstrip("/") strips all of them

Problem: The docstring (line 70) says "Collapse a single trailing slash," but path.rstrip("/") strips every trailing slash, so /health// → /health. Not exploitable (stripping trailing slashes can only make an already-public path match or produce a router 404 — it can never turn a protected path into an allowlisted one). Purely a doc/behavior mismatch.

Suggestion: Either match the docstring (removesuffix("/") for a single slash) or update the docstring to say all trailing slashes are stripped.

[dshoen619]

Fixed in 2a7c84b. Docstring now says it strips all trailing slashes (matching rstrip("/")), and notes stripping can only make an already-public path match or 404 — never turn a protected path into an allowlisted one.

[zeevmoney]

[LOW] test_enforce_allows_valid_token uses a loose != 401 assertion that a non-401 middleware block would also satisfy

Problem: With raise_server_exceptions=False, an authenticated request that reaches a route which then 500s offline still passes assert resp.status_code != 401. The test proves "not blocked by the middleware's 401" but would also pass if the middleware started blocking with 403/500. The tier-2 tests already use the precise _is_middleware_401(resp) helper.

Suggestion: Assert not _is_middleware_401(resp) here (and at the analogous != 401 assertions) for consistency and to catch a middleware that blocks with a non-401 status.

[dshoen619]

Fixed in 2a7c84b. test_enforce_allows_valid_token and the other != 401 assertions (public-routes, trailing-slash) now use the precise not _is_middleware_401(resp) helper, so a middleware block with any status would fail the test.

[dshoen619]

Thanks @zeevmoney — all 6 notes from the second review addressed in 2a7c84b, and the branch was rebased onto the latest (now integrating #317 / PER-15244 — the /kong gate + /health split + router-level enforcer auth).

MEDIUM

• Hot-path token cache — install_default_deny resolves the key once at install and fails loud on an empty/unresolved key, so the per-request check is always an O(1) cache hit (no on-loop blocking I/O, no per-request SystemExit). Per-request read kept so token tests still monkeypatch.
• Tier-2 verifier trust — loud WARNING at install if the OPAL verifier is disabled while tier-2 defers to it (WARN, not SystemExit, since dev PDPs legitimately disable it), plus a test with a real enabled JWTVerifier proving /policy-store/config returns a real (non-middleware) 401 — deferral verified, not assumed.

LOW — corrected the token-check comment (safety is pre-encoding to bytes, not the except) + error-class logging; fixed the _normalize_path docstring; tightened the loose != 401 assertions to _is_middleware_401; refreshed the stale OPTIONS module docstring.

✅ Full suite green on the integrated branch: 128 passed (the previously-failing aioresponses/aiohttp-3.14 tests now pass via the new conftest.py shim from main). ruff + format clean. install_default_deny remains correctly wired at the end of _configure_api_routes after #317's new health router.

[omer9564]

this is way overcomplicated, this should be done using the same dependency that enforces authentication everywhere else.
Backward compatibility isn't really needed because 1 - this API is almost for sure unused by any one of the PDP users, 2 it requires upgrade for the changes to happen, 3 we can send announcement that this is changed so users will know

[omer9564]

what is this config ? why do we need it ?
we can just declare breaking changes and whoever uses the apis unauthenticated and wants to upgrade should know he needs to add authentication

[dshoen619]

its more complicated but the idea was to change how authentication is enforced. We can add dependency for each route which is what we had previously and is definitely simpler, but it risks authentication holes when new routes are created. Thats what happened with the /kong endpoint.

Yes I agree the enforce/audit config is overkill, will remove.

[omer9564]

there are ways to enforce it on all routes or "group of routes" but simply setting the dependency on the router/app instead of the route itself.
The current implementation is far from any best practice seen on any project

[omer9564]

In general middleware is something that is applied on ALL routes without any way to exclude routes - forcing you to make the exclusion yourself.
dependency can be applied to routes selectively or group of routes / all routes if needed as a built in feature.

[dshoen619]

Please see new redesign using FastAPI dependencies on the exposed Opal Routes. I also added additional tests so ungated routes that are not public will not be exposed as this happened with /kong

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

[dshoen619]

Good catch — the PR description was stale from the original ASGI-middleware design. That approach was dropped after review feedback in favor of the FastAPI-native model you see here (per-router Depends(enforce_pdp_token) + _gate_opal_trigger_routes for the OPAL-mounted trigger routes). The PR description has been rewritten to match the actual implementation, so it no longer references a middleware.

[dshoen619]

horizon/tests/test_route_auth_audit.py now exists (added in 18ac4b1) and imports PUBLIC_ROUTE_PATHS: it builds the real app and asserts every non-public route carries a recognized auth gate, failing CI if a new route ships ungated. The comment is now accurate.

[dshoen619]

The route-audit test now exists (18ac4b1): test_opal_trigger_route_is_pdp_gated asserts both OPAL_TRIGGER_ROUTE_PATHS entries are present and gated by enforce_pdp_token — exactly what this comment describes.

[dshoen619]

Resolved in 18ac4b1 — test_route_auth_audit.py now exists and relies on PUBLIC_ROUTE_PATHS as the public allowlist, so the docstring reference is accurate.

[dshoen619]

Fixed in 6c7e526 — the sink now appends str(message), so records is genuinely list[str] and the substring checks no longer rely on loguru's Message being str-compatible.

[omer9564]

I now understand the problem in adding simple dependency here
LGTM