-
-
Notifications
You must be signed in to change notification settings - Fork 234
fix(sourcemaps): Avoid associating only sourcemap with all minified sources #2447
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
Conversation
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@loewenheim, when reviewing this change, please note that I split off some parts of #2438 into separate issues
``` uglifyjs captureException.js -o captureException.min.js --source-map url=captureException.min.js.map,includeSources sentry-cli sourcemaps inject captureException.min.js captureException.min.js.map ``` as mentioned here: getsentry/sentry-cli#2447 this indeed works, see: http://sokra.github.io/source-map-visualization/#custom
f952605 to
ff65a0b
Compare
|
@loewenheim I made some pretty significant changes to get the tests to pass here, so I'd appreciate a re-review |
src/utils/sourcemaps.rs
Outdated
|
|
||
| /// Adds a given source_file to sources, taking ownership of the source_file | ||
| /// but returning a reference to it. | ||
| fn add_file_to_sources(&mut self, source_file: SourceFile) -> &SourceFile { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT the returned reference is never used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed, a previous iteration did use the return value; now i'm trying to figure out why I changed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, I figured it out. I thought I would have to add the sourcemap (new parameter to unpack_ram_bundle) into the sourcemap processor, but I later realized that is unnecessary, since we anyways were overwriting its entry in the processor on line 478 by inserting at the same key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so I will remove the returned reference
…ources Remove the branch from `guess_sourcemap_reference` which handles the case of there only being one sourcemap. If there are multiple minified souces, they would all (erroneously) end up associated with the same single sourcemap. Also, since code for uploading bundles was relying on this branch (specifically when unpacking bundles), refactor so that we use the sourcemap which is passed to the command directly, rather thna "guessing" it. Fixes #2438 Fixes #2503
ff65a0b to
884faeb
Compare
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch on the review btw!
![snyk-io[bot]](https://badgen.net/badge/icon/snyk-io%5Bbot%5D/green?label=)  [<img width="16" alt="Powered by Pull Request Badge" src="https://user-images.githubusercontent.com/1393946/111216524-d2bb8e00-85d4-11eb-821b-ed4c00989c02.png">](https://pullrequestbadge.com/?utm_medium=github&utm_source=reisene&utm_campaign=badge_info)<!-- PR-BADGE: PLEASE DO NOT REMOVE THIS COMMENT -->  <h3>Snyk has created this PR to upgrade @sentry/cli from 2.46.0 to 2.47.1.</h3> :information_source: Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project. <hr/> - The recommended version is **2 versions** ahead of your current version. - The recommended version was released **24 days ago**. #### Issues fixed by the recommended upgrade: | | Issue | Score | Exploit Maturity | :-------------------------:|:-------------------------|:-------------------------|:-------------------------  | Regular Expression Denial of Service (ReDoS)<br/>[SNYK-JS-BRACEEXPANSION-9789073](https://snyk.io/vuln/SNYK-JS-BRACEEXPANSION-9789073) | **57** | Proof of Concept  | Regular Expression Denial of Service (ReDoS)<br/>[SNYK-JS-BRACEEXPANSION-9789073](https://snyk.io/vuln/SNYK-JS-BRACEEXPANSION-9789073) | **57** | Proof of Concept <details> <summary><b>Release notes</b></summary> <br/> <details> <summary>Package name: <b>@sentry/cli</b></summary> <ul> <li> <b>2.47.1</b> - <a href="https://redirect.github.com/getsentry/sentry-cli/releases/tag/2.47.1">2025-07-14</a></br><p>No user-facing changes.</p> </li> <li> <b>2.47.0</b> - <a href="https://redirect.github.com/getsentry/sentry-cli/releases/tag/2.47.0">2025-07-08</a></br><h3>Various fixes & improvements</h3> <ul> <li>ref: Exclude <code>mobile-app</code> command from release builds (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3212509860" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2582" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2582/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2582">#2582</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> <li>feat(login): Improve error output for login errors (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3212304793" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2581" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2581/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2581">#2581</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> <li>test(monitors): pass empty options to grep (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3153487576" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2562" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2562/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2562">#2562</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/lcian/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/lcian">@ lcian</a></li> <li>feat(login): Warn when overwriting existing auth token (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3136628527" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2554" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2554/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2554">#2554</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> <li>meta: Add .sentryclirc to gitignore (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3128551924" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2547" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2547/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2547">#2547</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/rbro112/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/rbro112">@ rbro112</a></li> <li>build: Bump MSRV to 1.86</li> </ul> </li> <li> <b>2.46.0</b> - <a href="https://redirect.github.com/getsentry/sentry-cli/releases/tag/2.46.0">2025-05-27</a></br><h3>Various fixes & improvements</h3> <ul> <li>feat: Mark <code>react-native appcenter</code> as deprecated (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3093593055" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2522" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2522/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2522">#2522</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/chromy/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/chromy">@ chromy</a></li> <li>docs: Fix typo "the the" -> "the" (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3093284739" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2519" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2519/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2519">#2519</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/chromy/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/chromy">@ chromy</a></li> <li>feat(npm): Add support for installing <code>npm</code> package on Android (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3093966444" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2524" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2524/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2524">#2524</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> <li>feat: Retry all HTTP requests (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3093822122" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2523" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2523/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2523">#2523</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> <li>ref: Only obtain max retry count once (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3093433658" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2521" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2521/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2521">#2521</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> <li>fix: Don't error if invalid value supplied for max retries (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3093374647" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2520" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2520/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2520">#2520</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> <li>fix: Explicitly deprecate <code>--started</code> flag (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3080611085" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2515" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2515/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2515">#2515</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> <li>fix: Use <code>orig_path</code> for bundle sources output file name (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3080431317" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2514" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2514/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2514">#2514</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> <li>feat: Mark all <code>files</code> subcommands as deprecated. (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3076318405" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2512" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2512/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2512">#2512</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/Swatinem/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/Swatinem">@ Swatinem</a></li> <li>Support multiple files in SENTRY_DOTENV_PATH (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2979203905" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2454" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2454/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2454">#2454</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/Kinqdos/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/Kinqdos">@ Kinqdos</a></li> <li>fix(sourcemaps): Avoid associating only sourcemap with all minified sources (<a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2953330799" data-permission-text="Title is private" data-url="getsentry/sentry-cli#2447" data-hovercard-type="pull_request" data-hovercard-url="/getsentry/sentry-cli/pull/2447/hovercard" href="https://redirect.github.com/getsentry/sentry-cli/pull/2447">#2447</a>) by <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/szokeasaurusrex/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://redirect.github.com/szokeasaurusrex">@ szokeasaurusrex</a></li> </ul> </li> </ul> from <a href="https://redirect.github.com/getsentry/sentry-cli/releases">@sentry/cli GitHub release notes</a> </details> </details> --- > [!IMPORTANT] > > - Check the changes in this PR to ensure they won't cause issues with your project. > - This PR was automatically created by Snyk using the credentials of a real user. > - Max score is 1000. Note that the real score may have changed since the PR was raised. --- **Note:** _You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs._ **For more information:** <img src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiJiN2IzMGM5Mi03ZDdkLTQxMGYtOWVjYi1kMTI3ZmIzZDcwNDkiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6ImI3YjMwYzkyLTdkN2QtNDEwZi05ZWNiLWQxMjdmYjNkNzA0OSJ9fQ==" width="0" height="0"/> > - 🧐 [View latest project report](https://app.snyk.io/org/reisene/project/55e114f8-489e-4f14-b900-20574b041e59?utm_source=github-cloud-app&utm_medium=referral&page=upgrade-pr) > - 📜 [Customise PR templates](https://docs.snyk.io/scan-using-snyk/pull-requests/snyk-fix-pull-or-merge-requests/customize-pr-templates?utm_source=&utm_content=fix-pr-template) > - 🛠 [Adjust upgrade PR settings](https://app.snyk.io/org/reisene/project/55e114f8-489e-4f14-b900-20574b041e59/settings/integration?utm_source=github-cloud-app&utm_medium=referral&page=upgrade-pr) > - 🔕 [Ignore this dependency or unsubscribe from future upgrade PRs](https://app.snyk.io/org/reisene/project/55e114f8-489e-4f14-b900-20574b041e59/settings/integration?pkg=@sentry/cli&utm_source=github-cloud-app&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades) [//]: # 'snyk:metadata:{"customTemplate":{"variablesUsed":[],"fieldsUsed":[]},"dependencies":[{"name":"@sentry/cli","from":"2.46.0","to":"2.47.1"}],"env":"prod","hasFixes":true,"isBreakingChange":false,"isMajorUpgrade":false,"issuesToFix":["SNYK-JS-BRACEEXPANSION-9789073","SNYK-JS-BRACEEXPANSION-9789073"],"prId":"b7b30c92-7d7d-410f-9ecb-d127fb3d7049","prPublicId":"b7b30c92-7d7d-410f-9ecb-d127fb3d7049","packageManager":"npm","priorityScoreList":[57],"projectPublicId":"55e114f8-489e-4f14-b900-20574b041e59","projectUrl":"https://app.snyk.io/org/reisene/project/55e114f8-489e-4f14-b900-20574b041e59?utm_source=github-cloud-app&utm_medium=referral&page=upgrade-pr","prType":"upgrade","templateFieldSources":{"branchName":"default","commitMessage":"default","description":"default","title":"default"},"templateVariants":["priorityScore"],"type":"auto","upgrade":["SNYK-JS-BRACEEXPANSION-9789073","SNYK-JS-BRACEEXPANSION-9789073"],"upgradeInfo":{"versionsDiff":2,"publishedDate":"2025-07-14T14:36:33.514Z"},"vulns":["SNYK-JS-BRACEEXPANSION-9789073","SNYK-JS-BRACEEXPANSION-9789073"]}'
Remove the branch from
guess_sourcemap_referencewhich handles the case of there only being one sourcemap. If there are multiple minified souces, they would all (erroneously) end up associated with the same single sourcemap.Also, since code for uploading bundles was relying on this branch (specifically when unpacking bundles), refactor so that we use the sourcemap which is passed to the command directly, rather thna "guessing" it.
Fixes #2438
Fixes #2503