Wasmtime GitHub notifications bot (Mar 28 2024 at 19:49): Wasmtime GitHub notifications bot (Mar 28 2024 at 19:49):alexcrichton requested cfallin for a review on PR #8259.
Wasmtime GitHub notifications bot (Mar 28 2024 at 19:49):alexcrichton opened PR #8259 from alexcrichton:update-vet-policy to bytecodealliance:main:
This was discussed at today's Wasmtime meeting out of some concerns around our current policies. Namely I felt the current state of affairs is not striking the right balance between cost and benefit with our usage of
cargo vet. After discussion we've reached consensus around two changes to ourcargo vetpolicy documented here in this PR:
An exemption can be added for "popular crates" at any time with no review required. This should handle most big crates that are needed for various dependencies. The thinking behind this is that a supply-chain attack against these crates is highly likely to be detected in a short time due to their popularity. Coupled with the fact that changes to Wasmtime take a minimum of two weeks to get released means that it's an unlikely exploitation vector.
Maintainers are recommended to push directly to contributor's PRs for
cargo vetentries instead of making separate PRs. This avoids the need for contributor rebasing and additionally solves the problem where thevetentries land in a separate PR but then the contributor's PR takes much longer to land. In the interim somevetentries have been cleaned up by accident which requires re-landing the PR to add the entries.<!--
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
-->
Wasmtime GitHub notifications bot (Mar 28 2024 at 19:49):alexcrichton requested wasmtime-default-reviewers for a review on PR #8259.
Wasmtime GitHub notifications bot (Mar 28 2024 at 19:53):alexcrichton commented on PR #8259:
Some points of note:
- I kept the 10,000 crate download metric since spot checking a few dependencies seemed to show that as a resonable threshold. For example from the ONNX PR the
compact_strdependency clears this threshold but thecapstonedependency does not. That makes sense to me since I don't expect disassembling to be all that popular on crates.io- I briefly looked into what it might take to "comment on a PR if a github actions fails". I'd love to drop a comment on PRs from contributors pointing to this documentation when the
cargo vetentry fails in CI. Turns out GitHub Actions has an example of doing exactly that (well, mostly), but it's pretty nasty so I didn't pursue it yet.
Wasmtime GitHub notifications bot (Mar 28 2024 at 19:54):alexcrichton edited a comment on PR #8259:
Some points of note:
- I kept the 10,000 crate download metric since spot checking a few dependencies seemed to show that as a reasonable threshold. For example from the ONNX PR the
compact_strdependency clears this threshold but thecapstonedependency does not. That makes sense to me since I don't expect disassembling to be all that popular on crates.io- I briefly looked into what it might take to "comment on a PR if a github actions fails". I'd love to drop a comment on PRs from contributors pointing to this documentation when the
cargo vetentry fails in CI. Turns out GitHub Actions has an example of doing exactly that (well, mostly), but it's pretty nasty so I didn't pursue it yet.
Wasmtime GitHub notifications bot (Mar 28 2024 at 20:19):cfallin submitted PR review:
LGTM -- this accurately represents what we came out of the discussion with, I think. Thanks very much for writing up the details!
Wasmtime GitHub notifications bot (Mar 28 2024 at 20:19):cfallin submitted PR review:
LGTM -- this accurately represents what we came out of the discussion with, I think. Thanks very much for writing up the details!
Wasmtime GitHub notifications bot (Mar 28 2024 at 20:19):cfallin created PR review comment:
Let's add some explicit note heres too -- maybe something like "... or don't overwrite your vet entries. Also verify that if the PR branch is rebased or force-pushed, the details of your previously pushed vetting remain the same: e.g., versions were not bumped and descriptive reasons remain the same. If pushing a vetting commit to a contributor's PR and also asking for more changes, request that the contributor make the requested fixes in an additional commit rather than force-pushing a rewritten history, so your existing vetting commit remains untouched. These guidelines make it easier to verify no tampering has occurred."
Wasmtime GitHub notifications bot (Mar 28 2024 at 20:20):cfallin edited PR review comment.
Wasmtime GitHub notifications bot (Mar 29 2024 at 16:26):alexcrichton updated PR #8259.
Wasmtime GitHub notifications bot (Mar 29 2024 at 16:27):alexcrichton has enabled auto merge for PR #8259.
Wasmtime GitHub notifications bot (Mar 29 2024 at 17:15):alexcrichton merged PR #8259.
Last updated: Nov 22 2024 at 16:03 UTC