fix: csv formula injection + local web server hardening#5
Merged
Conversation
A session title, project dir, or model name starting with =, +, -, @, tab, or CR is executed as a formula when the exported CSV is opened in Excel/LibreOffice/Sheets. Prefix such strings with an apostrophe on export; strings that are plain numbers (a negative cost) pass through untouched, and non-string cells are never affected.
Harden --serve/--web against hostile webpages: /api/reload mutated state on a plain GET, so any page could fire it cross-origin (CSRF / cheap DoS -- every hit re-parses the sources); it is POST-only now, GET answers 405 and the page's refresh button posts. On a loopback bind the Host header must name localhost/127.0.0.1/[::1] (port optional) or the request is refused with 403, closing DNS rebinding; an explicit --bind beyond loopback keeps working and already warns. Every response carries a deny-everything CSP fitting the self-contained page (inline JS/CSS, data: favicon, same-origin fetches) plus X-Content-Type-Options.
Owner
|
Nice work - thanks for this. The CSV formula-injection one is a genuinely good catch; titles being prompt-derived makes it a real vector, and I like that the sanitizer leaves negative-number cells alone. Host-check + POST-only reload are the right call too. Pulled it locally, tests green - merging. 🙏 |
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
2 participants
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.
Two security fixes:
CSV export formula injection — session titles, project dirs, and model names are written verbatim into the
e-key export, so a cell starting with=,+,-,@, tab, or CR executes as a formula when the CSV is opened in Excel/LibreOffice/Sheets (titles come straight from prompts, so the content is attacker-influenced when browsing shared transcripts). Strings that would be read as formulas now get a leading apostrophe; plain-number strings (negative costs) and non-string cells pass through untouched, applied centrally inexport_currentso every dataset is covered.--serve/--webhardening —/api/reloadmutated state on a plain GET, so any webpage could fire it cross-origin (<img src="http://localhost:8321/api/reload">— CSRF plus a cheap DoS since each hit re-parses the sources). It is POST-only now (GET answers 405, the page's refresh button posts). On the default loopback bind the Host header must name localhost/127.0.0.1/[::1] or the request gets 403, closing DNS rebinding; an explicit--bindbeyond loopback keeps working as before. Every response now carries a CSP fitting the self-contained page plusX-Content-Type-Options: nosniff.Tested: 3 new tests (sanitizer matrix incl. negative numbers, end-to-end export write, live server checks for 405/POST/403-on-foreign-Host); suite 307/307, ruff clean. Manually verified with
--demo --serve+ curl.