Require API authentication when reachable over the network - #110
Require API authentication when reachable over the network#110AAtomical wants to merge 1 commit into
Conversation
The action and status endpoints under /api operate on the authenticated user's Gmail (delete, archive, unsubscribe, mark read, read scan results, download a CSV backup) but had no authentication of any kind. With the default 0.0.0.0 bind this let any network-adjacent host drive the victim's mailbox using the stored OAuth credentials. Fix: - Bind to 127.0.0.1 by default (configurable via HOST); the API is no longer exposed to the local network out of the box. - Add an API token guard (Depends(require_api_auth)) on both API routers. A loopback-only bind needs no token (local single-user use is unchanged); any non-loopback bind requires one. If API_TOKEN is unset, a random token is generated at startup so the API is never left open by accident. - The same-origin web UI authenticates automatically: the root page sets the token as an HttpOnly cookie, so no client-side changes are needed. - Docker binds 0.0.0.0 internally (required for port publishing) and is therefore token-protected; compose publishes to host loopback by default. Token check uses constant-time comparison and accepts the token via Authorization: Bearer, X-API-Token, or the api_token cookie.
WalkthroughThis PR adds configurable API token authentication to the application. It introduces host binding configuration, auto-generates secure tokens for network-reachable deployments without explicit setup, extracts tokens from standard HTTP mechanisms (Bearer headers, custom headers, or cookies), enforces validation on API endpoints, sets secure cookies for same-origin UI access, and reports authentication status at startup. ChangesAPI Token Authentication
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/core/config.py`:
- Around line 59-61: The is_loopback_host() helper currently treats an empty
host string as loopback which incorrectly disables auth; update is_loopback_host
(in app/core/config.py) to only consider explicit loopback addresses
("127.0.0.1", "::1", "localhost") and remove "" from that set so that an empty
settings.host is treated as binding to all interfaces (not loopback). Ensure any
logic in resolve_effective_token() that calls is_loopback_host() continues to
rely on that corrected interpretation (so auth is not disabled when
settings.host is empty) and leave main.py which passes settings.host into
uvicorn.run(host=host) unchanged.
In `@app/main.py`:
- Around line 159-175: The root handler currently sets an "api_token" cookie for
any caller (TemplateResponse block using get_effective_token), which lets
anonymous requests mint sessions; change the logic so the cookie is only set
after a proven UI authentication step or when serving loopback-only clients:
update the code around get_effective_token() and response.set_cookie(...) to (a)
verify the token is tied to an authenticated UI session (e.g., check
session/user state or the same auth flag your UI login routine sets) before
calling response.set_cookie, or (b) if you need a shortcut for local-only
deployments, only set the cookie when request.client.host is loopback (127.0.0.1
or ::1); do not rely on presence of any token alone and do not set the cookie
for anonymous callers so require_api_auth() cannot be bypassed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d3f3d28d-af6a-4b99-9e49-dc3a1c8d7dc8
📒 Files selected for processing (6)
Dockerfileapp/api/auth.pyapp/core/config.pyapp/main.pydocker-compose.ymlmain.py
| def is_loopback_host(self) -> bool: | ||
| """Return True when the server only listens on the local machine.""" | ||
| return self.host.strip().lower() in ("127.0.0.1", "::1", "localhost", "") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
rg -n "class .*Settings|is_loopback_host|host\s*:" app/core/config.py || true
sed -n '1,120p' app/core/config.py || true
rg -n "uvicorn\.run|settings\.host|host=" -S . || true
# show likely main entrypoints without dumping huge files
for f in $(fd -t f -a -e py "main.py" .); do
echo "---- $f ----"
sed -n '1,200p' "$f" || true
done
# show other places host is referenced (keep it small)
rg -n "Settings\.host|settings\.host|is_loopback_host\(" -S . || trueRepository: Gururagavendra/gmail-cleaner
Length of output: 16494
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' app/api/auth.py
rg -n "is_loopback_host\(" app/api/auth.py
sed -n '1,220p' app/api/auth.pyRepository: Gururagavendra/gmail-cleaner
Length of output: 6591
🌐 Web query:
How does Uvicorn interpret host="" (empty string) passed to uvicorn.run(host=...) bind address?
💡 Result:
Uvicorn does not special-case an empty string host. If you pass host="" to uvicorn.run(host=...), Uvicorn will pass that value down to its socket-binding logic, which ultimately binds the listening socket(s) according to Python/asyncio semantics for host="". Concretely, Uvicorn’s config stores the host value and its bind_socket calls sock.bind((self.host, self.port)) (for the normal TCP case), without checking whether self.host is empty. Therefore, host="" is treated as “bind to all interfaces” by the underlying Python networking/asyncio behavior rather than being converted to 0.0.0.0/:: inside Uvicorn.[1][2] Notes/implications: 1) The “all interfaces” behavior comes from host="" meaning “bind to all interfaces” in the underlying socket/create_server implementation (and is consistent with asyncio’s documented behavior); some asyncio edge cases exist when host is provided as a sequence containing only "" (e.g., binding issues), but that’s about passing [''] to asyncio rather than passing "" directly to Uvicorn.[3] 2) Uvicorn’s own documented default host is 127.0.0.1, not empty string; use host="0.0.0.0" explicitly if you want an unambiguous IPv4-only bind, or host="::" for IPv6.[4][5] If your goal is “listen on all IPv4 interfaces” reliably, prefer uvicorn.run(..., host="0.0.0.0",...); if you want “all interfaces” (v4+v6), host="" (or None) will generally bind accordingly, but the exact address family behavior can depend on the underlying platform/asyncio behavior.[4][1][3]
Citations:
- 1: https://github.com/encode/uvicorn/blob/0.34.0/uvicorn/config.py
- 2: https://github.com/encode/uvicorn/blob/f6239169708adcf27433cb8d133e48f8835f0fa2/uvicorn/config.py
- 3: https://bugs.python.org/issue42795
- 4: https://www.uvicorn.org/settings/
- 5: https://uvicorn.dev/settings/
Don’t treat blank HOST ("") as loopback—this can disable API auth while binding to all interfaces.
app/core/config.py’s is_loopback_host() returns True for "", which makes resolve_effective_token() disable auth; then main.py passes settings.host directly into uvicorn.run(host=host). Uvicorn doesn’t special-case host="", so it binds as “all interfaces” at the socket layer—re-exposing the API without a token.
🛡️ Suggested hardening
+import ipaddress
import os
from pydantic import Field, field_validator
from pydantic_settings import BaseSettings, SettingsConfigDict
@@
def is_loopback_host(self) -> bool:
"""Return True when the server only listens on the local machine."""
- return self.host.strip().lower() in ("127.0.0.1", "::1", "localhost", "")
+ host = self.host.strip()
+ if not host:
+ return False
+ if host.lower() == "localhost":
+ return True
+ try:
+ return ipaddress.ip_address(host).is_loopback
+ except ValueError:
+ return False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/core/config.py` around lines 59 - 61, The is_loopback_host() helper
currently treats an empty host string as loopback which incorrectly disables
auth; update is_loopback_host (in app/core/config.py) to only consider explicit
loopback addresses ("127.0.0.1", "::1", "localhost") and remove "" from that set
so that an empty settings.host is treated as binding to all interfaces (not
loopback). Ensure any logic in resolve_effective_token() that calls
is_loopback_host() continues to rely on that corrected interpretation (so auth
is not disabled when settings.host is empty) and leave main.py which passes
settings.host into uvicorn.run(host=host) unchanged.
| response = templates.TemplateResponse( | ||
| request, | ||
| "index.html", | ||
| {"cache_bust": STARTUP_CACHE_BUST, "version": settings.app_version}, | ||
| ) | ||
| # Hand the same-origin browser its API token via a cookie so the UI | ||
| # keeps working without any client-side changes. Sent only when auth is | ||
| # actually in effect. | ||
| token = get_effective_token() | ||
| if token: | ||
| response.set_cookie( | ||
| "api_token", | ||
| token, | ||
| httponly=True, | ||
| samesite="strict", | ||
| secure=request.url.scheme == "https", | ||
| ) |
There was a problem hiding this comment.
Anonymous / requests can mint a valid API session.
This hands the shared token to anyone who can load the root page. Because require_api_auth() in app/api/auth.py accepts api_token cookies, a network-adjacent caller can just GET /, receive the cookie, and then use the protected /api/* routes. That means the new guard is still bypassable from the LAN; it only adds one extra request.
Only set this cookie after a real UI authentication step, or gate / behind the same token when the app is bound non-loopback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/main.py` around lines 159 - 175, The root handler currently sets an
"api_token" cookie for any caller (TemplateResponse block using
get_effective_token), which lets anonymous requests mint sessions; change the
logic so the cookie is only set after a proven UI authentication step or when
serving loopback-only clients: update the code around get_effective_token() and
response.set_cookie(...) to (a) verify the token is tied to an authenticated UI
session (e.g., check session/user state or the same auth flag your UI login
routine sets) before calling response.set_cookie, or (b) if you need a shortcut
for local-only deployments, only set the cookie when request.client.host is
loopback (127.0.0.1 or ::1); do not rely on presence of any token alone and do
not set the cookie for anonymous callers so require_api_auth() cannot be
bypassed.
1 participant
The action and status endpoints under /api operate on the authenticated user's Gmail (delete, archive, unsubscribe, mark read, read scan results, download a CSV backup) but had no authentication of any kind. With the default 0.0.0.0 bind this let any network-adjacent host drive the victim's mailbox using the stored OAuth credentials.
Fix:
Token check uses constant-time comparison and accepts the token via Authorization: Bearer, X-API-Token, or the api_token cookie.
Related Issues
Security: API authentication for network-reachable instances
Depends(require_api_auth)dependency on both/api/statusand/api/actionsrouters0.0.0.0to127.0.0.1(loopback only), making the API inaccessible from the network by defaultHOSTenvironment variable (Dockerfile sets it to0.0.0.0for container deployments)Token handling
API_TOKENenvironment variable is set, that value is usedAPI_TOKENis unset, a secure random token is auto-generated at startupAuthorization: Bearer <token>header,X-API-Tokenheader, orapi_tokencookieWeb UI integration
api_tokenas an HttpOnly, Strict SameSite, Secure cookieDocker deployment
0.0.0.0internally (required for port publishing)docker-compose.ymlupdated to publish ports only to host loopback (127.0.0.1:8766and127.0.0.1:8767)Configuration & startup
app/core/config.pysettings:host(defaults to127.0.0.1) andapi_token(defaults to empty string)is_loopback_host()method to identify loopback-only bindings