From 992ad68517f4f95d5c321190fe7af565efa6d7fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20B=C3=BCchele?= Date: Thu, 11 Oct 2018 07:00:58 -0700 Subject: [PATCH] error plugin import from outside Summary: Plugins can not require files outside their folder, to make sure they don't pull in any dependencies from Flipper which are not exported by the main app. However, those imports simply resolve to `undefined`. This diff adds a check in the babel-transform for plugins and throws an error if something from outside the plugin is required. Reviewed By: passy Differential Revision: D10297980 fbshipit-source-id: 1606f3211103281f9f4aa7bb2f3ca4d085d0ea1b --- .../__tests__/flipper-requires.node.js | 70 +++++++++++++++++++ static/transforms/flipper-requires.js | 41 ++++++----- static/transforms/index.js | 1 + 3 files changed, 95 insertions(+), 17 deletions(-) create mode 100644 static/transforms/__tests__/flipper-requires.node.js diff --git a/static/transforms/__tests__/flipper-requires.node.js b/static/transforms/__tests__/flipper-requires.node.js new file mode 100644 index 000000000..401428b88 --- /dev/null +++ b/static/transforms/__tests__/flipper-requires.node.js @@ -0,0 +1,70 @@ +/** + * Copyright 2018-present Facebook. + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * @format + */ + +import {parse} from '@babel/parser'; +import {transformFromAstSync} from '@babel/core'; +import generate from '@babel/generator'; + +import flipperRequires from '../flipper-requires'; + +const babelOptions = { + ast: true, + plugins: [flipperRequires], + filename: 'index.js', +}; + +test('transform react requires to global window', () => { + const src = 'require("react")'; + const ast = parse(src); + const transformed = transformFromAstSync(ast, src, babelOptions).ast; + const {code} = generate(transformed); + expect(code).toBe('window.React;'); +}); + +test('transform react-dom requires to global window', () => { + const src = 'require("react-dom")'; + const ast = parse(src); + const transformed = transformFromAstSync(ast, src, babelOptions).ast; + const {code} = generate(transformed); + expect(code).toBe('window.ReactDOM;'); +}); + +test('transform flipper requires to global window', () => { + const src = 'require("flipper")'; + const ast = parse(src); + const transformed = transformFromAstSync(ast, src, babelOptions).ast; + const {code} = generate(transformed); + expect(code).toBe('window.Flipper;'); +}); + +test('transform React identifier to window.React', () => { + const src = 'React;'; + const ast = parse(src); + const transformed = transformFromAstSync(ast, src, babelOptions).ast; + const {code} = generate(transformed); + expect(code).toBe('window.React;'); +}); + +test('throw error when requiring outside the plugin', () => { + const src = 'require("../test.js")'; + const ast = parse(src); + expect(() => { + transformFromAstSync(ast, src, babelOptions); + }).toThrow(); +}); + +test('allow requiring from parent folder as long as we stay in plugin folder', () => { + const src = 'require("../test.js")'; + const ast = parse(src); + const transformed = transformFromAstSync(ast, src, { + ...babelOptions, + root: '/path/to/plugin', + filename: '/path/to/plugin/subfolder/index.js', + }).ast; + const {code} = generate(transformed); + expect(code).toBe('require("../test.js");'); +}); diff --git a/static/transforms/flipper-requires.js b/static/transforms/flipper-requires.js index 0268f2daa..d38601886 100644 --- a/static/transforms/flipper-requires.js +++ b/static/transforms/flipper-requires.js @@ -5,6 +5,8 @@ * @format */ +const {resolve, dirname} = require('path'); + // do not apply this transform for these paths const EXCLUDE_PATHS = [ '/node_modules/react-devtools-core/', @@ -28,27 +30,32 @@ module.exports = ({types: t}) => ({ } const node = path.node; const args = node.arguments || []; + if ( node.callee.name === 'require' && args.length === 1 && - t.isStringLiteral(args[0]) && - args[0].value === 'flipper' + t.isStringLiteral(args[0]) ) { - path.replaceWith(t.identifier('window.Flipper')); - } else if ( - node.callee.name === 'require' && - args.length > 0 && - t.isStringLiteral(args[0]) && - args[0].value === 'react' - ) { - path.replaceWith(t.identifier('window.React')); - } else if ( - node.callee.name === 'require' && - args.length > 0 && - t.isStringLiteral(args[0]) && - args[0].value === 'react-dom' - ) { - path.replaceWith(t.identifier('window.ReactDOM')); + if (args[0].value === 'flipper') { + path.replaceWith(t.identifier('window.Flipper')); + } else if (args[0].value === 'react') { + path.replaceWith(t.identifier('window.React')); + } else if (args[0].value === 'react-dom') { + path.replaceWith(t.identifier('window.ReactDOM')); + } else if ( + // require a file not a pacakge + args[0].value.indexOf('/') > -1 && + // in the plugin itself and not inside one of its dependencies + state.file.opts.filename.indexOf('node_modules') === -1 && + // the resolved path for this file is outside the plugins root + !resolve(dirname(state.file.opts.filename), args[0].value).startsWith( + state.file.opts.root, + ) + ) { + throw new Error( + 'Plugins cannot require files from outside their folder.', + ); + } } }, Identifier(path, state) { diff --git a/static/transforms/index.js b/static/transforms/index.js index 77c1e823e..90a41a4fb 100644 --- a/static/transforms/index.js +++ b/static/transforms/index.js @@ -57,6 +57,7 @@ function transform({filename, options, src}) { code: false, comments: false, compact: false, + root: options.projectRoot, filename, plugins, presets,