feat(refacto): get provider endpoint toward clean architecture#767
feat(refacto): get provider endpoint toward clean architecture#767leoguillaume merged 8 commits intomainfrom
Conversation
benjaminpilia
force-pushed
the
refacto/get_provider_endpoint
branch
from
5c5cb94 to
a239066
Compare
benjaminpilia
force-pushed
the
refacto/get_provider_endpoint
branch
from
a239066 to
57826fa
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| extra={ | ||
| "user_id": command.user_id, | ||
| "router_id": router_id, | ||
| "offset": command.offset, | ||
| "limit": command.limit, | ||
| "sort_by": command.sort_by, | ||
| "sort_order": command.sort_order, | ||
| "error_type": type(e).__name__, | ||
| }, |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 29 days ago
In general, to fix log injection, any user-controlled values included in log messages (either in the main message string or in structured fields such as extra) should be sanitized before logging. Minimal sanitization for plain-text logs is to strip or replace newline (\n) and carriage-return (\r) characters so an attacker cannot split a single log entry into multiple lines or otherwise alter the log structure.
Here, the only tainted value is router_id (coming from the query parameter). The other fields in the extra dict (user_id, offset, limit, sort_by, sort_order) are not directly user-controlled in this snippet or are enums/validated types, and CodeQL has not flagged them. The most targeted, non-invasive fix is therefore:
- Create a sanitized version of
router_idbefore logging, removing any\rand\ncharacters. - Use that sanitized value in the
extradict.
Since router_id is annotated as int | None, but CodeQL treats it as tainted, it is safest to treat it as possibly string-like when sanitizing. We can:
- Check if
router_idis notNone. - Convert it to
str. - Replace
\rand\nwith empty strings. - Use the sanitized string in the log entry.
This change should be made in api/infrastructure/fastapi/endpoints/admin/providers.py inside the get_providers endpoint, in the except block where logger.exception is called. No new imports are needed; basic string methods are sufficient. The runtime behavior of the endpoint (its responses and use case execution) is unchanged; only the representation of router_id in the error log is normalized.
| @@ -258,11 +258,14 @@ | ||
| try: | ||
| result = await get_providers_use_case.execute(command) | ||
| except Exception as e: | ||
| sanitized_router_id = None | ||
| if router_id is not None: | ||
| sanitized_router_id = str(router_id).replace("\r", "").replace("\n", "") | ||
| logger.exception( | ||
| "Unexpected error while executing get_providers use case", | ||
| extra={ | ||
| "user_id": command.user_id, | ||
| "router_id": router_id, | ||
| "router_id": sanitized_router_id, | ||
| "offset": command.offset, | ||
| "limit": command.limit, | ||
| "sort_by": command.sort_by, |
benjaminpilia
changed the title
3 participants
No description provided.