thoughtworks / talisman

Using a pre-commit hook, Talisman validates the outgoing changeset for things that look suspicious — such as tokens, passwords, and private keys.
https://thoughtworks.github.io/talisman/
MIT License
1.89k stars 241 forks source link

Can not download binary talisman_darwin_amd64 (v1.29.1) #401

Closed coadtantw closed 1 year ago

coadtantw commented 1 year ago

Describe the bug Can not download binary talisman_darwin_amd64 (v1.29.1) on the release page

To Reproduce

  1. Go to the release page
  2. Download talisman_darwin_amd64 (v1.29.1)
  3. It would fail to download (Failed - Download error)

Expected behavior It should be downloaded successfully

Screenshots Screen Shot 2565-10-25 at 11 22 21 AM

Desktop (please complete the following information):

Additional context

Scatchell commented 1 year ago

A few members of my team and myself have had a similar problem with this issue, but it is more extreme as it is stopping us from using talisman at all. It seems related to this same issue.

Downloading the above mentioned file manually from bithub fails for me in the same way, but the bigger problem is that when running talisman locally, it detects it is out of date and automatically downloads this binary as an update. The download seems to succeed from the terminal, but then when trying to run the pre-commit hooks it just breaks with a segmentation fault:

git commit -am "Test for github issue"

Your version of Talisman is outdated. Updating Talisman to v1.29.1
Downloading latest talisman binary...
talisman_darwin_amd64: OK

Talisman binary updated successfully!
Downloading latest talisman hook script...
Talisman hook script updated successfully!
.git/hooks/pre-commit: line 92: 34940 Segmentation fault: 11  ${CMD}

All talisman commands then fail in the same way. Here is an example of finding which talisman binary is being used and trying to execute it with another segmentation fault.

I also think it might be odd that this binary is only 2.5M as previous versions have been much larger ( ~8M, I think )

└─[$] which talisman
talisman: aliased to ~/.talisman/bin/talisman_darwin_amd64
└─[$] ls -lh ~/.talisman/bin/talisman_darwin_amd64
-rwxr-xr-x  1 xxxxxx  xxxx   2.5M Oct 31 16:48 ~/.talisman/bin/talisman_darwin_amd64
└─[$] talisman --version
[1]    35128 segmentation fault  ~/.talisman/bin/talisman_darwin_amd64 --version

This is stopping us from being able to use talisman at all, and is forcing us to remove the hooks from our repo's - so any help or guidance would be much appreciated! May be related to the recent Ventura upgrade - OS info is attached below:

Machine: Mac 16 inch Intel Core i7 OS: Ventura 13.0 (22A380)

Happy to provide any more info that might be useful!

dcRUSTy commented 1 year ago

It should be a bug/ compatibility issue with upx and Ventura. upx was added by me to compress talisman binaries. :( Creating a new release with uncompressed binary should fix it.

coadtantw commented 1 year ago

Hi @Scatchell ,

In our case, we have a custom installation script and the workaround was to pin the version to its previous version https://github.com/thoughtworks/talisman/releases/download/v1.29.0

We can still use Talisman on our pipeline but would definitely change the version to latest as soon as the compress bug/compatibility issue is gone.

@dcRUSTy , thanks for the info! I'm using Monterey... not sure if you are referring to Ventura compatibility on the building phrase?

dcRUSTy commented 1 year ago

@coadtantw currently macOS amd64(Intel/non M1) variant is compressed with upx

upx generally has bad forward compatibility for generated binaries on freshly launched macOS versions https://github.com/upx/upx/issues/612 https://github.com/upx/upx/issues/613

Screenshot 2022-11-01 at 9 08 45 AM

The short term fastest solution would be create a new release where talisman_darwin_amd64 is not compressed with upx

https://github.com/thoughtworks/talisman/blob/main/compress-binary.sh#L14

https://github.com/thoughtworks/talisman/pull/248

Rajat-Sharma-thoughtworks commented 1 year ago

@dcRUSTy Is this resolved? I tried this today and still got this same error.

Screenshot 2022-11-21 at 6 23 58 PM

macOS: 13.0.1 (ventura) CPU: intel i7

JamborJan commented 1 year ago

As far as I can tell, the problem is still the same. Getting the error in all repositories. Current workaround to be able to push without having talisman check the code:

mv .git/hooks/pre-push .git/ 
git push 
mv .git/pre-push .git/hooks 
jmatias commented 1 year ago

thanks for reporting, i’ll take a look

Rajat-Sharma-thoughtworks commented 1 year ago

@jmatias any updates ?

Scatchell commented 1 year ago

Yes, still same with me as well - have removed talisman from all repo's but from time to time I check to see if it auto updates and downloads a new working binary - checking this morning I get the usual segmentation fault

[1] 34765 segmentation fault /Users/xxxx/.talisman/bin/talisman_darwin_amd64 --version

jmatias commented 1 year ago

Yes, still same with me as well - have removed talisman from all repo's but from time to time I check to see if it auto updates and downloads a new working binary - checking this morning I get the usual segmentation fault

[1] 34765 segmentation fault /Users/xxxx/.talisman/bin/talisman_darwin_amd64 --version

I'm going to debug this on an Intel Mac, I'll post an update a bit later today. @Rajat-Sharma-thoughtworks @Scatchell

jmatias commented 1 year ago

@Scatchell @Rajat-Sharma-thoughtworks Could you give it a try now?

image

Scatchell commented 1 year ago

Awesome! Thanks jmatias for the help! I haven't had time to properly test yet (can do later today) but I did a quick test to see if it updated and it seems to run for me now! 👍

image
Rajat-Sharma-thoughtworks commented 1 year ago

It worked for me as well. Thanks @jmatias

Screenshot 2022-12-09 at 3 06 29 PM
jmatias commented 1 year ago

Workaround released in version v1.29.4.

https://github.com/thoughtworks/talisman/releases/tag/v1.29.4