Harden FARM Django security configuration defaults#912
Conversation
WalkthroughThe PR converts Django settings from hardcoded development defaults to environment-driven configuration. It adds an ChangesDjango Settings Environment-Driven Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 1 high |
🟢 Metrics 0 complexity
Metric Results Complexity 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements environment-driven configuration for DEBUG, ALLOWED_HOSTS, and security settings in settings.py, including a check to ensure a secret key is set in production. Review feedback points out that SECURE_SSL_REDIRECT may cause infinite loops behind proxies without SECURE_PROXY_SSL_HEADER and notes that SECURE_BROWSER_XSS_FILTER is deprecated, suggesting the use of Content Security Policy (CSP) instead.
| SECURE_REFERRER_POLICY = "same-origin" | ||
| CSRF_COOKIE_SECURE = env_bool("DJANGO_CSRF_COOKIE_SECURE", default=not DEBUG) | ||
| SESSION_COOKIE_SECURE = env_bool("DJANGO_SESSION_COOKIE_SECURE", default=not DEBUG) | ||
| SECURE_SSL_REDIRECT = env_bool("DJANGO_SECURE_SSL_REDIRECT", default=not DEBUG) |
There was a problem hiding this comment.
Enabling SECURE_SSL_REDIRECT by default in production (DEBUG=False) is a strong security measure, but it frequently causes infinite redirect loops when the application is deployed behind a reverse proxy (like Nginx, Traefik, or a Cloud Load Balancer) that terminates SSL and forwards requests via HTTP.
To prevent this, you typically need to configure SECURE_PROXY_SSL_HEADER.
Warning: Only enable this if you are behind a trusted proxy that is configured to strip the X-Forwarded-Proto header from incoming client requests to prevent spoofing.
| SECURE_SSL_REDIRECT = env_bool("DJANGO_SECURE_SSL_REDIRECT", default=not DEBUG) | |
| SECURE_SSL_REDIRECT = env_bool("DJANGO_SECURE_SSL_REDIRECT", default=not DEBUG) | |
| # If the app is behind a trusted proxy that terminates SSL, you likely need the following: | |
| # SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https") |
|
|
||
|
|
||
| # Basic production security hardening. These can be tuned via environment. | ||
| SECURE_BROWSER_XSS_FILTER = True |
There was a problem hiding this comment.
The SECURE_BROWSER_XSS_FILTER setting is deprecated in Django as modern browsers no longer support the X-XSS-Protection header. While it remains harmless, it provides minimal security benefit today. For modern protection against XSS, it is recommended to implement a Content-Security-Policy (CSP) header instead.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@FARM/FARM/settings.py`:
- Line 28: Run ruff format to auto-fix the blank-line formatting errors in the
FARM settings module: invoke ruff format on the settings module (or run ruff
format across the repo), verify the module-level blank lines and import/section
spacing in the settings module are corrected to satisfy ruff, and commit the
updated file so the CI ruff format --check passes.
- Line 138: Remove the deprecated setting SECURE_BROWSER_XSS_FILTER from
settings.py: locate the line setting SECURE_BROWSER_XSS_FILTER = True and delete
it (no replacement is needed because Django 4.0+ ignores this setting).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 284b0067-0337-4c2c-a272-c2cb6db356a0
📒 Files selected for processing (1)
FARM/FARM/settings.py
| # SECURITY WARNING: keep the secret key used in production secret! | ||
| SECRET_KEY = os.environ.get("DJANGO_SECRET_KEY", "dev-insecure-key-change-me") | ||
|
|
||
|
|
There was a problem hiding this comment.
Fix Ruff formatting issues.
The pipeline indicates ruff format --check failed due to blank line formatting. Run ruff format FARM/FARM/settings.py to auto-fix.
🔧 How to fix
ruff format FARM/FARM/settings.py🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@FARM/FARM/settings.py` at line 28, Run ruff format to auto-fix the blank-line
formatting errors in the FARM settings module: invoke ruff format on the
settings module (or run ruff format across the repo), verify the module-level
blank lines and import/section spacing in the settings module are corrected to
satisfy ruff, and commit the updated file so the CI ruff format --check passes.
|
|
||
|
|
||
| # Basic production security hardening. These can be tuned via environment. | ||
| SECURE_BROWSER_XSS_FILTER = True |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Django version and check for SECURE_BROWSER_XSS_FILTER references
rg -n "django.*version|Django.*version" --type=py -g 'requirements*.txt' -g 'setup.py' -g 'pyproject.toml'Repository: Smartappli/AIMER
Length of output: 42
🌐 Web query:
Is SECURE_BROWSER_XSS_FILTER deprecated in Django 6.0?
💡 Result:
No. SECURE_BROWSER_XSS_FILTER is not deprecated in Django 6.0 because it was already removed from Django (so it is not available to deprecate there). [1][2] Evidence: Django core includes a change titled “Fixed #32678 — Removed SECURE_BROWSER_XSS_FILTER setting.” [1] The associated ticket records the setting as removed (“Resolution: fixed”). [2]
Citations:
- 1: Fixed #32678 -- Removed SECURE_BROWSER_XSS_FILTER setting. django/django#14306
- 2: https://code.djangoproject.com/ticket/32678
🏁 Script executed:
head -20 FARM/FARM/settings.pyRepository: Smartappli/AIMER
Length of output: 564
Remove deprecated SECURE_BROWSER_XSS_FILTER.
SECURE_BROWSER_XSS_FILTER was removed from Django in version 4.0 and has no effect in Django 6.0.1. This setting should be removed.
Proposed fix
-SECURE_BROWSER_XSS_FILTER = True
SECURE_CONTENT_TYPE_NOSNIFF = True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@FARM/FARM/settings.py` at line 138, Remove the deprecated setting
SECURE_BROWSER_XSS_FILTER from settings.py: locate the line setting
SECURE_BROWSER_XSS_FILTER = True and delete it (no replacement is needed because
Django 4.0+ ignores this setting).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24892638c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not DEBUG and SECRET_KEY == "dev-insecure-key-change-me": | ||
| msg = "DJANGO_SECRET_KEY must be set in non-debug environments." | ||
| raise RuntimeError(msg) |
There was a problem hiding this comment.
With no environment variables set, this new guard now raises RuntimeError during settings import because DEBUG defaults to False while SECRET_KEY still defaults to the insecure placeholder. That means a fresh checkout cannot start ASGI or run any Django management command unless callers preconfigure DJANGO_DEBUG or DJANGO_SECRET_KEY, which is a regression from the previous out-of-the-box behavior and will break local/dev automation that relies on defaults.
Useful? React with 👍 / 👎.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ] | ||
|
|
||
| ALLOWED_HOSTS = [] | ||
| if not DEBUG and SECRET_KEY == "dev-insecure-key-change-me": |
2 participants
Motivation
DEBUG=True, emptyALLOWED_HOSTS, and a development fallbackSECRET_KEY) and lacked baseline production security flags, risking accidental insecure deployments.Description
FARM/FARM/settings.pyto add anenv_bool()helper and readDJANGO_DEBUGandDJANGO_ALLOWED_HOSTSfrom the environment instead of using hardcoded values.DEBUG = TruewithDEBUG = env_bool("DJANGO_DEBUG", default=False)and setALLOWED_HOSTSto a safe default oflocalhost,127.0.0.1when not provided.RuntimeErrorif the default insecure secret key is still present whileDEBUGis disabled.SECURE_BROWSER_XSS_FILTER,SECURE_CONTENT_TYPE_NOSNIFF,X_FRAME_OPTIONS = "DENY",SECURE_REFERRER_POLICY, secure CSRF/session cookies, andSECURE_SSL_REDIRECT, all tunable via environment variables.Testing
python -m py_compile FARM/FARM/settings.py, which completed without errors.Codex Task
Summary by CodeRabbit
Release Notes
Security
Improvements