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..e14fb4a61b 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,99 @@ 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_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_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'), + 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, + }); + + // The sub_newline row carries an embedded newline inside a quoted + // cell, so a naive split on \n yields 6 pieces, not 5. + const lines = csv.split('\n'); + 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"`); + expect(lines[1]).toContain('"\'+SUM(A1:A2)"'); + expect(lines[1]).toContain('"\'-10+20"'); + // 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(`"'\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[5]).toContain('"Bob and Carol"'); + expect(lines[5]).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..7f090295c4 100644 --- a/apps/api/src/evidence-forms/evidence-forms.service.ts +++ b/apps/api/src/evidence-forms/evidence-forms.service.ts @@ -58,8 +58,28 @@ 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. Line feed is included: some parsers trim +// leading whitespace before evaluating, so a "\n=..." value could still +// 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 (FORMULA_PREFIX_PATTERN.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 { 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' }; - } -}