diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 7902c442..33d630cf 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -30,3 +30,7 @@ **Vulnerability:** Known high-severity vulnerabilities discovered by the audit in `js-yaml` and `nanoid` packages. **Learning:** Deeply nested dependencies (`js-yaml` via `eslint`, `nanoid` via `vitest/vite`) may expose the application to DoS or logic loops. **Prevention:** Use `pnpm.overrides` in the root `package.json` to enforce patched versions across all transitive paths in a pnpm workspace. +## 2026-08-20 - [Fix CSV Formula Injection] +**Vulnerability:** When exporting CSV, fields starting with `=`, `+`, `-`, `@`, ` `, or ` ` were not escaped which led to Spreadsheet Macro Injection vulnerabilities when the exported CSV file is opened in tools like Excel. +**Learning:** When creating CSV files, untrusted input must be sanitized. If an entry begins with a character that could be interpreted as a macro, a single quote should be prepended to force the spreadsheet to read it as a string. +**Prevention:** Create a shared `csvField` utility function and consistently apply it across all CSV endpoints, ensuring that values that could trigger macros are prepended with a single quote. diff --git a/package.json b/package.json index d085ba62..66dbf948 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,8 @@ "undici": "^7.29.0", "minimatch": "^10.0.0", "@hono/node-server": "^2.0.5", - "body-parser": "^2.3.0" + "body-parser": "^2.3.0", + "deepmerge-ts": "8.0.1" } } } diff --git a/packages/web/src/app/api/orgs/[orgSlug]/dashboard/sessions/route.ts b/packages/web/src/app/api/orgs/[orgSlug]/dashboard/sessions/route.ts index 7d6a4d4a..83c23c8d 100644 --- a/packages/web/src/app/api/orgs/[orgSlug]/dashboard/sessions/route.ts +++ b/packages/web/src/app/api/orgs/[orgSlug]/dashboard/sessions/route.ts @@ -10,6 +10,7 @@ import { resolveOrgScopedProjectIds, } from '@/lib/server/dashboard-route-helper' import { canAccessIndividualData, forbiddenByRole } from '@/lib/server/rbac' +import { csvField } from '@/lib/server/csv/export' export const runtime = 'nodejs' export const dynamic = 'force-dynamic' @@ -71,11 +72,6 @@ function mapSessionItem(session: SessionWithInclude): SessionItem { } } -function csvField(value: string | number | null | undefined) { - if (value === null || value === undefined) return '' - const text = String(value) - return /[",\r\n]/.test(text) ? `"${text.replaceAll('"', '""')}"` : text -} function buildSessionsCsv(sessions: SessionWithInclude[]) { const headers = [ diff --git a/packages/web/src/lib/server/csv/export.test.ts b/packages/web/src/lib/server/csv/export.test.ts new file mode 100644 index 00000000..dbf2f6a6 --- /dev/null +++ b/packages/web/src/lib/server/csv/export.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from 'vitest' +import { csvField } from './export' + +describe('csvField', () => { + it('returns empty string for null or undefined', () => { + expect(csvField(null)).toBe('') + expect(csvField(undefined)).toBe('') + }) + + it('escapes quotes and wraps in quotes if text contains a quote', () => { + expect(csvField('hello "world"')).toBe('"hello ""world"""') + }) + + it('wraps in quotes if text contains comma', () => { + expect(csvField('hello, world')).toBe('"hello, world"') + }) + + it('wraps in quotes if text contains newlines', () => { + expect(csvField('hello\nworld')).toBe('"hello\nworld"') + expect(csvField('hello\rworld')).toBe('"hello\rworld"') + }) + + it('prepends a single quote to string values starting with formula chars', () => { + expect(csvField('=1+2')).toBe("'=1+2") + expect(csvField('+1+2')).toBe("'+1+2") + expect(csvField('-1+2')).toBe("'-1+2") + expect(csvField('@1+2')).toBe("'@1+2") + expect(csvField('\t1+2')).toBe("'\t1+2") + expect(csvField('\r1+2')).toBe('"\'\r1+2"') + }) + + it('does not prepend single quote to raw numbers', () => { + expect(csvField(123)).toBe('123') + expect(csvField(-123)).toBe('-123') + expect(csvField(0)).toBe('0') + }) + + it('handles formula characters combined with quotes or commas properly', () => { + expect(csvField('="hello"')).toBe('"\'=""hello"""') + }) +}) diff --git a/packages/web/src/lib/server/csv/export.ts b/packages/web/src/lib/server/csv/export.ts new file mode 100644 index 00000000..90b5e4d1 --- /dev/null +++ b/packages/web/src/lib/server/csv/export.ts @@ -0,0 +1,12 @@ +export function csvField(value: string | number | null | undefined) { + if (value === null || value === undefined) return '' + let text = String(value) + + // Prevent CSV Formula Injection (Spreadsheet Macro Injection) + // Ensure that numbers retain their original formatting without injection prepending + if (typeof value !== 'number' && /^[=+\-@\t\r]/.test(text)) { + text = "'" + text + } + + return /[",\r\n]/.test(text) ? `"${text.replaceAll('"', '""')}"` : text +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6dfd315f..dabf7706 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -22,6 +22,7 @@ overrides: minimatch: ^10.0.0 '@hono/node-server': ^2.0.5 body-parser: ^2.3.0 + deepmerge-ts: 8.0.1 pnpmfileChecksum: qsp27c6veblwg3gxusbbzrumtm @@ -2278,8 +2279,8 @@ packages: deep-is@0.1.4: resolution: {integrity: sha512-oIPzksmTg4/MriiaYGO+okXDT7ztn/w3Eptv/+gSIdMdKsJo0u4CfYNFJPy+4SKMuCqGw2wxnA+URMg3t8a/bQ==} - deepmerge-ts@7.1.5: - resolution: {integrity: sha512-HOJkrhaYsweh+W+e74Yn7YStZOilkoPb6fycpwNLKzSPtruFs48nYis0zy5yJz1+ktUhHxoRDJ27RQAWLIJVJw==} + deepmerge-ts@8.0.1: + resolution: {integrity: sha512-szCXE7YLCvLKR9bFPJcvsezOShdalctSvrgN/LM/QGUEPZQajwjmsMObZ6/DuANT5lxzM/wtO8Feubwdkz8myA==} engines: {node: '>=16.0.0'} deepmerge@4.3.1: @@ -5659,7 +5660,7 @@ snapshots: '@prisma/config@6.19.3(magicast@0.3.5)': dependencies: c12: 3.1.0(magicast@0.3.5) - deepmerge-ts: 7.1.5 + deepmerge-ts: 8.0.1 effect: 3.21.0 empathic: 2.0.0 transitivePeerDependencies: @@ -6660,7 +6661,7 @@ snapshots: deep-is@0.1.4: {} - deepmerge-ts@7.1.5: {} + deepmerge-ts@8.0.1: {} deepmerge@4.3.1: {}