Skip to content

Relax checksum requirement - #323

Merged
mitsuhiko merged 2 commits into
masterfrom
cwlw/relax-checksum-requirement
Nov 11, 2021
Merged

Relax checksum requirement#323
mitsuhiko merged 2 commits into
masterfrom
cwlw/relax-checksum-requirement

Conversation

@chadwhitacre

Copy link
Copy Markdown
Contributor

This is a workaround for #322, where we're finding that our checksum comparison algorithm is not always sufficient. Better to log and proceed rather than block deployment in such cases.

We're finding cases where our checksum comparison algorithm is
insufficient. Better to log and proceed rather than block deployment in
those cases.
@AbhiPrasad

Copy link
Copy Markdown
Contributor

I'm in favour of merging this in to unblock, but this could cause issues with compliance, @getsentry/security do y'all have any thoughts before we push forward with it?

@chadwhitacre

Copy link
Copy Markdown
Contributor Author

Original reason to do the checksum was less security-related and more that we uploaded an old version as if it were the new version (if I remember right, would have to dig).

@iker-barriocanal

Copy link
Copy Markdown
Contributor

Tbh I don't feel really comfortable merging this since we may be releasing the wrong files. And I also wonder about security implications; if there aren't any, I guess we could merge this to unblock new releases.

@chadwhitacre

Copy link
Copy Markdown
Contributor Author

@iker-barriocanal Fair enough. Do you have any bandwidth to work on a better solution?

@mitsuhiko

mitsuhiko commented Nov 11, 2021

Copy link
Copy Markdown
Contributor

Ship it. This is good enough of a solution for now and there are no concerns from a compliance perspective.

@iker-barriocanal

Copy link
Copy Markdown
Contributor

@chadwhitacre yes, in progress.

@mitsuhiko
mitsuhiko merged commit ea24f4d into master Nov 11, 2021
@mitsuhiko
mitsuhiko deleted the cwlw/relax-checksum-requirement branch November 11, 2021 09:28
@marandaneto

Copy link
Copy Markdown
Contributor

having the same issue, can we get a patch release? otherwise I'm blocked to finish the release

@AbhiPrasad

Copy link
Copy Markdown
Contributor

@marandaneto We use latest craft as per https://github.com/getsentry/publish/blob/main/.github/workflows/publish.yml#L49, so re-rerunning publish should work.

@chadwhitacre

Copy link
Copy Markdown
Contributor Author

Ah, nice @AbhiPrasad. Coming here to say that I did kick off an 0.27.1 release as well:

https://github.com/getsentry/craft/actions/runs/1449358202

@marandaneto

Copy link
Copy Markdown
Contributor

Ah, nice @AbhiPrasad. Coming here to say that I did kick off an 0.27.1 release as well:

getsentry/craft/actions/runs/1449358202

that works, sweet, thanks for the quick release

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants