Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 97 additions & 0 deletions apps/api/src/evidence-forms/evidence-forms.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ jest.mock('@db', () => {
db: {
evidenceSubmission: {
findFirst: jest.fn(),
findMany: jest.fn(),
groupBy: jest.fn(),
update: jest.fn(),
create: jest.fn(),
Expand All @@ -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;
Expand Down Expand Up @@ -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();
});
});
});
22 changes: 21 additions & 1 deletion apps/api/src/evidence-forms/evidence-forms.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, '""')}"`)
Comment thread
dennisofficial marked this conversation as resolved.
.join(',');
}

function flattenValue(value: unknown): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
62 changes: 0 additions & 62 deletions apps/portal/src/actions/accept-policies.ts

This file was deleted.