From c93a84adac5135b9edb69804535de138828bdc1f Mon Sep 17 00:00:00 2001 From: Nicolas Goutay Date: Wed, 13 Mar 2019 10:39:32 +0100 Subject: [PATCH] chore: Upgrade minimatch to fix RegEx DoS security issue (#10338) Fixes #10282 - [x] Switch to `recursive-readdir` library - [x] Implement async readdir behavior - [x] Fix existing tests - [x] Test error handling - [x] Test on a `gatsby-starter-blog` with few examples of REPLs - [x] Try and implement the `Promise.all` refactor --- .../gatsby-remark-code-repls/package.json | 2 +- .../src/__tests__/gatsby-node.js | 122 ++++++++++-------- .../src/gatsby-node.js | 90 +++++++------ yarn.lock | 34 ++--- 4 files changed, 128 insertions(+), 120 deletions(-) diff --git a/packages/gatsby-remark-code-repls/package.json b/packages/gatsby-remark-code-repls/package.json index 92efdda9892f1..7703115645b27 100644 --- a/packages/gatsby-remark-code-repls/package.json +++ b/packages/gatsby-remark-code-repls/package.json @@ -10,7 +10,7 @@ "@babel/runtime": "^7.0.0", "lz-string": "^1.4.4", "normalize-path": "^2.1.1", - "recursive-readdir-synchronous": "^0.0.3", + "recursive-readdir": "^2.2.2", "unist-util-map": "^1.0.3", "urijs": "^1.19.0" }, diff --git a/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js b/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js index 76f6d1b1a0e55..be21fb86b260e 100644 --- a/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js +++ b/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js @@ -4,10 +4,17 @@ jest.mock(`fs`, () => { readFileSync: jest.fn(), } }) -jest.mock(`recursive-readdir-synchronous`, () => jest.fn()) +jest.mock(`recursive-readdir`, () => jest.fn()) +jest.mock(`gatsby-cli/lib/reporter`, () => { + return { + panic: jest.fn(), + } +}) const fs = require(`fs`) -const recursiveReaddir = require(`recursive-readdir-synchronous`) +const readdir = require(`recursive-readdir`) + +const reporter = require(`gatsby-cli/lib/reporter`) const { OPTION_DEFAULT_HTML, @@ -20,6 +27,7 @@ const createPagesParams = { actions: { createPage, }, + reporter, } describe(`gatsby-remark-code-repls`, () => { @@ -30,20 +38,17 @@ describe(`gatsby-remark-code-repls`, () => { fs.readFileSync.mockReset() fs.readFileSync.mockReturnValue(`const foo = "bar";`) - recursiveReaddir.mockReset() - recursiveReaddir.mockReturnValue([`file.js`]) + readdir.mockReset() + readdir.mockResolvedValue([`file.js`]) createPage.mockReset() }) describe(`gatsby-node`, () => { - it(`should iterate over all JavaScript files in the examples directory`, () => { - recursiveReaddir.mockReturnValue([ - `root-file.js`, - `path/to/nested/file.jsx`, - ]) + it(`should iterate over all JavaScript files in the examples directory`, async () => { + readdir.mockResolvedValue([`root-file.js`, `path/to/nested/file.jsx`]) - createPages(createPagesParams) + await createPages(createPagesParams) expect(fs.readFileSync).toHaveBeenCalledTimes(2) expect(fs.readFileSync).toHaveBeenCalledWith(`root-file.js`, `utf8`) @@ -53,52 +58,51 @@ describe(`gatsby-remark-code-repls`, () => { ) }) - it(`should ignore non JavaScript files in the examples directory`, () => { - recursiveReaddir.mockReturnValue([`javascript.js`, `not-javascript.html`]) + it(`should ignore non JavaScript files in the examples directory`, async () => { + readdir.mockResolvedValue([`javascript.js`, `not-javascript.html`]) - createPages(createPagesParams) + await createPages(createPagesParams) expect(fs.readFileSync).toHaveBeenCalledTimes(1) expect(fs.readFileSync).toHaveBeenCalledWith(`javascript.js`, `utf8`) }) - it(`should error if provided an invalid examples directory`, () => { + it(`should error if provided an invalid examples directory`, async () => { fs.existsSync.mockReturnValue(false) - expect(() => createPages(createPagesParams)).toThrow( - `Invalid REPL directory specified: "REPL/"` - ) + try { + await createPages(createPagesParams) + } catch (err) { + expect(err).toEqual(Error(`Invalid REPL directory specified: "REPL/"`)) + } }) - it(`should warn about an empty examples directory`, () => { - recursiveReaddir.mockReturnValue([]) + it(`should warn about an empty examples directory`, async () => { + readdir.mockResolvedValue([]) spyOn(console, `warn`) // eslint-disable-line no-undef - createPages(createPagesParams) + await createPages(createPagesParams) expect(console.warn).toHaveBeenCalledWith( `Specified REPL directory "REPL/" contains no files` ) }) - it(`should create redirect pages for the code in each example file`, () => { - recursiveReaddir.mockReturnValue([ - `root-file.js`, - `path/to/nested/file.jsx`, - ]) + it(`should create redirect pages for the code in each example file`, async () => { + readdir.mockResolvedValue([`root-file.js`, `path/to/nested/file.jsx`]) - createPages(createPagesParams) + await createPages(createPagesParams) expect(createPage).toHaveBeenCalledTimes(2) expect(createPage.mock.calls[0][0].path).toContain(`root-file`) expect(createPage.mock.calls[1][0].path).toContain(`path/to/nested/file`) }) - it(`should use a default redirect template`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should use a default redirect template`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams) + await createPages(createPagesParams) expect(createPage).toHaveBeenCalledTimes(1) expect(createPage.mock.calls[0][0].component).toContain( @@ -106,27 +110,43 @@ describe(`gatsby-remark-code-repls`, () => { ) }) - it(`should use a specified redirect template override`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should use a specified redirect template override`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams, { redirectTemplate: `foo/bar.js` }) + await createPages(createPagesParams, { redirectTemplate: `foo/bar.js` }) expect(createPage).toHaveBeenCalledTimes(1) expect(createPage.mock.calls[0][0].component).toContain(`foo/bar.js`) }) - it(`should error if an invalid redirect template is specified`, () => { + it(`should error if an invalid redirect template is specified`, async () => { fs.existsSync.mockImplementation(path => path !== `foo/bar.js`) - expect(() => - createPages(createPagesParams, { redirectTemplate: `foo/bar.js` }) - ).toThrow(`Invalid REPL redirectTemplate specified`) + try { + await createPages(createPagesParams, { redirectTemplate: `foo/bar.js` }) + } catch (err) { + expect(err).toEqual( + Error(`Invalid REPL redirectTemplate specified: "foo/bar.js"`) + ) + } + }) + + it(`should propagate any Error from recursive-readdir`, async () => { + const error = TypeError(`glob pattern string required`) + + readdir.mockRejectedValue(error) + + try { + await createPages(createPagesParams) + } catch (err) { + expect(err).toBe(error) + } }) - it(`should not load any external packages by default`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should not load any external packages by default`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams) + await createPages(createPagesParams) const { js_external } = JSON.parse( createPage.mock.calls[0][0].context.payload @@ -135,10 +155,10 @@ describe(`gatsby-remark-code-repls`, () => { expect(js_external).toBe(``) }) - it(`should load custom externals if specified`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should load custom externals if specified`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams, { externals: [`foo.js`, `bar.js`] }) + await createPages(createPagesParams, { externals: [`foo.js`, `bar.js`] }) const { js_external } = JSON.parse( createPage.mock.calls[0][0].context.payload @@ -148,10 +168,10 @@ describe(`gatsby-remark-code-repls`, () => { expect(js_external).toContain(`bar.js`) }) - it(`should inject the required prop-types for the Codepen prefill API`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should inject the required prop-types for the Codepen prefill API`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams) + await createPages(createPagesParams) const { action, payload } = createPage.mock.calls[0][0].context @@ -159,20 +179,20 @@ describe(`gatsby-remark-code-repls`, () => { expect(payload).toBeTruthy() }) - it(`should render default HTML for index page if no override specified`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should render default HTML for index page if no override specified`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams, {}) + await createPages(createPagesParams, {}) const { html } = JSON.parse(createPage.mock.calls[0][0].context.payload) expect(html).toBe(OPTION_DEFAULT_HTML) }) - it(`should support custom, user-defined HTML for index page`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should support custom, user-defined HTML for index page`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams, { html: `` }) + await createPages(createPagesParams, { html: `` }) const { html } = JSON.parse(createPage.mock.calls[0][0].context.payload) diff --git a/packages/gatsby-remark-code-repls/src/gatsby-node.js b/packages/gatsby-remark-code-repls/src/gatsby-node.js index e17c71e009317..a18e02fb333a7 100644 --- a/packages/gatsby-remark-code-repls/src/gatsby-node.js +++ b/packages/gatsby-remark-code-repls/src/gatsby-node.js @@ -2,7 +2,7 @@ const fs = require(`fs`) const { extname, resolve } = require(`path`) -const recursiveReaddir = require(`recursive-readdir-synchronous`) +const readdir = require(`recursive-readdir`) const normalizePath = require(`normalize-path`) const { @@ -11,8 +11,8 @@ const { OPTION_DEFAULT_REDIRECT_TEMPLATE_PATH, } = require(`./constants`) -exports.createPages = ( - { actions }, +exports.createPages = async ( + { actions, reporter }, { directory = OPTION_DEFAULT_LINK_TEXT, externals = [], @@ -27,54 +27,60 @@ exports.createPages = ( const { createPage } = actions if (!fs.existsSync(directory)) { - throw Error(`Invalid REPL directory specified: "${directory}"`) + reporter.panic(`Invalid REPL directory specified: "${directory}"`) } if (!fs.existsSync(redirectTemplate)) { - throw Error( + reporter.panic( `Invalid REPL redirectTemplate specified: "${redirectTemplate}"` ) } - // TODO We could refactor this to use 'recursive-readdir' instead, - // And wrap with Promise.all() to execute createPage() in parallel. - // I'd need to find a way to reliably test error handling though. - const files = recursiveReaddir(directory) + try { + const files = await readdir(directory) + if (files.length === 0) { + console.warn(`Specified REPL directory "${directory}" contains no files`) - if (files.length === 0) { - console.warn(`Specified REPL directory "${directory}" contains no files`) - - return - } + return + } - files.forEach(file => { - if (extname(file) === `.js` || extname(file) === `.jsx`) { - const slug = file - .substring(0, file.length - extname(file).length) - .replace(new RegExp(`^${directory}`), `redirect-to-codepen/`) - const code = fs.readFileSync(file, `utf8`) + files.forEach(file => { + if (extname(file) === `.js` || extname(file) === `.jsx`) { + const slug = file + .substring(0, file.length - extname(file).length) + .replace(new RegExp(`^${directory}`), `redirect-to-codepen/`) + const code = fs.readFileSync(file, `utf8`) - // Codepen configuration. - // https://blog.codepen.io/documentation/api/prefill/ - const action = `https://codepen.io/pen/define` - const payload = JSON.stringify({ - editors: `0010`, - html, - js: code, - js_external: externals.join(`;`), - js_pre_processor: `babel`, - layout: `left`, - }) + // Codepen configuration. + // https://blog.codepen.io/documentation/api/prefill/ + const action = `https://codepen.io/pen/define` + const payload = JSON.stringify({ + editors: `0010`, + html, + js: code, + js_external: externals.join(`;`), + js_pre_processor: `babel`, + layout: `left`, + }) - createPage({ - path: slug, - // Normalize the path so tests pass on Linux + Windows - component: normalizePath(resolve(redirectTemplate)), - context: { - action, - payload, - }, - }) - } - }) + createPage({ + path: slug, + // Normalize the path so tests pass on Linux + Windows + component: normalizePath(resolve(redirectTemplate)), + context: { + action, + payload, + }, + }) + } + }) + } catch (error) { + reporter.panic( + ` + Error in gatsby-remark-code-repls plugin: cannot read directory ${directory}. + More details can be found in the error reporting below. + `, + error + ) + } } diff --git a/yarn.lock b/yarn.lock index dfb89ddbf34ed..b1cd38aae1e8d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12705,11 +12705,6 @@ lpad-align@^1.0.1: longest "^1.0.0" meow "^3.3.0" -lru-cache@2: - version "2.7.3" - resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-2.7.3.tgz#6d4524e8b955f95d4f5b58851ce21dd72fb4e952" - integrity sha1-bUUk6LlV+V1PW1iFHOId1y+06VI= - lru-cache@4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-4.0.0.tgz#b5cbf01556c16966febe54ceec0fb4dc90df6c28" @@ -13234,15 +13229,7 @@ minimalistic-crypto-utils@^1.0.0, minimalistic-crypto-utils@^1.0.1: resolved "https://registry.yarnpkg.com/minimalistic-crypto-utils/-/minimalistic-crypto-utils-1.0.1.tgz#f6c00c1c0b082246e5c4d99dfb8c7c083b2b582a" integrity sha1-9sAMHAsIIkblxNmd+4x8CDsrWCo= -minimatch@0.3.0: - version "0.3.0" - resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-0.3.0.tgz#275d8edaac4f1bb3326472089e7949c8394699dd" - integrity sha1-J12O2qxPG7MyZHIInnlJyDlGmd0= - dependencies: - lru-cache "2" - sigmund "~1.0.0" - -"minimatch@2 || 3", minimatch@^3.0.0, minimatch@^3.0.2, minimatch@^3.0.3, minimatch@^3.0.4: +"minimatch@2 || 3", minimatch@3.0.4, minimatch@^3.0.0, minimatch@^3.0.2, minimatch@^3.0.3, minimatch@^3.0.4: version "3.0.4" resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083" integrity sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA== @@ -16293,13 +16280,6 @@ rechoir@^0.6.2: dependencies: resolve "^1.1.6" -recursive-readdir-synchronous@^0.0.3: - version "0.0.3" - resolved "https://registry.yarnpkg.com/recursive-readdir-synchronous/-/recursive-readdir-synchronous-0.0.3.tgz#d5e5a096ad56cf9666241c22a30b4f338bb7ed88" - integrity sha1-1eWglq1Wz5ZmJBwiowtPM4u37Yg= - dependencies: - minimatch "0.3.0" - recursive-readdir@2.2.1: version "2.2.1" resolved "https://registry.yarnpkg.com/recursive-readdir/-/recursive-readdir-2.2.1.tgz#90ef231d0778c5ce093c9a48d74e5c5422d13a99" @@ -16307,6 +16287,13 @@ recursive-readdir@2.2.1: dependencies: minimatch "3.0.3" +recursive-readdir@^2.2.2: + version "2.2.2" + resolved "https://registry.yarnpkg.com/recursive-readdir/-/recursive-readdir-2.2.2.tgz#9946fb3274e1628de6e36b2f6714953b4845094f" + integrity sha512-nRCcW9Sj7NuZwa2XvH9co8NPeXUBhZP7CRKJtU+cS6PW9FpCIFoI5ib0NT1ZrbNuPoRy0ylyCaUL8Gih4LSyFg== + dependencies: + minimatch "3.0.4" + redent@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/redent/-/redent-1.0.0.tgz#cf916ab1fd5f1f16dfb20822dd6ec7f730c2afde" @@ -17450,11 +17437,6 @@ sift@^6.0.0: resolved "https://registry.yarnpkg.com/sift/-/sift-6.0.0.tgz#f93a778e5cbf05a5024ebc391e6b32511a6d1f82" integrity sha1-+Tp3jly/BaUCTrw5HmsyURptH4I= -sigmund@~1.0.0: - version "1.0.1" - resolved "https://registry.yarnpkg.com/sigmund/-/sigmund-1.0.1.tgz#3ff21f198cad2175f9f3b781853fd94d0d19b590" - integrity sha1-P/IfGYytIXX587eBhT/ZTQ0ZtZA= - signal-exit@^3.0.0, signal-exit@^3.0.2: version "3.0.2" resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.2.tgz#b5fdc08f1287ea1178628e415e25132b73646c6d"