-
Notifications
You must be signed in to change notification settings - Fork 131
Add missing security headers (X-Content-Type-Options, X-Frame-Options, CSP, Referrer-Policy, Permissions-Policy) #431
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: staging
Are you sure you want to change the base?
Changes from 1 commit
36749dc
55be232
ec043c4
fe088a0
b9a1f68
af30c03
1e60cb0
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -44,12 +44,37 @@ async def dispatch(self, request: Request, call_next): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = await call_next(request) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Add HSTS header to prevent man-in-the-middle attacks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # max-age=31536000: 1 year in seconds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # includeSubDomains: apply to all subdomains | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # preload: eligible for browser HSTS preload lists | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hsts_value = "max-age=31536000; includeSubDomains; preload" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.headers["Strict-Transport-Security"] = hsts_value | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # HSTS: prevent man-in-the-middle attacks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.headers["Strict-Transport-Security"] = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "max-age=31536000; includeSubDomains; preload" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Prevent MIME-sniffing attacks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.headers["X-Content-Type-Options"] = "nosniff" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Prevent clickjacking | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.headers["X-Frame-Options"] = "DENY" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # XSS mitigation via Content Security Policy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.headers["Content-Security-Policy"] = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "default-src 'self'; " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "script-src 'self'; " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "style-src 'self' 'unsafe-inline'; " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "img-src 'self' data:; " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "font-src 'self'; " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "connect-src 'self'; " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. [major]: The new CSP limits |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "frame-ancestors 'none'; " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "object-src 'none'; " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "base-uri 'self'" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.headers["Content-Security-Policy"] = ( | |
| "default-src 'self'; " | |
| "script-src 'self'; " | |
| "style-src 'self' 'unsafe-inline'; " | |
| "img-src 'self' data:; " | |
| "font-src 'self'; " | |
| "connect-src 'self'; " | |
| "frame-ancestors 'none'; " | |
| "object-src 'none'; " | |
| "base-uri 'self'" | |
| ) | |
| path = request.url.path | |
| # Apply a more permissive CSP for FastAPI's interactive API docs and schema, | |
| # which rely on CDN assets and inline scripts/styles. | |
| if path.startswith("/docs") or path.startswith("/redoc") or path.startswith( | |
| "/openapi" | |
| ): | |
| response.headers["Content-Security-Policy"] = ( | |
| "default-src 'self'; " | |
| "script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cdn.jsdelivr.net https://unpkg.com; " | |
| "style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://fonts.googleapis.com; " | |
| "img-src 'self' data: https://cdn.jsdelivr.net; " | |
| "font-src 'self' https://fonts.gstatic.com data:; " | |
| "connect-src 'self' https://api.github.com; " | |
| "frame-ancestors 'none'; " | |
| "object-src 'none'; " | |
| "base-uri 'self'" | |
| ) | |
| else: | |
| # Default CSP for the SPA and other routes: | |
| # - Allow GitHub API for fetching repository stars. | |
| # - Allow Google Fonts styles and font files. | |
| response.headers["Content-Security-Policy"] = ( | |
| "default-src 'self'; " | |
| "script-src 'self'; " | |
| "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; " | |
| "img-src 'self' data:; " | |
| "font-src 'self' https://fonts.gstatic.com; " | |
| "connect-src 'self' https://api.github.com; " | |
| "frame-ancestors 'none'; " | |
| "object-src 'none'; " | |
| "base-uri 'self'" | |
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,13 @@ | ||
| """ | ||
| Test for HSTS header presence in responses. | ||
| Test for security header presence in responses. | ||
| """ | ||
| import pytest | ||
| from fastapi.testclient import TestClient | ||
| from api.index import app | ||
|
|
||
|
|
||
| class TestHSTSHeader: | ||
| """Test HSTS security header.""" | ||
| class TestSecurityHeaders: | ||
| """Test security headers.""" | ||
|
|
||
| @pytest.fixture | ||
| def client(self): | ||
|
|
@@ -16,28 +16,63 @@ def client(self): | |
|
|
||
| def test_hsts_header_present(self, client): | ||
| """Test that the HSTS header is present in responses.""" | ||
| # Make a request to the root endpoint | ||
| response = client.get("/") | ||
|
|
||
| # Verify HSTS header is present | ||
| assert "strict-transport-security" in response.headers | ||
|
|
||
| # Verify header value contains required directives | ||
| hsts_header = response.headers["strict-transport-security"] | ||
| assert "max-age=31536000" in hsts_header | ||
| assert "includeSubDomains" in hsts_header | ||
| assert "preload" in hsts_header | ||
|
|
||
| def test_hsts_header_on_api_endpoints(self, client): | ||
| """Test that the HSTS header is present on API endpoints.""" | ||
| # Test on graphs endpoint | ||
| response = client.get("/graphs") | ||
|
|
||
| # Verify HSTS header is present | ||
| assert "strict-transport-security" in response.headers | ||
|
|
||
| # Verify header value contains required directives | ||
| hsts_header = response.headers["strict-transport-security"] | ||
| assert "max-age=31536000" in hsts_header | ||
| assert "includeSubDomains" in hsts_header | ||
| assert "preload" in hsts_header | ||
|
|
||
| def test_x_content_type_options(self, client): | ||
| """Test that X-Content-Type-Options is set to nosniff.""" | ||
| response = client.get("/") | ||
| assert response.headers.get("x-content-type-options") == "nosniff" | ||
|
|
||
| def test_x_frame_options(self, client): | ||
| """Test that X-Frame-Options is set to DENY.""" | ||
| response = client.get("/") | ||
| assert response.headers.get("x-frame-options") == "DENY" | ||
|
|
||
| def test_content_security_policy(self, client): | ||
| """Test that Content-Security-Policy header is present.""" | ||
| response = client.get("/") | ||
| csp = response.headers.get("content-security-policy") | ||
| assert csp is not None | ||
| assert "default-src 'self'" in csp | ||
| assert "script-src 'self'" in csp | ||
| assert "frame-ancestors 'none'" in csp | ||
| assert "object-src 'none'" in csp | ||
|
|
||
| def test_referrer_policy(self, client): | ||
| """Test that Referrer-Policy header is present.""" | ||
| response = client.get("/") | ||
| assert response.headers.get("referrer-policy") == "strict-origin-when-cross-origin" | ||
|
|
||
| def test_permissions_policy(self, client): | ||
| """Test that Permissions-Policy header is present.""" | ||
| response = client.get("/") | ||
| policy = response.headers.get("permissions-policy") | ||
| assert policy is not None | ||
| assert "camera=()" in policy | ||
| assert "microphone=()" in policy | ||
|
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. [major]: Permissions-Policy directives not fully asserted. The new permissions-policy test only checks camera, microphone, and geolocation, so if the middleware stops sending the required payment=() directive the regression would go unnoticed; please assert that the payment control is present to keep the test aligned with the middleware contract. Suggested fix: Add |
||
| assert "geolocation=()" in policy | ||
|
|
||
| def test_security_headers_on_api_endpoints(self, client): | ||
| """Test that all security headers are present on API endpoints.""" | ||
| response = client.get("/graphs") | ||
| assert "x-content-type-options" in response.headers | ||
| assert "x-frame-options" in response.headers | ||
| assert "content-security-policy" in response.headers | ||
| assert "referrer-policy" in response.headers | ||
| assert "permissions-policy" in response.headers | ||
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.
SecurityMiddleware returns early for forbidden static paths (403 JSONResponse) before the header-setting block, so those responses won’t get HSTS/CSP/etc. Since this PR is expanding security headers, consider refactoring header application into a small helper and calling it both on early-return responses and on the normal
call_nextresponse, so the security headers are consistently present on all responses (including 403/401/etc.).