Skip to content

fix: address critical security and performance findings from audit #001#3

Merged
mrjunos merged 2 commits into
mainfrom
fix/audit-critical
Apr 7, 2026
Merged

fix: address critical security and performance findings from audit #001#3
mrjunos merged 2 commits into
mainfrom
fix/audit-critical

Conversation

@mrjunos

@mrjunos mrjunos commented Apr 7, 2026

Copy link
Copy Markdown
Owner

Summary

Resolves all 5 critical findings from `audits/001_2026-04-06_audit_report.md`.

  • CORS: replaced `allow_origins=["*"]` with env-configured allowlist (`COMPASS_ALLOWED_ORIGINS`). Defaults to `localhost:3000` and `localhost:5173`.
  • Path traversal: sanitized upload filename with `Path(file.filename).name` — strips any directory components.
  • Upload size limit: added 50MB cap, returns HTTP 413 if exceeded.
  • SQLite index: added `CREATE INDEX IF NOT EXISTS idx_messages_session_id` — eliminates full table scan on every chat request.
  • LLM timeout: added `timeout=60` to `litellm.acompletion` — prevents requests from hanging indefinitely if Ollama is unresponsive.

Also includes:

  • Bug fix: `get_session_messages` was using `ORDER BY timestamp DESC` which is non-deterministic when messages share the same second. Changed to `ORDER BY id DESC` (AUTOINCREMENT, always unique).

Tests added (38 total, up from 26)

`test_api.py` — 3 new upload tests:

  • Path traversal: `../../etc/passwd.md` → sanitized to `passwd.md` ✅ covered by `test_path_traversal_filename_is_sanitized`
  • File too large: >50MB returns HTTP 413 ✅ covered by `test_413_when_file_too_large`
  • File at exactly 50MB: accepted (boundary check) ✅ covered by `test_file_at_size_limit_is_accepted`

`test_database.py` — 7 new tests (new file):

  • Schema: messages table and `session_id` index exist after `init_db`
  • Save/get: session isolation, chronological order, limit behavior, empty session

Test plan

  • 38 tests passing locally
  • CI green
  • Upload rejects files > 50MB — verified via `test_413_when_file_too_large`
  • Path traversal sanitized — verified via `test_path_traversal_filename_is_sanitized`

🤖 Generated with Claude Code

Juan José Cano Duque and others added 2 commits April 7, 2026 07:21
- CORS: restrict origins to env-configured list (default: localhost:3000/5173)
- Upload: sanitize filename with Path.name to prevent path traversal
- Upload: reject files larger than 50MB (HTTP 413)
- SQLite: add index on messages(session_id) to avoid full table scans
- LLM: add 60s timeout to litellm.acompletion to prevent hung requests
- .env.example: document new COMPASS_ALLOWED_ORIGINS variable

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Upload tests:
- path traversal: verify ../../etc/passwd.md is sanitized to passwd.md
- file size: verify 413 on >50MB, 201 at exactly 50MB

Database tests (new test_database.py, 7 tests):
- schema: messages table and session_id index exist after init_db
- save/get: isolation by session, chronological order, limit behavior

Also fix: change ORDER BY timestamp DESC to ORDER BY id DESC in
get_session_messages — timestamp has second precision and is
non-deterministic when messages are inserted rapidly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mrjunos
mrjunos merged commit db0a72b into main Apr 7, 2026
1 check passed
@mrjunos
mrjunos deleted the fix/audit-critical branch April 7, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant