diff --git a/.eslintrc.js b/.eslintrc.js
index 63b2cf1d6167..0a397b6b47f8 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -604,6 +604,19 @@ module.exports = {
name: 'lodash/fp/assocPath',
message: 'Please use @elastic/safer-lodash-set instead',
},
+ {
+ name: 'lodash',
+ importNames: ['template'],
+ message: 'lodash.template is not allowed due to security concerns',
+ },
+ {
+ name: 'lodash.template',
+ message: 'lodash.template is not allowed due to security concerns',
+ },
+ {
+ name: 'lodash/template',
+ message: 'lodash.template is not allowed due to security concerns',
+ },
],
},
],
@@ -627,6 +640,14 @@ module.exports = {
name: 'lodash/setWith',
message: 'Please use @elastic/safer-lodash-set instead',
},
+ {
+ name: 'lodash.template',
+ message: 'lodash.template is not allowed due to security concerns',
+ },
+ {
+ name: 'lodash/template',
+ message: 'lodash.template is not allowed due to security concerns',
+ },
],
},
],
@@ -672,6 +693,16 @@ module.exports = {
property: 'assocPath',
message: 'Please use @elastic/safer-lodash-set instead',
},
+ {
+ object: 'lodash',
+ property: 'template',
+ message: 'lodash.template is not allowed due to security concerns',
+ },
+ {
+ object: '_',
+ property: 'template',
+ message: 'lodash.template is not allowed due to security concerns',
+ },
],
},
},
@@ -741,6 +772,21 @@ module.exports = {
'error',
{
patterns: ['lodash/*', '!lodash/fp'],
+ paths: [
+ {
+ name: 'lodash',
+ importNames: ['template'],
+ message: 'lodash.template is not allowed due to security concerns',
+ },
+ {
+ name: 'lodash.template',
+ message: 'lodash.template is not allowed due to security concerns',
+ },
+ {
+ name: 'lodash/template',
+ message: 'lodash.template is not allowed due to security concerns',
+ },
+ ],
},
],
},
diff --git a/changelogs/fragments/10920.yml b/changelogs/fragments/10920.yml
new file mode 100644
index 000000000000..10b110bffc38
--- /dev/null
+++ b/changelogs/fragments/10920.yml
@@ -0,0 +1,5 @@
+breaking:
+- Remove usage of lodash template ([#10920](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/10920))
+
+infra:
+- Add in eslint rule to prevent the use of lodash template ([#10920](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/10920))
\ No newline at end of file
diff --git a/src/plugins/data/common/field_formats/converters/color.ts b/src/plugins/data/common/field_formats/converters/color.ts
index e5a2449afeee..05ef22e88c66 100644
--- a/src/plugins/data/common/field_formats/converters/color.ts
+++ b/src/plugins/data/common/field_formats/converters/color.ts
@@ -29,15 +29,13 @@
*/
import { i18n } from '@osd/i18n';
-import { findLast, cloneDeep, template, escape } from 'lodash';
+import { findLast, cloneDeep, escape } from 'lodash';
import { OSD_FIELD_TYPES } from '../../osd_field_types/types';
import { FieldFormat } from '../field_format';
import { HtmlContextTypeConvert, FIELD_FORMAT_IDS } from '../types';
import { asPrettyString } from '../utils';
import { DEFAULT_CONVERTER_COLOR } from '../constants/color_default';
-const convertTemplate = template('<%- val %>');
-
export class ColorFormat extends FieldFormat {
static id = FIELD_FORMAT_IDS.COLOR;
static title = i18n.translate('data.fieldFormats.color.title', {
@@ -78,6 +76,6 @@ export class ColorFormat extends FieldFormat {
let style = '';
if (color.text) style += `color: ${color.text};`;
if (color.background) style += `background-color: ${color.background};`;
- return convertTemplate({ val, style });
+ return `${escape(asPrettyString(val))}`;
};
}
diff --git a/src/plugins/data/common/field_formats/converters/source.test.ts b/src/plugins/data/common/field_formats/converters/source.test.ts
index 763987f05054..727faab0b4f9 100644
--- a/src/plugins/data/common/field_formats/converters/source.test.ts
+++ b/src/plugins/data/common/field_formats/converters/source.test.ts
@@ -53,4 +53,156 @@ describe('Source Format', () => {
'{"foo":"bar","number":42,"hello":"<h1>World</h1>","also":"with \\"quotes\\" or 'single quotes'"}'
);
});
+
+ describe('templateHtml rendering', () => {
+ test('should render a definition list with field name and value', () => {
+ const mockIndexPattern = {
+ formatHit: jest.fn().mockReturnValue({
+ fieldName: 'field value',
+ }),
+ };
+
+ const hit = {
+ _source: { fieldName: 'field value' },
+ };
+
+ const field = { name: 'fieldName' };
+
+ const result = convertHtml(hit, { field, hit, indexPattern: mockIndexPattern });
+
+ expect(result).toContain('');
+ expect(result).toContain('- fieldName:
');
+ expect(result).toContain('- field value
');
+ expect(mockIndexPattern.formatHit).toHaveBeenCalledWith(hit);
+ });
+
+ test('should render multiple fields in the definition list', () => {
+ const mockIndexPattern = {
+ formatHit: jest.fn().mockReturnValue({
+ firstName: 'John',
+ lastName: 'Doe',
+ age: '30',
+ }),
+ };
+
+ const hit = {
+ _source: { firstName: 'John', lastName: 'Doe', age: 30 },
+ };
+
+ const field = { name: 'firstName' };
+
+ const result = convertHtml(hit, { field, hit, indexPattern: mockIndexPattern });
+
+ expect(result).toContain('- firstName:
');
+ expect(result).toContain('- John
');
+ expect(result).toContain('- lastName:
');
+ expect(result).toContain('- Doe
');
+ expect(result).toContain('- age:
');
+ expect(result).toContain('- 30
');
+ });
+
+ test('should escape HTML in field names to prevent XSS', () => {
+ const mockIndexPattern = {
+ formatHit: jest.fn().mockReturnValue({
+ '': 'malicious field name',
+ }),
+ };
+
+ const hit = {
+ _source: { '': 'malicious field name' },
+ };
+
+ const field = { name: 'test' };
+
+ const result = convertHtml(hit, { field, hit, indexPattern: mockIndexPattern });
+
+ // Field name should be escaped
+ expect(result).toContain('<script>alert("xss")</script>');
+ // Should NOT contain executable script tag
+ expect(result).not.toContain('');
+ });
+
+ test('should preserve HTML markup in pre-formatted field values', () => {
+ const mockIndexPattern = {
+ formatHit: jest.fn().mockReturnValue({
+ colorField: 'red text',
+ urlField: 'link',
+ highlightField: 'text with highlighted term',
+ }),
+ };
+
+ const hit = {
+ _source: {
+ colorField: 'red text',
+ urlField: 'http://example.com',
+ highlightField: 'text with highlighted term',
+ },
+ };
+
+ const field = { name: 'test' };
+
+ const result = convertHtml(hit, { field, hit, indexPattern: mockIndexPattern });
+
+ // HTML markup from formatters should be preserved (not escaped)
+ expect(result).toContain('red text');
+ expect(result).toContain('link');
+ expect(result).toContain('text with highlighted term');
+ });
+
+ test('should use indexPattern.formatHit to format field values', () => {
+ const mockFormatHit = jest.fn().mockReturnValue({
+ formattedField: 'formatted value',
+ });
+
+ const mockIndexPattern = {
+ formatHit: mockFormatHit,
+ };
+
+ const hit = {
+ _source: { formattedField: 'raw value' },
+ };
+
+ const field = { name: 'formattedField' };
+
+ convertHtml(hit, { field, hit, indexPattern: mockIndexPattern });
+
+ // Verify that indexPattern.formatHit was called with the hit
+ expect(mockFormatHit).toHaveBeenCalledWith(hit);
+ expect(mockFormatHit).toHaveBeenCalledTimes(1);
+ });
+
+ test('should include highlighted fields from hit.highlight', () => {
+ const mockIndexPattern = {
+ formatHit: jest.fn().mockReturnValue({
+ highlightedField: 'text with match',
+ regularField: 'regular value',
+ }),
+ };
+
+ const hit = {
+ _source: {
+ highlightedField: 'text with match',
+ regularField: 'regular value',
+ },
+ highlight: {
+ highlightedField: ['text with match'],
+ },
+ };
+
+ const field = { name: 'test' };
+
+ const result = convertHtml(hit, { field, hit, indexPattern: mockIndexPattern });
+
+ // Both highlighted and regular fields should be present
+ expect(result).toContain('- highlightedField:
');
+ expect(result).toContain('- text with match
');
+ expect(result).toContain('- regularField:
');
+ expect(result).toContain('- regular value
');
+
+ // Highlighted field should appear before regular field
+ const highlightedIndex = result.indexOf('highlightedField');
+ const regularIndex = result.indexOf('regularField');
+ expect(highlightedIndex).toBeLessThan(regularIndex);
+ });
+ });
});
diff --git a/src/plugins/data/common/field_formats/converters/source.ts b/src/plugins/data/common/field_formats/converters/source.ts
index 67a5a0bfde40..a50275138cb5 100644
--- a/src/plugins/data/common/field_formats/converters/source.ts
+++ b/src/plugins/data/common/field_formats/converters/source.ts
@@ -28,41 +28,13 @@
* under the License.
*/
-import { template, escape, keys } from 'lodash';
+import { escape, keys } from 'lodash';
import { shortenDottedString } from '../../utils';
import { OSD_FIELD_TYPES } from '../../osd_field_types/types';
import { FieldFormat } from '../field_format';
import { TextContextTypeConvert, HtmlContextTypeConvert, FIELD_FORMAT_IDS } from '../types';
import { UI_SETTINGS } from '../../constants';
-/**
- * Remove all of the whitespace between html tags
- * so that inline elements don't have extra spaces.
- *
- * If you have inline elements (span, a, em, etc.) and any
- * amount of whitespace around them in your markup, then the
- * browser will push them apart. This is ugly in certain
- * scenarios and is only fixed by removing the whitespace
- * from the html in the first place (or ugly css hacks).
- *
- * @param {string} html - the html to modify
- * @return {string} - modified html
- */
-function noWhiteSpace(html: string) {
- const TAGS_WITH_WS = />\s+<');
-}
-
-const templateHtml = `
-
- <% defPairs.forEach(function (def) { %>
- - <%- def[0] %>:
- - <%= def[1] %>
- <%= ' ' %>
- <% }); %>
-
`;
-const doTemplate = template(noWhiteSpace(templateHtml));
-
export class SourceFormat extends FieldFormat {
static id = FIELD_FORMAT_IDS._SOURCE;
static title = '_source';
@@ -92,6 +64,21 @@ export class SourceFormat extends FieldFormat {
pairs.push([newField, val]);
}, []);
- return doTemplate({ defPairs: highlightPairs.concat(sourcePairs) });
+ const defPairs = highlightPairs.concat(sourcePairs);
+ /**
+ * Build HTML without whitespace between tags to prevent extra spacing.
+ *
+ * If you have inline elements (span, a, em, etc.) and any amount of
+ * whitespace around them in your markup, then the browser will push
+ * them apart. This is ugly in certain scenarios and is only fixed by
+ * removing the whitespace from the html in the first place (or ugly css hacks).
+ *
+ * Note: The space after is intentional to provide proper spacing between
+ * definition pairs.
+ */
+ const pairHtml = defPairs
+ .map((def) => `- ${escape(def[0])}:
- ${def[1]}
`)
+ .join('');
+ return `${pairHtml}
`;
};
}
diff --git a/src/plugins/vis_type_vislib/public/vislib/partials/touchdown.tmpl.html b/src/plugins/vis_type_vislib/public/vislib/partials/touchdown.tmpl.html
deleted file mode 100644
index ee95eef68f3b..000000000000
--- a/src/plugins/vis_type_vislib/public/vislib/partials/touchdown.tmpl.html
+++ /dev/null
@@ -1,7 +0,0 @@
-
diff --git a/src/plugins/vis_type_vislib/public/vislib/visualizations/point_series.js b/src/plugins/vis_type_vislib/public/vislib/visualizations/point_series.js
index 80108521c1e0..d9ba62abfbad 100644
--- a/src/plugins/vis_type_vislib/public/vislib/visualizations/point_series.js
+++ b/src/plugins/vis_type_vislib/public/vislib/visualizations/point_series.js
@@ -36,10 +36,8 @@ import { Tooltip } from '../components/tooltip';
import { Chart } from './_chart';
import { TimeMarker } from './time_marker';
import { seriesTypes } from './point_series/series_types';
-import touchdownTmplHtml from '../partials/touchdown.tmpl.html';
const seriTypes = seriesTypes;
-const touchdownTmpl = _.template(touchdownTmplHtml);
/**
* Line Chart Visualization
*
@@ -191,7 +189,14 @@ export class PointSeries extends Chart {
}
function textFormatter() {
- return touchdownTmpl(callPlay(d3.event));
+ const { wholeBucket } = callPlay(d3.event);
+ return ``;
}
const endzoneTT = new Tooltip('endzones', this.handler.el, textFormatter, null);
diff --git a/src/setup_node_env/harden/index.js b/src/setup_node_env/harden/index.js
index 05f759c9c4bb..15194d80f52d 100644
--- a/src/setup_node_env/harden/index.js
+++ b/src/setup_node_env/harden/index.js
@@ -29,4 +29,3 @@
*/
require('./child_process');
-require('./lodash_template');
diff --git a/src/setup_node_env/harden/lodash_template.js b/src/setup_node_env/harden/lodash_template.js
deleted file mode 100644
index 2589f70acd62..000000000000
--- a/src/setup_node_env/harden/lodash_template.js
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- * SPDX-License-Identifier: Apache-2.0
- *
- * The OpenSearch Contributors require contributions made to
- * this file be licensed under the Apache-2.0 license or a
- * compatible open source license.
- *
- * Any modifications Copyright OpenSearch Contributors. See
- * GitHub history for details.
- */
-
-/*
- * Licensed to Elasticsearch B.V. under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch B.V. licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-var hook = require('require-in-the-middle');
-var isIterateeCall = require('lodash/_isIterateeCall');
-
-hook(['lodash'], function (lodash) {
- lodash.template = createProxy(lodash.template);
- return lodash;
-});
-
-hook(['lodash/template'], function (template) {
- return createProxy(template);
-});
-
-hook(['lodash/fp'], function (fp) {
- fp.template = createFpProxy(fp.template);
- return fp;
-});
-
-hook(['lodash/fp/template'], function (template) {
- return createFpProxy(template);
-});
-
-function createProxy(template) {
- return new Proxy(template, {
- apply: function (target, thisArg, args) {
- if (args.length === 1 || isIterateeCall(args)) {
- return target.apply(thisArg, [args[0], { sourceURL: '' }]);
- }
-
- var options = Object.assign({}, args[1]);
- options.sourceURL = (options.sourceURL + '').replace(/\s/g, ' ');
- var newArgs = args.slice(0); // copy
- newArgs.splice(1, 1, options); // replace options in the copy
- return target.apply(thisArg, newArgs);
- },
- });
-}
-
-function createFpProxy(template) {
- // we have to do the require here, so that we get the patched version
- var _ = require('lodash');
- return new Proxy(template, {
- apply: function (target, thisArg, args) {
- // per https://github.com/lodash/lodash/wiki/FP-Guide
- // > Iteratee arguments are capped to avoid gotchas with variadic iteratees.
- // this means that we can't specify the options in the second argument to fp.template because it's ignored.
- // Instead, we're going to use the non-FP _.template with only the first argument which has already been patched
- return _.template(args[0]);
- },
- });
-}
diff --git a/test/harden/lodash_template.js b/test/harden/lodash_template.js
deleted file mode 100644
index cc4c26afff55..000000000000
--- a/test/harden/lodash_template.js
+++ /dev/null
@@ -1,192 +0,0 @@
-/*
- * SPDX-License-Identifier: Apache-2.0
- *
- * The OpenSearch Contributors require contributions made to
- * this file be licensed under the Apache-2.0 license or a
- * compatible open source license.
- *
- * Any modifications Copyright OpenSearch Contributors. See
- * GitHub history for details.
- */
-
-/*
- * Licensed to Elasticsearch B.V. under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch B.V. licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-require('../../src/setup_node_env');
-const _ = require('lodash');
-const template = require('lodash/template');
-const fp = require('lodash/fp');
-const fpTemplate = require('lodash/fp/template');
-const test = require('tape');
-
-Object.prototype.sourceURL = '\u2028\u2029\n;global.whoops=true'; // eslint-disable-line no-extend-native
-
-test.onFinish(() => {
- delete Object.prototype.sourceURL;
-});
-
-test('test setup ok', (t) => {
- t.equal({}.sourceURL, '\u2028\u2029\n;global.whoops=true');
- t.end();
-});
-
-[_.template, template].forEach((fn) => {
- test(`_.template('<%= foo %>')`, (t) => {
- const output = fn('<%= foo %>')({ foo: 'bar' });
- t.equal(output, 'bar');
- t.equal(global.whoops, undefined);
- t.end();
- });
-
- test(`_.template('<%= foo %>', {})`, (t) => {
- const output = fn('<%= foo %>', Object.freeze({}))({ foo: 'bar' });
- t.equal(output, 'bar');
- t.equal(global.whoops, undefined);
- t.end();
- });
-
- test(`_.template('<%= data.foo %>', { variable: 'data' })`, (t) => {
- const output = fn('<%= data.foo %>', Object.freeze({ variable: 'data' }))({ foo: 'bar' });
- t.equal(output, 'bar');
- t.equal(global.whoops, undefined);
- t.end();
- });
-
- test(`_.template('<%= foo %>', { sourceURL: '/foo/bar' })`, (t) => {
- // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do
- const template = fn('<% throw new Error() %>', Object.freeze({ sourceURL: '/foo/bar' }));
- t.plan(2);
- try {
- template();
- } catch (err) {
- const path = parsePathFromStack(err.stack);
- t.equal(path, '/foo/bar');
- t.equal(global.whoops, undefined);
- }
- });
-
- test(`_.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true' })`, (t) => {
- // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do
- const template = fn(
- '<% throw new Error() %>',
- Object.freeze({ sourceURL: '\u2028\u2029\nglobal.whoops=true' })
- );
- t.plan(2);
- try {
- template();
- } catch (err) {
- const path = parsePathFromStack(err.stack);
- t.equal(path, 'global.whoops=true');
- t.equal(global.whoops, undefined);
- }
- });
-
- test(`_.template used as an iteratee call(`, (t) => {
- const templateStrArr = ['<%= data.foo %>', 'example <%= data.foo %>'];
- const output = _.map(templateStrArr, fn);
-
- t.equal(output[0]({ data: { foo: 'bar' } }), 'bar');
- t.equal(output[1]({ data: { foo: 'bar' } }), 'example bar');
- t.equal(global.whoops, undefined);
- t.end();
- });
-});
-
-[fp.template, fpTemplate].forEach((fn) => {
- test(`fp.template('<%= foo %>')`, (t) => {
- const output = fn('<%= foo %>')({ foo: 'bar' });
- t.equal(output, 'bar');
- t.equal(global.whoops, undefined);
- t.end();
- });
-
- test(`fp.template('<%= foo %>', {})`, (t) => {
- // fp.template ignores the second argument, this is negligible in this situation since options is an empty object
- const output = fn('<%= foo %>', Object.freeze({}))({ foo: 'bar' });
- t.equal(output, 'bar');
- t.equal(global.whoops, undefined);
- t.end();
- });
-
- test(`fp.template('<%= data.foo %>', { variable: 'data' })`, (t) => {
- // fp.template ignores the second argument, this causes an error to be thrown
- t.plan(2);
- try {
- fn('<%= data.foo %>', Object.freeze({ variable: 'data' }))({ foo: 'bar' });
- } catch (err) {
- t.equal(err.message, 'data is not defined');
- t.equal(global.whoops, undefined);
- }
- });
-
- test(`fp.template('<%= foo %>', { sourceURL: '/foo/bar' })`, (t) => {
- // fp.template ignores the second argument, the sourceURL is ignored
- // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do
- // our patching to hard-code the sourceURL and use non-FP _.template does slightly alter the stack-traces but it's negligible
- const template = fn('<% throw new Error() %>', Object.freeze({ sourceURL: '/foo/bar' }));
- t.plan(3);
- try {
- template();
- } catch (err) {
- const path = parsePathFromStack(err.stack);
- t.match(path, /^eval at /);
- t.doesNotMatch(path, /\/foo\/bar/);
- t.equal(global.whoops, undefined);
- }
- });
-
- test(`fp.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true' })`, (t) => {
- // fp.template ignores the second argument, the sourceURL is ignored
- // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do
- // our patching to hard-code the sourceURL and use non-FP _.template does slightly alter the stack-traces but it's negligible
- const template = fn(
- '<% throw new Error() %>',
- Object.freeze({ sourceURL: '\u2028\u2029\nglobal.whoops=true' })
- );
- t.plan(3);
- try {
- template();
- } catch (err) {
- const path = parsePathFromStack(err.stack);
- t.match(path, /^eval at /);
- t.doesNotMatch(path, /\/foo\/bar/);
- t.equal(global.whoops, undefined);
- }
- });
-
- test(`fp.template used as an iteratee call(`, (t) => {
- const templateStrArr = ['<%= data.foo %>', 'example <%= data.foo %>'];
- const output = fp.map(fn)(templateStrArr);
-
- t.equal(output[0]({ data: { foo: 'bar' } }), 'bar');
- t.equal(output[1]({ data: { foo: 'bar' } }), 'example bar');
- t.equal(global.whoops, undefined);
- t.end();
- });
-});
-
-function parsePathFromStack(stack) {
- const lines = stack.split('\n');
- // the frame starts at the second line
- const frame = lines[1];
-
- // the path is in parathensis, and ends with a colon before the line/column numbers
- const [, path] = /\(([^:]+)/.exec(frame);
- return path;
-}