-
Notifications
You must be signed in to change notification settings - Fork 1
security fixes + codegen improvement #47
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
Changes from all commits
d7d16c5
cad5ffb
a0a7596
4c211e5
a7700af
d191626
a72428f
548f0f3
ce1dab9
74a3f9b
be278e1
8c1fd69
5579d50
045b0aa
50fe207
11be5a8
3673b88
5cfeb3b
0d0aa20
1cd005a
43d61bd
2362395
0a6250b
c179d1a
a6b0ade
38abc48
5d39626
4a0a9dc
1862b00
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 | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,23 @@ | ||||||||||||||||||||||||||||||||||||||||||||
| # GitHub Actions workflow to clean up uploaded files on Render | ||||||||||||||||||||||||||||||||||||||||||||
| # This runs every 5 hours | ||||||||||||||||||||||||||||||||||||||||||||
| # The workflow calls the cleanup API endpoint | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| name: Cleanup Uploaded Files | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||||||||
| schedule: | ||||||||||||||||||||||||||||||||||||||||||||
| # Run every 5 hours | ||||||||||||||||||||||||||||||||||||||||||||
| - cron: '0 */5 * * *' | ||||||||||||||||||||||||||||||||||||||||||||
| workflow_dispatch: # Allow manual triggering | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||||||||
| cleanup: | ||||||||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||||||||
| - name: Trigger cleanup endpoint | ||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||
| curl -X POST \ | ||||||||||||||||||||||||||||||||||||||||||||
| -H "Content-Type: application/json" \ | ||||||||||||||||||||||||||||||||||||||||||||
| "${{ secrets.CLEANUP_ENDPOINT_URL }}" \ | ||||||||||||||||||||||||||||||||||||||||||||
| -d '{"secret": "${{ secrets.CLEANUP_SECRET }}"}' | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+20
to
+23
|
||||||||||||||||||||||||||||||||||||||||||||
| curl -X POST \ | |
| -H "Content-Type: application/json" \ | |
| "${{ secrets.CLEANUP_ENDPOINT_URL }}" \ | |
| -d '{"secret": "${{ secrets.CLEANUP_SECRET }}"}' | |
| set -euo pipefail | |
| echo "Calling cleanup endpoint..." | |
| response=$( | |
| curl --fail --show-error -sS -X POST \ | |
| -H "Content-Type: application/json" \ | |
| "${{ secrets.CLEANUP_ENDPOINT_URL }}" \ | |
| -d '{"secret": "${{ secrets.CLEANUP_SECRET }}"}' | |
| ) | |
| echo "Cleanup response: $response" | |
| success=$(echo "$response" | jq -r '.success // empty') | |
| if [ "$success" != "true" ]; then | |
| echo "Cleanup endpoint reported failure (success=$success)." | |
| exit 1 | |
| fi |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,10 +25,12 @@ | |||||||||||||||||||||||||||
| # See https://docs.djangoproject.com/en/5.2/howto/deployment/checklist/ | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # SECURITY WARNING: keep the secret key used in production secret! | ||||||||||||||||||||||||||||
| SECRET_KEY = os.getenv('DJANGO_SECRET_KEY', 'django-insecure-oy%j%4%)w%7#sx@e!h+m-hai9zvl*)-5$5uz%wlro4ry1*4vc-') | ||||||||||||||||||||||||||||
| SECRET_KEY = os.getenv('DJANGO_SECRET_KEY') | ||||||||||||||||||||||||||||
| if not SECRET_KEY: | ||||||||||||||||||||||||||||
| raise ValueError("DJANGO_SECRET_KEY environment variable must be set") | ||||||||||||||||||||||||||||
|
Comment on lines
+28
to
+30
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # SECURITY WARNING: don't run with debug turned on in production! | ||||||||||||||||||||||||||||
| DEBUG = os.getenv('DJANGO_DEBUG', 'True') == 'True' | ||||||||||||||||||||||||||||
| DEBUG = os.getenv('DJANGO_DEBUG', 'False') == 'True' | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Environment Variable Validation (Production Only) | ||||||||||||||||||||||||||||
| REQUIRED_ENV_VARS = [ | ||||||||||||||||||||||||||||
|
|
@@ -106,7 +108,12 @@ | |||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # CORS configuration - read from environment variable for production | ||||||||||||||||||||||||||||
| cors_origins = os.getenv('CORS_ALLOWED_ORIGINS', 'http://localhost:3000,http://localhost:5173,http://localhost:5000') | ||||||||||||||||||||||||||||
| if DEBUG: | ||||||||||||||||||||||||||||
| cors_origins = os.getenv('CORS_ALLOWED_ORIGINS', 'http://localhost:3000,http://localhost:5173,http://localhost:5000') | ||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||
| cors_origins = os.getenv('CORS_ALLOWED_ORIGINS') | ||||||||||||||||||||||||||||
| if not cors_origins: | ||||||||||||||||||||||||||||
| raise ValueError("CORS_ALLOWED_ORIGINS environment variable must be set in production") | ||||||||||||||||||||||||||||
| CORS_ALLOWED_ORIGINS = [origin.strip() for origin in cors_origins.split(',')] | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| CORS_ALLOW_CREDENTIALS = True | ||||||||||||||||||||||||||||
|
|
@@ -125,12 +132,19 @@ | |||||||||||||||||||||||||||
| 'x-firebase-token', | ||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| csrf_origins = os.getenv('CSRF_TRUSTED_ORIGINS', 'http://localhost:3000,http://localhost:5173,http://localhost:5000') | ||||||||||||||||||||||||||||
| if DEBUG: | ||||||||||||||||||||||||||||
| csrf_origins = os.getenv('CSRF_TRUSTED_ORIGINS', 'http://localhost:3000,http://localhost:5173,http://localhost:5000') | ||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||
| csrf_origins = os.getenv('CSRF_TRUSTED_ORIGINS') | ||||||||||||||||||||||||||||
| if not csrf_origins: | ||||||||||||||||||||||||||||
| raise ValueError("CSRF_TRUSTED_ORIGINS environment variable must be set in production") | ||||||||||||||||||||||||||||
| CSRF_TRUSTED_ORIGINS = [origin.strip() for origin in csrf_origins.split(',')] | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # CSRF configuration: keep cookie HTTP-only; frontend should use X-CSRFToken header pattern | ||||||||||||||||||||||||||||
| # CSRF Protection | ||||||||||||||||||||||||||||
| CSRF_COOKIE_HTTPONLY = True | ||||||||||||||||||||||||||||
| CSRF_USE_SESSIONS = False # Use cookie-based CSRF tokens | ||||||||||||||||||||||||||||
| CSRF_USE_SESSIONS = False | ||||||||||||||||||||||||||||
| CSRF_COOKIE_SAMESITE = 'Lax' | ||||||||||||||||||||||||||||
| CSRF_HEADER_NAME = 'HTTP_X_CSRFTOKEN' | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Environment mode configuration | ||||||||||||||||||||||||||||
| # Controls API key behavior: PROD/missing = BYOK, DEV/LOCAL = server keys | ||||||||||||||||||||||||||||
|
|
@@ -139,18 +153,16 @@ | |||||||||||||||||||||||||||
| REQUIRES_USER_API_KEY = IS_PRODUCTION | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
| # Django REST Framework configuration | |
| # NOTE: | |
| # - We require authentication by default for all modifying requests via | |
| # IsAuthenticatedOrReadOnly. This aligns with the "authentication by default" | |
| # guideline and recent security hardening PRs. | |
| # - Endpoints that truly need to be public (e.g. certain validation, chat, or | |
| # export endpoints) may explicitly use @permission_classes([AllowAny]) in | |
| # their view modules, but such use must be rare and documented in that | |
| # module's docstring explaining why the endpoint is unauthenticated. | |
| # - When adding new views, prefer the global default and avoid AllowAny unless | |
| # there is a clear, reviewed product requirement for a public endpoint. |
Copilot
AI
Dec 28, 2025
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.
CSP_SCRIPT_SRC includes both 'unsafe-inline' and 'unsafe-eval', which significantly weaken the Content Security Policy and allow XSS attacks. These directives permit inline scripts and eval(), which are common XSS vectors. If these are required for specific libraries (e.g., Firebase), consider using nonces or hashes for inline scripts, or refactor the code to eliminate the need for these unsafe directives. Document which dependencies require these settings.
| CSP_SCRIPT_SRC = ("'self'", "'unsafe-inline'", "'unsafe-eval'", "https://www.gstatic.com", "https://apis.google.com") | |
| # NOTE: 'unsafe-inline' is currently retained for existing inline scripts in the frontend. | |
| # No known dependency requires eval(), so 'unsafe-eval' is intentionally omitted to reduce XSS risk. | |
| CSP_SCRIPT_SRC = ("'self'", "'unsafe-inline'", "https://www.gstatic.com", "https://apis.google.com") |
Copilot
AI
Dec 28, 2025
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.
The CSP_IMG_SRC directive allows 'https:' which permits images from ANY https origin. This is overly permissive and defeats much of the purpose of CSP. Specify explicit trusted domains instead, such as the domains actually used by your application (Firebase Storage, etc.). If a wildcard is truly necessary, document the security rationale.
| CSP_IMG_SRC = ("'self'", "data:", "https:", "blob:") | |
| CSP_IMG_SRC = ("'self'", "data:", "blob:", "https://firebasestorage.googleapis.com", "https://www.gstatic.com") |
Copilot
AI
Dec 28, 2025
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.
Creating the upload directory at module import time (during settings.py execution) can cause issues in several scenarios: 1) Settings may be imported in contexts where filesystem operations are not appropriate (e.g., during migrations, management commands), 2) The BASE_DIR may not be writable in containerized deployments, 3) This violates the principle of lazy initialization. Move this directory creation to application startup (e.g., in AppConfig.ready()) or to the first use within the file cleanup utilities.
| # Create upload directory if it doesn't exist | |
| TEMP_UPLOAD_DIR.mkdir(exist_ok=True) |
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.
The GitHub Actions workflow secret CLEANUP_ENDPOINT_URL is used directly without validation. If this URL is misconfigured or points to an unintended endpoint, the cleanup secret could be exposed to a third party. Add validation in the workflow or documentation to ensure the URL must be an HTTPS endpoint and should match expected domain patterns. Consider using environment-specific secrets or adding a verification step.