Skip to content

Commit

Permalink
chore: Upgrade minimatch to fix RegEx DoS security issue (gatsbyjs#10338
Browse files Browse the repository at this point in the history 
)

Fixes gatsbyjs#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
  • Loading branch information
@phacks @wardpeet
phacks authored and wardpeet committed Mar 13, 2019
1 parent 89cd032 commit c93a84a
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 120 deletions.
2 changes: 1 addition & 1 deletion packages/gatsby-remark-code-repls/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
122 changes: 71 additions & 51 deletions packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -20,6 +27,7 @@ const createPagesParams = {
actions: {
createPage,
},
reporter,
}

describe(`gatsby-remark-code-repls`, () => {
Expand All @@ -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`)
Expand All @@ -53,80 +58,95 @@ 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(
OPTION_DEFAULT_REDIRECT_TEMPLATE_PATH
)
})

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
Expand All @@ -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
Expand All @@ -148,31 +168,31 @@ 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

expect(action).toBeTruthy()
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: `<span id="foo"></span>` })
await createPages(createPagesParams, { html: `<span id="foo"></span>` })

const { html } = JSON.parse(createPage.mock.calls[0][0].context.payload)

Expand Down
90 changes: 48 additions & 42 deletions packages/gatsby-remark-code-repls/src/gatsby-node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 = [],
Expand All @@ -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
)
}
}
Loading

0 comments on commit c93a84a

Please sign in to comment.