security: harden production settings for GraphQL, HSTS, and HTTP headers (fixes #3783)#3799
security: harden production settings for GraphQL, HSTS, and HTTP headers (fixes #3783)#3799arkid15r merged 18 commits intoOWASP:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary by CodeRabbit
WalkthroughAdds several Django security settings to base and environment configs, conditionally disables GraphQL introspection in non-DEBUG/non-FUZZ production, and relaxes those security settings in local/e2e/fuzz/test environments; staging only has whitespace cleanup. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
Confidence score: 2/5
- Missing
SECURE_PROXY_SSL_HEADERinbackend/settings/production.pycan cause Django to mis-detect HTTPS behind a proxy, which may break SSL redirects or security assumptions in production. enable_introspectionis not a validStrawberryConfigoption inbackend/settings/graphql.py, so introspection disabling may not work as intended at runtime.- These are high-severity, production-affecting configuration issues, so the merge risk is elevated.
- Pay close attention to
backend/settings/production.pyandbackend/settings/graphql.py- proxy HTTPS detection and GraphQL introspection configuration.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/settings/production.py">
<violation number="1" location="backend/settings/production.py:62">
P1: Missing `SECURE_PROXY_SSL_HEADER` setting. When running behind a load balancer or reverse proxy (common in AWS deployments), Django needs this setting to properly detect HTTPS connections. Without it, `SECURE_SSL_REDIRECT = True` can cause infinite redirect loops because Django sees all requests as HTTP.
Add this setting to inform Django about the proxy's forwarded protocol header.</violation>
</file>
<file name="backend/settings/graphql.py">
<violation number="1" location="backend/settings/graphql.py:51">
P1: `enable_introspection` is not a valid parameter for `StrawberryConfig`. According to Strawberry GraphQL documentation, introspection should be disabled using the `DisableIntrospection` extension instead.
To conditionally disable introspection in production:
```python
from strawberry.extensions import DisableIntrospection
extensions = [QueryDepthLimiter(max_depth=5), DjangoOptimizerExtension()]
if not settings.DEBUG and not getattr(settings, "IS_FUZZ_ENVIRONMENT", False):
extensions.append(DisableIntrospection())
schema = strawberry.Schema(
mutation=Mutation,
query=Query,
extensions=extensions,
)
```</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
This PR implements security hardening improvements aligned with OWASP recommendations to strengthen the application's production security posture. The changes focus on disabling GraphQL introspection in production, enforcing HTTPS/HSTS, and preventing MIME sniffing attacks. Additionally, unrelated Dockerfile optimizations consolidate RUN commands to reduce Docker layers.
Changes:
- Disabled GraphQL introspection when DEBUG=False, with exception for fuzz testing environments
- Added HSTS and SSL redirect settings to production environment (1 year HSTS with subdomains and preload)
- Enabled X-Content-Type-Options: nosniff header globally via SECURE_CONTENT_TYPE_NOSNIFF
- Consolidated multiple RUN commands in frontend Dockerfile (unrelated to security hardening)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/settings/graphql.py | Adds StrawberryConfig to disable GraphQL introspection in production while preserving it for DEBUG and fuzz testing |
| backend/settings/production.py | Configures HSTS with 1-year expiration, subdomain inclusion, preload support, and SSL redirect enforcement |
| backend/settings/base.py | Adds SECURE_CONTENT_TYPE_NOSNIFF globally to prevent MIME type sniffing |
| docker/frontend/Dockerfile | Consolidates RUN commands with && operators to reduce Docker layers (unrelated to security scope) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
backend/settings/production.py
Outdated
| # Security | ||
| SECURE_HSTS_SECONDS = 31536000 | ||
| SECURE_HSTS_INCLUDE_SUBDOMAINS = True | ||
| SECURE_HSTS_PRELOAD = True | ||
| SECURE_SSL_REDIRECT = True |
There was a problem hiding this comment.
The staging environment (backend/settings/staging.py) should also include the same HSTS and SSL security settings as production. Staging environments typically serve over HTTPS with valid certificates and should mirror production security configurations to catch issues before they reach production. Consider adding the same SECURE_HSTS_SECONDS, SECURE_HSTS_INCLUDE_SUBDOMAINS, SECURE_HSTS_PRELOAD, and SECURE_SSL_REDIRECT settings to staging.py.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
Could you resolve copilot comments?
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="docker/frontend/Dockerfile">
<violation number="1" location="docker/frontend/Dockerfile:13">
P0: Setting `PATH="$PNPM_HOME:$PATH"` in the same ENV instruction where `PNPM_HOME` is defined will not work correctly. In Docker, variables set in a single ENV instruction are not available for expansion within that same instruction. The `$PNPM_HOME` variable will be empty (undefined), so PATH will not include `/pnpm`, causing pnpm commands to fail.
Split this into two separate ENV instructions to ensure PNPM_HOME is defined before being used in PATH expansion.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docker/frontend/Dockerfile`:
- Around line 8-13: The ENV block uses PNPM_HOME and PATH in the same
instruction so $PNPM_HOME won't expand; split the ENV into two instructions:
first define PNPM_HOME (and any cache vars that don't reference PNPM_HOME) using
ENV PNPM_HOME="/pnpm" (and keep NPM_CACHE, APK_CACHE_DIR, APK_SYMLINK_DIR,
FORCE_COLOR if you prefer), then a second ENV to set PATH="$PNPM_HOME:$PATH" so
the PATH expansion picks up the just-defined PNPM_HOME; update the Dockerfile's
ENV instructions accordingly (look for PNPM_HOME and PATH).
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="docker/frontend/Dockerfile">
<violation number="1">
P1: The `-f` flag was removed from `rm .env`. Without it, the build will fail if `.env` doesn't exist in the standalone output. The original code used `rm -f .env` to handle this case gracefully.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Added security settings for HSTS and SSL redirection.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/settings/staging.py`:
- Around line 54-59: The staging settings are missing SECURE_PROXY_SSL_HEADER
which causes SSL redirect loops behind a TLS-terminating proxy; add the same
header tuple used in production (SECURE_PROXY_SSL_HEADER =
("HTTP_X_FORWARDED_PROTO", "https")) into staging.py alongside
SECURE_HSTS_SECONDS / SECURE_SSL_REDIRECT so Django treats X-Forwarded-Proto as
the scheme when SECURE_SSL_REDIRECT is True, matching the behavior of
production.py.
Refactor GraphQL schema extensions and configuration.
|
Let me know is this correct, i apply all the required changes into them.. |
backend/settings/production.py
Outdated
| SLACK_COMMANDS_ENABLED = True | ||
| SLACK_EVENTS_ENABLED = True | ||
|
|
||
| # Security |
There was a problem hiding this comment.
A believe a more reasonable default would be to have security settings shared by staging/production via base.py and have them overridden in local as needed.
backend/settings/graphql.py
Outdated
| DjangoOptimizerExtension(), | ||
| ] | ||
|
|
||
| if not settings.DEBUG and not getattr(settings, "IS_FUZZ_ENVIRONMENT", False): |
There was a problem hiding this comment.
Let's make settings.IS_FUZZ_ENVIRONMENT work.
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/settings/base.py">
<violation number="1" location="backend/settings/base.py:239">
P1: `SECURE_SSL_REDIRECT = True` in the Base configuration is not overridden in `test.py`. The local, e2e, and fuzz configurations all correctly override this to `False`, but the test configuration does not. This will cause Django's `SecurityMiddleware` to 301-redirect all HTTP requests to HTTPS during tests, which can break test assertions expecting direct responses.
Either add `SECURE_SSL_REDIRECT = False` to `backend/settings/test.py`, or move `SECURE_SSL_REDIRECT = True` out of `base.py` and into only the configurations that need it (production, staging).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| SECURE_HSTS_PRELOAD = True | ||
| SECURE_HSTS_SECONDS = 31536000 | ||
| SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https") | ||
| SECURE_SSL_REDIRECT = True |
There was a problem hiding this comment.
P1: SECURE_SSL_REDIRECT = True in the Base configuration is not overridden in test.py. The local, e2e, and fuzz configurations all correctly override this to False, but the test configuration does not. This will cause Django's SecurityMiddleware to 301-redirect all HTTP requests to HTTPS during tests, which can break test assertions expecting direct responses.
Either add SECURE_SSL_REDIRECT = False to backend/settings/test.py, or move SECURE_SSL_REDIRECT = True out of base.py and into only the configurations that need it (production, staging).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/settings/base.py, line 239:
<comment>`SECURE_SSL_REDIRECT = True` in the Base configuration is not overridden in `test.py`. The local, e2e, and fuzz configurations all correctly override this to `False`, but the test configuration does not. This will cause Django's `SecurityMiddleware` to 301-redirect all HTTP requests to HTTPS during tests, which can break test assertions expecting direct responses.
Either add `SECURE_SSL_REDIRECT = False` to `backend/settings/test.py`, or move `SECURE_SSL_REDIRECT = True` out of `base.py` and into only the configurations that need it (production, staging).</comment>
<file context>
@@ -231,3 +232,8 @@ class Base(Configuration):
+ SECURE_HSTS_PRELOAD = True
+ SECURE_HSTS_SECONDS = 31536000
+ SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https")
+ SECURE_SSL_REDIRECT = True
</file context>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3799 +/- ##
=======================================
Coverage 95.39% 95.39%
=======================================
Files 463 463
Lines 14540 14540
Branches 2017 2017
=======================================
Hits 13871 13871
Misses 328 328
Partials 341 341
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/settings/base.py`:
- Line 31: Update the test and fuzz settings to disable production-only security
flags that break HTTP test runs: in test.py add overrides
CSRF_COOKIE_SECURE=False, SESSION_COOKIE_SECURE=False,
SECURE_SSL_REDIRECT=False, SECURE_HSTS_SECONDS=0 and
SECURE_PROXY_SSL_HEADER=None (matching local.py/e2e.py pattern) so tests won’t
be redirected or drop cookies; in fuzz.py add CSRF_COOKIE_SECURE=False and
SESSION_COOKIE_SECURE=False in addition to its existing
SECURE_SSL_REDIRECT/SECURE_HSTS_SECONDS overrides to ensure fuzz tests run over
plain HTTP.
In `@backend/settings/fuzz.py`:
- Around line 18-20: The fuzz settings inherit CSRF_COOKIE_SECURE=True and
SESSION_COOKIE_SECURE=True from Base which prevents cookies over plain HTTP; in
backend/settings/fuzz.py add explicit overrides CSRF_COOKIE_SECURE = False and
SESSION_COOKIE_SECURE = False (alongside SECURE_HSTS_SECONDS,
SECURE_PROXY_SSL_HEADER, SECURE_SSL_REDIRECT) so the fuzz environment will send
cookies over http://localhost:9500 and CSRF/session handling works during tests.
|
Proposed change
Resolves #3783
This PR implements several security hardening improvements aligned with OWASP recommendations to strengthen the application’s production security posture.
Summary of changes
GraphQL Introspection
DEBUG = Falseinbackend/settings/graphql.pyHTTPS & HSTS Enforcement
SECURE_SSL_REDIRECT,SECURE_HSTS_SECONDS,SECURE_HSTS_INCLUDE_SUBDOMAINS, andSECURE_HSTS_PRELOADinbackend/settings/production.pyMIME Sniffing Protection
SECURE_CONTENT_TYPE_NOSNIFF = Trueinbackend/settings/base.pyX-Content-Type-Options: nosniffheader to prevent content-type sniffing attacksThese changes improve security without affecting existing functionality and bring the project closer to OWASP best practices.
Checklist
make check-testlocally: all warnings addressed, tests passed