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

[Privacy] Use of __contentFilePath query param for gatsby-plugin-mdx leaks absolute file paths #37677

Closed
2 tasks done
aschultz opened this issue Feb 20, 2023 · 3 comments · Fixed by #37687
Closed
2 tasks done
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby

Comments

@aschultz
Copy link

aschultz commented Feb 20, 2023

Preliminary Checks

Description

The documentation for gatsby-plugin-mdx describes attaching a query param when creating a page so that the plugin can access the path to the original MDX file.

createPage({
    path: node.frontmatter.slug,
    component: `${path.resolve(`./src/templates/posts.jsx`)}?__contentFilePath=${node.internal.contentFilePath}`,
    context: { id: node.id },
})

Normally when Gatsby generates component chunk names, it converts the absolute component path to a path relative to the project root before applying the kebab transformation. However, it doesn't have any special handling to alter or remove query parameters. Since contentFilePath is an absolute path, attaching it as a query param results in the chunk names containing the entire absolute path structure. Chunk names are referenced repeatedly in the HTML and JS files meant for clients, so this leaks the names of the developer or build machine's directories outside the project folder.

For example,
component = C:\my\privates\mywebsite\src\components\page.tsx?__contentFilePath=C:/my/privates/mywebsite/src/pages/about.md creates the chunk name component---src-components-page-tsx-content-file-path-c-my-privates-mywebsite-src-pages-about-md, creates public\<chunkName>.js and references the string in the window.___chunkMapping of every HTML file.

While the issue of Gatsby including paths in component names has come up before (#18791), this case is particularly egregious since it includes path information outside of the project directory and results in excessively long chunk names.

Reproduction Link

https://github.com/gatsbyjs/gatsby/tree/master/examples/using-mdx

Steps to Reproduce

  1. Download the sample
  2. npm i --legacy-peer-deps
  3. npm run build

Expected Result

Generated files and chunk names do not contain absolute path information

Actual Result

Generated files and chunk names contain absolute path information

Environment

  System:
    OS: Windows 10 10.0.22621
    CPU: (8) x64 Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
  Binaries:
    Node: 18.14.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.21.0 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 9.3.1 - C:\Program Files\nodejs\npm.CMD
  Languages:
    Python: 3.8.5
  Browsers:
    Chrome: 110.0.5481.77
    Edge: Spartan (44.22621.1265.0), Chromium (110.0.1587.41)
  npmPackages:
    gatsby: next => 5.8.0-next.0 
    gatsby-plugin-mdx: next => 5.8.0-next.0 
    gatsby-plugin-sharp: next => 5.8.0-next.0 
    gatsby-remark-images: next => 7.8.0-next.0 
    gatsby-source-filesystem: next => 5.8.0-next.0 
  npmGlobalPackages:
    gatsby-cli: 2.12.109

Config Flags

No response

@aschultz aschultz added the type: bug An issue or pull request relating to a bug in Gatsby label Feb 20, 2023
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 20, 2023
@pieh pieh added status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Feb 22, 2023
@pieh
Copy link
Contributor

pieh commented Feb 22, 2023

Started work on #37687 to resolve this leakage

@pieh
Copy link
Contributor

pieh commented Feb 22, 2023

this case is particularly egregious since it includes path information outside of the project directory and results in excessively long chunk names.

Does "excessively long chunk names" cause real problems or it's just "not nice looking"? We do truncate on names that are long enough that would exceed limitations of various filesystems:

/**
* File names should not exceed 255 characters
* minus 12 for `component---`
* minus 7 for `.js.map`
* minus 20 for `-[hash].js`
*/
const maxLength = 215
const shouldTruncate = name.length > maxLength
/**
* To prevent long file name errors, we truncate the name to a maximum of 60 characters.
*/
if (shouldTruncate) {
const hash = murmurhash(name, 0)
name = `${hash}-${name.substring(name.length - 60)}`
}

Leaking absolute path is valid to fix, but other than that the long names shouldn't be a problem - it's implementation detail that users shouldn't care about unless I'm missing something?

@pieh
Copy link
Contributor

pieh commented Feb 22, 2023

[email protected] was released with fix to replace absolute path with relative path for __contentFilePath. If you could confirm that it handles the issue you reported for you I would backport this to stable / latest tag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants