security: fix VAPT vulnerabilities — WebSocket CSWSH, rate limiting, …

[shuveb]

…cookie Secure flag, CSP headers

• Add Origin header validation on WebSocket endpoint to prevent cross-site hijacking (High)
• Add slowapi rate limiting on auth endpoints: login, register, me/token, resend-verification (Medium)
• Add Secure flag to frontend-set mfbt_session cookie on HTTPS (Low)
• Add Content-Security-Policy, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy headers to frontend (Next.js) and backend (FastAPI) (Low)

What

Why

How

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

Database Migrations

Env Config

Relevant Docs

Related Issues or PRs

Dependencies Versions

Notes on Testing

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

[greptile-apps]

Greptile Summary

This PR addresses four VAPT-identified vulnerabilities: Cross-Site WebSocket Hijacking (CSWSH), missing rate limiting on auth endpoints, unset Secure flag on the session cookie, and absent security headers (CSP, X-Frame-Options, etc.). The changes are generally well-structured and follow existing patterns in the codebase.

Key observations:

• Rate limiter backing store: backend/app/rate_limit.py creates the Limiter with the default in-memory store. In any multi-worker deployment (multiple uvicorn processes), each worker maintains its own independent counter, so the effective per-client limit is configured_limit × worker_count. Since Settings.redis_url is already available, the limiter should be pointed at Redis to share state across workers — otherwise the rate limiting is largely ineffective in production.
• WebSocket origin validation: The CSWSH fix is solid — localhost origins are now gated to is_development, and the decision to permit absent Origin headers for non-browser MCP/CLI clients is clearly documented.
• CSP: unsafe-eval is correctly gated to NODE_ENV === "development" only. The connect-src is derived from NEXT_PUBLIC_API_URL rather than using wildcards. Explicit object-src 'none' and media-src 'self' directives are missing but the default-src 'self' fallback covers them.
• /me/token rate limit at 30/minute may be tight for a SPA that calls this endpoint on every page load with multiple tabs open.
• Proxy trust: The TRUSTED_PROXY_IPS configuration for ProxyHeadersMiddleware is now env-driven, which addresses the previous concern about wildcard trust being a deploy-time decision.

Confidence Score: 3/5

• Mergeable but the rate limiting will not enforce correctly in multi-worker production deployments without a shared backing store.
• The security intent of each fix is sound, but the rate limiter's default in-memory store means the feature is largely ineffective in any production setup running more than one worker — which is the primary environment the feature is meant to protect. All other changes (CSWSH origin check, Secure cookie flag, security headers, CSP) are correct and low-risk.
• backend/app/rate_limit.py needs a shared Redis backing store before rate limiting will work correctly in production. backend/app/routers/auth.py — the 30/minute limit on /me/token warrants a second look given SPA usage patterns.

Important Files Changed

Filename	Overview
backend/app/rate_limit.py	New file introducing SlowAPI rate limiter — uses in-process memory by default, which will not enforce limits correctly across multiple uvicorn workers in production; a shared Redis backing store is needed.
backend/app/routers/auth.py	Rate-limit decorators added to register, login, me/token, and resend-verification; request parameter correctly injected; resend_verification parameter rename from request to verification_request is correct. The 30/minute limit on get_session_token may be too tight for normal SPA usage.
backend/app/routers/websocket.py	CSWSH mitigation added via Origin header validation; localhost origins gated behind is_development flag; deliberate decision to allow absent Origin for non-browser MCP/CLI clients is documented.
backend/app/main.py	ProxyHeadersMiddleware added with configurable trusted hosts; rate limiter wired to app state; CORS localhost origins gated to development; security headers middleware adds X-Content-Type-Options and X-Frame-Options to all responses.
frontend/next.config.ts	Security headers added including CSP (with dev-only unsafe-eval), X-Frame-Options, X-Content-Type-Options, Referrer-Policy, and Permissions-Policy. CSP connect-src correctly derived from NEXT_PUBLIC_API_URL. Missing explicit object-src/media-src directives.
frontend/lib/api/client.ts	Adds Secure flag to mfbt_session cookie when the page is served over HTTPS; two call sites updated consistently.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant NextJS as Next.js Frontend
    participant Nginx as Reverse Proxy
    participant FastAPI as FastAPI Backend
    participant WS as WebSocket Handler

    Note over Browser,NextJS: CSP + security headers on every page response

    Browser->>Nginx: POST /api/v1/auth/login
    Nginx->>FastAPI: Forwards real client IP via proxy header
    FastAPI->>FastAPI: ProxyHeadersMiddleware sets real client IP
    FastAPI->>FastAPI: SlowAPI checks rate bucket (10 per min per IP)
    alt Limit exceeded
        FastAPI-->>Browser: 429 Too Many Requests
    else Within limit
        FastAPI-->>Browser: 200 + JWT token
        Browser->>Browser: Set mfbt_session cookie (Secure flag on HTTPS)
    end

    Browser->>Nginx: WebSocket upgrade to /api/v1/ws/orgs/id/jobs
    Nginx->>FastAPI: WebSocket upgrade request
    FastAPI->>WS: Dispatch to websocket_jobs_endpoint
    WS->>WS: Read Origin header
    alt Origin absent
        WS->>WS: Allow - non-browser client, JWT auth still required
    else Origin present but not in allowed set
        WS-->>Browser: Close 1008 - CSWSH blocked
    else Origin allowed
        WS->>WS: Validate JWT token
        WS-->>Browser: WebSocket connection accepted
    end

Loading

Comments Outside Diff (1)

1. backend/app/routers/auth.py, line 136-139 (link)

   /me/token rate limit may cause false positives for normal SPA usage

   30/minute translates to one call every 2 seconds. The get_session_token endpoint is called by the frontend at initialization and likely on every page navigation in the SPA (to exchange the mfbt_session cookie for a fresh JWT). With multiple browser tabs open or background polling, a normal user can plausibly hit this limit, resulting in a 429 Too Many Requests that logs them out.

   Consider increasing this limit (e.g. 60/minute) or scoping it per-session/cookie rather than per-IP, since the cookie already binds the request to a legitimate user. Alternatively, cache the issued JWT client-side and only call this endpoint when the token is close to expiry.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: backend/app/routers/auth.py
   Line: 136-139
   
   Comment:
   **`/me/token` rate limit may cause false positives for normal SPA usage**
   
   `30/minute` translates to one call every 2 seconds. The `get_session_token` endpoint is called by the frontend at initialization and likely on every page navigation in the SPA (to exchange the `mfbt_session` cookie for a fresh JWT). With multiple browser tabs open or background polling, a normal user can plausibly hit this limit, resulting in a `429 Too Many Requests` that logs them out.
   
   Consider increasing this limit (e.g. `60/minute`) or scoping it per-session/cookie rather than per-IP, since the cookie already binds the request to a legitimate user. Alternatively, cache the issued JWT client-side and only call this endpoint when the token is close to expiry.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: backend/app/routers/auth.py
Line: 136-139

Comment:
**`/me/token` rate limit may cause false positives for normal SPA usage**

`30/minute` translates to one call every 2 seconds. The `get_session_token` endpoint is called by the frontend at initialization and likely on every page navigation in the SPA (to exchange the `mfbt_session` cookie for a fresh JWT). With multiple browser tabs open or background polling, a normal user can plausibly hit this limit, resulting in a `429 Too Many Requests` that logs them out.

Consider increasing this limit (e.g. `60/minute`) or scoping it per-session/cookie rather than per-IP, since the cookie already binds the request to a legitimate user. Alternatively, cache the issued JWT client-side and only call this endpoint when the token is close to expiry.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: frontend/next.config.ts
Line: 13-17

Comment:
**CSP missing `media-src` and `object-src` directives**

The current policy relies on `default-src 'self'` to cover `media-src` and `object-src`, which is correct, but it's best practice to explicitly set `object-src 'none'` to block Flash/plugin content and `media-src 'self'` to be explicit about intent. Without `object-src 'none'`, older browsers that pre-date the `default-src` fallback for plugins could still load untrusted plugin objects.

```typescript
"object-src 'none'",
"media-src 'self'",
```

This also makes the CSP's intent clearer for future reviewers.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: backend/app/rate_limit.py
Line: 6

Comment:
**In-memory rate limiting broken under multiple workers**

The `Limiter` is constructed here without a shared backing store, so it defaults to an in-process memory counter. In a production deployment with multiple uvicorn workers (e.g. `--workers 4`), each process holds its own independent counter. A client can therefore make `limit × worker_count` attempts per window — e.g. 5 register calls × 4 workers = 20 effective attempts per minute — defeating the purpose of the limit entirely.

The app already has a Redis connection configured in `Settings`. SlowAPI's `Limiter` can be pointed at Redis so that all workers share a single rate-limit bucket per client. Without a shared backing store, the rate limiting added in this PR will not function correctly in any multi-worker production environment.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 3d4a5c3}

[greptile-apps]

In-memory rate limiting broken under multiple workers

The Limiter is constructed here without a shared backing store, so it defaults to an in-process memory counter. In a production deployment with multiple uvicorn workers (e.g. --workers 4), each process holds its own independent counter. A client can therefore make limit × worker_count attempts per window — e.g. 5 register calls × 4 workers = 20 effective attempts per minute — defeating the purpose of the limit entirely.

The app already has a Redis connection configured in Settings. SlowAPI's Limiter can be pointed at Redis so that all workers share a single rate-limit bucket per client. Without a shared backing store, the rate limiting added in this PR will not function correctly in any multi-worker production environment.

Prompt To Fix With AI

This is a comment left during a code review.
Path: backend/app/rate_limit.py
Line: 6

Comment:
**In-memory rate limiting broken under multiple workers**

The `Limiter` is constructed here without a shared backing store, so it defaults to an in-process memory counter. In a production deployment with multiple uvicorn workers (e.g. `--workers 4`), each process holds its own independent counter. A client can therefore make `limit × worker_count` attempts per window — e.g. 5 register calls × 4 workers = 20 effective attempts per minute — defeating the purpose of the limit entirely.

The app already has a Redis connection configured in `Settings`. SlowAPI's `Limiter` can be pointed at Redis so that all workers share a single rate-limit bucket per client. Without a shared backing store, the rate limiting added in this PR will not function correctly in any multi-worker production environment.

How can I resolve this? If you propose a fix, please make it concise.