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

Setting cleanArtifacts: true fails with a 403 #481

Open
Wedvich opened this issue Feb 6, 2024 · 15 comments
Open

Setting cleanArtifacts: true fails with a 403 #481

Wedvich opened this issue Feb 6, 2024 · 15 comments

Comments

@Wedvich
Copy link

Wedvich commented Feb 6, 2024

Environment

Steps to Reproduce

I added cleanArtifacts: true to my config, and tried to overwrite the same release name.

release: {
  cleanArtifacts: true,
  name: sentryReleaseId,
  setCommits: {
    auto: true,
  },
},

Expected Result

The existing artifact bundle should be deleted, and the new artifacts should be uploaded. When Webpack is done I expect to only see one artifact bundle under the release.

Actual Result

The upload with the new artifact bundle succeeds, but the old artifacts aren't deleted, and I get the following error in the logs:

  DEBUG   2024-02-06 11:53:05.136196 +01:00 > DELETE /api/0/projects/<redacted>/<redacted>/files/source-maps/?name=local-tmp HTTP/1.1                                       
  DEBUG   2024-02-06 11:53:05.136216 +01:00 > Host: sentry.io                                                                                                                
  DEBUG   2024-02-06 11:53:05.136221 +01:00 > Accept: */*                                                                                                                    
  DEBUG   2024-02-06 11:53:05.136225 +01:00 > Connection: TE                                                                                                                 
  DEBUG   2024-02-06 11:53:05.136228 +01:00 > TE: gzip                                                                                                                       
  DEBUG   2024-02-06 11:53:05.136231 +01:00 > User-Agent: sentry-cli/2.26.0 webpack-plugin/2.10.3                                                                            
  DEBUG   2024-02-06 11:53:05.136975 +01:00 > Authorization: Bearer sntrys_e***                                                                                              
  DEBUG   2024-02-06 11:53:05.292127 +01:00 < HTTP/1.1 403 Forbidden                                                                                                         
  DEBUG   2024-02-06 11:53:05.292146 +01:00 < server: nginx                                                                                                                  
  DEBUG   2024-02-06 11:53:05.292150 +01:00 < date: Tue, 06 Feb 2024 10:53:05 GMT                                                                                            
  DEBUG   2024-02-06 11:53:05.292153 +01:00 < content-type: application/json                                                                                                 
  DEBUG   2024-02-06 11:53:05.292156 +01:00 < Content-Length: 63                                                                                                             
  DEBUG   2024-02-06 11:53:05.292159 +01:00 < allow: GET, DELETE, HEAD, OPTIONS                                                                                              
  DEBUG   2024-02-06 11:53:05.292162 +01:00 < access-control-allow-methods: GET, DELETE, HEAD, OPTIONS                                                                       
  DEBUG   2024-02-06 11:53:05.292170 +01:00 < access-control-allow-headers: X-Sentry-Auth, X-Requested-With, Origin, Accept, Content-Type, Authentication, Authorization, Con
tent-Encoding, sentry-trace, baggage, X-CSRFToken                                                                                                                            
  DEBUG   2024-02-06 11:53:05.292173 +01:00 < access-control-expose-headers: X-Sentry-Error, X-Sentry-Direct-Hit, X-Hits, X-Max-Hits, Endpoint, Retry-After, Link            
  DEBUG   2024-02-06 11:53:05.292177 +01:00 < access-control-allow-origin: *                                                                                                 
  DEBUG   2024-02-06 11:53:05.292179 +01:00 < x-sentry-rate-limit-remaining: 39                                                                                              
  DEBUG   2024-02-06 11:53:05.292182 +01:00 < x-sentry-rate-limit-limit: 40                                                                                                  
  DEBUG   2024-02-06 11:53:05.292184 +01:00 < x-sentry-rate-limit-reset: 1707216786                                                                                          
  DEBUG   2024-02-06 11:53:05.292187 +01:00 < x-sentry-rate-limit-concurrentremaining: 24                                                                                    
  DEBUG   2024-02-06 11:53:05.292223 +01:00 < x-sentry-rate-limit-concurrentlimit: 25                                                                                        
  DEBUG   2024-02-06 11:53:05.292226 +01:00 < vary: Accept-Language, Cookie                                                                                                  
  DEBUG   2024-02-06 11:53:05.292228 +01:00 < content-language: en                                                                                                           
  DEBUG   2024-02-06 11:53:05.292231 +01:00 < x-frame-options: deny                                                                                                          
  DEBUG   2024-02-06 11:53:05.292233 +01:00 < x-content-type-options: nosniff                                                                                                
  DEBUG   2024-02-06 11:53:05.292236 +01:00 < x-xss-protection: 1; mode=block                                                                                                
  DEBUG   2024-02-06 11:53:05.292244 +01:00 < content-security-policy: frame-ancestors 'self' *.sentry.io; object-src 'self'; connect-src 'self' *.algolia.net *.algolianet.c
om *.algolia.io sentry.io *.sentry.io s1.sentry-cdn.com o1.ingest.sentry.io api2.amplitude.com app.pendo.io data.pendo.io reload.getsentry.net t687h3m0nh65.statuspage.io sen
try.zendesk.com ekr.zdassets.com maps.googleapis.com; img-src blob: data: *; script-src 'self' 'unsafe-inline' 'report-sample' 'unsafe-eval' s1.sentry-cdn.com js.sentry-cdn.
com browser.sentry-cdn.com statuspage-production.s3.amazonaws.com static.zdassets.com aui-cdn.atlassian.com connect-cdn.atl-paas.net js.stripe.com 'strict-dynamic' cdn.pendo
.io data.pendo.io pendo-io-static.storage.googleapis.com pendo-static-5634074999128064.storage.googleapis.com; style-src 'unsafe-inline' *; default-src 'none'; media-src *; 
base-uri 'none'; font-src * data:; frame-src app.pendo.io demo.arcade.software js.stripe.com sentry.io; report-uri https://o1.ingest.sentry.io/api/54785/security/?sentry_key
=f724a8a027db45f5b21507e7142ff78e&sentry_release=68fefd51e8c0a3cc6d9e7f584ec72790f09f72e7                                                                                    
  DEBUG   2024-02-06 11:53:05.292279 +01:00 < x-envoy-attempt-count: 1                                                                                                       
  DEBUG   2024-02-06 11:53:05.292282 +01:00 < x-envoy-upstream-service-time: 23                                                                                              
  DEBUG   2024-02-06 11:53:05.292284 +01:00 < x-served-by: getsentry-web-default-common-production-579846cdcd-tnmb7                                                          
  DEBUG   2024-02-06 11:53:05.292287 +01:00 < x-served-by: frontend-default-8677bf596b-7krcw                                                                                 
  DEBUG   2024-02-06 11:53:05.292290 +01:00 < strict-transport-security: max-age=31536000; includeSubDomains; preload                                                        
  DEBUG   2024-02-06 11:53:05.292292 +01:00 < via: 1.1 google                                                                                                                
  DEBUG   2024-02-06 11:53:05.292294 +01:00 < Alt-Svc: h3=":443"; ma=2592000,h3-29=":443"; ma=2592000                                                                        
  DEBUG   2024-02-06 11:53:05.292315 +01:00 response status: 403                                                                                                             
  DEBUG   2024-02-06 11:53:05.292347 +01:00 body: {"detail":"You do not have permission to perform this action."}                                                            
error: API request failed                                                                                                                                                    
  caused by: sentry reported an error: You do not have permission to perform this action. (http status: 403)

I have the Manager role in my Sentry org, and I can manually delete artifact bundles and releases in the Sentry UI.

@lforst
Copy link
Member

lforst commented Feb 7, 2024

Thanks for reporting this! We need to adjust the endpoint in sentry to work with the org:ci scope.

@szokeasaurusrex
Copy link
Member

@Wedvich could you please try again, but without the cleanArtifacts: true option, and let us know whether the upload succeeds?

We think that the upload might work without the option, since the endpoint should simply overwrite any existing files with the same name.

@Wedvich
Copy link
Author

Wedvich commented Feb 22, 2024

Without cleanArtifacts: true it acts the same way as with it, except it doesn't give the 403. If I run two builds in a row, I now have two artifact bundles under the same release, and both bundles contain the same files/filenames.

Is this intentional behavior? I tried reading the docs on Artifact Bundles but it doesn't say anything about multiple bundles with the same files for the same release?

@lforst
Copy link
Member

lforst commented Feb 23, 2024

Is this intentional behavior? I tried reading the docs on Artifact Bundles but it doesn't say anything about multiple bundles with the same files for the same release?

@Wedvich I believe the files that were uploaded last take priority in our resolving logic. But I'll have @Swatinem confirm that!

@Swatinem
Copy link
Member

The logic is indeed very complex and there is way too many moving parts, but we should always prefer the most recently uploaded bundle that contains the file being requested.

If there is some problem with this, can you point me to a specific event and the release with bundles so I can take a closer look?

@szokeasaurusrex szokeasaurusrex removed their assignment Mar 5, 2024
@newhouse
Copy link

Thanks for reporting this! We need to adjust the endpoint in sentry to work with the org:ci scope.

@lforst your comment really seems to be the root cause, IMO. The suggested method for creating an Auth Token for use in this plugin will only have the org:ci scope, and it appears that is not sufficient for the delete --all CLI command it tries to run.

Here are some extra logs showing the command:

sentry-cli was invoked with the following command line: "/app/node_modules/@sentry/cli-linux-x64/bin/sentry-cli" "releases" "files" "some:release-name-here" "delete" "--all"
 error: API request failed
   caused by: sentry reported an error: You do not have permission to perform this action. (http status: 403)

It appears that the cleanArtifacts: true will attempt to run that command before uploading the maps. It's nice that the subsequent sourcemap upload succeeds, but it's still not correct that the pre-existing artifacts are not deleted and it's not great seeing errors in the logs.

Hoping for a fix...thanks!

@Lms24
Copy link
Member

Lms24 commented Apr 12, 2024

then we need to check if we can adjust the permissions for deleting so that this works with the org based auth token. If you use an old user auth token (Settings -> Account -> API -> User Auth Tokens), does it work then?

@newhouse
Copy link

then we need to check if we can adjust the permissions for deleting so that this works with the org based auth token. If you use an old user auth token (Settings -> Account -> API -> User Auth Tokens), does it work then?

I/we don't use any User Auth Tokens in these scenarios, so I can't tell you about that specifically, but we do have an organization Custom Integration setup that we are currently using with the correct permissions where things seem to be working fine.

@Lms24
Copy link
Member

Lms24 commented Apr 17, 2024

Hmm yes, afaik these integration tokens and user tokens work very similarly, so that makes sense. Thanks for testing!

@lforst
Copy link
Member

lforst commented Apr 18, 2024

Ok so after some pondering we decided to deprecate and delete cleanArtifacts (for now it will just noop). There was a historical need for it but nowadays you can just override files in Sentry and Sentry will pick up the most recent one for source mapping. For 99.999% of people this should be fine. For all the other users that want to actually delete artifacts from Sentry, it is recommended to use the UI (it is admittedly shitty though), the CLI with a non-organization-auth-token having the right permissions, or by using the API.

@lforst
Copy link
Member

lforst commented Apr 18, 2024

Lemme know if you think this is stupid or not 😄

@newhouse
Copy link

Ok so after some pondering we decided to deprecate and delete cleanArtifacts (for now it will just noop). There was a historical need for it but nowadays you can just override files in Sentry and Sentry will pick up the most recent one for source mapping. For 99.999% of people this should be fine. For all the other users that want to actually delete artifacts from Sentry, it is recommended to use the UI (it is admittedly shitty though), the CLI with a non-organization-auth-token having the right permissions, or by using the API.

If what you said is true - that overwritten files will just be picked up/prioritized as one might expect - then I think you're right that this sounds fine for me and probably most users who don't actually really care whether the old files are deleted, as long as having them around:

  • Won't cause problems
  • Won't end up adding towards some storage quota accidentally and hurting us that way.

I personally don't feel like we need our files deleted before a new set of maps are pushed up. Others may disagree but sounds unlikely to me.

Curious as to why the choice to remove/deprecate vs fix? Seems like there are some issues between the various tokens being supported. I have seen (and experienced) some other issues with tokens being rejected by the CLI for not having a valid format sometimes...but for us right now our Custom Integration token seems to work everywhere we need it to.

@lforst
Copy link
Member

lforst commented Apr 18, 2024

  • Won't cause problems
  • Won't end up adding towards some storage quota accidentally and hurting us that way.

It should not cause problems (very generic I know 😄) and sourcemaps also don't count towards any quota and we also do not have plans to make them count towards quota. Enjoy the free storage lol 😄

Curious as to why the choice to remove/deprecate vs fix? Seems like there are some issues between the various tokens being supported.

Exactly. So recently we introduced organization auth tokens with very limited scope to drastically reduce the complexity when setting up source maps. In our documentation, we now heavily tell you to use these new auth tokens because they are much simpler to set up and also have upsides like encoding the org and your self-hosted instance. The thing is, we didn't consider the full capabilities of our bundler plugins, meaning for some features the limited scope of these tokens was not enough. We thought about widening the permissions of the tokens but didn't really want to do that for security reasons, so we instead opted to remove the unnecessary feature.

@newhouse
Copy link

Gotcha...makes sense. This will be in some upcoming release then?

Thank you for your support and explanations!

@lforst
Copy link
Member

lforst commented Apr 18, 2024

@newhouse Yep this will land soon. For now, please just remove the cleanArtifacts and everything should work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

6 participants