pchickey opened PR #6784 from bytecodealliance:pch/consistient_rights_reporting
to bytecodealliance:main
:
We have three preview1 implementations in tree:
wasi-common
wasi-preview1-component-adapter
wasmtime-wasi::preview2::preview1
This PR makes all of them report the exact same set of rights for directories.
wasi-libc based userlands (e.g. wasi-sdk, tinygo, some parts of rust std, and others!) will sometimes check whether they have a preview1 Right to perform an import function call, and return early if they do not.
Last attempt at adjusting how we report Rights https://github.com/bytecodealliance/wasmtime/pull/6463
hard-coded the set of rights in wasi-common. That PR also added a wasi-testpath_open_preopen
that was intended to check whether wasi-libc's expectations were met. This test focused on the bug report we had wasi-libc around opening directories, and short of testing for the full expected set or rights.We got a bug report on zulip that tinygo, which uses wasi-libc, failed in the go call
os.Create
: https://bytecodealliance.zulipchat.com/#narrow/stream/217126-wasmtime/topic/Problem.20with.20WasiCtxBuilder.2Epush_preopened_dir.20for.20component . It appeared this was getting rejected in wasi-libc, using the rights reported from the wasi-preview1-component-adapter.To avoid chasing these individual wasi-libc behaviors much further, I decided to add the expectation of the set of rights to
path_open_preopen
, and fixwasi-preview1-component-adapter
andwasmtime-wasi::preview2::preview1
to always report those rights. This does mean duplicating that list of rights in a few more places in the source code, but theres no good way to do code sharing among these crates because the constants are coming from different crates/binding generators across these projects.I'd love for this to be the last time we have to kick the Rights can down the road. However, I'm now paranoid that, even with directories taken care of, we will run into the a similar problem with wasi-libc's expectation of file rights. I believe the behavior for file rights is consistent across all three implementations, so since we haven't gotten any file related bug reports since #6463 I am hopeful we have it right everywhere.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
pchickey requested alexcrichton for a review on PR #6784.
pchickey requested wasmtime-core-reviewers for a review on PR #6784.
pchickey requested sunfishcode for a review on PR #6784.
pchickey requested jameysharp for a review on PR #6784.
pchickey requested wasmtime-core-reviewers for a review on PR #6784.
alexcrichton submitted PR review.
alexcrichton merged PR #6784.
Last updated: Dec 23 2024 at 12:05 UTC