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

Move to using common container for building WASM #3714

Open
mhdawson opened this issue Oct 10, 2024 · 7 comments
Open

Move to using common container for building WASM #3714

mhdawson opened this issue Oct 10, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@mhdawson
Copy link
Member

mhdawson commented Oct 10, 2024

This would solve...

The security wg has been looking at auditing build dependencies for Node.js. nodejs/security-wg#1236

One area we thought that we could improve is how we build WASM. Today it's done differently, in some cases with different tools, different versions of tools and the versions used may or may not be known/captured in a way that would allow us to be sure we could reproduce a build.

I've looked at creating a common container that would be shared across the dependencies that build WASM and which captures the versions of tools etc. In addition in using a shared container that we store in the project we know we can build with the same tools because they are in the container stored within the registry.

The approach is based on how undici was building, with the main change being to use a pre-existing container. I've already talked to maintainers of amaro and cjs-module-lexer which are the other deps that build WASM blobs and they seem willing to adopt the approach.

I still have a bit of work in terms of how the container would be published (as I believe GitHub has a migration from ghcr of some sort) and the repo mhdawson/wasm-builder would need to move under the nodejs org.

The PR to adopt in undici would look like: main...mhdawson:undici:wasm-build-experiment

You can see what it looks like for the other deps from the links in - nodejs/security-wg#1236 (comment)

The reason for this issue is to ask if undici would be willing to adopt the approach as well.

The implementation should look like...

I have also considered...

Additional context

@mhdawson mhdawson added the enhancement New feature or request label Oct 10, 2024
@Uzlopak
Copy link
Contributor

Uzlopak commented Oct 10, 2024

Why do we need to use your wasm-builder image?
Would the repo for this image be added to nodejs org?

@mcollina
Copy link
Member

mcollina commented Oct 10, 2024

I'm +1 but the image needs to be hosted in the org.

@mhdawson
Copy link
Member Author

needs to be hosted in the org.

absolutely. I would move mhdawson/wasm-builder to nodejs/wasm-builder

@mhdawson
Copy link
Member Author

Why do we need to use your wasm-builder image?

There are a number of benefits on using a common approach/container across the dependencies.

  • Fewer tools and different versions of tools used to build Node.js ( ie smaller supply chain)
  • It will be easier to ensure that builds capture all information related to tools and versions used and that we do things in the way we believe it good for supply chain security and repeatability.
    • For example I had PR'd in a change to generate a file with the tools/versions into undici but later changes broke that
  • By using a container that we store instead of building one on the fly we will be sure we have the exact environment available if we need to rebuild at a later time. For example, in the current undici build process if the external link to install a particular version of BINARYEN no longer exists we would not be able to rebuild in the same way.

So while we could use a different approach and tools in each dependency and meet all of the same goals, it should be easier by re-using one common approach and build container.

@mhdawson
Copy link
Member Author

I'll also that in line with Matteo's comment it would be the "The Node.js project's" container not mine.

@mcollina
Copy link
Member

Send the PR!

@mhdawson
Copy link
Member Author

@mcollina thanks, will start the process of moving over the repo for the container generation and then I'll do that.

mhdawson added a commit to mhdawson/undici that referenced this issue Oct 31, 2024
mhdawson added a commit to mhdawson/undici that referenced this issue Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants