security-audit

name	security-audit
description	Run OpenAlgo's periodic security audit across backend, frontend, database, cache, routes and dependencies, producing a dated xlsx report. Use for the monthly or twice-monthly review, before a release, after a dependency bump, or when the user asks for a security check, vulnerability scan, or audit report.

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

Area	Checks
Secrets	APP_KEY/API_KEY_PEPPER present, full-entropy, and not one of the publicly-known leaked values shipped before v2.0.0.6; no credentials tracked in git; detect-secrets sweep
Runtime posture	FLASK_DEBUG off; remote MCP never with debug; CSRF on; CSP enforcing (not report-only); CORS not wildcard-with-credentials; HTTPS; ngrok off; rate limits set
Routes	All 461 blueprint routes checked for @check_session_validity / rate limiting against an allowlist of intentionally-public paths; state-changing routes reported separately; test/debug surfaces flagged
Database	File permissions; Fernet encryption of broker tokens; peppered API keys; NullPool (never StaticPool); engines created via engine_factory; backup presence
Cache	Credential caches must have an explicit bounded TTL; auth-change invalidation available; the ZMQ invalidation publisher must connect(), never bind()
Backend code	eval/exec, shell=True, pickle/yaml deserialization, raw-SQL f-strings, path traversal, SSRF, open redirect, weak hashes, verify=False, missing HTTP timeouts
Frontend	dangerouslySetInnerHTML, innerHTML, eval/new Function, credentials in browser storage, hardcoded key literals, plain-http endpoints, source maps in dist/, npm audit
Dependencies	pip-audit (Python) and npm audit (JS) — two separate ecosystems, both required
Static analysis	bandit, triaged rather than dumped

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.

Manual checks the script cannot do

The Manual review sheet lists twelve items that need a human and an authenticated session — they are part of the audit, not optional extras. The ones most specific to this platform:

• Static IP whitelisting. Confirm the broker portal still lists only this server's IP. Under the SEBI mandate stolen keys are unusable off-IP, but anything routed through this server still works — so server compromise, not key theft, is the threat that matters.
• API keys. Revoke keys for integrations no longer in use. TradingView/Chartink send keys in the body or query string and cannot set headers, so a stale key is a standing grant.
• Network exposure. Ports 5000/8765/5555 must not be internet-reachable. ZMQ 5555 carries the raw tick feed with no authentication.
• Webhook tokens. Flow and strategy webhook URLs are bearer credentials — anyone holding the URL can trigger the strategy.
• Pepper discipline. Never hand-edit API_KEY_PEPPER on a populated database; it invalidates every password hash and encrypted token. Use upgrade/rotate_pepper.py.

Threat model this is calibrated to

OpenAlgo is single-user and self-hosted: one user, one broker session per instance, no privilege escalation and no SaaS component. Server access equals full control, which is why filesystem permissions, secret hygiene and network exposure carry more weight here than classic multi-tenant concerns like IDOR or role bypass. Weight your triage accordingly.

Maintaining the script

Add a check by writing one function that calls add(check, severity, status, detail, remediation, evidence) and wiring it into main(). Keep three properties:

1. Never print a secret — use fingerprint().
2. Never write outside tmp/security-audits/ — scratch goes to tempfile.
3. Give every finding a remediation, not just a description. A finding nobody knows how to fix gets skipped next month.

When a REVIEW item is confirmed benign and will stay benign, encode that as an allowlist entry or a narrowed pattern rather than re-triaging it every month.