Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ban use of lodash.template #100277

Merged
merged 3 commits into from
May 19, 2021
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
64 changes: 49 additions & 15 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,33 @@ module.exports = {
name: 'lodash/fp/assocPath',
message: 'Please use @elastic/safer-lodash-set instead',
},
{
name: 'lodash',
importNames: ['template'],
message:
'lodash.template is unsafe, and not compatible with our content security policy.',
},
{
name: 'lodash.template',
message:
'lodash.template is unsafe, and not compatible with our content security policy.',
},
{
name: 'lodash/template',
message:
'lodash.template is unsafe, and not compatible with our content security policy.',
},
{
name: 'lodash/fp',
importNames: ['template'],
message:
'lodash.template is unsafe, and not compatible with our content security policy.',
},
{
name: 'lodash/fp/template',
message:
'lodash.template is unsafe, and not compatible with our content security policy.',
},
{
name: 'react-use',
message: 'Please use react-use/lib/{method} instead.',
Expand All @@ -730,6 +757,11 @@ module.exports = {
name: 'lodash.setwith',
message: 'Please use @elastic/safer-lodash-set instead',
},
{
name: 'lodash.template',
message:
'lodash.template is unsafe, and not compatible with our content security policy.',
},
{
name: 'lodash/set',
message: 'Please use @elastic/safer-lodash-set instead',
Expand All @@ -738,6 +770,11 @@ module.exports = {
name: 'lodash/setWith',
message: 'Please use @elastic/safer-lodash-set instead',
},
{
name: 'lodash/template',
message:
'lodash.template is unsafe, and not compatible with our content security policy.',
},
],
},
],
Expand All @@ -753,6 +790,18 @@ module.exports = {
property: 'set',
message: 'Please use @elastic/safer-lodash-set instead',
},
{
object: 'lodash',
property: 'template',
message:
'lodash.template is unsafe, and not compatible with our content security policy.',
},
{
object: '_',
property: 'template',
message:
'lodash.template is unsafe, and not compatible with our content security policy.',
},
{
object: 'lodash',
property: 'setWith',
Expand Down Expand Up @@ -1576,20 +1625,5 @@ module.exports = {
'@typescript-eslint/prefer-ts-expect-error': 'error',
},
},
{
files: [
'**/public/**/*.{js,mjs,ts,tsx}',
'**/common/**/*.{js,mjs,ts,tsx}',
'packages/**/*.{js,mjs,ts,tsx}',
],
rules: {
'no-restricted-imports': [
'error',
{
patterns: ['lodash/*', '!lodash/fp', 'rxjs/internal-compatibility'],
},
],
},
},
Comment on lines -1579 to -1593
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this rule because it prevented the other no-restricted-imports rule above from firing. Even combining them together did not fix the problem. This is apparently desired behavior: eslint/eslint#14220

I opted to remove this rule in favor of the definition above because the other definitions are security-related, which is [subjectively] much more important.

],
};
5 changes: 5 additions & 0 deletions src/setup_node_env/harden/lodash_template.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ var hook = require('require-in-the-middle');
var isIterateeCall = require('lodash/_isIterateeCall');

hook(['lodash'], function (lodash) {
// we use lodash.template here to harden third-party usage of this otherwise banned function.
// eslint-disable-next-line no-restricted-properties
lodash.template = createProxy(lodash.template);
return lodash;
});
Expand Down Expand Up @@ -52,6 +54,9 @@ function createFpProxy(template) {
// > 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

// we use lodash.template here to harden third-party usage of this otherwise banned function.
// eslint-disable-next-line no-restricted-properties
return _.template(args[0]);
},
});
Expand Down
2 changes: 2 additions & 0 deletions test/harden/lodash_template.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

require('../../src/setup_node_env');
const _ = require('lodash');
// eslint-disable-next-line no-restricted-modules
const template = require('lodash/template');
const fp = require('lodash/fp');
const fpTemplate = require('lodash/fp/template');
Expand All @@ -24,6 +25,7 @@ test('test setup ok', (t) => {
t.end();
});

// eslint-disable-next-line no-restricted-properties
[_.template, template].forEach((fn) => {
test(`_.template('<%= foo %>')`, (t) => {
const output = fn('<%= foo %>')({ foo: 'bar' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import createContainer from 'constate';
import { useCallback, useState } from 'react';
import { useDebounce } from 'react-use';
import useDebounce from 'react-use/lib/useDebounce';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes an eslint rule that should have been firing, but wasn't. I've updated this import to comply with the fixed rule.

import { esQuery, IIndexPattern, Query } from '../../../../../../../src/plugins/data/public';

type ParsedQuery = ReturnType<typeof esQuery.buildEsQuery>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
*/

import { i18n } from '@kbn/i18n';
// Prefer importing entire lodash library, e.g. import { get } from "lodash"
// eslint-disable-next-line no-restricted-imports
import flowRight from 'lodash/flowRight';
import { flowRight } from 'lodash';
import React from 'react';
import { Redirect, RouteComponentProps } from 'react-router-dom';
import useMount from 'react-use/lib/useMount';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import equal from 'fast-deep-equal';
import { useCallback, useMemo, useState } from 'react';
import { useAsync } from 'react-use';
import useAsync from 'react-use/lib/useAsync';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes an eslint rule that should have been firing, but wasn't. I've updated this import to comply with the fixed rule.

import { ObjectEntries } from '../../../../common/utility_types';
import { ChildFormValidationError, GenericValidationError } from './validation_errors';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React, { useCallback, useMemo, useState } from 'react';
import { useThrottle } from 'react-use';
import useThrottle from 'react-use/lib/useThrottle';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes an eslint rule that should have been firing, but wasn't. I've updated this import to comply with the fixed rule.

import { FormattedMessage } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';
import { EuiFieldSearch } from '@elastic/eui';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import React, { useState } from 'react';

import { useDebounce } from 'react-use';
import useDebounce from 'react-use/lib/useDebounce';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes an eslint rule that should have been firing, but wasn't. I've updated this import to comply with the fixed rule.

import { useValuesList } from '../../../hooks/use_values_list';
import { FieldValueSelection } from './field_value_selection';
import { FieldValueSuggestionsProps } from './types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { capitalize, union } from 'lodash';
import { useEffect, useState } from 'react';
import { useDebounce } from 'react-use';
import useDebounce from 'react-use/lib/useDebounce';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes an eslint rule that should have been firing, but wasn't. I've updated this import to comply with the fixed rule.

import { IndexPattern } from '../../../../../src/plugins/data/common';
import { ESFilter } from '../../../../../typings/elasticsearch';
import { createEsParams, useEsSearch } from './use_es_search';
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/osquery/public/agents/agents_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { find } from 'lodash/fp';
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { EuiComboBox, EuiHealth, EuiHighlight, EuiSpacer } from '@elastic/eui';

import { useDebounce } from 'react-use';
import useDebounce from 'react-use/lib/useDebounce';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes an eslint rule that should have been firing, but wasn't. I've updated this import to comply with the fixed rule.

import { useAllAgents } from './use_all_agents';
import { useAgentGroups } from './use_agent_groups';
import { useOsqueryPolicies } from './use_osquery_policies';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
* 2.0.
*/

// eslint-disable-next-line no-restricted-imports
import isEmpty from 'lodash/isEmpty';
import { isEmpty } from 'lodash';
import { SourcererModel, SourcererScopeName } from './model';
import { TimelineEventsType } from '../../../../common/types/timeline';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
import { EuiButtonEmpty, EuiFormRow, EuiSpacer } from '@elastic/eui';
import React, { FC, memo, useCallback, useState, useEffect } from 'react';
import styled from 'styled-components';
// Prefer importing entire lodash library, e.g. import { get } from "lodash"
// eslint-disable-next-line no-restricted-imports
import isEqual from 'lodash/isEqual';
import { isEqual } from 'lodash';

import { IndexPattern } from 'src/plugins/data/public';
import { DEFAULT_INDEX_KEY } from '../../../../../common/constants';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
import { fold } from 'fp-ts/lib/Either';
import { identity } from 'fp-ts/lib/function';
import { pipe } from 'fp-ts/lib/pipeable';
// Prefer importing entire lodash library, e.g. import { get } from "lodash"
// eslint-disable-next-line no-restricted-imports
import isEmpty from 'lodash/isEmpty';
import { isEmpty } from 'lodash';

import { throwErrors } from '../../../../cases/common';
import {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React, { useCallback, useState } from 'react';
import { useDebounce } from 'react-use';
import useDebounce from 'react-use/lib/useDebounce';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes an eslint rule that should have been firing, but wasn't. I've updated this import to comply with the fixed rule.

import { useDispatch } from 'react-redux';
import { Query } from 'src/plugins/data/common';
import { useGetUrlParams, useUpdateKueryString, useUrlParams } from '../../../hooks';
Expand Down