From 2114033b6ea57d939f6b978bb16d56c34aa94970 Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Wed, 23 Sep 2026 15:06:12 -0400 Subject: [PATCH 1/4] fix(security): neutralize formula prefixes in evidence-form CSV export (GH-097) --- .../evidence-forms.service.spec.ts | 62 +++++++++++++++++++ .../evidence-forms/evidence-forms.service.ts | 15 ++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/apps/api/src/evidence-forms/evidence-forms.service.spec.ts b/apps/api/src/evidence-forms/evidence-forms.service.spec.ts index 9093e6c305..041189bec0 100644 --- a/apps/api/src/evidence-forms/evidence-forms.service.spec.ts +++ b/apps/api/src/evidence-forms/evidence-forms.service.spec.ts @@ -40,6 +40,7 @@ jest.mock('@db', () => { db: { evidenceSubmission: { findFirst: jest.fn(), + findMany: jest.fn(), groupBy: jest.fn(), update: jest.fn(), create: jest.fn(), @@ -55,6 +56,7 @@ jest.mock('@db', () => { type MockDb = { evidenceSubmission: { findFirst: jest.Mock; + findMany: jest.Mock; groupBy: jest.Mock; update: jest.Mock; create: jest.Mock; @@ -336,4 +338,64 @@ describe('EvidenceFormsService', () => { }); }); }); + + describe('exportCsv', () => { + it('neutralizes spreadsheet formula prefixes in submitted free text (GH-097)', async () => { + mockedDb.evidenceSubmission.findMany.mockResolvedValue([ + { + id: 'sub_attack', + submittedAt: new Date('2026-09-18T00:00:00.000Z'), + submittedBy: { name: 'Mallory', email: 'mallory@example.com' }, + data: { + submissionDate: '2026-09-18', + incidentDate: '2026-09-18', + complaintDetails: "=cmd|' /C calc'!A0", + individualsInvolved: '+SUM(A1:A2)', + evidence: '-10+20', + }, + }, + { + id: 'sub_benign', + submittedAt: new Date('2026-09-18T01:00:00.000Z'), + submittedBy: { name: 'Alice', email: 'alice@example.com' }, + data: { + submissionDate: '2026-09-18', + incidentDate: '2026-09-18', + complaintDetails: '@channel please review', + individualsInvolved: 'Bob and Carol', + evidence: 'He said "hi"', + }, + }, + ]); + + const csv = await service.exportCsv({ + organizationId: 'org_123', + formType: 'whistleblower-report', + authContext, + }); + + const lines = csv.split('\n'); + expect(lines).toHaveLength(3); + // Every formula-leading value is prefixed with a single quote so + // Excel/Sheets render it as text instead of evaluating it. + expect(lines[1]).toContain(`"'=cmd|' /C calc'!A0"`); + expect(lines[1]).toContain('"\'+SUM(A1:A2)"'); + expect(lines[1]).toContain('"\'-10+20"'); + expect(lines[2]).toContain(`"'@channel please review"`); + // Benign values stay untouched; embedded-quote escaping is unchanged. + expect(lines[2]).toContain('"Bob and Carol"'); + expect(lines[2]).toContain('He said ""hi""'); + }); + + it('rejects export for reviewers without privileged evidence access', async () => { + await expect( + service.exportCsv({ + organizationId: 'org_123', + formType: 'whistleblower-report', + authContext: { ...authContext, userRoles: ['employee'] }, + }), + ).rejects.toThrow(); + expect(mockedDb.evidenceSubmission.findMany).not.toHaveBeenCalled(); + }); + }); }); diff --git a/apps/api/src/evidence-forms/evidence-forms.service.ts b/apps/api/src/evidence-forms/evidence-forms.service.ts index 86050d2c55..8eed2b0c0d 100644 --- a/apps/api/src/evidence-forms/evidence-forms.service.ts +++ b/apps/api/src/evidence-forms/evidence-forms.service.ts @@ -58,8 +58,21 @@ const EVIDENCE_FORM_DELETE_ROLES = ['owner', 'admin'] as const; const MAX_UPLOAD_FILE_SIZE_BYTES = 100 * 1024 * 1024; const MAX_UPLOAD_BASE64_LENGTH = Math.ceil(MAX_UPLOAD_FILE_SIZE_BYTES / 3) * 4; +// Prefix values that spreadsheet apps would parse as formulas so exported +// CSVs can't carry executable content (CSV formula injection, GH-097). +// Excel/Sheets strip the surrounding quotes before evaluating, so quoting +// alone is not a mitigation. +function neutralizeFormula(value: string): string { + if (/^[=+\-@\t\r]/.test(value)) { + return `'${value}`; + } + return value; +} + function toCsvRow(values: string[]): string { - return values.map((value) => `"${value.replace(/"/g, '""')}"`).join(','); + return values + .map((value) => `"${neutralizeFormula(value).replace(/"/g, '""')}"`) + .join(','); } function flattenValue(value: unknown): string { From 1f8fb1215e5d6417dc40ecae4bd1e1849885911a Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Wed, 23 Sep 2026 15:06:17 -0400 Subject: [PATCH 2/4] fix(security): delete unauthenticated accept-policies server action (GH-266) --- .../policy-acknowledgment-digest-helpers.ts | 2 +- apps/portal/src/actions/accept-policies.ts | 62 ------------------- 2 files changed, 1 insertion(+), 63 deletions(-) delete mode 100644 apps/portal/src/actions/accept-policies.ts diff --git a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts index 8cea31d1de..2415dd06e2 100644 --- a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts +++ b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts @@ -109,7 +109,7 @@ export function computePendingPolicies( policies: DigestPolicy[], ): DigestPolicy[] { return policies.filter((policy) => { - // signedBy stores member ids (see apps/portal/src/actions/accept-policies.ts), + // signedBy stores member ids (see apps/portal accept-policies API route), // not user ids — every other consumer checks against member.id. if (policy.signedBy.includes(member.id)) return false; if (policy.visibility === 'ALL') return true; diff --git a/apps/portal/src/actions/accept-policies.ts b/apps/portal/src/actions/accept-policies.ts deleted file mode 100644 index aa19d11cbd..0000000000 --- a/apps/portal/src/actions/accept-policies.ts +++ /dev/null @@ -1,62 +0,0 @@ -'use server'; - -import { db } from '@db/server'; - -export async function acceptPolicy(policyId: string, memberId: string) { - try { - // Add the member ID to the signedBy array if not already present - const policy = await db.policy.findUnique({ - where: { id: policyId }, - }); - - if (!policy) { - throw new Error('Policy not found'); - } - - if (!policy.signedBy.includes(memberId)) { - await db.policy.update({ - where: { id: policyId }, - data: { - signedBy: { - push: memberId, - }, - }, - }); - } - - return { success: true }; - } catch (error) { - console.error('Error accepting policy:', error); - return { success: false, error: 'Failed to accept policy' }; - } -} - -export async function acceptAllPolicies(policyIds: string[], memberId: string) { - try { - // Update all policies to include the member ID in signedBy array - const updatePromises = policyIds.map(async (policyId) => { - const policy = await db.policy.findUnique({ - where: { id: policyId }, - }); - - if (policy && !policy.signedBy.includes(memberId)) { - return db.policy.update({ - where: { id: policyId }, - data: { - signedBy: { - push: memberId, - }, - }, - }); - } - return null; - }); - - await Promise.all(updatePromises); - - return { success: true }; - } catch (error) { - console.error('Error accepting all policies:', error); - return { success: false, error: 'Failed to accept all policies' }; - } -} From 3c9cd011c8fc9d41e4b5c9d0fc7ec0ebf7cb02cd Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Wed, 23 Sep 2026 16:36:51 -0400 Subject: [PATCH 3/4] fix(security): include line feed in CSV formula-prefix neutralization (GH-097) --- .../evidence-forms.service.spec.ts | 26 ++++++++++++++++--- .../evidence-forms/evidence-forms.service.ts | 6 +++-- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/apps/api/src/evidence-forms/evidence-forms.service.spec.ts b/apps/api/src/evidence-forms/evidence-forms.service.spec.ts index 041189bec0..789387fd68 100644 --- a/apps/api/src/evidence-forms/evidence-forms.service.spec.ts +++ b/apps/api/src/evidence-forms/evidence-forms.service.spec.ts @@ -354,6 +354,18 @@ describe('EvidenceFormsService', () => { evidence: '-10+20', }, }, + { + id: 'sub_newline', + submittedAt: new Date('2026-09-18T00:30:00.000Z'), + submittedBy: { name: 'Mallory', email: 'mallory@example.com' }, + data: { + submissionDate: '2026-09-18', + incidentDate: '2026-09-18', + complaintDetails: '\n=HYPERLINK("https://evil.example","x")', + individualsInvolved: 'Mallory', + evidence: 'x', + }, + }, { id: 'sub_benign', submittedAt: new Date('2026-09-18T01:00:00.000Z'), @@ -374,17 +386,23 @@ describe('EvidenceFormsService', () => { authContext, }); + // The sub_newline row carries an embedded newline inside a quoted + // cell, so a naive split on \n yields 5 pieces, not 4. const lines = csv.split('\n'); - expect(lines).toHaveLength(3); + expect(lines).toHaveLength(5); // Every formula-leading value is prefixed with a single quote so // Excel/Sheets render it as text instead of evaluating it. expect(lines[1]).toContain(`"'=cmd|' /C calc'!A0"`); expect(lines[1]).toContain('"\'+SUM(A1:A2)"'); expect(lines[1]).toContain('"\'-10+20"'); - expect(lines[2]).toContain(`"'@channel please review"`); + // A leading line feed must not smuggle a formula past the check: + // the quote prefix lands before the \n so nothing evaluates it. + expect(lines[2].endsWith(',"\'')).toBe(true); + expect(lines[3]).toContain('=HYPERLINK(""https://evil.example"",""x"")'); + expect(lines[4]).toContain(`"'@channel please review"`); // Benign values stay untouched; embedded-quote escaping is unchanged. - expect(lines[2]).toContain('"Bob and Carol"'); - expect(lines[2]).toContain('He said ""hi""'); + expect(lines[4]).toContain('"Bob and Carol"'); + expect(lines[4]).toContain('He said ""hi""'); }); it('rejects export for reviewers without privileged evidence access', async () => { diff --git a/apps/api/src/evidence-forms/evidence-forms.service.ts b/apps/api/src/evidence-forms/evidence-forms.service.ts index 8eed2b0c0d..9e8d884062 100644 --- a/apps/api/src/evidence-forms/evidence-forms.service.ts +++ b/apps/api/src/evidence-forms/evidence-forms.service.ts @@ -61,9 +61,11 @@ const MAX_UPLOAD_BASE64_LENGTH = Math.ceil(MAX_UPLOAD_FILE_SIZE_BYTES / 3) * 4; // Prefix values that spreadsheet apps would parse as formulas so exported // CSVs can't carry executable content (CSV formula injection, GH-097). // Excel/Sheets strip the surrounding quotes before evaluating, so quoting -// alone is not a mitigation. +// alone is not a mitigation. Line feed is included: some parsers trim +// leading whitespace before evaluating, so a "\n=..." value could still +// execute. function neutralizeFormula(value: string): string { - if (/^[=+\-@\t\r]/.test(value)) { + if (/^[=+\-@\t\r\n]/.test(value)) { return `'${value}`; } return value; From 88ce8ab8796ecd6b42c691d9aa6cdf48b05a808a Mon Sep 17 00:00:00 2001 From: Dennis Lysenko Date: Thu, 24 Sep 2026 12:35:06 -0400 Subject: [PATCH 4/4] fix(security): neutralize full-width CSV formula prefixes (GH-097) --- .../evidence-forms.service.spec.ts | 27 +++++++++++++++---- .../evidence-forms/evidence-forms.service.ts | 9 +++++-- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/apps/api/src/evidence-forms/evidence-forms.service.spec.ts b/apps/api/src/evidence-forms/evidence-forms.service.spec.ts index 789387fd68..e14fb4a61b 100644 --- a/apps/api/src/evidence-forms/evidence-forms.service.spec.ts +++ b/apps/api/src/evidence-forms/evidence-forms.service.spec.ts @@ -366,6 +366,20 @@ describe('EvidenceFormsService', () => { evidence: 'x', }, }, + { + id: 'sub_fullwidth', + submittedAt: new Date('2026-09-18T00:45:00.000Z'), + submittedBy: { name: 'Mallory', email: 'mallory@example.com' }, + data: { + submissionDate: '2026-09-18', + incidentDate: '2026-09-18', + // Full-width formula prefixes (U+FF1D etc.): OWASP notes some + // locales may treat these as formula starters. + complaintDetails: '\uFF1DSUM(A1:A2)', + individualsInvolved: '\uFF0B1+2', + evidence: '\uFF0D10', + }, + }, { id: 'sub_benign', submittedAt: new Date('2026-09-18T01:00:00.000Z'), @@ -387,9 +401,9 @@ describe('EvidenceFormsService', () => { }); // The sub_newline row carries an embedded newline inside a quoted - // cell, so a naive split on \n yields 5 pieces, not 4. + // cell, so a naive split on \n yields 6 pieces, not 5. const lines = csv.split('\n'); - expect(lines).toHaveLength(5); + expect(lines).toHaveLength(6); // Every formula-leading value is prefixed with a single quote so // Excel/Sheets render it as text instead of evaluating it. expect(lines[1]).toContain(`"'=cmd|' /C calc'!A0"`); @@ -399,10 +413,13 @@ describe('EvidenceFormsService', () => { // the quote prefix lands before the \n so nothing evaluates it. expect(lines[2].endsWith(',"\'')).toBe(true); expect(lines[3]).toContain('=HYPERLINK(""https://evil.example"",""x"")'); - expect(lines[4]).toContain(`"'@channel please review"`); + expect(lines[4]).toContain(`"'\uFF1DSUM(A1:A2)"`); + expect(lines[4]).toContain(`"'\uFF0B1+2"`); + expect(lines[4]).toContain(`"'\uFF0D10"`); + expect(lines[5]).toContain(`"'@channel please review"`); // Benign values stay untouched; embedded-quote escaping is unchanged. - expect(lines[4]).toContain('"Bob and Carol"'); - expect(lines[4]).toContain('He said ""hi""'); + expect(lines[5]).toContain('"Bob and Carol"'); + expect(lines[5]).toContain('He said ""hi""'); }); it('rejects export for reviewers without privileged evidence access', async () => { diff --git a/apps/api/src/evidence-forms/evidence-forms.service.ts b/apps/api/src/evidence-forms/evidence-forms.service.ts index 9e8d884062..7f090295c4 100644 --- a/apps/api/src/evidence-forms/evidence-forms.service.ts +++ b/apps/api/src/evidence-forms/evidence-forms.service.ts @@ -63,9 +63,14 @@ const MAX_UPLOAD_BASE64_LENGTH = Math.ceil(MAX_UPLOAD_FILE_SIZE_BYTES / 3) * 4; // Excel/Sheets strip the surrounding quotes before evaluating, so quoting // alone is not a mitigation. Line feed is included: some parsers trim // leading whitespace before evaluating, so a "\n=..." value could still -// execute. +// execute. The full-width variants (\uFF1D =, \uFF0B +, \uFF0D -, +// \uFF20 @) are covered too: OWASP's CSV Injection guidance notes some +// locales (e.g. Japanese environments) may interpret them as formula +// starters. +const FORMULA_PREFIX_PATTERN = /^[=+\-@\t\r\n\uFF1D\uFF0B\uFF0D\uFF20]/; + function neutralizeFormula(value: string): string { - if (/^[=+\-@\t\r\n]/.test(value)) { + if (FORMULA_PREFIX_PATTERN.test(value)) { return `'${value}`; } return value;