forked from Rockachopa/Timmy-time-dashboard
* feat: upgrade primary model from llama3.1:8b to qwen2.5:14b - Swap OLLAMA_MODEL_PRIMARY to qwen2.5:14b for better reasoning - llama3.1:8b-instruct becomes fallback - Update .env default and README quick start - Fix hardcoded model assertions in tests qwen2.5:14b provides significantly better multi-step reasoning and tool calling reliability while still running locally on modest hardware. The 8B model remains as automatic fallback. * security: centralize config, harden uploads, fix silent exceptions - Add 9 pydantic Settings fields (skip_embeddings, disable_csrf, rqlite_url, brain_source, brain_db_path, csrf_cookie_secure, chat_api_max_body_bytes, timmy_test_mode) to centralize env-var access - Migrate 8 os.environ.get() calls across 5 source files to use `from config import settings` per project convention - Add path traversal defense-in-depth to file upload endpoint - Add 1MB request body size limit to chat API - Make CSRF cookie secure flag configurable via settings - Replace 2 silent `except: pass` blocks with debug logging in session.py - Remove unused `import os` from brain/memory.py and csrf.py - Update 5 CSRF test fixtures to patch settings instead of os.environ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Trip T <trip@local> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
112 lines
4.1 KiB
Python
112 lines
4.1 KiB
Python
"""Tests for CSRF protection middleware bypass vulnerabilities."""
|
|
|
|
import pytest
|
|
from fastapi import FastAPI
|
|
from fastapi.testclient import TestClient
|
|
from dashboard.middleware.csrf import CSRFMiddleware
|
|
|
|
class TestCSRFBypassVulnerability:
|
|
"""Test CSRF bypass via path normalization and suffix matching."""
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def enable_csrf(self):
|
|
"""Re-enable CSRF for these tests."""
|
|
from config import settings
|
|
original = settings.timmy_disable_csrf
|
|
settings.timmy_disable_csrf = False
|
|
yield
|
|
settings.timmy_disable_csrf = original
|
|
|
|
def test_csrf_bypass_via_traversal_to_exempt_pattern(self):
|
|
"""Test if a non-exempt route can be accessed by traversing to an exempt pattern.
|
|
|
|
The middleware uses os.path.normpath() on the request path and then checks
|
|
if it starts with an exempt pattern. If the request is to '/webhook/../api/chat',
|
|
normpath makes it '/api/chat', which DOES NOT start with '/webhook'.
|
|
|
|
Wait, the vulnerability is actually the OTHER way around:
|
|
If I want to access '/api/chat' (protected) but I use '/webhook/../api/chat',
|
|
normpath makes it '/api/chat', which is NOT exempt.
|
|
|
|
HOWEVER, if the middleware DOES NOT use normpath, then '/webhook/../api/chat'
|
|
WOULD start with '/webhook' and be exempt.
|
|
|
|
The current code DOES use normpath:
|
|
```python
|
|
normalized_path = os.path.normpath(path)
|
|
if not normalized_path.startswith("/"):
|
|
normalized_path = "/" + normalized_path
|
|
```
|
|
|
|
Let's look at the exempt patterns again:
|
|
exempt_patterns = [
|
|
"/webhook",
|
|
"/api/v1/",
|
|
"/lightning/webhook",
|
|
"/_internal/",
|
|
]
|
|
|
|
If I have a route '/webhook_attacker' that is NOT exempt,
|
|
but it starts with '/webhook', it WILL be exempt.
|
|
"""
|
|
app = FastAPI()
|
|
app.add_middleware(CSRFMiddleware)
|
|
|
|
@app.post("/webhook_attacker")
|
|
def sensitive_endpoint():
|
|
return {"message": "sensitive data accessed"}
|
|
|
|
client = TestClient(app)
|
|
|
|
# This route should NOT be exempt, but it starts with '/webhook'
|
|
# CSRF validation should fail (403) because we provide no token.
|
|
response = client.post("/webhook_attacker")
|
|
|
|
# If it's 200, it's a bypass!
|
|
assert response.status_code == 403, "Route /webhook_attacker should be protected by CSRF"
|
|
|
|
def test_csrf_bypass_via_api_v1_prefix(self):
|
|
"""Test if a route like /api/v1_secret is exempt because it starts with /api/v1/."""
|
|
# Wait, the pattern is "/api/v1/", with a trailing slash.
|
|
# So "/api/v1_secret" does NOT start with "/api/v1/".
|
|
# But "/webhook" does NOT have a trailing slash.
|
|
pass
|
|
|
|
def test_csrf_bypass_via_webhook_prefix(self):
|
|
"""Test if /webhook_secret is exempt because it starts with /webhook."""
|
|
app = FastAPI()
|
|
app.add_middleware(CSRFMiddleware)
|
|
|
|
@app.post("/webhook_secret")
|
|
def sensitive_endpoint():
|
|
return {"message": "sensitive data accessed"}
|
|
|
|
client = TestClient(app)
|
|
|
|
# Should be 403
|
|
response = client.post("/webhook_secret")
|
|
assert response.status_code == 403, "Route /webhook_secret should be protected by CSRF"
|
|
|
|
def test_legitimate_exempt_paths(self):
|
|
"""Test that legitimate exempt paths still work."""
|
|
app = FastAPI()
|
|
app.add_middleware(CSRFMiddleware)
|
|
|
|
@app.post("/webhook")
|
|
def webhook():
|
|
return {"message": "webhook received"}
|
|
|
|
@app.post("/api/v1/chat")
|
|
def api_chat():
|
|
return {"message": "api chat"}
|
|
|
|
client = TestClient(app)
|
|
|
|
# Legitimate /webhook (exact match)
|
|
response = client.post("/webhook")
|
|
assert response.status_code == 200
|
|
|
|
# Legitimate /api/v1/chat (prefix match)
|
|
response = client.post("/api/v1/chat")
|
|
assert response.status_code == 200
|