fix(api): enforce configured inference bearer token #15
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| import hmac | ||
| import os | ||
|
|
||
| from fastapi import Depends, FastAPI, HTTPException, Header, Request | ||
| from slowapi import Limiter, _rate_limit_exceeded_handler | ||
| from slowapi.util import get_remote_address | ||
|
|
@@ -22,10 +25,21 @@ class PromptRequest(BaseModel): | |
|
|
||
|
|
||
| def _require_bearer(authorization: Optional[str] = Header(default=None)) -> str: | ||
| """Require a configured bearer token for the public inference stub.""" | ||
|
|
||
| expected_token = os.getenv("API_BEARER_TOKEN") | ||
| if not expected_token: | ||
| raise HTTPException( | ||
| status_code=503, | ||
| detail="Inference authentication is not configured.", | ||
| ) | ||
| if not authorization or not authorization.startswith("Bearer "): | ||
| raise HTTPException(status_code=401, detail="Missing or invalid Authorization header") | ||
|
|
||
| token = authorization.removeprefix("Bearer ").strip() | ||
|
Comment on lines
36
to
+39
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)(main\.py|test_auth\.py)$|api/|tests/' | sed -n '1,120p'
echo "== api/main.py around auth =="
if [ -f api/main.py ]; then
nl -ba api/main.py | sed -n '1,80p'
fi
echo "== test_auth.py =="
if [ -f tests/test_auth.py ]; then
nl -ba tests/test_auth.py | sed -n '1,220p'
fi
echo "== search authorization usage =="
rg -n "Authorization|removeprefix|bearer|Bearer|HTTPException|status_code=401" api tests -S || trueRepository: CoreyLeath-code/SentinelAI Length of output: 589 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== api/main.py =="
awk '{printf "%6d\t%s\n", NR, $0}' api/main.py | sed -n '1,90p'
echo "== api/auth.py =="
awk '{printf "%6d\t%s\n", NR, $0}' api/auth.py | sed -n '1,160p'
echo "== tests/test_auth.py =="
awk '{printf "%6d\t%s\n", NR, $0}' tests/test_auth.py | sed -n '1,220p'
echo "== deterministic header parser behavior =="
python3 - <<'PY'
def current(authorization):
if not authorization or not authorization.startswith("Bearer "):
return "401"
token = authorization.removeprefix("Bearer ").strip()
return ("ok", token)
def proposed(authorization):
parts = authorization.split(maxsplit=1) if authorization else []
if len(parts) != 2 or parts[0].lower() != "bearer":
return "401"
token = parts[1].strip()
return ("ok", token)
cases = [
"Bearer test-token",
"bearer test-token",
"BeArEr test-token",
"bearer test-token",
"Bearer",
"",
"Basic abc",
]
for header in cases:
print(repr(header), "current=", current(header), "proposed=", proposed(header))
PYRepository: CoreyLeath-code/SentinelAI Length of output: 4641 Accept case-insensitive Bearer token schemes. A lowercase or mixed-case scheme should not fail authentication when the token is valid. Use the proposed parser for scheme/token fields, and add a cover test such as 🤖 Prompt for AI Agents |
||
| if not hmac.compare_digest(token, expected_token): | ||
|
There was a problem hiding this comment.
When either the configured secret or a received credential contains a non-ASCII character, Python's Useful? React with 👍 / 👎. |
||
| raise HTTPException(status_code=401, detail="Missing or invalid Authorization header") | ||
|
Comment on lines
37
to
+41
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)api/main\.py$|(^|/)tests/test_auth\.py$|pyproject\.toml$|requirements.*\.txt$' || true
echo "== api/main.py relevant =="
if [ -f api/main.py ]; then
cat -n api/main.py | sed -n '1,120p'
fi
echo "== tests/test_auth.py relevant =="
if [ -f tests/test_auth.py ]; then
cat -n tests/test_auth.py | sed -n '1,220p'
fi
echo "== search auth challenge usages =="
rg -n "WWW-Authenticate|Bearer|Authorization|HTTPException|401" .Repository: CoreyLeath-code/SentinelAI Length of output: 5519 🌐 Web query:
💡 Result: To include a WWW-Authenticate Bearer challenge in a FastAPI response, use the HTTPException class with the headers parameter [1][2]. When an authentication error occurs (typically a 401 Unauthorized), the HTTP specification requires that the server includes a WWW-Authenticate header to inform the client about the authentication scheme [1][3][4]. For Bearer token authentication, you should provide the following: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Not authenticated", headers={"WWW-Authenticate": "Bearer"},) Key points regarding this implementation: 1. Status Code: Use status.HTTP_401_UNAUTHORIZED [1][2]. Starting with FastAPI version 0.122.0, built-in security utilities (like OAuth2PasswordBearer or HTTPBearer) automatically return this 401 status code along with the appropriate WWW-Authenticate header when authentication fails [5][4]. 2. Headers Parameter: The HTTPException class accepts a headers dictionary, which allows you to pass the WWW-Authenticate instruction directly to the client [6][7]. 3. Standard Compliance: Including this header ensures your API follows HTTP specifications (such as RFC 9110), signaling to clients that they should attempt authentication using the Bearer scheme [1][3][4]. For advanced use cases or when overriding default security behavior, you can extend classes like HTTPBearer and override the make_not_authenticated_error method to customize how these 401 responses are generated [5][8]. Citations:
Include the Bearer challenge on both 401 responses. Both 🤖 Prompt for AI Agents |
||
| return token | ||
|
|
||
|
|
||
| @app.get("/") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,17 @@ | ||
| def test_infer_fails_closed_when_bearer_token_is_unconfigured(client, monkeypatch): | ||
| monkeypatch.delenv("API_BEARER_TOKEN", raising=False) | ||
|
|
||
| response = client.post("/infer", json={"prompt": "Hello"}) | ||
|
|
||
| assert response.status_code == 503 | ||
|
|
||
|
|
||
| def test_infer_rejects_missing_and_invalid_bearer_tokens(client, monkeypatch): | ||
| monkeypatch.setenv("API_BEARER_TOKEN", "test-token") | ||
|
|
||
| assert client.post("/infer", json={"prompt": "Hello"}).status_code == 401 | ||
| assert client.post( | ||
| "/infer", | ||
| json={"prompt": "Hello"}, | ||
| headers={"Authorization": "Bearer wrong-token"}, | ||
| ).status_code == 401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rewrite the bearer-token description.
Use
Shared bearer token required for ...instead ofRequired shared bearer token .... This removes the awkward wording without changing the meaning.The provided LanguageTool hint identifies this wording issue.
Proposed wording
📝 Committable suggestion
🧰 Tools
🪛 LanguageTool
[style] ~154-~154: The double modal “Required shared” is nonstandard (only accepted in certain dialects). Consider “to be shared”.
Context: ...PI_BEARER_TOKEN
| *(unset)* | Required shared bearer token forPOST /infer`; the end...(NEEDS_FIXED)
🤖 Prompt for AI Agents
Source: Linters/SAST tools