Fix CSV export field escaping and injection protection#956
Open
Ridanshi wants to merge 1 commit into
Open
Conversation
All exported fields are now serialized through a shared escapeCSVField helper that applies RFC 4180 quoting rules: every value is wrapped in double quotes, embedded double quotes are doubled, and null/undefined produce an empty quoted field so commas and newlines can never split columns or rows. Values whose first character is a spreadsheet formula trigger (=, +, -, @, tab, carriage return) are prefixed with a tab before quoting so spreadsheet applications treat the content as plain text rather than executing it as a formula. Previously only the notes column had any escaping; the other seven columns (id, subject_name, title, due_at, status, priority, confidence_score) were interpolated raw into the CSV output. Row separators are changed from LF to CRLF in line with RFC 4180. Adds tests/exportCsv.test.js with 26 tests covering normal values, commas, newlines, double quotes, all formula-prefix characters, null, undefined, empty strings, multi-row output, and regression cases for each previously unescaped field.
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
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.
Closes #361
Root cause
downloadDatainbackend/controllers/csvDownload.controller.jsbuilt each CSV row by interpolating raw field values and joining them with commas. Only thenotescolumn had any escaping (a manual double-quote wrapper). The other seven exported fields —id,subject_name,title,due_at,status,priority, andconfidence_score— were written without quoting, so:=,+,-, or@could be executed as spreadsheet formulas.Changes
backend/controllers/csvDownload.controller.jsAdds
escapeCSVField(value), a single reusable helper that implements RFC 4180 serialization:null/undefined→""(empty quoted field)"→""), satisfying RFC 4180 §2.7.=,+,-,@, tab, carriage return) are prefixed with a tab character before quoting, preventing spreadsheet applications from interpreting the content as a formula.downloadDatanow passes every field throughescapeCSVFieldvia a.map()call instead of the previous manual mix of raw values and a hand-rolled notes escaper. Row separators are changed from\nto\r\nper RFC 4180.tests/exportCsv.test.js(new file)26 tests using Node's built-in
node:testrunner (same runner used by the existingexportIcssuite) covering:null,undefined, empty string=,+,-,@, tab, carriage return)All 26 new tests pass. The 5 existing ICS tests are unaffected.
Verification