Add security headers - #94
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces a security headers middleware for FastAPI that automatically appends security-related HTTP headers to all API responses. The implementation includes a middleware module, integration into the API application, comprehensive test coverage, and a detailed planning document. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/plans/2026-02-19_security_headers_482917/plan.md`:
- Line 124: The sentence "**CSP header**: Omitted for the API since responses
are JSON; CSP is most useful for HTML (e.g. Swagger UI)." lacks an explicit
subject—change it to include one (for example, "The CSP header is omitted for
the API because responses are JSON; CSP is most useful for HTML (e.g. Swagger
UI).") so the sentence reads with a clear subject; update the sentence in the
line that begins with "**CSP header**:" accordingly.
- Around line 38-45: The markdown ordered list in the plan (the block starting
"1. Request hits any API route (e.g. `GET /health`...") has inconsistent
numbering and triggers MD029; update the list so its numeric markers are
consistent—either make every item start with "1." (preferred for stable diffs)
or renumber sequentially from 1 to 5—ensure the items mentioning
RequestIdMiddleware, SecurityHeadersMiddleware, call_next(request), added
headers (X-Content-Type-Options, X-Frame-Options, Strict-Transport-Security /
ENABLE_HSTS) and the final return are updated accordingly.
- Around line 131-135: The code fence in
docs/plans/2026-02-19_security_headers_482917/ is missing a fenced language and
the file lacks a trailing newline; update the opening fence to include a
language token (e.g., change ``` to ```text) and make the closing fence match,
and ensure the file ends with exactly one newline character so the document
conforms to MD040/MD047.
Comment on lines
+38
to
+45
| 1. Request hits any API route (e.g. `GET /health` or `POST /v1/simulations/run`). | ||
| 2. `RequestIdMiddleware` runs first (existing), then `SecurityHeadersMiddleware`. | ||
| 3. `SecurityHeadersMiddleware` calls `call_next(request)`, obtains the response. | ||
| 4. Middleware adds headers to the response: | ||
| - `X-Content-Type-Options: nosniff` | ||
| - `X-Frame-Options: DENY` | ||
| - `Strict-Transport-Security: max-age=31536000; includeSubDomains` (only when `ENABLE_HSTS` is set and truthy, for HTTPS deployments). | ||
| 5. Response is returned to the client with all security headers present. |
Contributor
There was a problem hiding this comment.
Fix ordered list numbering to satisfy MD029.
📝 Suggested fix
-1. Request hits any API route (e.g. `GET /health` or `POST /v1/simulations/run`).
-2. `RequestIdMiddleware` runs first (existing), then `SecurityHeadersMiddleware`.
-3. `SecurityHeadersMiddleware` calls `call_next(request)`, obtains the response.
-4. Middleware adds headers to the response:
- - `X-Content-Type-Options: nosniff`
- - `X-Frame-Options: DENY`
- - `Strict-Transport-Security: max-age=31536000; includeSubDomains` (only when `ENABLE_HSTS` is set and truthy, for HTTPS deployments).
-5. Response is returned to the client with all security headers present.
+1. Request hits any API route (e.g. `GET /health` or `POST /v1/simulations/run`).
+1. `RequestIdMiddleware` runs first (existing), then `SecurityHeadersMiddleware`.
+1. `SecurityHeadersMiddleware` calls `call_next(request)`, obtains the response.
+1. Middleware adds headers to the response:
+ - `X-Content-Type-Options: nosniff`
+ - `X-Frame-Options: DENY`
+ - `Strict-Transport-Security: max-age=31536000; includeSubDomains` (only when `ENABLE_HSTS` is set and truthy, for HTTPS deployments).
+1. Response is returned to the client with all security headers present.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. Request hits any API route (e.g. `GET /health` or `POST /v1/simulations/run`). | |
| 2. `RequestIdMiddleware` runs first (existing), then `SecurityHeadersMiddleware`. | |
| 3. `SecurityHeadersMiddleware` calls `call_next(request)`, obtains the response. | |
| 4. Middleware adds headers to the response: | |
| - `X-Content-Type-Options: nosniff` | |
| - `X-Frame-Options: DENY` | |
| - `Strict-Transport-Security: max-age=31536000; includeSubDomains` (only when `ENABLE_HSTS` is set and truthy, for HTTPS deployments). | |
| 5. Response is returned to the client with all security headers present. | |
| 1. Request hits any API route (e.g. `GET /health` or `POST /v1/simulations/run`). | |
| 1. `RequestIdMiddleware` runs first (existing), then `SecurityHeadersMiddleware`. | |
| 1. `SecurityHeadersMiddleware` calls `call_next(request)`, obtains the response. | |
| 1. Middleware adds headers to the response: | |
| - `X-Content-Type-Options: nosniff` | |
| - `X-Frame-Options: DENY` | |
| - `Strict-Transport-Security: max-age=31536000; includeSubDomains` (only when `ENABLE_HSTS` is set and truthy, for HTTPS deployments). | |
| 1. Response is returned to the client with all security headers present. |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 45-45: Ordered list item prefix
Expected: 1; Actual: 5; Style: 1/1/1
(MD029, ol-prefix)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-02-19_security_headers_482917/plan.md` around lines 38 - 45,
The markdown ordered list in the plan (the block starting "1. Request hits any
API route (e.g. `GET /health`...") has inconsistent numbering and triggers
MD029; update the list so its numeric markers are consistent—either make every
item start with "1." (preferred for stable diffs) or renumber sequentially from
1 to 5—ensure the items mentioning RequestIdMiddleware,
SecurityHeadersMiddleware, call_next(request), added headers
(X-Content-Type-Options, X-Frame-Options, Strict-Transport-Security /
ENABLE_HSTS) and the final return are updated accordingly.
|
|
||
| - **Per-route decorator**: Would require decorating every route; middleware is DRY and applies to all responses. | ||
| - `**fastapi-security` or third-party package**: Adds a dependency for a simple middleware; custom middleware is minimal and matches existing patterns (e.g. `RequestIdMiddleware`). | ||
| - **CSP header**: Omitted for the API since responses are JSON; CSP is most useful for HTML (e.g. Swagger UI). Can be added later if needed. |
Contributor
There was a problem hiding this comment.
Add a subject to the sentence (LanguageTool).
📝 Suggested fix
-- **CSP header**: Omitted for the API since responses are JSON; CSP is most useful for HTML (e.g. Swagger UI). Can be added later if needed.
+- **CSP header**: Omitted for the API since responses are JSON; CSP is most useful for HTML (e.g. Swagger UI). It can be added later if needed.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **CSP header**: Omitted for the API since responses are JSON; CSP is most useful for HTML (e.g. Swagger UI). Can be added later if needed. | |
| - **CSP header**: Omitted for the API since responses are JSON; CSP is most useful for HTML (e.g. Swagger UI). It can be added later if needed. |
🧰 Tools
🪛 LanguageTool
[style] ~124-~124: To form a complete sentence, be sure to include a subject.
Context: ...most useful for HTML (e.g. Swagger UI). Can be added later if needed. - `**X-XSS-Pr...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-02-19_security_headers_482917/plan.md` at line 124, The
sentence "**CSP header**: Omitted for the API since responses are JSON; CSP is
most useful for HTML (e.g. Swagger UI)." lacks an explicit subject—change it to
include one (for example, "The CSP header is omitted for the API because
responses are JSON; CSP is most useful for HTML (e.g. Swagger UI).") so the
sentence reads with a clear subject; update the sentence in the line that begins
with "**CSP header**:" accordingly.
Comment on lines
+131
to
+135
| ``` | ||
| docs/plans/2026-02-19_security_headers_482917/ | ||
| ``` | ||
|
|
||
| No UI changes; no before/after screenshots required. No newline at end of file |
Contributor
There was a problem hiding this comment.
Specify a fenced code language and ensure trailing newline (MD040/MD047).
📝 Suggested fix
-```
+```text
docs/plans/2026-02-19_security_headers_482917/
-```
+```Also ensure the file ends with a single newline.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` | |
| docs/plans/2026-02-19_security_headers_482917/ | |
| ``` | |
| No UI changes; no before/after screenshots required. |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 131-131: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 135-135: Files should end with a single newline character
(MD047, single-trailing-newline)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-02-19_security_headers_482917/plan.md` around lines 131 -
135, The code fence in docs/plans/2026-02-19_security_headers_482917/ is missing
a fenced language and the file lacks a trailing newline; update the opening
fence to include a language token (e.g., change ``` to ```text) and make the
closing fence match, and ensure the file ends with exactly one newline character
so the document conforms to MD040/MD047.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes