Repository navigation
Conversation
eacdfde to
fda8f75
Compare
Review — 4 findingsLow
Verified against |
the-gigi-pplx
left a comment
There was a problem hiding this comment.
Approved. 4 non-blocking findings in the review comment above.
## Summary Install and remove the pinned Cloud Hypervisor v54.0 ARM64 and x86_64 static executables through the existing bounded download, verification, publication and durable operation pipeline. Register its own source and managed store, and add a runtime filter to the CLI's available-release listing. Firecracker paths, pins and default execution policy remain unchanged. The transfer and executable SHA-256 pins were checked against the downloaded upstream release asset. The fs-verity measurement uses SHA-256, 4096-byte blocks and no salt. This establishes byte identity, not independent build provenance or guest compatibility. Installation does not advertise a runnable CH environment or snapshot support. ## Validation - Review follow-up: runtime-manager 66 passed (two existing external-network tests ignored); service CLI credential regression 1 passed; runtime-core 111 passed; execution-spec 29 passed. Credential tests failed before the fix. Formatting and all-target Linux ARM64 Clippy for these four packages passed with warnings denied. - Catalog tests: 8 passed, including complete executable and fs-verity pins for both architectures. Runtime-manager tests with `github`: 65 passed; one existing upstream-network test ignored. CLI tests: 13 passed. - Formatting and all-target Linux ARM64 Clippy for runtime-core, runtime-manager and the CLI passed with warnings denied. - The node-manager routing regression passed in an unprivileged Linux ARM64 container. - Both content pins match upstream v54.0 release-asset metadata. The x86_64 content and fs-verity pins were moved unchanged from the dependent boot-environment PR; their earlier Linux measurements remain the provenance for the verity values. This update does not add live service-driven install/uninstall evidence. Run that acceptance check before merging; compilation and unit tests do not establish deployed publication or guest execution. Release-source URLs reject userinfo, query parameters and fragments. Rejected values are omitted from parse errors and service CLI diagnostics; the public GitHub defaults remain valid. Credential-bearing mirror configurations now fail at startup and must be replaced with credential-free endpoints. ## Severity and risk Severity: does not apply; new runtime installation support. Risk is routing or verification regressions in artifact management. Separate runtime roots and unchanged FC URL/layout coverage protect existing installs. Both architectures are installable; no automatic execution selection is added. Upgrade node-manager first, then runtime-manager, before submitting CH installs. The status audit reports the new root as absent until node-manager provisions it. An older node-manager refuses the unknown pin; resubmit a failed install after upgrading. Rolling back the artifact services leaves CH bytes unused and does not require deleting them. ## Stack Cloud Hypervisor installation: #915 → #922 → #923 → #924, based on main after #921. This group ends at verified runtime installation; it does not enable sandbox execution or change Firecracker/E0 defaults. Upgrade node-manager and runtime-manager before requesting Cloud Hypervisor installs. The dependent execution stack begins at #925. AI assistance: Codex drafted code, tests and this description.
|
Addressed the documentation findings in
I am deferring a configurable CH store root. An rt-mgr-only flag would not move node-manager's privileged publication store; this needs coordinated configuration and overlap validation on both sides. The existing compiled roots agree, and configured overlaps continue to fail closed. Also fixed the reported release-source credential issue: HTTPS URLs reject userinfo, query parameters and fragments. Parse errors and CLI diagnostics omit the rejected input, so refusing a credential-bearing URL does not itself leak it. Credential-free public defaults remain valid; an @ in a URL path remains allowed. Validation at the installation head: runtime-manager 66 passed (two existing external-network tests ignored), service CLI regression 1 passed, runtime-core 111 passed, execution-spec 29 passed; formatting and Linux ARM64 all-target Clippy passed. The credential regressions failed before the fix. After resolving dependent-stack conflicts, runtime-manager 73 and service CLI 2 passed. This is still not live publication evidence: the real service-driven CH install/uninstall check remains a pre-merge gate, and pin verification is not independent build attestation. |
…aims (perplexityai#915) ## Summary Recognize Cloud Hypervisor's single executable in runtime-manager diagnostic scans without requiring or inventing a snapshot format. Keep Firecracker's existing layout and state-format checks. Cloud Hypervisor's baseline no longer advertises unimplemented snapshot, warm-pool, pause, networking or volume capabilities. This is the discovery foundation for managed Cloud Hypervisor installation. It does not add installable releases or enable the execution adapter; diagnostics remain separate from execution readiness. No SQL or protobuf changes. ## Validation - Review follow-up: all three runtime capability tests passed, including the complete CH baseline. Diagnostic docs distinguish missing layouts from runtimes without memory-state support. - Execution-spec unit tests: 19 passed. - Runtime-core diagnostic tests: 27 passed, including a mixed Firecracker/Cloud Hypervisor listing with an arm64 ELF and no state-format metadata. - Clippy for execution-spec and runtime-core, all targets: passed on macOS; runtime-core also passed for aarch64 Linux/musl. - Runtime-manager and its tests cross-compile for aarch64 Linux/musl. - `cargo arch`: no required findings; existing advisory findings remain. ## Severity and risk Severity does not apply: this prepares diagnostic discovery and the capability model for a later execution adapter. No production caller currently consumes the narrowed capability baseline. Low implementation risk: the change adds a diagnostic layout and narrows the model to supported operations. Firecracker selection and artifact identities are unchanged. This does not qualify Cloud Hypervisor for sandbox execution. ## Stack Cloud Hypervisor installation: perplexityai#915 → perplexityai#922 → perplexityai#923 → perplexityai#924, based on main after perplexityai#921. This group ends at verified runtime installation; it does not enable sandbox execution or change Firecracker/E0 defaults. Upgrade node-manager and runtime-manager before requesting Cloud Hypervisor installs. The dependent execution stack begins at perplexityai#925. AI assistance: Codex drafted the code, tests and description.
) ## Summary Runtime releases can now arrive as a single executable or a tar.gz archive. Both use the existing bounded download, pinned payload and binary digests, ELF architecture checks, private staging and privileged publication. This supports static Cloud Hypervisor release packaging without changing Firecracker's installed layout. Sources describe packaging; only catalog layout paths can become installation destinations. Missing, duplicate or foreign destination mappings fail before downloading. Durable job payloads and RPCs are unchanged. This PR does not add an installable Cloud Hypervisor release or enable its execution. ## Validation - Review regressions failed before the fix for repeated archive members and the redundant standalone payload copy; the final runtime-manager suite passed 63 tests (one upstream-network test ignored). Standalone staging now moves the synced executable, and production/test sources share layout-derived Firecracker packaging. - Runtime-manager library suite with `github`: 63 passed; one upstream network test remains ignored. - Linux arm64 all-target Clippy with `github` and warnings denied passed. - Formatting and `cargo arch` passed; archcheck reports existing advisory size warnings. New coverage exercises standalone downloads with transfer and binary mismatches, wrong architecture, size bounds, cancellation and invalid layouts. Existing Firecracker install, retry, delete and lost-response tests pass. No live runtime installation was performed. ## Severity and risk Severity does not apply: this adds a delivery format. Risk is in shared release staging; compiled pins and node-manager publication remain independent checks. No configuration or execution policy changes. Custom Rust release-source implementations must supply the new packaging description. ## Stack Cloud Hypervisor installation: perplexityai#915 → perplexityai#922 → perplexityai#923 → perplexityai#924, based on main after perplexityai#921. This group ends at verified runtime installation; it does not enable sandbox execution or change Firecracker/E0 defaults. Upgrade node-manager and runtime-manager before requesting Cloud Hypervisor installs. The dependent execution stack begins at perplexityai#925. AI assistance: Codex drafted the implementation, tests and this description.
## Summary Runtime-manager now routes catalog listings, installs, retries and removals through a source/store pair for the requested runtime. Node-manager selects its privileged publication store from that same runtime identity instead of sending every release to Firecracker's directory. Diagnostic inventory can scan several managed roots without merging releases that share a version. Configuration rejects duplicate runtime sources and overlapping stores, including aliases through existing ancestors. Missing routing is an explicit refusal; it never falls back to another runtime. Store overlap checks are scoped to the requested store so an unrelated broken layout does not disable Firecracker provisioning. Existing Firecracker paths, download configuration, job payloads and RPCs are unchanged. Production configuration still registers only Firecracker; Cloud Hypervisor's pinned source is a separate change. Kernel storage remains shared. ## Validation - Review follow-up documents the execution gate: additional runtimes must extend inventory filtering and execution-root overlap checks before execution is enabled. The follow-up changes comments only; the previously executed Linux routing regression remains the behavior evidence. - Runtime-manager library suite with `github`: 64 passed; one upstream network test remains ignored. - Runtime-core diagnostic suite: 28 passed, including coexisting same-version runtimes and an independently damaged CH entry. - Linux arm64 all-target Clippy for node-manager, runtime-manager and its service passed with warnings denied. The node-manager routing regression also passed in an unprivileged Linux ARM64 container using the installation-stack head; the routing implementation is unchanged from this PR. - Formatting and `cargo arch` passed; existing advisory size warnings remain. No live installation or KVM test was performed. No SQL or protobuf changes. ## Severity and risk Severity does not apply: this adds routing for additional runtime families. The main risk is source/store disagreement. Immutable artifact identities and privileged byte verification are preserved, and unconfigured runtimes are refused. Custom Rust configurations now provide a collection of sources; store paths must be absolute without parent traversal. ## Stack Cloud Hypervisor installation: perplexityai#915 → perplexityai#922 → perplexityai#923 → perplexityai#924, based on main after perplexityai#921. This group ends at verified runtime installation; it does not enable sandbox execution or change Firecracker/E0 defaults. Upgrade node-manager and runtime-manager before requesting Cloud Hypervisor installs. The dependent execution stack begins at perplexityai#925. AI assistance: Codex drafted code, tests and this description.
Summary
Install and remove the pinned Cloud Hypervisor v54.0 ARM64 and x86_64 static executables through the existing bounded download, verification, publication and durable operation pipeline. Register its own source and managed store, and add a runtime filter to the CLI's available-release listing. Firecracker paths, pins and default execution policy remain unchanged.
The transfer and executable SHA-256 pins were checked against the downloaded upstream release asset. The fs-verity measurement uses SHA-256, 4096-byte blocks and no salt. This establishes byte identity, not independent build provenance or guest compatibility. Installation does not advertise a runnable CH environment or snapshot support.
Validation
github: 65 passed; one existing upstream-network test ignored. CLI tests: 13 passed.This update does not add live service-driven install/uninstall evidence. Run that acceptance check before merging; compilation and unit tests do not establish deployed publication or guest execution.
Release-source URLs reject userinfo, query parameters and fragments. Rejected values are omitted from parse errors and service CLI diagnostics; the public GitHub defaults remain valid. Credential-bearing mirror configurations now fail at startup and must be replaced with credential-free endpoints.
Severity and risk
Severity: does not apply; new runtime installation support. Risk is routing or verification regressions in artifact management. Separate runtime roots and unchanged FC URL/layout coverage protect existing installs. Both architectures are installable; no automatic execution selection is added.
Upgrade node-manager first, then runtime-manager, before submitting CH installs. The status audit reports the new root as absent until node-manager provisions it. An older node-manager refuses the unknown pin; resubmit a failed install after upgrading. Rolling back the artifact services leaves CH bytes unused and does not require deleting them.
Stack
Cloud Hypervisor installation: #915 → #922 → #923 → #924, based on main after #921. This group ends at verified runtime installation; it does not enable sandbox execution or change Firecracker/E0 defaults. Upgrade node-manager and runtime-manager before requesting Cloud Hypervisor installs. The dependent execution stack begins at #925.
AI assistance: Codex drafted code, tests and this description.
Stack created with Sapling. Best reviewed with ReviewStack.
Base: main; #921 is merged.