Security Audit

OpenAlgo security audit

Run monthly (or twice monthly), and additionally before any release and after any dependency bump.

uv run --with openpyxl python .claude/skills/security-audit/audit.py

Writes tmp/security-audits/<YYYY-MM>/security_audit_<date>_<time>.xlsx. tmp/ is gitignored and holds reports only — the script keeps its own scratch files in the OS temp dir. Exit code is 0 when no CRITICAL/HIGH check is failing, 1 otherwise, so it drops into CI unchanged.

The script is read-only. It never rotates a key, edits config, or touches the database, and the report never contains a secret value — only pass/fail, a sha256: fingerprint, and a remediation pointer. That means the xlsx is safe to share with someone helping you triage.

What it covers

Reading the report

Six sheets: Summary, Findings (colour-coded by status), Code patterns, Frontend patterns, Routes (unprotected), Bandit (triaged), Manual review.

Statuses mean different things and should be worked in this order:

• FAIL — a control is objectively wrong. Fix it.
• ERROR — a check could not run (usually a missing tool). Fix the tooling; an unrun check is not a pass.
• REVIEW — a pattern that is often fine and sometimes catastrophic. A human must look. This is where the real findings live.
• WARN — weaker posture, defensible depending on deployment.
• PASS / SKIP — no action.

Do not treat counts as scores. Bandit produces ~930 raw findings on this repo and roughly 5 matter; the rest are asserts and try/except/pass. The script does that triage for you and reports all three numbers so you can see the ratio. Same for the route check: 461 routes, ~119 without decorators, and almost all of those are legitimately public — which is why the allowlist exists.

Triage guidance for the recurring REVIEW items

Raw SQL f-strings. All current hits interpolate table and column names (SQLite cannot parameterize those) from module-level literals in migration code, e.g. _migrate_mode_unique(ScalpingSLState, "scalping_sl_state"). That is safe. The check exists to catch the day someone interpolates a request value. Trace each new hit to its source; if it is not a developer-controlled constant, it is a real injection.

Direct create_engine calls. Mostly in broker database/master_contract_db.py modules. These bypass engine_factory and therefore the NullPool guarantee — an FD-hygiene and availability issue rather than a breach. Worth converging over time; see the fd-audit skill.

Credential caches. database/telegram_db.py holds _user_credentials_cache with a 30-minute TTL. Broker tokens roll over at ~3 AM IST, so confirm no auth cache outlives that boundary and that logout or revoke invalidates it.

Unprotected routes. Genuinely public ones (React SPA shells, /login, .well-known, broker callbacks, token-authenticated webhooks and postbacks) are on PUBLIC_ROUTE_ALLOWLIST in the script. When you add a legitimately public route, add it there with a reason — that keeps the check meaningful instead of noisy.

detect-secrets candidates. High false-positive rate on this repo. Triage once, then commit a .secrets.baseline so subsequent audits only surface new candidates.

Credentials in browser storage. The current hit is localStorage.setItem('pocketful_oauth_state', ...), which is an OAuth CSRF state token — the correct use of localStorage. An auth token or API key there would not be.