Set APP_ENV to force HTTPS in production #164
Conversation
WalkthroughIntroduces APP_ENV-based session cookie HTTPS toggle in the application factory. Updates documentation and example configuration to describe and demonstrate APP_ENV usage, OAuth development settings, and sample email configuration. Adds APP_ENV to E2E testing env notes. No exported/public APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Proc as Process Startup
participant Env as Environment
participant App as App Factory
participant MW as SessionMiddleware
participant Client as Client
Proc->>Env: Read APP_ENV
Env-->>App: "production" / "staging" / "development"
App->>App: Compute is_https = APP_ENV in {production, staging}
App->>MW: Initialize with https_only = is_https
note right of MW: https_only=True enforces HTTPS cookies
rect rgba(230,245,255,0.5)
Client->>MW: HTTP(S) Request
MW-->>Client: Set/Send session cookie (HTTPS-only if enabled)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/app_factory.py (2)
27-31: Silence Pylint R0903 on middleware class.This class intentionally has a single public method.
-class SecurityMiddleware(BaseHTTPMiddleware): +class SecurityMiddleware(BaseHTTPMiddleware): # pylint: disable=too-few-public-methods
115-119: Fix Pylint W0613 and remove FIXMEs.Rename the unused request parameter and downgrade the TODO to a note to pass lint.
- async def handle_oauth_error(request: Request, exc: Exception): + async def handle_oauth_error(_request: Request, exc: Exception): """Handle OAuth-related errors gracefully""" # Check if it's an OAuth-related error - # TODO check this scenario + # Note: consider refining this condition to specific OAuth exceptions
🧹 Nitpick comments (9)
tests/e2e/README.md (1)
118-134: Add a dev-only hint for local OAuth runs.Include OAUTHLIB_INSECURE_TRANSPORT to reduce confusion when folks try OAuth locally.
# Required for FastAPI FASTAPI_SECRET_KEY=your-secret-key APP_ENV=development FASTAPI_DEBUG=False + +# For local OAuth testing only (NEVER set in staging/production) +# OAUTHLIB_INSECURE_TRANSPORT=1api/app_factory.py (2)
58-59: Avoid trusting all proxy headers in production.Reading trusted proxies from an env var is safer than "*".
- app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*") + app.add_middleware( + ProxyHeadersMiddleware, + trusted_hosts=os.getenv("TRUSTED_PROXY_HOSTS", "*") + )Consider documenting TRUSTED_PROXY_HOSTS in .env.example as a comma-separated list (e.g., "127.0.0.1,::1,nginx").
114-129: Narrow the OAuth error catch or add metrics.Catching Exception may hide unrelated errors containing the words “oauth” or “token”. Consider catching specific exceptions from authlib or emit a metric/structured log to aid debugging.
.github/copilot-instructions.md (1)
140-160: Document dev-only OAuth flag alongside APP_ENV.Small doc tweak to help contributors succeed locally.
# Optional: set application environment (development, staging, production) # Default: development (affects session cookie security for OAuth) APP_ENV=development + +# For local OAuth over HTTP only (never in staging/production) +# OAUTHLIB_INSECURE_TRANSPORT=1.env.example (2)
23-29: Add proxy hardening knob.Expose TRUSTED_PROXY_HOSTS to avoid trusting all X-Forwarded-* headers in prod.
# Default: development # APP_ENV=development + +# Optional: restrict which reverse proxies' headers are trusted (comma-separated) +# TRUSTED_PROXY_HOSTS=127.0.0.1,::1,nginx
23-29: Consider documenting cookie domain override.If you ever need to share cookies across subdomains, a domain override is useful.
# APP_ENV=development + +# Optional: session cookie domain override (e.g., .queryweaver.ai) +# SESSION_COOKIE_DOMAIN=.queryweaver.aiREADME.md (3)
42-51: Good call adding APP_ENV to docker run. Also mention proxy header.Add a short note reminding operators to set X-Forwarded-Proto at the proxy when terminating TLS.
-e AZURE_API_KEY=your_azure_api_key \ falkordb/queryweaver + +# Note: If running behind a reverse proxy with TLS termination, +# ensure it sets X-Forwarded-Proto=https so the app sees HTTPS.
294-307: Tighten the prod OAuth guidance.Add a reminder to update IdP redirect URIs to https:///... when switching to production/staging.
**Important**: If you're getting "mismatching_state: CSRF Warning!" errors on staging/production, ensure `APP_ENV` is set to `production` or `staging` to enable secure session handling. +Also confirm your OAuth app redirect URIs use your HTTPS host +(e.g., https://staging.queryweaver.ai/login/google/authorized).
363-363: Add a quick cookie-debug tip.One-liner to help diagnose lost session cookies.
- **OAuth "mismatching_state: CSRF Warning!" errors**: Set `APP_ENV=production` (or `staging`) in your environment for HTTPS deployments, or `APP_ENV=development` for HTTP development environments. This ensures session cookies are configured correctly for your deployment type. + - **OAuth "mismatching_state: CSRF Warning!" errors**: Set `APP_ENV=production` (or `staging`) in your environment for HTTPS deployments, or `APP_ENV=development` for HTTP development environments. This ensures session cookies are configured correctly for your deployment type. In the browser devtools, verify the `qw_session` cookie has `Secure` on HTTPS and is present on the callback response.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.env.example(2 hunks).github/copilot-instructions.md(2 hunks)README.md(4 hunks)api/app_factory.py(1 hunks)tests/e2e/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-24T17:15:21.337Z
Learnt from: CR
PR: FalkorDB/QueryWeaver#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-24T17:15:21.337Z
Learning: Applies to .env.example : Ensure `.env.example` includes all required environment variables (FastAPI secret/debug, FalkorDB host/port, OAuth client IDs/secrets) with placeholder values
Applied to files:
tests/e2e/README.md.github/copilot-instructions.mdREADME.md.env.example
📚 Learning: 2025-08-24T17:15:21.337Z
Learnt from: CR
PR: FalkorDB/QueryWeaver#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-24T17:15:21.337Z
Learning: Applies to Dockerfile : Keep Docker configuration consistent with documented run commands (exposing port 5000, running uvicorn with `api.index:app`)
Applied to files:
README.md
🪛 GitHub Actions: Pylint
api/app_factory.py
[error] 118-118: W0511: TODO check this scenario (fixme)
[error] 27-27: R0903: Too few public methods (1/2) (too-few-public-methods)
[error] 63-63: R1703: The if statement can be replaced with 'var = bool(test)' (simplifiable-if-statement)
[error] 115-115: W0613: Unused argument 'request' (unused-argument)
🔇 Additional comments (4)
api/app_factory.py (1)
72-74: Confirm SameSite policy won’t break OAuth in your deployment.SameSite="lax" typically works for top-level OAuth GET redirects; if your IdP flow involves cross-site POSTs or if you embed flows, you may need SameSite=None (with https_only=True).
.github/copilot-instructions.md (1)
162-169: LGTM: minimal .env example now includes APP_ENV..env.example (1)
78-88: Confirm or remove unused email config variables. I couldn’t find any references to MAIL_SERVER, MAIL_PORT, MAIL_USE_TLS, MAIL_USERNAME, MAIL_PASSWORD, MAIL_DEFAULT_SENDER, or EMAIL_AUTH_ENABLED in the codebase; please implement their consumption or remove them from .env.example.README.md (1)
255-256: LGTM: clear guidance to set APP_ENV=development locally.
| app_env = os.getenv("APP_ENV", "").lower() | ||
| if app_env in ("production", "staging"): | ||
| is_https = True | ||
| else: | ||
| is_https = False | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify boolean and add startup log; fixes Pylint R1703.
Also adds a helpful log for debugging env-related cookie issues.
- app_env = os.getenv("APP_ENV", "").lower()
- if app_env in ("production", "staging"):
- is_https = True
- else:
- is_https = False
+ app_env = os.getenv("APP_ENV", "").lower()
+ is_https = app_env in ("production", "staging")
+ logging.info("Session cookie Secure=%s (APP_ENV=%s)", is_https, app_env or "(unset)")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| app_env = os.getenv("APP_ENV", "").lower() | |
| if app_env in ("production", "staging"): | |
| is_https = True | |
| else: | |
| is_https = False | |
| app_env = os.getenv("APP_ENV", "").lower() | |
| is_https = app_env in ("production", "staging") | |
| logging.info("Session cookie Secure=%s (APP_ENV=%s)", is_https, app_env or "(unset)") |
🧰 Tools
🪛 GitHub Actions: Pylint
[error] 63-63: R1703: The if statement can be replaced with 'var = bool(test)' (simplifiable-if-statement)
🤖 Prompt for AI Agents
In api/app_factory.py around lines 62 to 67, replace the verbose if/else boolean
assignment with a direct boolean expression (is_https = app_env in
("production", "staging")) to address Pylint R1703, and add a startup log
statement that records the resolved APP_ENV and resulting is_https value to help
debug environment/cookie issues (use the module or app logger at startup so this
runs once during app creation).
Users are getting this error when trying to login with Google
on staging.queryweaver.ai
Summary by CodeRabbit