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

fix(gatsby-plugin-mdx): Do not leak frontmatter into page #35859

Merged
merged 4 commits into from Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 91 additions & 0 deletions e2e-tests/mdx/cypress/integration/frontmatter.js
@@ -0,0 +1,91 @@
const page = {
js: `/frontmatter-js`,
javascript: `/frontmatter-javascript`,
yaml: `/frontmatter-yaml`,
json: `/frontmatter-json`,
graphqlQuery: `/frontmatter-graphql-query`,
}

// Attribute selector for element we assert against in pages
const selector = `[data-cy="frontmatter"]`

// Strings used for frontmatter titles
const titles = {
notParsed: `I should not be parsed`,
parsed: `I am parsed`,
}

// Frontmatter that should not be rendered
const otherKey = `Some other key`

describe(`webpack loader`, () => {
describe(`---yaml frontmatter`, () => {
beforeEach(() => {
cy.visit(page.yaml).waitForRouteChange()
})

it(`should parse`, () => {
cy.get(selector).invoke(`text`).should(`eq`, titles.parsed)
})

it(`should not leak into the page`, () => {
cy.contains(otherKey).should(`not.exist`)
})
})

describe(`---json frontmatter`, () => {
beforeEach(() => {
cy.visit(page.json).waitForRouteChange()
})

it(`should parse`, () => {
cy.get(selector).invoke(`text`).should(`eq`, titles.parsed)
})

it(`should not leak into the page`, () => {
cy.contains(otherKey).should(`not.exist`)
})
})

describe(`---js frontmatter`, () => {
beforeEach(() => {
cy.visit(page.js).waitForRouteChange()
})

it(`should parse`, () => {
cy.get(selector).invoke(`text`).should(`eq`, `disabled`)
})

it(`should not leak into the page`, () => {
cy.contains(otherKey).should(`not.exist`)
})
})

describe(`---javascript frontmatter`, () => {
beforeEach(() => {
cy.visit(page.javascript).waitForRouteChange()
})

it(`should parse`, () => {
cy.get(selector).invoke(`text`).should(`eq`, `disabled`)
})

it(`should not leak into the page`, () => {
cy.contains(otherKey).should(`not.exist`)
})
})
})

describe(`data layer`, () => {
it(`---js or ---javascript frontmatter should not parse by default`, () => {
cy.visit(page.graphqlQuery).waitForRouteChange()
cy.contains(titles.notParsed).should(`not.exist`)
})
})

it(`---js and ---javascript frontmatter should not allow remote code execution`, () => {
cy.readFile(`cypress/fixtures/file-to-attempt-rce-on.txt`).should(
`eq`,
`Nothing here, do not remove`
)
})
29 changes: 0 additions & 29 deletions e2e-tests/mdx/cypress/integration/js-frontmatter.js

This file was deleted.

Expand Up @@ -3,14 +3,15 @@
require(`fs`).writeFileSync(`${process.cwd()}/cypress/fixtures/file-to-attempt-rce-on.txt`, (new Error('Helpful stack trace if this does execute. It should not execute.')).stack)
console.trace()
return {
title: `I should not be parsed`
title: `I should not be parsed`,
otherKey: `Some other key`
}
})()

---

<h1>JS frontmatter engine is disabled by default</h1>

<span data-cy="js-frontmatter">
<span data-cy="frontmatter">
{props.pageContext.frontmatter?.title || `disabled`}
</span>
Expand Up @@ -3,14 +3,15 @@
require(`fs`).writeFileSync(`${process.cwd()}/cypress/fixtures/file-to-attempt-rce-on.txt`, (new Error('Helpful stack trace if this does execute. It should not execute.')).stack)
console.trace()
return {
title: `I should not be parsed`
title: `I should not be parsed`,
otherKey: `Some other key`
}
})()

---

<h1>JS frontmatter engine is disabled by default</h1>

<span data-cy="js-frontmatter">
<span data-cy="frontmatter">
{props.pageContext.frontmatter?.title || `disabled`}
</span>
8 changes: 8 additions & 0 deletions e2e-tests/mdx/src/pages/frontmatter-json.mdx
@@ -0,0 +1,8 @@
---json
{ "title": "I am parsed", "otherKey": "Some other key" }

---

<h1>A page with JSON frontmatter</h1>

<span data-cy="frontmatter">{props.pageContext.frontmatter?.title}</span>
9 changes: 9 additions & 0 deletions e2e-tests/mdx/src/pages/frontmatter-yaml.mdx
@@ -0,0 +1,9 @@
---yaml
title: I am parsed
otherKey: Some other key

---

<h1>A page with YAML frontmatter</h1>

<span data-cy="frontmatter">{props.pageContext.frontmatter?.title}</span>
7 changes: 2 additions & 5 deletions packages/gatsby-plugin-mdx/loaders/mdx-loader.js
Expand Up @@ -182,12 +182,9 @@ export const _frontmatter = ${JSON.stringify(data)};`
// check needs to happen first.
if (!hasDefaultExport(content, options) && !!defaultLayout) {
debug(`inserting default layout`, defaultLayout)
const { content: contentWithoutFrontmatter, matter } = grayMatter(
content,
options
)
const { content: contentWithoutFrontmatter } = grayMatter(content, options)

code = `${matter ? matter : ``}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When grayMatter was not passed options prior to this change, matter would always be undefined, resulting in no frontmatter rendered to the page. This changes removes matter entirely from the output node raw body.

Unsure if this is the correct fix since it's unclear to me why matter was included here in the first place (maybe it is stripped in later code paths?).

Copy link
Contributor

Choose a reason for hiding this comment

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

I tracked first introduction of it to ChristopherBiscardi/gatsby-mdx#329 - there is 0 context on it - to me it seem more like debugging code than anything else - the code is in the MDX format, so it would just output any content put there as content which doesn;t make much sense (to me anyway).

Especially with fact that matter in returned object means:

file.matter {String}: the raw, un-parsed front-matter string

code = `

import DefaultLayout from "${slash(defaultLayout)}"

Expand Down