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

Version 4.03, integration of stCarolas/setup-maven module #189

Closed
gst-de opened this issue Apr 22, 2024 · 5 comments · Fixed by #191
Closed

Version 4.03, integration of stCarolas/setup-maven module #189

gst-de opened this issue Apr 22, 2024 · 5 comments · Fixed by #191

Comments

@gst-de
Copy link

gst-de commented Apr 22, 2024

As part of the 4.0.3 release (which is currently just a tag) you added the release.yml file. This points to a module stCarolas/setup-maven.
setup-maven includes the node_modules/%40actions lib, which has in its tool-cache/scripts/externals path a binary of a zip decoder. With regard to the xz-lib-backdoor desaster, integrating a lib which provides a binary seems at least a bit careless to me.

@manovotn
Copy link
Collaborator

As part of the 4.0.3 release (which is currently just a tag)

It's not just a tag, it's in Central. See for instance https://central.sonatype.com/artifact/org.jboss.weld/weld-junit4/versions
But I can see it's not under https://github.com/weld/weld-testing/releases which I'll fix in a bit; thanks for the heads up.

you added the release.yml file

Yes, this is a file that's only used during the release process by GH actions.
It's output can be seen here - https://github.com/weld/weld-testing/actions/workflows/release.yml

This points to a module stCarolas/setup-maven.
setup-maven includes the node_modules/%40actions lib, which has in its tool-cache/scripts/externals path a binary of a zip decoder. With regard to the xz-lib-backdoor desaster, integrating a lib which provides a binary seems at least a bit careless to me.

This is something you should bring up with the setup-maven project itself (which I saw you did).
But note that the release artifacts of Weld project won't contain the script in any shape or form.
Plus, looking at their code, it looks like this is a decoder usable only on Windows while the CI here runs Linux.

@gst-de
Copy link
Author

gst-de commented Apr 23, 2024

Thanks for the clarification.
The concern I have with this is that the xz-lib-backdoor worked in a similar fashion. (And I am just semi-educated on this, so I might be wrong).
The backdoor wasn't in the sources. It was part of the build procedure and changed the result of the build procedure quite significantly. So having an unverified binary, which is a windows binary -or might just disguise itself as one- would be the exact equivalent of this backdoor.

But this is just guessing from my side and quite similar to looking into a crystal ball. So if this doesn't raise any concerns with you, then this is a good start.

The only useful way forward with this issue right now seems to be to close it. But I'd like to ask you to have this issue in mind when you look at your build pipeline the next time. I am pretty certain that the xz-lib-backdoor was just the tip of the iceberg and a good many relevant projects might have been hacked in a similar fashion.

So after reading this comment, feel free to close the issue.

@manovotn
Copy link
Collaborator

To be clear, I am not especially educated on this as well and I definitely understand your concern.
I did not know a binary is part of this GH actions module when I used it and I am genuinely curious to see any replies in stCarolas/setup-maven#39
Most of the release job setup was taken over from SmallRye projects which is where I came across this module.

That being said, we might be able to drop that module from our CI job (and from other Weld projects using this) and simply use whatever the default Maven version is for that given container. It was just a convenient way to setup deterministic maven version and hence make the build more robust.
Looking at the Ubuntu image used during builds, the maven versions match with what we requested anyway (3.8.8).

So after reading this comment, feel free to close the issue.

I'll keep it open for now; if I have some free cycles any time soon, I'll try to remove this dependency if we truly don't need it.

@manovotn
Copy link
Collaborator

I've sent PRs to Weld projects where this was used and removed the action as we don't seem to require it:

@gst-de
Copy link
Author

gst-de commented May 13, 2024

Thanks for doing this! I appreciate your dedication!

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 a pull request may close this issue.

2 participants