Fix the Linux runtime contract end to end - #38
Merged
Conversation
📝 WalkthroughWalkthroughRuntime v2 introduces a digest-qualified manifest shared by installation and execution, namespaced image and cache paths, contract-based cache validation and repair, nested-LXC subordinate-ID allocation, stricter systemd helper resolution, and host contract lifecycle updates. ChangesLXC runtime v2
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Installer
participant RuntimeManifest
participant LxcRuntime
participant ActiveCache
Installer->>RuntimeManifest: install validated runtime-v2 manifest
LxcRuntime->>RuntimeManifest: load runtime and image contract
LxcRuntime->>ActiveCache: validate digest and contract
ActiveCache-->>LxcRuntime: reuse or require repair
LxcRuntime->>ActiveCache: prepare repaired cache
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/1helm-lxc-runtime`:
- Around line 162-166: Extend the cache readiness validation around the rootfs
checksum and cache_contract check to also verify the extracted metadata
representation before accepting the cache. Persist or reuse a metadata digest,
and mark the cache invalid when metadata is corrupted even if the marker and
rootfs remain intact; add a regression test covering metadata corruption with
both preserved.
In `@site/public/install-lxc-runtime.sh`:
- Around line 74-85: Extend the manifest validation condition in the install
script to validate ONEHELM_LXC_IMAGE_BUILD before assigning IMAGE_BUILD and
completing installation. Use the same build-format validation expected by
scripts/1helm-lxc-runtime, while preserving the existing checks and
invalid-manifest failure behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eae76d42-11fa-4b06-bf02-f1634628161c
📒 Files selected for processing (12)
CHANGELOG.mddeploy/1helm-lxc-runtime-v2.confscripts/1helm-lxc-runtimesite/public/apply-linux-release.shsite/public/install-lxc-runtime.shsite/public/install.shsite/public/migrate-linux-host-contract.shsite/public/uninstall-host.shsite/public/update-host.shsrc/server/channel-computers.tstest/fake-lxc-runtime.mjstest/site.mjs
Comment on lines
+162
to
+166
| if [[ -f "$cache_dir/rootfs.tar.xz" ]] \ | ||
| && printf '%s %s\n' "$rootfs_sha" "$cache_dir/rootfs.tar.xz" | sha256sum -c - >/dev/null 2>&1; then | ||
| cache_rootfs_ok=1 | ||
| fi | ||
| if [[ "$cache_contract" != "$expected_contract" || "$cache_rootfs_ok" -ne 1 ]]; then |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Repair metadata corruption, not only rootfs corruption.
The readiness check validates only cached rootfs.tar.xz; it never verifies the metadata extracted at Line 171. A damaged metadata payload with an intact rootfs and 1helm-runtime-contract marker skips repair, leaving lxc-create to consume a corrupt cache. Validate the cached metadata representation too, or persist and verify its archive digest before accepting the cache. Add a regression test that corrupts metadata while preserving the marker and rootfs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/1helm-lxc-runtime` around lines 162 - 166, Extend the cache readiness
validation around the rootfs checksum and cache_contract check to also verify
the extracted metadata representation before accepting the cache. Persist or
reuse a metadata digest, and mark the cache invalid when metadata is corrupted
even if the marker and rootfs remain intact; add a regression test covering
metadata corruption with both preserved.
Comment on lines
+74
to
+85
| [[ "${ONEHELM_LXC_RUNTIME_VERSION:-}" == "1helm-lxc-runtime-v2" \ | ||
| && "${ONEHELM_LXC_STATE_PATH:-}" == "/var/lib/1helm-lxc/machines" \ | ||
| && "${ONEHELM_LXC_IMAGE_SET:-}" =~ ^ubuntu-noble-[0-9]{8}-[0-9]{4}$ \ | ||
| && "${ONEHELM_LXC_IMAGE_RELEASE:-}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ \ | ||
| && "${ONEHELM_LXC_AMD64_ROOTFS_SHA256:-}" =~ ^[a-f0-9]{64}$ \ | ||
| && "${ONEHELM_LXC_AMD64_META_SHA256:-}" =~ ^[a-f0-9]{64}$ \ | ||
| && "${ONEHELM_LXC_ARM64_ROOTFS_SHA256:-}" =~ ^[a-f0-9]{64}$ \ | ||
| && "${ONEHELM_LXC_ARM64_META_SHA256:-}" =~ ^[a-f0-9]{64}$ ]] \ | ||
| || { echo "The verified release has an invalid LXC runtime manifest." >&2; exit 1; } | ||
| LXC_PATH="$ONEHELM_LXC_STATE_PATH" | ||
| IMAGE_BUILD="$ONEHELM_LXC_IMAGE_BUILD" | ||
| IMAGE_RELEASE="$ONEHELM_LXC_IMAGE_RELEASE" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate ONEHELM_LXC_IMAGE_BUILD before declaring installation successful.
Line 84 consumes this value, but Lines 74-81 do not validate it. A malformed manifest can pass installation and the version probe at Line 278, then fail every helper ready call because scripts/1helm-lxc-runtime rejects invalid build formats.
Proposed fix
&& "${ONEHELM_LXC_STATE_PATH:-}" == "/var/lib/1helm-lxc/machines" \
&& "${ONEHELM_LXC_IMAGE_SET:-}" =~ ^ubuntu-noble-[0-9]{8}-[0-9]{4}$ \
+ && "${ONEHELM_LXC_IMAGE_BUILD:-}" =~ ^[0-9]{8}_[0-9]{2}:[0-9]{2}$ \
&& "${ONEHELM_LXC_IMAGE_RELEASE:-}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| [[ "${ONEHELM_LXC_RUNTIME_VERSION:-}" == "1helm-lxc-runtime-v2" \ | |
| && "${ONEHELM_LXC_STATE_PATH:-}" == "/var/lib/1helm-lxc/machines" \ | |
| && "${ONEHELM_LXC_IMAGE_SET:-}" =~ ^ubuntu-noble-[0-9]{8}-[0-9]{4}$ \ | |
| && "${ONEHELM_LXC_IMAGE_RELEASE:-}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ \ | |
| && "${ONEHELM_LXC_AMD64_ROOTFS_SHA256:-}" =~ ^[a-f0-9]{64}$ \ | |
| && "${ONEHELM_LXC_AMD64_META_SHA256:-}" =~ ^[a-f0-9]{64}$ \ | |
| && "${ONEHELM_LXC_ARM64_ROOTFS_SHA256:-}" =~ ^[a-f0-9]{64}$ \ | |
| && "${ONEHELM_LXC_ARM64_META_SHA256:-}" =~ ^[a-f0-9]{64}$ ]] \ | |
| || { echo "The verified release has an invalid LXC runtime manifest." >&2; exit 1; } | |
| LXC_PATH="$ONEHELM_LXC_STATE_PATH" | |
| IMAGE_BUILD="$ONEHELM_LXC_IMAGE_BUILD" | |
| IMAGE_RELEASE="$ONEHELM_LXC_IMAGE_RELEASE" | |
| [[ "${ONEHELM_LXC_RUNTIME_VERSION:-}" == "1helm-lxc-runtime-v2" \ | |
| && "${ONEHELM_LXC_STATE_PATH:-}" == "/var/lib/1helm-lxc/machines" \ | |
| && "${ONEHELM_LXC_IMAGE_SET:-}" =~ ^ubuntu-noble-[0-9]{8}-[0-9]{4}$ \ | |
| && "${ONEHELM_LXC_IMAGE_BUILD:-}" =~ ^[0-9]{8}_[0-9]{2}:[0-9]{2}$ \ | |
| && "${ONEHELM_LXC_IMAGE_RELEASE:-}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ \ | |
| && "${ONEHELM_LXC_AMD64_ROOTFS_SHA256:-}" =~ ^[a-f0-9]{64}$ \ | |
| && "${ONEHELM_LXC_AMD64_META_SHA256:-}" =~ ^[a-f0-9]{64}$ \ | |
| && "${ONEHELM_LXC_ARM64_ROOTFS_SHA256:-}" =~ ^[a-f0-9]{64}$ \ | |
| && "${ONEHELM_LXC_ARM64_META_SHA256:-}" =~ ^[a-f0-9]{64}$ ]] \ | |
| || { echo "The verified release has an invalid LXC runtime manifest." >&2; exit 1; } | |
| LXC_PATH="$ONEHELM_LXC_STATE_PATH" | |
| IMAGE_BUILD="$ONEHELM_LXC_IMAGE_BUILD" | |
| IMAGE_RELEASE="$ONEHELM_LXC_IMAGE_RELEASE" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@site/public/install-lxc-runtime.sh` around lines 74 - 85, Extend the manifest
validation condition in the install script to validate ONEHELM_LXC_IMAGE_BUILD
before assigning IMAGE_BUILD and completing installation. Use the same
build-format validation expected by scripts/1helm-lxc-runtime, while preserving
the existing checks and invalid-manifest failure behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome
Fixes Linux channel provisioning by installing one immutable runtime manifest shared by the installer, root lifecycle helper, application, updater, image assets, and cache. Corrects nested unprivileged ID-map selection and makes same-version readiness repair a corrupted cache.
Real Linux acceptance
1:65535.agentin/workspace, reached the internet, and retained its file across stop/start./api/channelswithcomputer_ready=true.run_commandand Terminal both executed inside the same channel machine and mirrored files to the Files API.Source checks
npm run typechecknpm run buildnpm run test:sitenode --test test/channel-computers.mjsnpm run test:onboarding-browser(20/20)npm test: native-world completed 125/125 earlier in the run; later reruns exposed unrelated mock-provider fetch timing flakes at different assertions; isolated cowork rerun passed.git diff --checkSummary by CodeRabbit
New Features
Bug Fixes