Repository navigation
π‘οΈ Sentinel: [MEDIUM] HTTP μμΈ μλ΅μ λλ½λ 보μ ν€λ μΆκ° #1045
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,3 +90,7 @@ | |
| **Vulnerability:** The `_safe_upload_filename` function used `filename.replace`, `PurePosixPath`, and `re.sub` on unbounded client input, making it vulnerable to ReDoS or CPU/memory exhaustion (DoS) when fed extremely long strings. | ||
| **Learning:** Even fast standard library functions like `PurePosixPath` and string replacements can cause significant lag when chained on strings in the megabytes. String processing operations should always bound their inputs first if the input is untrusted and can be arbitrarily large. | ||
| **Prevention:** Cap the length of client-provided filename strings early by slicing them (e.g. `filename = filename[-512:]`) before doing more complex string parsing or regex replacements, especially when only the basename suffix is relevant. | ||
| ## 2025-02-27 - [MEDIUM] Missing Security Headers on FastAPI Exception Responses | ||
| **Vulnerability:** HTTP exception responses (e.g. 404, 401, 422) raised by Starlette or FastAPI standard exceptions bypassed the custom `_apply_security_headers` middleware and global exception handler, leaking server information without strict security headers. | ||
| **Learning:** Default framework exception handlers bypass application middlewares that inject headers into responses unless explicitly overridden to inject those headers manually. | ||
|
Comment on lines
+94
to
+95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π― Functional Correctness | π‘ Minor | β‘ Quick win μμΈ νΈλ€λ¬κ° λ―Έλ€μ¨μ΄λ₯Ό μ°ννλ€λ μ€λͺ μ μμ νμΈμ. Starletteλ μ²λ¦¬λ HTTP μμΈλ₯Ό μ ν리μΌμ΄μ
λ―Έλ€μ¨μ΄ μμͺ½μμ μλ΅μΌλ‘ λ³νν©λλ€. λ°λΌμ π€ Prompt for AI Agents |
||
| **Prevention:** Override `StarletteHTTPException` and `RequestValidationError` default exception handlers to always append security headers to error responses, regardless of response content or status body allowances. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import pytest | ||
| from fastapi.testclient import TestClient | ||
| from starlette.exceptions import HTTPException as StarletteHTTPException | ||
| from src.newsdom_api.config import RuntimeSettings, AuthenticationMode, RuntimeProfile | ||
| from src.newsdom_api.main import create_app | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def test_client(): | ||
| settings = RuntimeSettings( | ||
| api_token=None, | ||
| authentication_mode=AuthenticationMode.DISABLED, | ||
| runtime_profile=RuntimeProfile.DEVELOPMENT, | ||
| ) | ||
| # Using real app router but we override readiness for testing | ||
| app = create_app(settings, runtime_readiness_probe=lambda: True) | ||
|
|
||
| # Add a dummy route that raises an exception with a status code that does not allow a body | ||
| @app.get("/no-content-exception") | ||
| def no_content_exception(): | ||
| raise StarletteHTTPException(status_code=204) | ||
|
|
||
| return TestClient(app, raise_server_exceptions=False) | ||
|
|
||
|
|
||
| def test_http_exception_no_body(test_client): | ||
| response = test_client.get("/no-content-exception") | ||
| assert response.status_code == 204 | ||
| assert response.headers.get("x-content-type-options") == "nosniff" | ||
| assert response.content == b"" | ||
|
Comment on lines
+26
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π μ£Όμ μμΈ μλ΅ κ²μ¦ λλ½ μ ν
μ€νΈλ 204μ ν€λ νλλ§ νμΈν©λλ€. 404Β·422μ μλ΅ λ³Έλ¬Έκ³Ό ν€λλ κ²μ¦νμ§ μμΌλ©°, Was this helpful? React with π or π to provide feedback. |
||
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.
π μμΈ μλ΅ μ°ν μ€λͺ νμΈ νμ
κΈ°λ‘μ κΈ°λ³Έ HTTP μμΈκ° 보μ ν€λ λ―Έλ€μ¨μ΄λ₯Ό μ°ννλ€κ³ μ€λͺ ν©λλ€. κ·Έλ¬λ security_boundary_middlewareλ
call_nextμ 404Β·422 μλ΅μλ ν€λλ₯Ό μΆκ°ν©λλ€.Was this helpful? React with π or π to provide feedback.