diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..317f15c --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,462 @@ +# Security Review — AI Gateway + +**Date:** 2026-04-02 +**Reviewer:** Claude Code (automated static analysis) +**Scope:** Full codebase (`d:\Antigravity Apps\ai_gateway`) + +--- + +## Summary + +| Severity | Count | +|---|---| +| Critical | 4 | +| High | 13 | +| Medium | 10 | +| Low | 9 | +| Informational | 3 | +| **Total** | **39** | + +--- + +## Immediate Action Items + +> Do these before any other work. + +1. **Separate `API_KEY` and `SECRET_KEY`** — they are currently set to the same value; a leaked API key allows forging admin JWTs. +2. **Remove hardcoded fallback credentials** in `app/core/config.py` (`"admin123"`, `"storyline-secret-key-123"`). +3. **Add rate limiting to login** (`/auth/login`) — currently bruteforceable. +4. **Consider runtime secret injection** (Docker secrets, AWS Secrets Manager) so credentials are never written to disk at all. + +--- + +## Critical + +### C-1 — Plaintext Secrets in `.env` File +**File:** `.env` lines 1–10 + +Multiple live credentials stored in plaintext. The `.env` file is correctly listed in `.gitignore` and is **not committed to the repository**, so there is no git history exposure. However, the secrets still warrant attention: + +| Variable | Issue | +|---|---| +| `API_KEY` | Same value as `SECRET_KEY` — two independent secrets should not share a value | +| `GOOGLE_API_KEY` | Live Google API key — should be scoped to minimum required APIs/quotas | +| `DATABASE_URL` | Full Supabase connection string with embedded password | +| `ADMIN_PASSWORD` | Plaintext in a file on disk; no rotation mechanism | +| `SECRET_KEY` | Identical to `API_KEY` — compromising one compromises both | + +**Remaining concerns:** +- Anyone with filesystem access to the server (compromised container, misconfigured volume mount, log leak of `DATABASE_URL`) gets all credentials at once. +- `SECRET_KEY` and `API_KEY` being the same value means a leaked API key also forges JWTs. + +**Fix:** Use a secrets manager or environment injection at runtime (Docker secrets, AWS Secrets Manager, Doppler) so credentials are never written to disk. At minimum, use separate values for `API_KEY` and `SECRET_KEY`. + +--- + +### C-2 — Weak Default Credentials in Config +**File:** [app/core/config.py](app/core/config.py) lines 8, 13, 16 + +```python +API_KEY: str = os.getenv("API_KEY", "storyline-secret-key-123") +SECRET_KEY: str = os.getenv("SECRET_KEY", "your-super-secret-key-change-me") +ADMIN_PASSWORD: str = os.getenv("ADMIN_PASSWORD", "admin123") +``` + +If environment variables are absent, the app falls back to trivially guessable values. + +**Fix:** Remove all default values. Raise a `RuntimeError` on startup if these are not set. + +--- + +### C-3 — Fully Permissive CORS +**File:** [app/main.py](app/main.py) lines 49–56 + +```python +CORSMiddleware( + allow_origins=["*"], + allow_credentials=True, + allow_methods=["*"], + allow_headers=["*"], +) +``` + +`allow_origins=["*"]` combined with `allow_credentials=True` is both a browser-rejected combination and a clear signal of over-permissive intent. Any domain can make cross-origin requests. + +**Fix:** +```python +allow_origins = os.getenv("CORS_ORIGINS", "https://your-frontend.com").split(",") +``` + +--- + +### C-4 — Hardcoded Admin Credentials with No Password Hashing +**File:** [app/api/endpoints/auth.py](app/api/endpoints/auth.py) lines 14–26 + +```python +if form_data.username == "admin" and form_data.password == settings.ADMIN_PASSWORD: +``` + +- Password is plaintext comparison — `passlib` is a dependency but never used. +- No rate limiting on the login endpoint → bruteforceable. +- Single static admin account with no database backing. + +**Fix:** Use a proper user table with bcrypt-hashed passwords. Add `@limiter.limit("5/minute")` to the login route. + +--- + +## High + +### H-1 — Debug Print Statements Leak Auth Info in Production +**File:** [app/api/deps.py](app/api/deps.py) lines 23–28 + +```python +print(f"DEBUG: API Key received (prefix): {api_key[:5]}...") +``` + +First 5 chars of each API key are printed to stdout on every authenticated request. `ENVIRONMENT=production` is set in `.env`. + +**Fix:** Remove all `print()` debug statements. Use structured logging gated on `DEBUG` level. + +--- + +### H-2 — Request Logging Leaks URLs and Origins +**File:** [app/main.py](app/main.py) lines 40–47 + +Every incoming request, including URL (which may contain query-param secrets) and `Origin` header, is logged at INFO level in production. + +**Fix:** Gate debug logging behind `settings.ENVIRONMENT == "development"`. Sanitize URLs before logging. + +--- + +### H-3 — No Rate Limiting on Admin Endpoints +**File:** [app/api/endpoints/admin.py](app/api/endpoints/admin.py) lines 24–120 + +`POST /modules`, `GET /modules`, `POST /modules/{id}/rotate`, `DELETE /modules/{id}` — none have `@limiter.limit()`. Regular API endpoints use `settings.RATE_LIMIT` but admin does not. + +**Fix:** Add `@limiter.limit("10/minute")` or stricter to all admin routes. + +--- + +### H-4 — Duplicate Admin Endpoints with Inconsistent Auth +**File:** [app/api/admin_backend.py](app/api/admin_backend.py) and [app/api/endpoints/admin.py](app/api/endpoints/admin.py) + +Two sets of module management endpoints are mounted to the same prefix. `admin_backend.py` uses API key auth (any valid module key can call it); `admin.py` uses JWT. This creates an auth bypass path. + +**Fix:** Remove `admin_backend.py` endpoints. Consolidate to one set requiring JWT with an admin role check. + +--- + +### H-5 — Exception Messages Returned to Clients +**File:** [app/api/endpoints/gemini.py](app/api/endpoints/gemini.py) lines 71–79, 142–143; similar in [app/api/endpoints/openai.py](app/api/endpoints/openai.py) + +```python +except Exception as e: + return {"status": "error", "detail": f"Failed to parse text/plain as JSON: {str(e)}"} +``` + +Raw Python exception messages reveal framework internals, library versions, and data structures to callers. + +**Fix:** Log the full exception server-side; return a generic message: `"Service error — please try again later"`. + +--- + +### H-6 — No HTTPS Enforcement at Application Level +**File:** [docker-compose.yml](docker-compose.yml), [Caddyfile](Caddyfile) + +TLS is terminated by Caddy, but the application itself has no HTTPS redirect or HSTS header. If Caddy is bypassed (direct port 8000 access), credentials are transmitted in plaintext. + +**Fix:** Add an HTTPS enforcement middleware and set `Strict-Transport-Security: max-age=31536000; includeSubDomains` in all responses. + +--- + +### H-7 — Unvalidated Query Parameter Casting +**File:** [app/api/endpoints/gemini.py](app/api/endpoints/gemini.py) lines 46–61 + +```python +temperature=float(params.get("temperature", 0.7)), +top_k=int(params.get("top_k", 40)), +``` + +Invalid input raises `ValueError` → 500. No bounds checking: `temperature` could be `-999`, `top_k` could be `2147483647`. + +**Fix:** Use Pydantic `Query()` with `Field(ge=0.0, le=2.0)` constraints and handle `ValueError` with a 422 response. + +--- + +### H-8 — Prompt Injection via User-Controlled System Prompt +**File:** [app/api/endpoints/gemini.py](app/api/endpoints/gemini.py) lines 110–112 + +```python +system_instruction = chat_data.system_prompt or GEMINI_SYSTEM_PROMPT +system_instruction += f"\n\nKnowledge Base:\n{chat_data.knowledge_base}" +``` + +Callers can fully replace the system prompt and append arbitrary content to it. This enables jailbreaking the model, data exfiltration via the LLM, and policy bypass. + +**Fix:** Restrict `system_prompt` to an allowlist of predefined values. Never concatenate raw user input into a system instruction. + +--- + +### H-9 — Stored XSS in Admin Panel (Module Names) +**File:** [app/static/admin.html](app/static/admin.html) line 608 + +```javascript +
${m.name}
+``` + +Module names from the API are interpolated directly into `innerHTML` via template literals. An attacker with a valid API key can create a module named `` to exfiltrate the admin JWT from `localStorage`. + +**Fix:** Use `element.textContent = m.name` or DOMPurify. Add a `Content-Security-Policy` header. + +--- + +### H-10 — XSS via Inline Event Handler with API Key +**File:** [app/static/admin.html](app/static/admin.html) lines 617, 620 + +```javascript +