maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[Reporting] Config flag to escape formula CSV values (#63645) (#63987)

Browse source 
* Adds a new xpack.reporting.csv.escapeFormulaValues boolean to auto-escape potentially bad cells

* Treat csvs with formulas differently than those that aren't escaped

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Joel Griffith 2020-04-21 10:03:45 -07:00 committed by GitHub
parent 90cd6486e9
commit 58aa4da1d2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 152 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker
View file

@ -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

1
x-pack/legacy/plugins/reporting/common/constants.ts
View file

@ -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',

74
x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.test.ts
View file

@ -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());

5
x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.ts
View file

@ -123,7 +123,7 @@ export const executeJobFactory: ExecuteJobFactory<ESQueueWorkerExecuteFn<
const generateCsv = createGenerateCsv(jobLogger);
const bom = config.get('csv', 'useByteOrderMarkEncoding') ? CSV_BOM_CHARS : '';
const { content, maxSizeReached, size, csvContainsFormulas } = await generateCsv({
const { content, maxSizeReached, size, csvContainsFormulas, warnings } = await generateCsv({
searchRequest,
fields,
metaFields,
@ -136,15 +136,18 @@ export const executeJobFactory: ExecuteJobFactory<ESQueueWorkerExecuteFn<
checkForFormulas: config.get('csv', 'checkForFormulas'),
maxSizeBytes: config.get('csv', 'maxSizeBytes'),
scroll: config.get('csv', 'scroll'),
escapeFormulaValues: config.get('csv', 'escapeFormulaValues'),
},
});
// @TODO: Consolidate these one-off warnings into the warnings array (max-size reached and csv contains formulas)
return {
content_type: 'text/csv',
content: bom + content,
max_size_reached: maxSizeReached,
size,
csv_contains_formulas: csvContainsFormulas,
warnings,
};
};
};

11
x-pack/legacy/plugins/reporting/export_types/csv/server/lib/cell_has_formula.ts Normal file
View file

@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { startsWith } from 'lodash';
import { CSV_FORMULA_CHARS } from '../../../../common/constants';
export const cellHasFormulas = (val: string) =>
CSV_FORMULA_CHARS.some(formulaChar => startsWith(val, formulaChar));

7
x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts
View file

@ -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));
};

34
x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.test.ts
View file

@ -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"`);
});
});
});
});
});

11
x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.ts
View file

@ -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, '""')}"`;
}
}

17
x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.ts
View file

@ -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<SavedSearchGeneratorResult> {
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,
};
};
}

2
x-pack/legacy/plugins/reporting/export_types/csv/types.d.ts vendored
View file

@ -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;
};
}

1
x-pack/legacy/plugins/reporting/export_types/csv_from_savedobject/server/lib/generate_csv_search.ts
View file

@ -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,
},
};

1
x-pack/plugins/reporting/server/config/schema.ts
View file

@ -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