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

[nextjs] Original source code visible in browser dev tools #4489

Open
4 of 9 tasks
kelvinndmo opened this issue Feb 2, 2022 · 7 comments
Open
4 of 9 tasks

[nextjs] Original source code visible in browser dev tools #4489

kelvinndmo opened this issue Feb 2, 2022 · 7 comments

Comments

@kelvinndmo
Copy link

kelvinndmo commented Feb 2, 2022

Package + Version

  • @sentry/browser
  • @sentry/node
  • raven-js
  • raven-node (raven for node)
  • other:

Version:

6.17.3

Description

Describe your issue in detail, ideally, you have a reproducible demo that you can show.

I have used sentry/nextjs to upload the source maps to sentry, however, i can be able to see my source code on production, attached here is a screenshot, if i remove sentry from the next config file, the folder doesn't exist anymore, i have tried all options but haven't succeeded, any help will be appreciated.

Screenshot 2022-02-02 at 10 46 12

@lobsterkatie
Copy link
Member

Hi, @kelvinndmo.

What you want is a combo of hidden-source-map (which removes the sourceMappingURL comment at the bottom of your bundles, so Chrome won't look for maps) and not deploying the maps (so that even if one went looking, they wouldn't be there). You can turn hidden-source-maps on in your sentry config). As for not hosting sourcemaps, it depends on your platform, but likely the easiest way is just to delete the files after your app builds but before it deploys (or, if you can control what gets deployed, only deploy the .js files).

Cheers!

@AlexBeauchemin
Copy link

I created a webpack plugin to achieve this with nextjs, it will build the sourcemaps, upload them to sentry and then remove them so they won't be there at the end of the build and won't be uploaded to your production server
https://github.com/AlexBeauchemin/webpack-delete-sourcemaps-plugin

@lobsterkatie
Copy link
Member

lobsterkatie commented Mar 30, 2022

Thanks for sharing! BTW, your readme says

For nextjs config using withSentryConfig, we can't set the devtool value to hidden-source-map as it's overriden by the sentry config wrapper

but that's no longer true: https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#use-hidden-source-map.

@dcramer dcramer reopened this Aug 19, 2022
@dcramer
Copy link
Member

dcramer commented Aug 19, 2022

Let's revisit this conversation and figure out if there's something we can do to improve DX.

  1. We need sourcmaps in Sentry
  2. We seemingly default them on (ok thats fine, we need them)
  3. If we are the responsible party for turning them on, what can we do to also restore the prior experience after we get what we need? e.g. delete them before app is published?

@lobsterkatie
Copy link
Member

We talked about this as a team, and we're going to deal with this in stages. There are a few different considerations here:

  • Default value of devtool (source-map or hiddden-source-map): Controls the existence of the sourceMappingURL comment at the bottom of bundles, affects whether or not chrome devtools looks for and auto-loads sourcemaps and sourcemapped code. Doesn't affect the existence of sourcemaps. (IOW, if you publish everything, they'll still be there if you know the correct URL.)
  • Existence of/potential deletion of sourcemaps: They can't be published if they don't exist because we delete them after uploading. That said, should we be in the business of deleting people's files?
  • Reason for sourcemaps' existence: Would they be there if not for us?
  • Order of config manipulation: We tell people to do module.exports = withSentryConfig(...) in their next.config.js, which means we're the last to touch the webpack config. If we are, it's easy to tell if we're the ones who set devtool. But can we guarantee that we're last?

Our first stage will be to assume we're last, check the incoming config, and if we're the ones to set devtool, set it to hidden-source-map by default. This is technically a breaking change, so we'll have to find the best way(s) to communicate this to users.

Our second stage will be to consider deleting sourcemaps after we upload them, again assuming that we're the last to change the webpack config.

Our third stage, if we decide to delete source maps, will be to actually parse next.config.js or run it in a sandbox or find some other way to determine if we actually are the last ones to modify the config and, depending on our approach, possibly figure out what happens after us if we're not the last.

We are going into a weeklong code freeze starting Monday (and will not make any releases before then), but will begin work on this today and pick it back up after after we return.

@lobsterkatie lobsterkatie changed the title Nextjs Source code on product under _N_E folder, how can i avoid this? [nextjs] Original source code visible in browser dev tools Aug 30, 2022
@lobsterkatie
Copy link
Member

lobsterkatie commented Aug 30, 2022

Update:

A slightly modified version of the first stage mentioned above has now been PR-ed.

  • Existing users who have not set the hideSourceMaps option one way or the other will get a warning, during both prod and dev build, letting them know that sourcemaps may make original source code visible in browser dev tools, and letting them know how to prevent that.
  • New users setting up their app either by using the wizard or by following the nextjs with-sentry example will have hideSourceMaps explicitly set to true in their boilerplate next.config.js.
  • In the next major (version 8 of the SDK), we will switch the default for hideSourceMaps to true, a fact reflected in the aforementioned warning message, in a comment above the aforementioned spots where we're setting it explicitly, and in the docs.

Further update: All of the above are merged and released. I'm going to move this to the backlog for potential future work on stages 2 and 3 mentioned above. Hopefully this stage 1 implementation solves the immediate problem, though.

Last update (I swear!): In order to prepare for a possible future in which we delete source maps, I've filed a feature request in the sentry-cli repo, for a flag which could be passed to upload-sourcemaps and which would cause the cli to delete any maps it finds after it's done uploading.

lobsterkatie added a commit to getsentry/sentry-docs that referenced this issue Aug 30, 2022
…5464)

This expands the explanation of the `hideSourceMaps` option in `next.config.js`, and notes that it will default to `true` starting in version 8 of the SDK.

In conjunction with getsentry/sentry-javascript#5649, getsentry/sentry-wizard#188, and vercel/next.js#40079, this is the first step in addressing the concerns raised in getsentry/sentry-javascript#4489. See getsentry/sentry-javascript#4489 (comment) for more details.
lobsterkatie added a commit to getsentry/sentry-wizard that referenced this issue Aug 30, 2022
This adds a default value for `hideSourceMaps` to the `next.config.js` the wizard provides, so that new users will use `hidden-source-map` as their Webpack `devtool` by default in client-side builds. (This prevents browser devtools from seeing sourcemaps and therefore prevents original source code from showing up automatically in the `Sources` tab. (More information on `devtool` values can be found at https://webpack.js.org/configuration/devtool/.)

In conjunction with getsentry/sentry-javascript#5649, getsentry/sentry-docs#5464, and vercel/next.js#40079, this is the first step in addressing the concerns raised in getsentry/sentry-javascript#4489. See getsentry/sentry-javascript#4489 (comment) for more details.
lobsterkatie added a commit that referenced this issue Aug 30, 2022
This adds a warning during nextjs app build (both in prod and dev) letting folks know that by default (for now), the sourcemaps we create can be seen by browser devtools.

In conjunction with getsentry/sentry-wizard#188, getsentry/sentry-docs#5464, and vercel/next.js#40079, this is the first step in addressing the concerns raised in #4489. See #4489 (comment) for more details.

Notes:

- The function doing the warning includes both the current warning and one for use in v8 and beyond (currently commented out), telling people they no longer need to set `hideSourceMaps` to `true` (because by that point it will be the default).

- The formatting of the warning matches the formatting of other nextjs warnings. Because nextjs vendors `chalk` themselves, that meant we needed to add it as a dependency. Though the latest version of `chalk` is 5.x, here we're using 3.x because it's the last to be compatible with node 8. (See the PR for a screenshot.)

- There are `TODO`s reflecting the changes which need to be made in v8 (change the default to `true`, switch the warning message) and v9 or v10 (get rid of the second warning once it's had plenty of time to do its job).
kodiakhq bot pushed a commit to vercel/next.js that referenced this issue Sep 5, 2022
This adds a default value for `hideSourceMaps` to the `next.config.js` the `with-sentry` example app provides, so that new users will use `hidden-source-map` as their Webpack `devtool` by default in client-side builds. (This prevents browser devtools from seeing sourcemaps and therefore prevents original source code from showing up automatically in the `Sources` tab. (More information on `devtool` values can be found [here](https://webpack.js.org/configuration/devtool/).)

In conjunction with getsentry/sentry-javascript#5649, getsentry/sentry-wizard#188, and getsentry/sentry-docs#5464, this is the first step in addressing the concerns raised in getsentry/sentry-javascript#4489. See [here](getsentry/sentry-javascript#4489 (comment)) for more details.
@lforst
Copy link
Member

lforst commented Oct 30, 2023

We can think about blocking requests for source maps with Next.js rewrites automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment