-
Notifications
You must be signed in to change notification settings - Fork 43
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
Support a user-prompt-less version of cargo vet certify
#293
Comments
I'm a bit concerned about bypassing the EULA in these situations, and would generally rather not have each bot implementer winging the UX. Perhaps we can also add a flag to print the same text that you get in the interactive screen, and then the bot could embed that as a comment on the bug when attaching the patch for review? |
Would that also update the audits.toml file automatically? |
I'm spitballing a bit here, but what if we had a command like:
This command would automatically generate audits for everything such that a {
"changes": [
// if an import was updated, this is generated, and will list any criteria which were
// changed upstream and need to be approved (may be none)
{ "type": "UpdateImport", "criteria_changes": [ ... ] },
// If an exemption was removed as it is no longer necessary (e.g. due to an import),
// it will be listed here.
{ "type": "ExemptionRemoved", "crate": "...", "version": "...", "criteria": [...] },
// If a crate was automatically marked as audit-as-crates-io = false, it could also be listed
{ "type": "SetAuditAsCratesIo", "crate": "...", "audit-as-crates-io": false },
// If an audit was added for a crate, it will be listed here.
{
"type": "FullAudit",
"crate": "...",
"version": "...",
"criteria": [...],
"dependency_criteria": ..., // we might infer this some day?
"eulas": { "safe-to-deploy": ... },
"inspect_url": "https://..."
},
{
"type": "DeltaAudit",
"crate": "...",
"from_version": "...",
"to_version": "...",
"criteria": [...],
"dependency_criteria": ...,
"eulas": { "safe-to-deploy": ... },
"inspect_url": "https://..."
}
]
} One potential issue I see here is the lack of an ability to add notes to the given audits, which presumably would need to happen in response to the user e.g. commenting on the phabricator revision. Perhaps when This is a bit sketchy, but perhaps we could add a new file to the store, like We'd presumably also report errors like violations which can't be solved automatically just like with |
So the fundamental issue here is that there are some interactive steps (reading the criteria, doing the audit, accepting the EULA, adding notes) that we'd like users to do. We have a command-line experience for this, but we'd also like to permit people to do it from the browser with a bot-based workflow. Engineering each of these interactive steps as an explicit affordance in the bot seems sub-optimal on a number of fronts. As such, I wonder if we could do something like this: (1) Automation invokes The engineering of (3) is of course non-trivial, but perhaps more tractable than it first appears with the help of off-the-shelf wasm tooling. |
So I think the above might actually be quite doable. The basic idea would be to abstract cargo-vet's interactive logic into a tiny standalone library that operates on serialized inputs and outputs and uses hooks to interact with the user. The existing implementation would attach the hooks to the CLI as they are now, and then we'd build a simple JS interface that talks to a WASM-ified version of the library (using something like this) and hooks it up to buttons and textareas. |
This sounds roughly like my concept (#293 (comment)), but with a specific blessed web service which provides the UX flow rather than spitting the results out as JSON, and asking the bot to provide the UX. If we decide to serialize the state directly in the URI, it probably limits our future options in the UI somewhat, as we probably can't serialize the entire audit universe to the UI. This would mean we couldn't e.g. refine audit suggestions as the user adds audits with custom Perhaps we could ask bots to host a CORS-accessible document with the full serialized state of the audit universe (as generated by a If we're clever about it we could potentially even offer this as a thing you can access locally, where we start a HTTP server as-if we were a bot from within All of this is probably out-of-scope for a proof of concept though :-) |
Strictly speaking the web service wouldn't be blessed — you'd just want to interact with a wasmified version of the cargo-vet code whose version matched the one the bot was running in automation.
I would be comfortable leaving such refinements as out-of-scope for now — particularly because the bot workflow will mostly be about updating existing packages, where it's less likely that you'll be bringing in a new subtree for which you want a different policy. If people want to do fancy stuff they can drop down to the CLI. |
If we're not doing refinement work, I don't know how much of the core The main advantage I could see would be to have a shared serde serialization definition, I'm not sure what else would benefit from being shared. Things like criteria descriptions, crate versions, and criteria selections probably need to be embedded in the query parameter anyway. |
I think the primary advantage of sharing the code is that we don't have to worry about keeping the implementations synchronized (so that they both behave consistently and interoperate correctly). If the generic driver operates on the level of "display this text", "get text input", etc, then the JS code doesn't need to know anything about cargo-vet-specific like criteria or crate versions. If we can ensure that the wasm code is always same-revision with the code in automation, we don't need to worry about interoperability between versions, allowing the serialization to be totally opaque. And using the same code-paths for regular cargo-vet (including the serialization) minimizes the chance that we'll accidentally break the less-frequently-used web flow. |
I threw together an (extremely rough) idea of what the UX could potentially look like for an app like this (https://band-organic-lion.glitch.me/, don't look at the JS it's awful). Something like this wouldn't require us to load a wasm module with Roughly for something like this we'd need to send down the following bits of information somehow (e.g. through a JSON blob):
The result could then be submitted back by wrapping everything in a I worry a bit about the original idea of passing the information down in query parameters specifically because of the need to serialize the descriptions for each criteria, which could be fairly long (e.g. This all doesn't provide any way to e.g. control |
This seems like a fine approach.
Looks like it brotli-compresses down to 377 bytes, which seems promising. Generally-speaking I'd prefer to wrap up the bidirectional payloads in an opaque bundle anyway.
Agreed. |
In order for Updatebot to be able to update rust crates (for crate owners who want updatebot to do so, like wgpu) we can detect when certification is needed and then run
cargo vet certify package v1 v2
- but this command expects user input. We would like to run it in automation without user input. We would need, at a minimum, a--username
flag we could use to populate as the user's email and username.Mozilla-process-wise: After the bot performs the certification-thats-not-really-a-certification, the audits.toml file will become part of the patch submitted to phabricator and the user can approve their patch on Phabricator. supply-chain-reviewers will also be flagged for review, and I think by policy we could require the user to add a comment noting they have actually reviewed the patch and attest to the certification that Updatebot wrote for them. (And Updatebot can add a comment to phabricator to tell them to do so.)
Related: #151
The text was updated successfully, but these errors were encountered: