Fix APE authentication to use FastAPI Depends pattern#371
Conversation
Lightheartdevs
left a comment
There was a problem hiding this comment.
Review: Fix APE authentication to use FastAPI Depends pattern
Good PR — fixes a real security gap (/policy and /metrics were unauthenticated) and replaces inconsistent manual auth with the idiomatic Depends() pattern per CLAUDE.md's DIP guidance.
Minor issue (non-blocking)
Type mismatch on verify_api_key return value vs. Depends() annotation
dream-server/extensions/services/ape/main.py lines 248-251, 275, etc.
verify_api_key returns True (a bool), but every endpoint annotates the injected value as api_key: str = Depends(verify_api_key). This is a type lie — api_key will be True at runtime, not a string. Since the return value is never actually used in any endpoint body, the cleanest fix is either:
- Return
None(or nothing) and annotate asNone = Depends(verify_api_key), or - Return the actual key (
return x_api_key) and keep thestrannotation — this is what dashboard-api does with itsverify_api_key, which returns the credential string.
Option 2 would be more consistent with dashboard-api's pattern and makes the dependency genuinely useful if a future endpoint needs to know which key was used.
What looks good
/verify: Correctly migrated from brokenawait verify_api_key(request)(was passingRequestwhereHeaderparam was expected) toDepends(). Pre-existing bug fixed./audit: Removed duplicated inline auth check in favor of the shared dependency. Clean./policy,/metrics: Now auth-protected. Security improvement.Requestparameter correctly retained on/verifysincerequest.client.hostis used for audit logging./healthcorrectly left unauthenticated (liveness probes must not require auth).
LGTM with the optional type annotation cleanup.
Summary
Fixes APE authentication to use FastAPI's
Depends()pattern consistently across all endpoints, matching the pattern used in dashboard-api.Security Issues Fixed
Before:
/policyand/metricsendpoints had NO authenticationAfter: All endpoints require API key via
Depends(verify_api_key)Changes
Dependsimport from fastapi/verify: Changed from manualawait verify_api_key(request)toDepends(verify_api_key)/audit: Changed from manual header check toDepends(verify_api_key)/policy: Added authentication (was unauthenticated)/metrics: Added authentication (was unauthenticated)Consistency
This matches the auth pattern used throughout dashboard-api routers:
The manual auth checks in APE were inconsistent and left security gaps.