Security: API key accepted from request body (ai_token) can leak via logs and intermediaries

[tuanaiseo]

Problem

The authentication helper accepts credentials from body['ai_token'] in addition to the Authorization header. Tokens in JSON bodies are more likely to be logged by application middleware, error handlers, or upstream gateways, and are not handled by standard auth-header redaction rules. This increases the risk of secret disclosure.

Severity: medium
File: acestep/api/http/auth.py

Solution

Prefer Authorization: Bearer <token> exclusively for API authentication. If backward compatibility is required, gate body-token support behind an explicit legacy flag and ensure body fields containing secrets are redacted from logs.

Changes

• acestep/api/http/auth.py (modified)

Testing

• Existing tests pass
• Manual review completed
• No new warnings/errors introduced

Summary by CodeRabbit

• Breaking Changes
  • Authentication now requires credentials exclusively via the Authorization header; the alternative request body method is no longer supported.
  • Updated error messaging to reflect that only the Authorization header is required for authentication.

[coderabbitai]

📝 Walkthrough

Walkthrough

The verify_token_from_request function in the authentication module no longer accepts authentication tokens from the request body. All credential validation now occurs exclusively through the Authorization header, with the corresponding error message updated to reflect this requirement.

Changes

Cohort / File(s)	Summary
Authentication Header Simplification acestep/api/http/auth.py	Removed request body token validation logic from verify_token_from_request, eliminating the ai_token parameter acceptance. Error message updated from "Missing ai_token or Authorization header" to "Missing Authorization header". Bearer token parsing and API-key validation behavior unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit once carried tokens two ways,
Through body and header in authentication's maze.
Now headers alone shall our credentials convey—
One path is cleaner, the simpler way! 🔐✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main security change: removing the ai_token authentication from request body to prevent API key leakage through logs and intermediaries, which directly aligns with the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

acestep/api/http/auth.py (1)

26-30: Clarify that body is intentionally ignored.

body remains in the public signature but no longer participates in authentication. Document that explicitly so future changes do not accidentally reintroduce body-token auth.

Proposed docstring clarification

-    """Validate request auth from Authorization header.
+    """Validate request auth from the Authorization header only.
 
     Args:
-        body: Parsed request payload dictionary.
+        body: Parsed request payload dictionary. Ignored for authentication.
         authorization: Optional ``Authorization`` header value.

As per coding guidelines, “Docstrings must be concise and include purpose plus key inputs/outputs and raised exceptions when relevant.”

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/api/http/auth.py` around lines 26 - 30, Update the function docstring
that begins "Validate request auth from Authorization header" to explicitly
state that the body parameter is intentionally ignored for authentication (kept
only for signature/backwards compatibility) and will not be used to derive
tokens or credentials; clearly list the inputs (authorization header used, body
ignored), outputs (e.g., authenticated principal or None) and any exceptions
raised so future maintainers of the function know not to reintroduce body-based
auth. Reference the existing parameter names body and authorization in the
description and keep the docstring concise and aligned with project docstring
guidelines.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@acestep/api/http/auth.py`:
- Around line 42-46: The current Authorization parsing accepts raw API keys
without the Bearer scheme; change the logic in the block that reads the
`authorization` header (variable `authorization`, extraction into `token`, and
comparison with `_api_key`) to reject any header that does not start with
"Bearer " by raising HTTPException(401) rather than stripping non-Bearer values,
and update the related `verify_api_key` function to use the same Bearer-only
check so only `Authorization: Bearer <token>` is accepted.

---

Nitpick comments:
In `@acestep/api/http/auth.py`:
- Around line 26-30: Update the function docstring that begins "Validate request
auth from Authorization header" to explicitly state that the body parameter is
intentionally ignored for authentication (kept only for signature/backwards
compatibility) and will not be used to derive tokens or credentials; clearly
list the inputs (authorization header used, body ignored), outputs (e.g.,
authenticated principal or None) and any exceptions raised so future maintainers
of the function know not to reintroduce body-based auth. Reference the existing
parameter names body and authorization in the description and keep the docstring
concise and aligned with project docstring guidelines.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bb127559-f8e7-4601-9863-501dfae082f3

📥 Commits

Reviewing files that changed from the base of the PR and between 1d9d2d3 and 01a2e24.

📒 Files selected for processing (1)

• acestep/api/http/auth.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Enforce Bearer if header-only auth is meant to be Bearer-only.

Line 43 still accepts Authorization: <raw-api-key> without the Bearer scheme. If the intended contract is exclusively Authorization: Bearer <token>, reject non-Bearer headers here and mirror the same behavior in verify_api_key.

Proposed Bearer-only parsing

     if authorization:
-        token = authorization[7:] if authorization.startswith("Bearer ") else authorization
+        if not authorization.startswith("Bearer "):
+            raise HTTPException(status_code=401, detail="Invalid Authorization header")
+        token = authorization.removeprefix("Bearer ")
         if token == _api_key:
             return token
         raise HTTPException(status_code=401, detail="Invalid API key")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/api/http/auth.py` around lines 42 - 46, The current Authorization
parsing accepts raw API keys without the Bearer scheme; change the logic in the
block that reads the `authorization` header (variable `authorization`,
extraction into `token`, and comparison with `_api_key`) to reject any header
that does not start with "Bearer " by raising HTTPException(401) rather than
stripping non-Bearer values, and update the related `verify_api_key` function to
use the same Bearer-only check so only `Authorization: Bearer <token>` is
accepted.