diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker index b0ab274ced77bf..9b7964462ad4f6 100755 --- a/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker +++ b/src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker @@ -195,6 +195,7 @@ kibana_vars=( xpack.reporting.capture.viewport.width xpack.reporting.capture.zoom xpack.reporting.csv.checkForFormulas + xpack.reporting.csv.escapeFormulaValues xpack.reporting.csv.enablePanelActionDownload xpack.reporting.csv.maxSizeBytes xpack.reporting.csv.scroll.duration diff --git a/x-pack/legacy/plugins/reporting/common/constants.ts b/x-pack/legacy/plugins/reporting/common/constants.ts index e3d6a4274e7df7..f30a7cc87f3187 100644 --- a/x-pack/legacy/plugins/reporting/common/constants.ts +++ b/x-pack/legacy/plugins/reporting/common/constants.ts @@ -20,6 +20,7 @@ export const API_GENERATE_IMMEDIATE = `${API_BASE_URL_V1}/generate/immediate/csv export const CONTENT_TYPE_CSV = 'text/csv'; export const CSV_REPORTING_ACTION = 'downloadCsvReport'; export const CSV_BOM_CHARS = '\ufeff'; +export const CSV_FORMULA_CHARS = ['=', '+', '-', '@']; export const WHITELISTED_JOB_CONTENT_TYPES = [ 'application/json', diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.test.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.test.ts index 0f7f01fa171215..ba527152a554a6 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.test.ts +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.test.ts @@ -300,7 +300,7 @@ describe('CSV Execute Job', function() { }); }); - describe('Cells with formula values', () => { + describe('Warning when cells have formulas', () => { it('returns `csv_contains_formulas` when cells contain formulas', async function() { configGetStub.withArgs('csv', 'checkForFormulas').returns(true); callAsCurrentUserStub.onFirstCall().returns({ @@ -353,6 +353,7 @@ describe('CSV Execute Job', function() { it('returns no warnings when cells have no formulas', async function() { configGetStub.withArgs('csv', 'checkForFormulas').returns(true); + configGetStub.withArgs('csv', 'escapeFormulaValues').returns(false); callAsCurrentUserStub.onFirstCall().returns({ hits: { hits: [{ _source: { one: 'foo', two: 'bar' } }], @@ -376,6 +377,33 @@ describe('CSV Execute Job', function() { expect(csvContainsFormulas).toEqual(false); }); + it('returns no warnings when cells have formulas but are escaped', async function() { + configGetStub.withArgs('csv', 'checkForFormulas').returns(true); + configGetStub.withArgs('csv', 'escapeFormulaValues').returns(true); + callAsCurrentUserStub.onFirstCall().returns({ + hits: { + hits: [{ _source: { '=SUM(A1:A2)': 'foo', two: 'bar' } }], + }, + _scroll_id: 'scrollId', + }); + + const executeJob = await executeJobFactory(mockReportingPlugin, mockLogger); + const jobParams = getJobDocPayload({ + headers: encryptedHeaders, + fields: ['=SUM(A1:A2)', 'two'], + conflictedTypesFields: [], + searchRequest: { index: null, body: null }, + }); + + const { csv_contains_formulas: csvContainsFormulas } = await executeJob( + 'job123', + jobParams, + cancellationToken + ); + + expect(csvContainsFormulas).toEqual(false); + }); + it('returns no warnings when configured not to', async () => { configGetStub.withArgs('csv', 'checkForFormulas').returns(false); callAsCurrentUserStub.onFirstCall().returns({ @@ -446,6 +474,50 @@ describe('CSV Execute Job', function() { }); }); + describe('Escaping cells with formulas', () => { + it('escapes values with formulas', async () => { + configGetStub.withArgs('csv', 'escapeFormulaValues').returns(true); + callAsCurrentUserStub.onFirstCall().returns({ + hits: { + hits: [{ _source: { one: `=cmd|' /C calc'!A0`, two: 'bar' } }], + }, + _scroll_id: 'scrollId', + }); + + const executeJob = await executeJobFactory(mockReportingPlugin, mockLogger); + const jobParams = getJobDocPayload({ + headers: encryptedHeaders, + fields: ['one', 'two'], + conflictedTypesFields: [], + searchRequest: { index: null, body: null }, + }); + const { content } = await executeJob('job123', jobParams, cancellationToken); + + expect(content).toEqual("one,two\n\"'=cmd|' /C calc'!A0\",bar\n"); + }); + + it('does not escapes values with formulas', async () => { + configGetStub.withArgs('csv', 'escapeFormulaValues').returns(false); + callAsCurrentUserStub.onFirstCall().returns({ + hits: { + hits: [{ _source: { one: `=cmd|' /C calc'!A0`, two: 'bar' } }], + }, + _scroll_id: 'scrollId', + }); + + const executeJob = await executeJobFactory(mockReportingPlugin, mockLogger); + const jobParams = getJobDocPayload({ + headers: encryptedHeaders, + fields: ['one', 'two'], + conflictedTypesFields: [], + searchRequest: { index: null, body: null }, + }); + const { content } = await executeJob('job123', jobParams, cancellationToken); + + expect(content).toEqual('one,two\n"=cmd|\' /C calc\'!A0",bar\n'); + }); + }); + describe('Elasticsearch call errors', function() { it('should reject Promise if search call errors out', async function() { callAsCurrentUserStub.rejects(new Error()); diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.ts index 376a398da274f9..dbe305bc452dbf 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.ts +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.ts @@ -123,7 +123,7 @@ export const executeJobFactory: ExecuteJobFactory + CSV_FORMULA_CHARS.some(formulaChar => startsWith(val, formulaChar)); diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts index 09f7cd2061ffbb..0ec39c527d6567 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts @@ -5,8 +5,7 @@ */ import * as _ from 'lodash'; - -const formulaValues = ['=', '+', '-', '@']; +import { cellHasFormulas } from './cell_has_formula'; interface IFlattened { [header: string]: string; @@ -14,7 +13,7 @@ interface IFlattened { export const checkIfRowsHaveFormulas = (flattened: IFlattened, fields: string[]) => { const pruned = _.pick(flattened, fields); - const csvValues = [..._.keys(pruned), ...(_.values(pruned) as string[])]; + const cells = [..._.keys(pruned), ...(_.values(pruned) as string[])]; - return _.some(csvValues, cell => _.some(formulaValues, char => _.startsWith(cell, char))); + return _.some(cells, cell => cellHasFormulas(cell)); }; diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.test.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.test.ts index 64b021a2aeea84..dd0f9d08b864b9 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.test.ts +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.test.ts @@ -11,7 +11,7 @@ describe('escapeValue', function() { describe('quoteValues is true', function() { let escapeValue: (val: string) => string; beforeEach(function() { - escapeValue = createEscapeValue(true); + escapeValue = createEscapeValue(true, false); }); it('should escape value with spaces', function() { @@ -46,7 +46,7 @@ describe('escapeValue', function() { describe('quoteValues is false', function() { let escapeValue: (val: string) => string; beforeEach(function() { - escapeValue = createEscapeValue(false); + escapeValue = createEscapeValue(false, false); }); it('should return the value unescaped', function() { @@ -54,4 +54,34 @@ describe('escapeValue', function() { expect(escapeValue(value)).to.be(value); }); }); + + describe('escapeValues', () => { + describe('when true', () => { + let escapeValue: (val: string) => string; + beforeEach(function() { + escapeValue = createEscapeValue(true, true); + }); + + ['@', '+', '-', '='].forEach(badChar => { + it(`should escape ${badChar} injection values`, function() { + expect(escapeValue(`${badChar}cmd|' /C calc'!A0`)).to.be( + `"'${badChar}cmd|' /C calc'!A0"` + ); + }); + }); + }); + + describe('when false', () => { + let escapeValue: (val: string) => string; + beforeEach(function() { + escapeValue = createEscapeValue(true, false); + }); + + ['@', '+', '-', '='].forEach(badChar => { + it(`should not escape ${badChar} injection values`, function() { + expect(escapeValue(`${badChar}cmd|' /C calc'!A0`)).to.be(`"${badChar}cmd|' /C calc'!A0"`); + }); + }); + }); + }); }); diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.ts index 563de563350e9e..60e75d74b2f98e 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.ts +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.ts @@ -5,15 +5,20 @@ */ import { RawValue } from './types'; +import { cellHasFormulas } from './cell_has_formula'; const nonAlphaNumRE = /[^a-zA-Z0-9]/; const allDoubleQuoteRE = /"/g; -export function createEscapeValue(quoteValues: boolean): (val: RawValue) => string { +export function createEscapeValue( + quoteValues: boolean, + escapeFormulas: boolean +): (val: RawValue) => string { return function escapeValue(val: RawValue) { if (val && typeof val === 'string') { - if (quoteValues && nonAlphaNumRE.test(val)) { - return `"${val.replace(allDoubleQuoteRE, '""')}"`; + const formulasEscaped = escapeFormulas && cellHasFormulas(val) ? "'" + val : val; + if (quoteValues && nonAlphaNumRE.test(formulasEscaped)) { + return `"${formulasEscaped.replace(allDoubleQuoteRE, '""')}"`; } } diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.ts index 1986e68917ba86..c7996ebf832a19 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.ts +++ b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { i18n } from '@kbn/i18n'; import { Logger } from '../../../../types'; import { GenerateCsvParams, SavedSearchGeneratorResult } from '../../types'; import { createFlattenHit } from './flatten_hit'; @@ -26,14 +27,17 @@ export function createGenerateCsv(logger: Logger) { cancellationToken, settings, }: GenerateCsvParams): Promise { - const escapeValue = createEscapeValue(settings.quoteValues); + const escapeValue = createEscapeValue(settings.quoteValues, settings.escapeFormulaValues); const builder = new MaxSizeStringBuilder(settings.maxSizeBytes); const header = `${fields.map(escapeValue).join(settings.separator)}\n`; + const warnings: string[] = []; + if (!builder.tryAppend(header)) { return { size: 0, content: '', maxSizeReached: true, + warnings: [], }; } @@ -82,11 +86,20 @@ export function createGenerateCsv(logger: Logger) { const size = builder.getSizeInBytes(); logger.debug(`finished generating, total size in bytes: ${size}`); + if (csvContainsFormulas && settings.escapeFormulaValues) { + warnings.push( + i18n.translate('xpack.reporting.exportTypes.csv.generateCsv.escapedFormulaValues', { + defaultMessage: 'CSV may contain formulas whose values have been escaped', + }) + ); + } + return { content: builder.getString(), - csvContainsFormulas, + csvContainsFormulas: csvContainsFormulas && !settings.escapeFormulaValues, maxSizeReached, size, + warnings, }; }; } diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/types.d.ts b/x-pack/legacy/plugins/reporting/export_types/csv/types.d.ts index 529c195486bc6d..40a42db3526352 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv/types.d.ts +++ b/x-pack/legacy/plugins/reporting/export_types/csv/types.d.ts @@ -87,6 +87,7 @@ export interface SavedSearchGeneratorResult { size: number; maxSizeReached: boolean; csvContainsFormulas?: boolean; + warnings: string[]; } export interface CsvResultFromSearch { @@ -109,5 +110,6 @@ export interface GenerateCsvParams { maxSizeBytes: number; scroll: ScrollConfig; checkForFormulas?: boolean; + escapeFormulaValues: boolean; }; } diff --git a/x-pack/legacy/plugins/reporting/export_types/csv_from_savedobject/server/lib/generate_csv_search.ts b/x-pack/legacy/plugins/reporting/export_types/csv_from_savedobject/server/lib/generate_csv_search.ts index 9757c71c19cf41..2611b74c83de92 100644 --- a/x-pack/legacy/plugins/reporting/export_types/csv_from_savedobject/server/lib/generate_csv_search.ts +++ b/x-pack/legacy/plugins/reporting/export_types/csv_from_savedobject/server/lib/generate_csv_search.ts @@ -173,6 +173,7 @@ export async function generateCsvSearch( ...uiSettings, maxSizeBytes: config.get('csv', 'maxSizeBytes'), scroll: config.get('csv', 'scroll'), + escapeFormulaValues: config.get('csv', 'escapeFormulaValues'), timezone, }, }; diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index 67d70c1513c15d..402fddcb5e0146 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -114,6 +114,7 @@ const CaptureSchema = schema.object({ const CsvSchema = schema.object({ checkForFormulas: schema.boolean({ defaultValue: true }), + escapeFormulaValues: schema.boolean({ defaultValue: false }), enablePanelActionDownload: schema.boolean({ defaultValue: true }), maxSizeBytes: schema.number({ defaultValue: 1024 * 1024 * 10, // 10MB