fix: security, parallelization, observability, and DX overhaul#1
Open
aezakmixdxd wants to merge 1 commit into
Open
fix: security, parallelization, observability, and DX overhaul#1aezakmixdxd wants to merge 1 commit into
aezakmixdxd wants to merge 1 commit into
Conversation
P0 — Security: - Replace permissive CORS (allow_origins=*+credentials) with env-driven whitelist - Add X-API-Key authentication via Depends(verify_api_key) on all /api endpoints - Enforce upload size limit (MAX_CONTRACT_SIZE_KB) with streamed read - Validate code size + LOC on /analyze and /batch-analyze P1 — Logic / reliability: - Phase 1 agents now run in true parallel via asyncio.gather (was sequential) - Per-chunk analysis also parallelized inside each agent - Token tracker: configurable DATA_DIR, replays JSONL on startup so stats survive restart - Daily budget enforcement (BUDGET_ENFORCE) returns HTTP 429 instead of silently overspending - Per-route slowapi rate limits + 429 handling P2 — Observability / quality: - Structured logging module (app.core.logging) replaces print() - Unhandled exception handler hides stack traces from clients - Tests: validator, pipeline (incl. concurrency assertion), token tracker (incl. persistence + budget) - ruff config + GitHub Actions CI (lint + test + docker build) P3 — Infra / DX: - Dockerfile (non-root, healthcheck) + docker-compose with frontend nginx - pyproject.toml with ruff, pytest.ini with asyncio_mode=auto - Remove unused tiktoken; add slowapi, dev deps (pytest, ruff) - Expand .env.example to document all knobs Verified: 17/17 tests pass, ruff clean, server boots, auth + rate limit confirmed via curl.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive security, performance, observability, and DX overhaul of ChainSentinel backend.
Verified locally: 17/17 tests pass, ruff lint clean, server boots, auth + rate limit confirmed via curl.
P0 — Security 🔴
allow_origins=["*"] + allow_credentials=True(invalid per spec, browsers reject) with env-drivenALLOWED_ORIGINSwhitelistX-API-Keyheader on every/api/*endpoint (constant-time comparison, dev mode opt-in viaAUTH_DISABLED=true)MAX_CONTRACT_SIZE_KBcap (was unbounded — DoS risk)P1 — Reliability / Performance 🟠
asyncio.gather()instead of sequentialforloop. Per-chunk analysis also parallelized — 3× speedup for long contracts./statssurvives restart (was in-memory only)DATA_DIRenv var (was hardcoded~/projects/chainsentinel/..., broke in Docker)BUDGET_ENFORCE): returns HTTP 429 instead of silent overspend10/minuteanalyze,2/minutebatch,30/minutechat — all configurable)P2 — Observability / Quality 🟡
app.core.logging) replacesprint()test_validator.py— pragma, contract/interface/library, brace balancetest_pipeline.py— concurrency assertion via fake MiMo client, chunking edge cases, complexity scoringtest_token_tracker.py— persistence, budget enforcement, edge casesP3 — Infra / DX 🔵
Dockerfile(non-root user, healthcheck, layer caching)docker-compose.ymlwith frontend nginxpyproject.toml(ruff config),pytest.ini(asyncio_mode=auto)requirements.txt: removed unusedtiktoken, addedslowapi+ dev deps (pytest, ruff).env.exampleexpanded to document all knobs/api/chat/streamendpoint exposing the existingstream_chat()(was unused)Stats
Verification
Smoke tested:
POST /api/analyzewithout key →401POST /api/analyzewith validX-API-Key→200200, remaining429(rate limit working)Notes
frontend/js/app.js) untouched; will need to sendX-API-Keyheader when consuming the API. Suggest a follow-up PR for that.netlify.tomlredirect URLchainsentinel-api.onrender.comleft as-is — should become a Netlify env var in a future change./api/*now requireX-API-KeyunlessAUTH_DISABLED=true(dev only).