Epic: Red-team hardening — authz boundary, MCP exec safety & audit integrity (Fable-5 audit 2026-07-05)

[Santisoutoo]

Epic tracking the fixes from an adversarial (red-team) security & concurrency review (Fable-5, 2026-07-05, static/read-only) of LexFlow's critical paths. Full findings (file:line, attack traces) in memory/fable_redteam_audit_2026-07-05.md (kept private — this issue text is deliberately non-exploitable).

⚠️ P0 items describe an unpatched RCE on a public repo. Consider handling P0-1/P0-2 via a private GitHub Security Advisory. Issue bodies here state the vulnerability class + affected endpoints + fix, without a weaponized payload.

Verdict

For the desktop default (uvicorn bound to 127.0.0.1:8000, single user, main.py:9) the posture is mostly sound on the classic sinks: no path traversal in law lookup, SPA serving containment-guarded, git commit hashes validated, secrets keyring-backed and never echoed, errors sanitized, the agentic tool loop bounded (5 iters) and every tool read-only + default-deny, react-markdown escapes HTML, np.load with allow_pickle=False. But the whole API has zero auth and zero CSRF protection while exposing highly side-effecting endpoints — the sharpest registers an MCP server and later spawns its command string as a subprocess. Worst case if the bind is ever moved off localhost / reverse-proxied: unauthenticated arbitrary command execution. Even on localhost the "localhost = only the user" assumption is defeated through the browser: the .mcpb bundle-install endpoint is a multipart upload (a CORS "simple request" — no preflight, no CSRF token, no Origin check), so a malicious site the user visits can register a server with an attacker-chosen command and then trigger its spawn with a simple GET /api/v1/mcp/tools. Headline: CSRF-to-RCE via unauthenticated MCP registration + external-tool discovery (direct unauth RCE if ever networked). The VforVitorio#769 TOCTOU class also persists in the audit-log append.

Top-3 must-fix

1. P0-1 MCP exec — gate every external-MCP-server spawn behind explicit in-SPA user consent + an Origin/Host (or custom-header) CSRF guard on the spawn-triggering + registration routes. Kills both browser CSRF→RCE and networked unauth-RCE.
2. P0/P1-2 — global Origin/Host allow-list middleware + keep the 127.0.0.1 bind as a documented, enforced security boundary; require real auth before any networked/multi-user mode.
3. P2-4 — validate whats-new?since= as a hex commit + add --end-of-options to the git diff (close the argument-injection file-write).

Sprints (findings P0-1 … P3-9)

Sprint 1 — MCP exec & CSRF boundary (P0):

• S1.1 security(mcp): consent-before-spawn for external MCP servers (no auto-connect on add/import) [M] [P0] — an added/imported server's command is spawned as a stdio subprocess on the first GET /api/v1/mcp/tools (chat/mcp_client.py:26-28,102-124,182-211); command/args have no allow-list (api/routers/mcp_servers.py:107, mcp_servers/schemas.py:69-98 validates only url); bundle path api/routers/mcp_servers.py:248, mcp_servers/bundle.py:226-251. AC: require explicit in-SPA consent before the first spawn of any newly added/imported server. (P0-1, RCE)
• S1.2 security(api): Origin/Host allow-list middleware on state-changing + spawn-triggering routes [M] [P0] — only RequestIdMiddleware is installed, no auth/CSRF gate anywhere (api/app.py:129); the browser-reachable subset (bodyless POST /sync, POST /system/semantic-install, GET /mcp/tools, multipart POST /mcp/bundles) is CSRF-able. AC: reject cross-origin state-changing requests (Origin/Host allow-list or a required custom header). (P0/P1-2)
• S1.3 security(deploy): document + assert the 127.0.0.1 bind as a hard security boundary; require auth before any networked mode [S] — main.py:9. AC: startup assertion / explicit opt-in + warning when bound off-loopback; note it in packaging ([epic] Legal chat — trust & UX (Fable 5 audit) #37) + CONTRIBUTING. (P0/P1-2)
• S1.4 (stretch) security(mcp): sandbox spawned MCP servers [S] — the _build_config seam (chat/mcp_client.py:127) is already flagged for this. (P0-1 defense-in-depth)

Sprint 2 — Trust-boundary input validation (P2/P3):

• S2.1 security(system): validate whats-new?since= as a hex commit + add --end-of-options to git diff [S] [P2] — since is unvalidated (api/routers/system.py:78-84) and flows into git diff -M "{since}..HEAD" with no --/--end-of-options guard (core/delta_sync.py:87-91); a --leading value is parsed as a git option (constrained file-write primitive), CSRF-reachable (plain GET). Contrast the correct hex-pin at api/routers/versions.py:52-63. AC: ^[0-9a-f]{7,40}$ at the Query boundary + --end-of-options before the revspec. (P2-4)
• S2.2 fix(models): wrap provider list_models() in _probe with except Exception [S] — OpenAIProvider.list_models catches only Auth/RateLimit (chat/providers/openai_provider.py:44-52); another SDK error escapes _probe (api/routers/models.py:113-121) → asyncio.gather re-raises → GET /models 500s (no confirmed key leak). AC: except Exception → placeholder. (P3-8, PLAUSIBLE)
• S2.3 fix(frontend): rel="noopener noreferrer" on LawMarkdown external links [S] — the markdown a renderer lacks rel on target="_blank" (frontend/src/components/domain/LawMarkdown.tsx), unlike SettingsPage/ErrorState; reverse-tabnabbing from corpus/law markdown (low impact, modern browsers imply noopener). AC: add rel. (P3-9)

Sprint 3 — Audit-log integrity (P2):

• S3.1 security(audit): HMAC/anchor the hash chain + verify() on startup with a tamper indicator [M] [P2] — the chain is KEYLESS SHA256 (chat/audit/canonical.py:74-82): anyone who can write mcp.log can edit a record + recompute downstream hashes → verify() passes; tail truncation is also undetectable; the docstring overstates the guarantee (chat/audit/log.py:1-13) and verify() (:137) is never called at runtime. AC: HMAC with a keyring key (edits require the key) or periodically anchor the tail; call verify() on startup + surface tamper; persist expected length/last-hash so truncation is detectable. (P2-5)
• S3.2 fix(audit): move append hashing inside the lock (TOCTOU, the #769 class) [S] [P2] — _audited computes previous_hash from read_last_hash() (lock released) then append() re-acquires and rejects on previous_hash != _last_hash (chat/mcp_server.py:96,108,127,139, chat/audit/log.py:114-119); a concurrent appender between read and append → ValueError("chain break") → spurious tool_error, tool never runs. AC: have append derive previous_hash + stamp entry_hash itself inside the critical section. (P2-6)

Sprint 4 — Availability & multi-tenant posture (P1/P2):

• S4.1 security(chat): off-load tool dispatch via asyncio.to_thread (availability DoS) [S] [P2] — each tool runs SYNC directly in the async SSE generator (chat/streaming.py:337); search_law scans 12k, search_semantic_top_k can trigger a full cold embed build (chat/mcp_server.py:191→search/service.py:22-33) — all on the event loop, freezing every other request/stream. AC: await asyncio.to_thread(_run_tool_call, call). Cross-ref Epic: Backend performance & scale at 12k laws — cold start, memory, event loop (Fable-5 audit 2026-07-05) VforVitorio/LexFlow#870/Epic: Frontend runtime perf layer 2 — input-driven cost, post #712/#713 (Fable-5 audit 2026-07-05) VforVitorio/LexFlow#875 (there it's perf; here it's the DoS angle) — coordinate so it's fixed once. (P2-7)
• S4.2 security(multi-tenant): scope threads/tags/secrets to a user OR formally document + enforce single-user [S] [P1] — _load_thread_or_404 is session.get(ChatThread, thread_id) with no owner filter (api/routers/chat_threads.py:76-81), ChatThread has no user column (chat/storage_models.py:46-68), GET /chat/threads lists ALL threads; same for user_tags + the single global keyring. Fine desktop, cross-tenant exposure if networked. AC: add a user/session scope + filter before any multi-user mode; until then document single-user as a security invariant. (P1-3)

Positive controls verified (NOT findings)

No path traversal in law_id (core/registry.py:91); SPA resolve()+relative_to (api/spa.py:68-75); git args validated (versions.py:52-63); Ollama tag + provider base URLs locked, McpServerCommand.url has a real SSRF validator (mcp_servers/schemas.py:100-139); bundle extraction rejects zip-slip + caps size (bundle.py:171-224); secrets never echoed (chat/secrets.py); no CORS middleware; no dangerouslySetInnerHTML/rehype-raw; tool loop bounded (streaming.py:56) + all tools read-only default-deny (policy.py:54-128); np.load allow_pickle=False.

Cross-references (not re-filed): perf/event-loop → VforVitorio#870 & VforVitorio#875 (S4.1 is the DoS framing); resilience/wedge → VforVitorio#808; stale cache → VforVitorio#771.

---

Upstream: VforVitorio#884