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

Add wait-for-them #514

Merged
merged 4 commits into from
Jun 8, 2024
Merged

Add wait-for-them #514

merged 4 commits into from
Jun 8, 2024

Conversation

jayvdb
Copy link
Contributor

@jayvdb jayvdb commented Jun 8, 2024

@jayvdb
Copy link
Contributor Author

jayvdb commented Jun 8, 2024

Raised shenek/wait-for-them#53

@jayvdb jayvdb marked this pull request as ready for review June 8, 2024 04:35
Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command looks pretty good, but I'm not sure if it's a good idea to add every rust crate support to this action, given that it fallbacks to cargo-binstall

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jun 8, 2024

Not saying that I'm oppose to adding rust crate to this action, it's that with the cargo-binstall fallback, we don't have to add support here manually and maintain it to get the support, so we may not need to add everything here.

I'll let @taiki-e makes the final decision though because I maintain cargo-binstall so have a cognitive bias.

@jayvdb
Copy link
Contributor Author

jayvdb commented Jun 8, 2024

I'll comment here once about your rejections of this, jaq, and xh. cargo-binstall is not good enough for my industry, healthcare, and other sectors I have worked in that are highly regulated, such as automotive. binstall uses the signature on the releases page, which can be changed at the same time a maintainer changes the binary. This is effectively useless security theater, that only protects against simplistic supply chain attacks, and only if the project supplies checksum files, which many Rust projects dont provide.

Also binstall uses quickinstall as a fallback. That means for me to use cargo-binstall, I need to get both cargo-binstall and cargo-quickinstall reviewed & approved - and both are much more complicated than install-action from a "trust" perspective, and both provide less guarantees than install-action. In short, they are not going to be approved.

So, cargo-binstall is not a fallback for me, and many other projects that have strict controls in place. cargo-binstall installs SOUP which cant be justified in Github workflows.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jun 8, 2024

cargo-binstall is not good enough for my industry, healthcare, and other sectors I have worked in that are highly regulated, such as automotive. binstall uses the signature on the releases page, which can be changed at the same time a maintainer changes the binary. This is effectively useless security theater, that only protects against simplistic supply chain attacks, and only if the project supplies checksum files, which many Rust projects dont provide.

Yeah that's indeed a problem for healthcare industry, though do note that cargo-binstall supports having a dedicated public key for every release, with the public key stored in the https://crates.io by putting it in Cargo.toml thus immutable and the private key is only used in the GHA CI and erased once the release is done.

https://github.com/cargo-bins/cargo-binstall/blob/main/SIGNING.md

Unfortunately cargo-binstall is used on client so we can't cache the hash sum like taiki-e/install-action, which like you say, is the main advantage of this action.

Though just TBF if someone takes over the GitHub account of any maintainer of these popular crates and obtain ability to publish it (maybe through GHA to auto-publish release to crates.io), or someone malicious becomes the maintainer, even taiki-e/install-action can't prevent it.

Maybe cargo-binstall could use the hashsum stores in taiki-e/install-action to verify the release?

Also binstall uses quickinstall as a fallback. That means for me to use cargo-binstall, I need to get both cargo-binstall and cargo-quickinstall reviewed & approved - and both are much more complicated than install-action from a "trust" perspective, and both provide less guarantees than install-action. In short, they are not going to be approved.

That can be disabled, though only through commandline parameters --disable-strategies quickinstall, though it does make it harder to use for your use case.

So, cargo-binstall is not a fallback for me, and many other projects that have strict controls in place. cargo-binstall installs SOUP which cant be justified in Github workflows.

That's true, cargo-binstall is designed to be a drop in replacement of cargo-install, though TBF, IIRC taiki-e/install-action previously fallback to cargo-install if it is not directly supported.

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

wait-for-them does sound like a pretty useful tool in CI, to wait for tasks/servers to startup.

@NobodyXu NobodyXu merged commit a0060a7 into taiki-e:main Jun 8, 2024
29 checks passed
@NobodyXu
Copy link
Collaborator

NobodyXu commented Jun 8, 2024

BTW cargo-binstall also has plan to support more signature schemes cargo-bins/cargo-binstall#1 (comment)

@jayvdb jayvdb deleted the wait-for-them branch June 8, 2024 07:26
@jayvdb jayvdb mentioned this pull request Jun 8, 2024
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.

2 participants