Skip to content

🛡️ Sentinel: [MEDIUM] Add HTTP security headers #605

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.

Sign up for GitHub

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

Open
anchapin wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🛡️ Sentinel: [MEDIUM] Add HTTP security headers #605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions backend/src/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from services.report_generator import ConversionReportGenerator
from services.error_handlers import register_exception_handlers
from services.rate_limiter import RateLimitMiddleware, get_rate_limiter, init_rate_limiter, close_rate_limiter, create_global_limiter
from services.security_headers import SecurityHeadersMiddleware

# Import API routers
from api import performance, behavioral_testing, validation, comparison, embeddings, feedback, experiments, behavior_files, behavior_templates, behavior_export, advanced_events, conversions, mod_imports
Expand Down Expand Up @@ -125,6 +126,9 @@ async def lifespan(app: FastAPI):
rate_limiter = create_global_limiter()
app.add_middleware(RateLimitMiddleware, rate_limiter=rate_limiter)

# Security headers middleware
app.add_middleware(SecurityHeadersMiddleware)

@app.on_event("startup")
async def startup_event():
"""Initialize rate limiter on startup"""
Expand Down
14 changes: 14 additions & 0 deletions backend/src/services/security_headers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from starlette.middleware.base import BaseHTTPMiddleware
from fastapi import Request

class SecurityHeadersMiddleware(BaseHTTPMiddleware):
"""
Middleware to add security headers to all responses.
"""
async def dispatch(self, request: Request, call_next):
response = await call_next(request)
response.headers["X-Content-Type-Options"] = "nosniff"
response.headers["X-Frame-Options"] = "DENY"
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The X-Frame-Options header value conflicts with the Nginx configuration. The backend sets "DENY" while Nginx sets "SAMEORIGIN" (frontend/nginx.conf:48). This creates inconsistent behavior where responses from the backend have stricter settings than those served by Nginx. The values should be aligned - either both use "DENY" for maximum security or both use "SAMEORIGIN" for flexibility.

Suggested change
response.headers["X-Frame-Options"] = "DENY"
response.headers["X-Frame-Options"] = "SAMEORIGIN"

Copilot uses AI. Check for mistakes.
response.headers["X-XSS-Protection"] = "1; mode=block"
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The X-XSS-Protection header is deprecated and should be removed. Modern browsers have deprecated this header in favor of Content-Security-Policy (CSP). Chrome, Edge, and Safari have removed support for this header, and it can introduce security vulnerabilities in older browsers. Since Nginx already sets a comprehensive CSP header (frontend/nginx.conf:52), this header is redundant and potentially harmful. Consider removing it or replacing it with CSP if not already present.

Suggested change
response.headers["X-XSS-Protection"] = "1; mode=block"

Copilot uses AI. Check for mistakes.
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a Content-Security-Policy (CSP) header to the backend middleware for defense in depth. While Nginx already sets CSP (frontend/nginx.conf:52), adding it at the backend level ensures API responses are protected even if accessed directly or if the Nginx layer is bypassed. This is especially important for the backend API running on port 8080 which may be accessed directly in development or certain deployment scenarios.

Suggested change
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
if "Content-Security-Policy" not in response.headers:
response.headers[
"Content-Security-Policy"
] = "default-src 'none'; frame-ancestors 'none'; base-uri 'none'; form-action 'none'"

Copilot uses AI. Check for mistakes.
Comment on lines +10 to +13
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding Strict-Transport-Security (HSTS) header for HTTPS deployments. While the current setup uses HTTP (port 80 in Nginx), production deployments should enforce HTTPS. Adding the header conditionally based on the request scheme or environment variable would improve security posture when deployed with TLS. Example: "Strict-Transport-Security: max-age=31536000; includeSubDomains".

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Referrer-Policy header value conflicts with the Nginx configuration. The backend sets "strict-origin-when-cross-origin" while Nginx sets "no-referrer-when-downgrade" (frontend/nginx.conf:51). This creates inconsistent security postures. The backend's value is more privacy-preserving and should be aligned across both layers. Consider updating Nginx to match this setting or documenting why different policies are needed.

Suggested change
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
# NOTE: This must stay aligned with the Referrer-Policy set in Nginx (frontend/nginx.conf).
response.headers["Referrer-Policy"] = "no-referrer-when-downgrade"

Copilot uses AI. Check for mistakes.
return response
14 changes: 14 additions & 0 deletions backend/src/tests/unit/test_security_headers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from fastapi.testclient import TestClient
from src.main import app

client = TestClient(app)

def test_security_headers():
Comment on lines +1 to +6
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should use the pytest fixture pattern instead of creating a TestClient directly. The codebase has a centralized client fixture in conftest.py (backend/src/tests/conftest.py:103-134) that properly handles database mocking and dependency overrides. Using this fixture ensures consistency across tests and avoids potential issues with database initialization and cleanup. Refactor to use the client fixture as a parameter.

Suggested change
from fastapi.testclient import TestClient
from src.main import app
client = TestClient(app)
def test_security_headers():
def test_security_headers(client):

Copilot uses AI. Check for mistakes.
response = client.get("/api/v1/health")
assert response.status_code == 200

headers = response.headers
assert headers["X-Content-Type-Options"] == "nosniff"
assert headers["X-Frame-Options"] == "DENY"
assert headers["X-XSS-Protection"] == "1; mode=block"
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should verify that the deprecated X-XSS-Protection header is not present, rather than asserting it equals "1; mode=block". Since this header is deprecated and modern browsers no longer support it, the test should be updated to reflect the corrected implementation once the security header is removed from the middleware.

Suggested change
assert headers["X-XSS-Protection"] == "1; mode=block"
assert "X-XSS-Protection" not in headers

Copilot uses AI. Check for mistakes.
assert headers["Referrer-Policy"] == "strict-origin-when-cross-origin"