Skip to content
This repository has been archived by the owner on Nov 7, 2019. It is now read-only.

Verify that the tests in CraneStation/wasi-common/misc_testsuite match this repository as a part of CI #18

Closed
marmistrz opened this issue Aug 13, 2019 · 2 comments · Fixed by CraneStation/wasi-common#70

Comments

@marmistrz
Copy link
Member

CraneStation/wasi-common only contains the prebuilt wasm binaries. This means it's not guaranteed that the binaries there match the sources in this repo; either due to a malicious actor or a plain human mistake.

I assume it's a design decision to include only the binaries in wasi-common, so that the wasm32-wasi Rust target is not required to run the testsuite.

I think that the verification takes place in the wasi-common CI. Otherwise we risk that the developers may accidentally run a malicious wasm binary.
I also think all the binaries are generated using cargo build --target=wasm32-wasi --release.

The main problem is: how to verify that the binaries actually match the sources? If the wasm32-wasi target is deterministic, we could just do a binary diff, but this probably holds insofar a single Rust release is concerned. We definitely don't want to add a new commit every 6 weeks, updating the binaries to match the newly released Rust version.

@kubkon
Copy link
Member

kubkon commented Aug 14, 2019

I agree that this is a problem and as wasi-common grows in size and contributors, it will only get worse. In fact, I've already initiated some discussion with @sunfishcode about this in #13, and as this PR has landed, it is now possible to build the tests using stable toolchain which opens up more doors for us in terms of syncing the tests with wasi-common.

Here's a thought. Do you think you could figure out a way to have the sources for the tests (i.e., this repo) pulled in to wasi-common and built on-the-fly whenever we trigger cargo test (and something actually changes that is)? Would you be keen to have a look at it @marmistrz ?

@marmistrz
Copy link
Member Author

marmistrz commented Aug 14, 2019

My current idea is something like this:

  • Add wasi-misc-tests to this repository as a submodule. The CI may optionally pull the submodule to make sure it's up to date, but I think that manual submodule updates would be better to get deterministic builds
  • Add wasi-misc-tests as a dev-depenency for the wasi-common crate.
  • Configure the default build target for wasi-misc-tests to wasm32-wasi, as described in this stackoverflow post. This means that the build will fail if the user doesn't have the target installed.
  • Modify build.rs to look for *.wasm files in the directory cargo places them in instead of misc_testsuite

Alternatively, we can just move the tests to this repository, so that no separate PR is needed to update the submodule here. The rest of the idea remains the same in that case.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants