Split app and channel image delivery - #71
Conversation
Reduce normal update downloads by separating immutable OCI image bytes, retain complete offline bundles, and add measured cache-safe artifact budgets. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Joseph Yaksich <gitcommit90@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR splits Linux desktop delivery into an application archive and a sealed, digest-addressed OCI channel image. It adds an artifact contract, split online/offline packaging, updated candidate manifest and promotion validation, platform acceptance checks, installer/update scripts, size reporting with budgets, and documentation updates across the release lifecycle. ChangesSplit Artifact Delivery & Sealed Channel Image
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CI as candidate.yml
participant Packaging as package-linux-host.mjs
participant Manifest as candidate-manifest.mjs
participant Promotion as promotion-lib.mjs
participant GitHub
CI->>Packaging: build OCI image, online/offline archives
Packaging->>Manifest: pass split manifest and archives
Manifest->>Manifest: validate channel-image consistency
Manifest-->>CI: candidate manifest with sealed OCI digest
CI->>Promotion: assemble promotion with channel image
Promotion->>GitHub: reuse or create immutable image release
GitHub-->>Promotion: release identity confirmed
Promotion-->>CI: stable manifest with linux_offline_tgz and channel_image
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: 10
🧹 Nitpick comments (11)
scripts/build-oci-channel-image.sh (2)
113-132: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueThe archive is hashed up to three times per run.
validate_cachehashesCACHE_TARat line 53, line 125 hashes it again for theOUT_SHAcomparison, and line 131 hashes it a third time. The archive is about 204 MB perconfig/artifact-budgets.json. Compute the digest once after the cache entry is selected and reuse the value.🤖 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/build-oci-channel-image.sh` around lines 113 - 132, Compute the SHA-256 digest of CACHE_TAR once immediately after the cache entry is selected, then reuse that value throughout the remaining flow. Update validate_cache and the OUT_SHA comparison and final OUT_SHA write to use the shared digest, removing all repeated sha256sum calls for the archive.
19-20: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCheck
python3in the prerequisite block.The script checks
podmanat line 20, butvalidate_cacheat line 44, the metadata writer at line 87, and the pointer writer at line 134 all requirepython3. Ifpython3is absent,validate_cachereturns non-zero and the run proceeds into a full image build, then fails at line 87 after the build cost. Fail closed early instead.🛡️ Proposed fix
command -v podman >/dev/null || { echo "podman is required to build the sealed channel image" >&2; exit 1; } +command -v python3 >/dev/null || { echo "python3 is required to validate and record channel image metadata" >&2; exit 1; }🤖 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/build-oci-channel-image.sh` around lines 19 - 20, Add a prerequisite check for python3 alongside the existing podman check in the script’s initial validation block, using a clear error message and exiting nonzero when unavailable. Ensure this occurs before validate_cache or any image build, while preserving the existing CONTAINERFILE and podman checks.scripts/package-linux-host.mjs (1)
269-275: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueThe offline stage copies the complete release root a second time.
cpSyncat line 270 duplicates the staged tree, including the prunednode_modules(budget 320 MB) and the client assets, only to add three files undercontainer/. This doubles peak temporary disk use and adds a full copy to every build. Consider building the offline archive from twotarsources, or link the shared files instead of copying bytes.🤖 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/package-linux-host.mjs` around lines 269 - 275, Update the offline packaging flow around offlineRoot and deterministicTar to avoid recursively copying the complete releaseRoot. Build the offline archive by combining the existing staged release contents with the three container artifacts through tar sources or another byte-sharing approach, while preserving the current archive layout and deterministic output.scripts/artifact-contract.mjs (1)
108-126: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate
production_dependenciesbefore it is passed through.Line 125 returns
{ ...value, ... }, so unvalidated fields survive.scripts/candidate-manifest.mjsline 110 copiessplit.production_dependenciesstraight into the candidate manifest. The split manifest producer atscripts/package-linux-host.mjslines 283-284 writeskey,reused,node_abi,architecture,builder_image_digest, andruntime_package_sha256. No validator checks those fields, so a malformed or absent block reaches the manifest silently.Add a shape check for the fields the contract records.
🤖 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/artifact-contract.mjs` around lines 108 - 126, The validateSplitArtifactManifest function must validate value.production_dependencies before returning the normalized manifest. Require the block to exist with the recorded key, reused, node_abi, architecture, builder_image_digest, and runtime_package_sha256 fields in the expected shape, and throw on malformed or missing data; preserve the validated block in the returned object.ops/dress-rehearsal/candidate-boundary.py (1)
89-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe offline bundle name is duplicated across languages.
Line 90 hardcodes
1Helm-{version}-linux-node-offline.tgz.offlineBundleNameinscripts/artifact-contract.mjslines 28-30 owns the same format. A future rename must change both. Add a test that compares the two formats, or derive the Python pattern from the shared contract.🤖 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 `@ops/dress-rehearsal/candidate-boundary.py` around lines 89 - 97, The offline bundle filename format is duplicated between the Python validation and the artifact contract. Update the candidate-boundary validation around the offline bundle check to consume or verify the format defined by scripts/artifact-contract.mjs, and add a cross-language test if direct reuse is unavailable, so future renames require only one coordinated contract change.config/linux-runtime-package.json (1)
27-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPruning by bare directory name can remove required runtime files.
scripts/package-linux-host.mjsslimDependencieswalks the wholenode_modulestree and deletes every directory whose name matchesdirectory_names. Some published packages place required modules under paths namedtest,docs, orexamples, and some resolve.d.tssiblings at runtime through re-exports. The current boot check proves the current dependency set works, but a future dependency bump can fail at runtime with no packaging error.Consider recording the pruned path list in the native manifest so a regression is attributable, or restrict pruning to top-level package directories.
🤖 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 `@config/linux-runtime-package.json` around lines 27 - 31, Update the slimDependencies configuration and packaging flow to avoid deleting required nested runtime files based solely on bare directory names, preferably by restricting directory pruning to top-level package directories. Alternatively, record the exact pruned paths in the native manifest so future runtime regressions can be traced; preserve existing exclusion behavior for files that are safe to remove.scripts/candidate-promotion-skeleton.mjs (1)
56-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the retained manifest name from the contract helper.
Line 57 rebuilds the manifest file name with a string replacement.
scripts/artifact-contract.mjsalready exportschannelImageManifestName, andscripts/publish-promotion.mjsuses it. If the contract naming changes, this local rewrite drifts silently.♻️ Proposed refactor
-import { normalizeChannelImageManifest, releasedChannelImageManifest } from "./artifact-contract.mjs"; +import { channelImageManifestName, normalizeChannelImageManifest, releasedChannelImageManifest } from "./artifact-contract.mjs";const channelImageSource = join(imageSource, candidate.sealed_oci.artifact.name); -const channelImageManifestSource = join(imageSource, candidate.sealed_oci.artifact.name.replace(/\.oci\.tar$/, ".json")); +const channelImageManifestSource = join(imageSource, channelImageManifestName(candidate.sealed_oci));🤖 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/candidate-promotion-skeleton.mjs` around lines 56 - 57, Update the manifest source construction near channelImageSource to use the exported channelImageManifestName contract helper from artifact-contract.mjs instead of replacing the .oci.tar suffix locally. Preserve the existing imageSource path joining and pass the candidate artifact name to the helper.site/public/install-oci-runtime.sh (2)
69-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare
IMAGE_FIELDSlocal and report the field-count failure.Line 69 assigns
IMAGE_FIELDSin the global scope, unlike every other variable inresolve_channel_image. Line 70 then returns 1 with no message, so an operator sees the installer abort with no explanation.♻️ Proposed refactor
- mapfile -t IMAGE_FIELDS <<<"$fields" - [[ "${`#IMAGE_FIELDS`[@]}" -eq 5 ]] || return 1 + local -a IMAGE_FIELDS + mapfile -t IMAGE_FIELDS <<<"$fields" + [[ "${`#IMAGE_FIELDS`[@]}" -eq 5 ]] \ + || { echo "The channel image manifest did not yield the expected identity fields." >&2; return 1; }🤖 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-oci-runtime.sh` around lines 69 - 70, Update resolve_channel_image so IMAGE_FIELDS is declared local before mapfile populates it, and replace the silent field-count return with an error message describing the expected and received field counts before returning 1.
86-97: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the image download with curl timeouts.
Lines 87 and 91 use
--retry 3but set no--connect-timeoutor--max-time. The image is about 204 MB. If a connection stalls without closing,curlwaits indefinitely and the root-run installer hangs with no output and no failure.--retrydoes not bound the total duration of a single stalled transfer.Add explicit timeouts, and prefer
--speed-limit/--speed-timeover a fixed--max-timeso that slow but progressing links are not cut off.🛡️ Proposed fix
- curl -fsSL --proto '=https' --tlsv1.2 --retry 3 -o "$temp/manifest.json" "$manifest_url" \ + curl -fsSL --proto '=https' --tlsv1.2 --retry 3 --connect-timeout 20 --max-time 120 \ + -o "$temp/manifest.json" "$manifest_url" \ || { rm -rf -- "$temp"; echo "The referenced channel image manifest could not be downloaded." >&2; return 1; } cmp -s "$temp/manifest.json" "$manifest" \ || { rm -rf -- "$temp"; echo "The downloaded channel image manifest does not match the application release." >&2; return 1; } - curl -fsSL --proto '=https' --tlsv1.2 --retry 3 -o "$temp/image.tar" "$image_url" \ + curl -fsSL --proto '=https' --tlsv1.2 --retry 3 --connect-timeout 20 \ + --speed-limit 10240 --speed-time 60 -o "$temp/image.tar" "$image_url" \ || { rm -rf -- "$temp"; echo "The referenced channel image could not be downloaded." >&2; return 1; }🤖 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-oci-runtime.sh` around lines 86 - 97, Update both curl downloads in the temporary-image flow—the manifest download and image download—to include explicit connection and low-throughput timeouts, using --connect-timeout together with --speed-limit and --speed-time alongside the existing retry options. Apply consistent timeout settings to both curl invocations while allowing slow but progressing transfers to continue.scripts/publish-promotion.mjs (1)
146-152: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winA failed
gh release createleaves a pushed tag that blocks every later rerun.Line 148 pushes
refs/tags/$imageTagto origin before line 149 creates the Release. Ifgh release createfails, the tag exists on origin and no Release exists. On the next promotion attempt,remoteTagAndReleasesees a tag with a 404 Release and throwsImmutable artifact <tag> has a partial tag/Release identity. BecauseimageTagis derived from the image digest, the same identical bytes can never be promoted again until an operator deletes the remote tag by hand.Consider creating the draft Release first and pushing the tag only after the draft exists, or deleting the pushed tag when
gh release createfails.🤖 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/publish-promotion.mjs` around lines 146 - 152, Update the immutable promotion flow around the git tag push and gh release create commands so a failed release creation cannot leave an orphaned remote tag that blocks retries. Create the draft release before pushing the tag, or otherwise clean up the remote tag when release creation fails, while preserving the existing tag, release metadata, and repository behavior.scripts/verify-promotion-attestation.mjs (1)
9-15: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the exact subject roles instead of counting three subjects.
Line 13 only checks
subjects.length !== 3. Twolinux_tgzentries plus the image artifact also produce a length of 3, andlinux_offline_tgzis then never attested. Two conditions also pass the count check in a degraded way: anullchannel_image.candidate.artifactstill contributes one array element.
scripts/promotion-lib.mjscatches a missing offline role separately, so this is not currently exploitable. This script is an independent fail-closed gate, so it should prove the role set itself.♻️ Proposed refactor
-const subjects = [ - ...(Array.isArray(promotion?.artifacts) ? promotion.artifacts.filter((item) => ["linux_tgz", "linux_offline_tgz"].includes(item?.role)) : []), - promotion?.channel_image?.candidate?.artifact, -]; -if (subjects.length !== 3 || !/^[a-f0-9]{40}$/.test(String(promotion?.commit || ""))) { +const artifacts = Array.isArray(promotion?.artifacts) ? promotion.artifacts : []; +const linuxRoles = ["linux_tgz", "linux_offline_tgz"]; +const roleSubjects = linuxRoles.map((role) => artifacts.filter((item) => item?.role === role)); +const imageSubject = promotion?.channel_image?.candidate?.artifact; +const subjects = [...roleSubjects.map((matches) => matches[0]), imageSubject]; +if (roleSubjects.some((matches) => matches.length !== 1) || !imageSubject + || !/^[a-f0-9]{40}$/.test(String(promotion?.commit || ""))) { throw new Error("Refusing incomplete Linux/image attestation subject set"); }🤖 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/verify-promotion-attestation.mjs` around lines 9 - 15, Update the subject validation in the attestation verification flow to assert exactly one artifact for each required role: linux_tgz, linux_offline_tgz, and the channel image candidate artifact. Reject duplicate or missing Linux roles and reject a null or missing image artifact rather than relying on subjects.length, while preserving the existing commit validation and fail-closed error behavior.
🤖 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 `@ops/dress-rehearsal/candidate-boundary.py`:
- Around line 111-121: Guard each archive.extractfile result before passing it
to json.load in the identity/package and embedded_image parsing blocks. Reuse
the existing boundary-refusal path via fail, ensuring missing or non-regular
members produce the corresponding invalid-candidate message and that identity
remains safely initialized after a failed parse; verify fail raises rather than
returns.
In `@scripts/artifact-contract.mjs`:
- Around line 108-126: Add validation in validateSplitArtifactManifest for
production_dependencies, requiring key, reused, node_abi, architecture,
builder_image_digest, and runtime_package_sha256 with their expected types
before returning the manifest. Update test/phase2-candidate.mjs lines 57-61 to
populate all six fields, matching the shape written by
scripts/package-linux-host.mjs; both locations require changes.
- Around line 48-58: Update the validation around the normalized url and
manifestUrl values in the artifact contract so artifact.url and
artifact.manifest_url must either both be present or both be null, including
when requireUrl is false. Reject any mixed state before the individual URL
mismatch checks, while preserving the existing expected-URL validation for
complete pairs.
In `@scripts/build-oci-channel-image.sh`:
- Around line 25-37: Update the CONTEXT_SHA computation in
build-oci-channel-image.sh to account for untracked, non-ignored files under
container, or invalidate cache reuse whenever such entries exist. Preserve
exclusion of the generated channel-machine OCI artifacts, and ensure CACHE_KEY
changes whenever any Containerfile context content changes.
In `@scripts/channel-image-gc-report.mjs`:
- Around line 13-16: Update the retained-manifest processing around JSON.parse
and the catch block to record unreadable, malformed, or invalid manifests
instead of silently ignoring them; make the report return a nonzero or
indeterminate result and avoid presenting cleanup-safe unreferenced byte totals
until all retained manifests validate. Add a fixture covering a malformed
manifest and verify the failure behavior.
In `@scripts/package-linux-host.mjs`:
- Line 158: Extend the overwrite guard near the existing output checks to also
inspect the `${output}.sha256` and `${offlineOutput}.sha256` sidecar paths
before any artifacts are written. Reuse the same refusal behavior and
relative-path error reporting, while preserving the existing checks for
`output`, `offlineOutput`, and `splitOutput`.
- Around line 100-108: Update deterministicTar to stream tar output directly
through gzip into destination rather than buffering complete stdout in
tar.stdout and packed.stdout; use the existing process execution approach to
preserve deterministic tar/gzip options and ensure failures are checked before
treating the destination as written. Keep explicit errors for unsuccessful tar
or gzip execution, and treat a successful gzip invocation that produces no
destination output as a write failure.
- Around line 172-179: Validate the result of run("git", ["ls-files", ...]) in
the local-worktree/rollback-fixture branch before invoking tar. If listed.status
is nonzero, fail immediately with the exact-source-archive error context, and
only pipe listed.stdout to tar when the Git listing succeeds; keep the existing
archive validation for tar failures.
In `@site/public/install-oci-runtime.sh`:
- Around line 102-103: The install flow must explicitly handle
resolve_channel_image failure and reuse its verified digest. Update
resolve_channel_image so every successful branch, including the legacy branch,
assigns RESOLVED_IMAGE_SHA to the validated manifest digest; guard the call at
the caller before using RESOLVED_IMAGE_TAR, and replace the sha256sum
recomputation with the exported RESOLVED_IMAGE_SHA value.
In `@site/public/update-host.sh`:
- Around line 149-151: Update the curl invocation for the release metadata
request to include explicit connection and overall operation timeouts, while
preserving the existing retry and failure handling around RELEASE_METADATA_URL.
---
Nitpick comments:
In `@config/linux-runtime-package.json`:
- Around line 27-31: Update the slimDependencies configuration and packaging
flow to avoid deleting required nested runtime files based solely on bare
directory names, preferably by restricting directory pruning to top-level
package directories. Alternatively, record the exact pruned paths in the native
manifest so future runtime regressions can be traced; preserve existing
exclusion behavior for files that are safe to remove.
In `@ops/dress-rehearsal/candidate-boundary.py`:
- Around line 89-97: The offline bundle filename format is duplicated between
the Python validation and the artifact contract. Update the candidate-boundary
validation around the offline bundle check to consume or verify the format
defined by scripts/artifact-contract.mjs, and add a cross-language test if
direct reuse is unavailable, so future renames require only one coordinated
contract change.
In `@scripts/artifact-contract.mjs`:
- Around line 108-126: The validateSplitArtifactManifest function must validate
value.production_dependencies before returning the normalized manifest. Require
the block to exist with the recorded key, reused, node_abi, architecture,
builder_image_digest, and runtime_package_sha256 fields in the expected shape,
and throw on malformed or missing data; preserve the validated block in the
returned object.
In `@scripts/build-oci-channel-image.sh`:
- Around line 113-132: Compute the SHA-256 digest of CACHE_TAR once immediately
after the cache entry is selected, then reuse that value throughout the
remaining flow. Update validate_cache and the OUT_SHA comparison and final
OUT_SHA write to use the shared digest, removing all repeated sha256sum calls
for the archive.
- Around line 19-20: Add a prerequisite check for python3 alongside the existing
podman check in the script’s initial validation block, using a clear error
message and exiting nonzero when unavailable. Ensure this occurs before
validate_cache or any image build, while preserving the existing CONTAINERFILE
and podman checks.
In `@scripts/candidate-promotion-skeleton.mjs`:
- Around line 56-57: Update the manifest source construction near
channelImageSource to use the exported channelImageManifestName contract helper
from artifact-contract.mjs instead of replacing the .oci.tar suffix locally.
Preserve the existing imageSource path joining and pass the candidate artifact
name to the helper.
In `@scripts/package-linux-host.mjs`:
- Around line 269-275: Update the offline packaging flow around offlineRoot and
deterministicTar to avoid recursively copying the complete releaseRoot. Build
the offline archive by combining the existing staged release contents with the
three container artifacts through tar sources or another byte-sharing approach,
while preserving the current archive layout and deterministic output.
In `@scripts/publish-promotion.mjs`:
- Around line 146-152: Update the immutable promotion flow around the git tag
push and gh release create commands so a failed release creation cannot leave an
orphaned remote tag that blocks retries. Create the draft release before pushing
the tag, or otherwise clean up the remote tag when release creation fails, while
preserving the existing tag, release metadata, and repository behavior.
In `@scripts/verify-promotion-attestation.mjs`:
- Around line 9-15: Update the subject validation in the attestation
verification flow to assert exactly one artifact for each required role:
linux_tgz, linux_offline_tgz, and the channel image candidate artifact. Reject
duplicate or missing Linux roles and reject a null or missing image artifact
rather than relying on subjects.length, while preserving the existing commit
validation and fail-closed error behavior.
In `@site/public/install-oci-runtime.sh`:
- Around line 69-70: Update resolve_channel_image so IMAGE_FIELDS is declared
local before mapfile populates it, and replace the silent field-count return
with an error message describing the expected and received field counts before
returning 1.
- Around line 86-97: Update both curl downloads in the temporary-image flow—the
manifest download and image download—to include explicit connection and
low-throughput timeouts, using --connect-timeout together with --speed-limit and
--speed-time alongside the existing retry options. Apply consistent timeout
settings to both curl invocations while allowing slow but progressing transfers
to continue.
🪄 Autofix
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: 2fb3d4c9-403d-4c0f-9123-0d79112c4de9
📒 Files selected for processing (45)
.github/workflows/candidate.yml.github/workflows/promote-stable.ymlconfig/artifact-budgets.jsonconfig/linux-runtime-package.jsondocs/GOVERNANCE.mddocs/USER_GUIDE.mddocs/artifact-size-and-split-delivery.mddocs/phase4-platform-acceptance.mddocs/release-checklist.mddocs/release-lifecycle.mddocs/release-notes-template.mdops/dress-rehearsal/1helm-candidate-installops/dress-rehearsal/candidate-boundary.pyops/platform-acceptance/linux.shops/platform-acceptance/windows.ps1package.jsonscripts/artifact-contract.mjsscripts/artifact-size-report.mjsscripts/build-oci-channel-image.shscripts/candidate-manifest.mjsscripts/candidate-promotion-skeleton.mjsscripts/channel-image-gc-report.mjsscripts/github-promotion-gates.mjsscripts/linux-acceptance-evidence.mjsscripts/package-linux-host.mjsscripts/pending-acceptance-evidence.mjsscripts/platform-acceptance-lib.mjsscripts/promotion-lib.mjsscripts/publish-promotion.mjsscripts/stable-manifest-lib.mjsscripts/verify-promotion-attestation.mjsscripts/windows-acceptance-evidence.mjssite/content.mjssite/public/apply-linux-release.shsite/public/install-oci-runtime.shsite/public/install.shsite/public/update-host.shsite/server.mjstest/connectors.mjstest/phase2-candidate.mjstest/phase3-promotion.mjstest/phase4-platform-acceptance.mjstest/phase5-artifacts.mjstest/release-governance.mjstest/site.mjs
| try: | ||
| identity = json.load(archive.extractfile(identity_members[0])) | ||
| package = json.load(archive.extractfile(package_members[0])) | ||
| except (TypeError, json.JSONDecodeError) as error: | ||
| fail(f"candidate embedded identity is invalid: {error}") | ||
| try: | ||
| embedded_image = json.load(archive.extractfile(image_manifests[0])) | ||
| except (TypeError, json.JSONDecodeError) as error: | ||
| fail(f"candidate channel image manifest is invalid: {error}") | ||
| if embedded_image != identity.get("channel_image") or embedded_image != manifest.get("sealed_oci"): | ||
| fail("channel image manifest does not match the candidate identity") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
extractfile can return None, and AttributeError is not caught here.
tarfile.TarFile.extractfile returns None when the member is not a regular file or a link to one. A crafted archive can contain a directory member named 1Helm-x/resources/candidate-build.json, which satisfies the re.fullmatch filters on lines 105-108. json.load(None) then raises AttributeError, which neither the local except (TypeError, json.JSONDecodeError) clauses nor the outer handler on line 248 catches. The result is an uncaught traceback instead of the boundary refusal message. Lines 132-133 already guard embedded_oci is None, so apply the same guard here.
🐛 Proposed fix
+ identity_stream = archive.extractfile(identity_members[0])
+ package_stream = archive.extractfile(package_members[0])
+ image_stream = archive.extractfile(image_manifests[0])
+ if identity_stream is None or package_stream is None or image_stream is None:
+ fail("candidate archive metadata members are not regular files")
try:
- identity = json.load(archive.extractfile(identity_members[0]))
- package = json.load(archive.extractfile(package_members[0]))
- except (TypeError, json.JSONDecodeError) as error:
+ identity = json.load(identity_stream)
+ package = json.load(package_stream)
+ except json.JSONDecodeError as error:
fail(f"candidate embedded identity is invalid: {error}")
try:
- embedded_image = json.load(archive.extractfile(image_manifests[0]))
- except (TypeError, json.JSONDecodeError) as error:
+ embedded_image = json.load(image_stream)
+ except json.JSONDecodeError as error:
fail(f"candidate channel image manifest is invalid: {error}")Confirm that fail raises rather than returns. If it returns, identity is unbound at line 120 and at line 150 after a failed parse.
#!/bin/bash
# Inspect the fail/expect helpers used by the boundary validator.
fd -t f 'candidate-boundary.py' | xargs -r rg -n -B2 -A6 '^def (fail|expect|load_json)\b'🤖 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 `@ops/dress-rehearsal/candidate-boundary.py` around lines 111 - 121, Guard each
archive.extractfile result before passing it to json.load in the
identity/package and embedded_image parsing blocks. Reuse the existing
boundary-refusal path via fail, ensuring missing or non-regular members produce
the corresponding invalid-candidate message and that identity remains safely
initialized after a failed parse; verify fail raises rather than returns.
| const url = value.artifact?.url == null ? null : String(value.artifact.url); | ||
| const manifestName = channelImageManifestName({ architecture, sha256 }); | ||
| const releaseTag = channelImageReleaseTag({ architecture, sha256 }); | ||
| const expectedUrl = `https://github.com/gitcommit90/1Helm/releases/download/${releaseTag}/${expectedName}`; | ||
| const manifestUrl = value.artifact?.manifest_url == null ? null : String(value.artifact.manifest_url); | ||
| const expectedManifestUrl = `https://github.com/gitcommit90/1Helm/releases/download/${releaseTag}/${manifestName}`; | ||
| if (requireUrl && (url !== expectedUrl || manifestUrl !== expectedManifestUrl)) { | ||
| throw new Error("sealed channel image release URLs are not immutable digest-addressed URLs"); | ||
| } | ||
| if (url != null && url !== expectedUrl) throw new Error("sealed channel image artifact URL mismatch"); | ||
| if (manifestUrl != null && manifestUrl !== expectedManifestUrl) throw new Error("sealed channel image manifest URL mismatch"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Require url and manifest_url together when either is present.
When requireUrl is false, a manifest that sets artifact.url but omits artifact.manifest_url passes. Line 58 only checks a non-null manifestUrl. The normalized output at line 78 then contains manifest_url: null next to a valid url. Downstream comparisons use exact JSON.stringify equality on this object, so a partially populated manifest becomes an accepted shape that no producer intends.
Reject the mixed state explicitly.
🛡️ Proposed fix
const expectedManifestUrl = `https://github.com/gitcommit90/1Helm/releases/download/${releaseTag}/${manifestName}`;
if (requireUrl && (url !== expectedUrl || manifestUrl !== expectedManifestUrl)) {
throw new Error("sealed channel image release URLs are not immutable digest-addressed URLs");
}
+ if ((url == null) !== (manifestUrl == null)) {
+ throw new Error("sealed channel image release URLs must be present together");
+ }
if (url != null && url !== expectedUrl) throw new Error("sealed channel image artifact URL mismatch");📝 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.
| const url = value.artifact?.url == null ? null : String(value.artifact.url); | |
| const manifestName = channelImageManifestName({ architecture, sha256 }); | |
| const releaseTag = channelImageReleaseTag({ architecture, sha256 }); | |
| const expectedUrl = `https://github.com/gitcommit90/1Helm/releases/download/${releaseTag}/${expectedName}`; | |
| const manifestUrl = value.artifact?.manifest_url == null ? null : String(value.artifact.manifest_url); | |
| const expectedManifestUrl = `https://github.com/gitcommit90/1Helm/releases/download/${releaseTag}/${manifestName}`; | |
| if (requireUrl && (url !== expectedUrl || manifestUrl !== expectedManifestUrl)) { | |
| throw new Error("sealed channel image release URLs are not immutable digest-addressed URLs"); | |
| } | |
| if (url != null && url !== expectedUrl) throw new Error("sealed channel image artifact URL mismatch"); | |
| if (manifestUrl != null && manifestUrl !== expectedManifestUrl) throw new Error("sealed channel image manifest URL mismatch"); | |
| const url = value.artifact?.url == null ? null : String(value.artifact.url); | |
| const manifestName = channelImageManifestName({ architecture, sha256 }); | |
| const releaseTag = channelImageReleaseTag({ architecture, sha256 }); | |
| const expectedUrl = `https://github.com/gitcommit90/1Helm/releases/download/${releaseTag}/${expectedName}`; | |
| const manifestUrl = value.artifact?.manifest_url == null ? null : String(value.artifact.manifest_url); | |
| const expectedManifestUrl = `https://github.com/gitcommit90/1Helm/releases/download/${releaseTag}/${manifestName}`; | |
| if (requireUrl && (url !== expectedUrl || manifestUrl !== expectedManifestUrl)) { | |
| throw new Error("sealed channel image release URLs are not immutable digest-addressed URLs"); | |
| } | |
| if ((url == null) !== (manifestUrl == null)) { | |
| throw new Error("sealed channel image release URLs must be present together"); | |
| } | |
| if (url != null && url !== expectedUrl) throw new Error("sealed channel image artifact URL mismatch"); | |
| if (manifestUrl != null && manifestUrl !== expectedManifestUrl) throw new Error("sealed channel image manifest URL mismatch"); |
🤖 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/artifact-contract.mjs` around lines 48 - 58, Update the validation
around the normalized url and manifestUrl values in the artifact contract so
artifact.url and artifact.manifest_url must either both be present or both be
null, including when requireUrl is false. Reject any mixed state before the
individual URL mismatch checks, while preserving the existing expected-URL
validation for complete pairs.
| export function validateSplitArtifactManifest(value, { version, architecture } = {}) { | ||
| if (!value || value.schema !== LINUX_SPLIT_SCHEMA || value.kind !== LINUX_SPLIT_KIND) { | ||
| throw new Error("Linux split artifact manifest schema or kind mismatch"); | ||
| } | ||
| if (version && value.version !== version) throw new Error("Linux split artifact version mismatch"); | ||
| const app = value.app || {}; | ||
| const offline = value.offline || {}; | ||
| if (!/^\d+\.\d+\.\d+$/.test(String(value.version || "")) | ||
| || app.name !== `1Helm-${value.version}-linux-node.tgz` | ||
| || offline.name !== offlineBundleName(value.version) | ||
| || !HEX64.test(String(app.sha256 || "")) || !HEX64.test(String(offline.sha256 || "")) | ||
| || !Number.isSafeInteger(app.bytes) || app.bytes < 1 | ||
| || !Number.isSafeInteger(offline.bytes) || offline.bytes < 1) { | ||
| throw new Error("Linux split artifact byte identities are incomplete"); | ||
| } | ||
| const channelImage = normalizeChannelImageManifest(value.channel_image); | ||
| if (architecture && channelImage.architecture !== architecture) throw new Error("Linux split channel image architecture mismatch"); | ||
| return { ...value, app: { ...app }, offline: { ...offline }, channel_image: channelImage }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
production_dependencies has no owner in the split-artifact contract. validateSplitArtifactManifest returns { ...value }, so the block passes through unchecked into the candidate manifest, and the fixture drifted from the producer without any test failing.
scripts/artifact-contract.mjs#L108-L126: validateproduction_dependenciesbefore returning, requiringkey,reused,node_abi,architecture,builder_image_digest, andruntime_package_sha256with their expected types.test/phase2-candidate.mjs#L57-L61: populate all six fields in the fixture so the test exercises the shape thatscripts/package-linux-host.mjslines 283-284 writes.
📍 Affects 2 files
scripts/artifact-contract.mjs#L108-L126(this comment)test/phase2-candidate.mjs#L57-L61
🤖 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/artifact-contract.mjs` around lines 108 - 126, Add validation in
validateSplitArtifactManifest for production_dependencies, requiring key,
reused, node_abi, architecture, builder_image_digest, and runtime_package_sha256
with their expected types before returning the manifest. Update
test/phase2-candidate.mjs lines 57-61 to populate all six fields, matching the
shape written by scripts/package-linux-host.mjs; both locations require changes.
| CONTEXT_SHA="$( | ||
| cd "$ROOT" | ||
| git ls-files -z container \ | ||
| | while IFS= read -r -d '' file; do | ||
| case "$file" in | ||
| container/channel-machine.oci.tar|container/channel-machine.oci.sha256|container/channel-machine.oci.json) continue ;; | ||
| esac | ||
| printf '%s\0' "$file" | ||
| sha256sum "$file" | awk '{printf "%s\0", $1}' | ||
| done \ | ||
| | sha256sum | awk '{print $1}' | ||
| )" | ||
| CACHE_KEY="$(printf '1helm-channel-image-v1\n%s\n%s\n%s\n%s\n' "$ARCH" "$BASE_DIGEST" "$CONTAINERFILE_SHA" "$CONTEXT_SHA" | sha256sum | awk '{print $1}')" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Untracked files in container/ do not change the cache key.
git ls-files container at line 27 lists tracked files only. If a developer adds an untracked file under container/ that the Containerfile copies, CONTEXT_SHA and therefore CACHE_KEY stay unchanged. validate_cache then succeeds and the script reuses a stale image that does not contain the new content. The clean-worktree requirement in scripts/package-linux-host.mjs line 144 applies only to trusted-main candidates, so local and rollback-fixture builds can hit this.
Include untracked, non-ignored context files in the hash, or refuse to reuse the cache when container/ has untracked entries.
🐛 Proposed fix
- git ls-files -z container \
+ git ls-files -z --cached --others --exclude-standard container \📝 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.
| CONTEXT_SHA="$( | |
| cd "$ROOT" | |
| git ls-files -z container \ | |
| | while IFS= read -r -d '' file; do | |
| case "$file" in | |
| container/channel-machine.oci.tar|container/channel-machine.oci.sha256|container/channel-machine.oci.json) continue ;; | |
| esac | |
| printf '%s\0' "$file" | |
| sha256sum "$file" | awk '{printf "%s\0", $1}' | |
| done \ | |
| | sha256sum | awk '{print $1}' | |
| )" | |
| CACHE_KEY="$(printf '1helm-channel-image-v1\n%s\n%s\n%s\n%s\n' "$ARCH" "$BASE_DIGEST" "$CONTAINERFILE_SHA" "$CONTEXT_SHA" | sha256sum | awk '{print $1}')" | |
| git ls-files -z --cached --others --exclude-standard container \ |
🤖 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/build-oci-channel-image.sh` around lines 25 - 37, Update the
CONTEXT_SHA computation in build-oci-channel-image.sh to account for untracked,
non-ignored files under container, or invalidate cache reuse whenever such
entries exist. Preserve exclusion of the generated channel-machine OCI
artifacts, and ensure CACHE_KEY changes whenever any Containerfile context
content changes.
| try { | ||
| const value = JSON.parse(readFileSync(manifest, "utf8")); | ||
| if (/^[a-f0-9]{64}$/.test(String(value.sha256 || ""))) referenced.add(value.sha256); | ||
| } catch {} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Fail closed when a retained release manifest cannot be validated.
catch {} at Line 16 discards an unreadable, malformed, or invalid manifest. Its digest is then absent from referenced, and Line 29 can include rollback image bytes in unreferenced_bytes. Even in report-only mode, this can direct an unsafe manual cleanup.
Record manifest failures and return a nonzero or indeterminate report until retained manifests validate. Add a malformed-manifest fixture.
🤖 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/channel-image-gc-report.mjs` around lines 13 - 16, Update the
retained-manifest processing around JSON.parse and the catch block to record
unreadable, malformed, or invalid manifests instead of silently ignoring them;
make the report return a nonzero or indeterminate result and avoid presenting
cleanup-safe unreferenced byte totals until all retained manifests validate. Add
a fixture covering a malformed manifest and verify the failure behavior.
| function deterministicTar(sourceDirectory, prefix, destination) { | ||
| const tar = spawnSync("tar", ["--sort=name", "--mtime=@0", "--owner=0", "--group=0", "--numeric-owner", "-cf", "-", "-C", sourceDirectory, prefix], { | ||
| encoding: "buffer", maxBuffer: 1024 * 1024 * 1024, | ||
| }); | ||
| if (tar.status !== 0) throw new Error(`Could not stage ${basename(destination)}`); | ||
| const packed = spawnSync("gzip", ["-n", "-c"], { input: tar.stdout, encoding: "buffer", maxBuffer: 1024 * 1024 * 1024 }); | ||
| if (packed.status === 0 && packed.stdout?.length) writeFileSync(destination, packed.stdout); | ||
| if (packed.status !== 0) throw new Error(`Could not write ${basename(destination)}`); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Buffer sizes are close to the limit for the offline archive.
deterministicTar buffers the complete uncompressed tar stream in tar.stdout and the complete gzip output in packed.stdout, both capped at 1 GB. The offline bundle already contains the 204 MB sealed image plus the pruned node_modules budget of 320 MB from config/artifact-budgets.json. When the uncompressed stream passes maxBuffer, spawnSync kills tar and the run fails with Could not stage …, which does not name the real cause.
Also note the write ordering at line 106: the file is written before the status check on line 107. If gzip exits 0 with empty output, nothing is written and no error is raised, so digestFile(output) fails later with ENOENT.
Pipe tar into gzip and redirect to the destination file instead of buffering both stages.
♻️ Minimal ordering fix
const packed = spawnSync("gzip", ["-n", "-c"], { input: tar.stdout, encoding: "buffer", maxBuffer: 1024 * 1024 * 1024 });
- if (packed.status === 0 && packed.stdout?.length) writeFileSync(destination, packed.stdout);
- if (packed.status !== 0) throw new Error(`Could not write ${basename(destination)}`);
+ if (packed.status !== 0 || !packed.stdout?.length) throw new Error(`Could not write ${basename(destination)}`);
+ writeFileSync(destination, packed.stdout);🤖 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/package-linux-host.mjs` around lines 100 - 108, Update
deterministicTar to stream tar output directly through gzip into destination
rather than buffering complete stdout in tar.stdout and packed.stdout; use the
existing process execution approach to preserve deterministic tar/gzip options
and ensure failures are checked before treating the destination as written. Keep
explicit errors for unsuccessful tar or gzip execution, and treat a successful
gzip invocation that produces no destination output as a write failure.
|
|
||
| // Build the client and sidecar bundles here, on the release builder, so the | ||
| // installed release is already runnable. Everything below only copies results. | ||
| for (const path of [output, offlineOutput, splitOutput]) if (existsSync(path)) throw new Error(`Refusing to overwrite existing artifact: ${relative(root, path)}`); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
The overwrite refusal omits the .sha256 sidecars.
Line 158 protects output, offlineOutput, and splitOutput. Lines 267 and 277 write ${output}.sha256 and ${offlineOutput}.sha256 unconditionally. A stale sidecar from an earlier run is overwritten without the fail-closed check that the archives get.
🛡️ Proposed fix
-for (const path of [output, offlineOutput, splitOutput]) if (existsSync(path)) throw new Error(`Refusing to overwrite existing artifact: ${relative(root, path)}`);
+for (const path of [output, `${output}.sha256`, offlineOutput, `${offlineOutput}.sha256`, splitOutput]) {
+ if (existsSync(path)) throw new Error(`Refusing to overwrite existing artifact: ${relative(root, path)}`);
+}📝 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.
| for (const path of [output, offlineOutput, splitOutput]) if (existsSync(path)) throw new Error(`Refusing to overwrite existing artifact: ${relative(root, path)}`); | |
| for (const path of [output, `${output}.sha256`, offlineOutput, `${offlineOutput}.sha256`, splitOutput]) { | |
| if (existsSync(path)) throw new Error(`Refusing to overwrite existing artifact: ${relative(root, path)}`); | |
| } |
🤖 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/package-linux-host.mjs` at line 158, Extend the overwrite guard near
the existing output checks to also inspect the `${output}.sha256` and
`${offlineOutput}.sha256` sidecar paths before any artifacts are written. Reuse
the same refusal behavior and relative-path error reporting, while preserving
the existing checks for `output`, `offlineOutput`, and `splitOutput`.
| if (["local-worktree", "rollback-fixture"].includes(candidateIdentity?.source_state)) { | ||
| const files = spawnSync("git", ["ls-files", "-z", "--cached", "--others", "--exclude-standard"], { | ||
| cwd: root, encoding: "buffer", maxBuffer: 64 * 1024 * 1024, | ||
| }); | ||
| if (files.status !== 0 || !files.stdout?.length) throw new Error("Could not enumerate the local candidate worktree"); | ||
| archive = spawnSync("tar", ["-cf", "-", "--null", "--files-from=-", `--transform=s,^,${prefix}/,`], { | ||
| cwd: root, input: files.stdout, encoding: "buffer", maxBuffer: 512 * 1024 * 1024, | ||
| }); | ||
| const listed = run("git", ["ls-files", "-z", "--cached", "--others", "--exclude-standard"], { encoding: "buffer", maxBuffer: 64 * 1024 * 1024 }); | ||
| sourceArchive = spawnSync("tar", ["-cf", "-", "--null", "--files-from=-"], { cwd: root, input: listed.stdout, encoding: "buffer", maxBuffer: 512 * 1024 * 1024 }); | ||
| } else { | ||
| archive = spawnSync("git", ["archive", "--format=tar", `--prefix=${prefix}/`, "HEAD"], { | ||
| cwd: root, | ||
| encoding: "buffer", | ||
| maxBuffer: 512 * 1024 * 1024, | ||
| }); | ||
| } | ||
| if (archive.status !== 0) throw new Error("Could not package the exact Git candidate source"); | ||
| if (!archive.stdout?.length) throw new Error("Exact Git candidate source archive was empty"); | ||
| const sourceArchiveSha256 = createHash("sha256").update(archive.stdout).digest("hex"); | ||
| const extract = spawnSync("tar", ["-xf", "-", "-C", stage], { input: archive.stdout, stdio: ["pipe", "inherit", "inherit"] }); | ||
| if (extract.status !== 0) throw new Error("Could not extract the Git release source for packaging"); | ||
|
|
||
| const stagedPackage = resolve(stage, prefix, "package.json"); | ||
| if (!existsSync(stagedPackage) || String(JSON.parse(readFileSync(stagedPackage, "utf8")).version || "") !== version) { | ||
| throw new Error("Exact Git release source archive is missing its versioned package contract"); | ||
| } | ||
|
|
||
| const containerDir = join(stage, prefix, "container"); | ||
| mkdirSync(containerDir, { recursive: true }); | ||
| for (const rel of sealed) { | ||
| const src = resolve(root, rel); | ||
| if (!existsSync(src)) continue; | ||
| copyFileSync(src, join(stage, prefix, rel)); | ||
| } | ||
| if (candidateIdentity) { | ||
| const imageSha256 = String(readFileSync(resolve(root, "container/channel-machine.oci.sha256"), "utf8")).trim(); | ||
| if (!/^[a-f0-9]{64}$/.test(imageSha256)) throw new Error("Candidate packaging requires the sealed OCI image SHA-256"); | ||
| const identityFile = join(stage, prefix, "resources", "candidate-build.json"); | ||
| mkdirSync(dirname(identityFile), { recursive: true }); | ||
| writeFileSync(identityFile, `${JSON.stringify({ | ||
| ...candidateIdentity, | ||
| source_archive_sha256: sourceArchiveSha256, | ||
| sealed_oci_sha256: imageSha256, | ||
| }, null, 2)}\n`); | ||
| sourceArchive = run("git", ["archive", "--format=tar", "HEAD"], { encoding: "buffer", maxBuffer: 512 * 1024 * 1024 }); | ||
| } | ||
| const archive = sourceArchive; | ||
| if (archive.status !== 0 || !archive.stdout?.length) throw new Error("Could not create exact Git source identity archive"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Check the exit status of git ls-files before piping it into tar.
Line 179 checks archive.status, which is the status of tar, not of the git ls-files call on line 173. If git ls-files fails, listed.stdout is empty, tar still exits 0, and archive.stdout holds a non-empty empty-archive block, so the check on line 179 passes. The failure is caught only at line 186 with the misleading message Git source archive is missing its versioned package contract.
Fail at the real cause.
🐛 Proposed fix
const listed = run("git", ["ls-files", "-z", "--cached", "--others", "--exclude-standard"], { encoding: "buffer", maxBuffer: 64 * 1024 * 1024 });
+ if (listed.status !== 0 || !listed.stdout?.length) throw new Error("Could not list the exact local worktree source identity");
sourceArchive = spawnSync("tar", ["-cf", "-", "--null", "--files-from=-"], { cwd: root, input: listed.stdout, encoding: "buffer", maxBuffer: 512 * 1024 * 1024 });📝 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.
| if (["local-worktree", "rollback-fixture"].includes(candidateIdentity?.source_state)) { | |
| const files = spawnSync("git", ["ls-files", "-z", "--cached", "--others", "--exclude-standard"], { | |
| cwd: root, encoding: "buffer", maxBuffer: 64 * 1024 * 1024, | |
| }); | |
| if (files.status !== 0 || !files.stdout?.length) throw new Error("Could not enumerate the local candidate worktree"); | |
| archive = spawnSync("tar", ["-cf", "-", "--null", "--files-from=-", `--transform=s,^,${prefix}/,`], { | |
| cwd: root, input: files.stdout, encoding: "buffer", maxBuffer: 512 * 1024 * 1024, | |
| }); | |
| const listed = run("git", ["ls-files", "-z", "--cached", "--others", "--exclude-standard"], { encoding: "buffer", maxBuffer: 64 * 1024 * 1024 }); | |
| sourceArchive = spawnSync("tar", ["-cf", "-", "--null", "--files-from=-"], { cwd: root, input: listed.stdout, encoding: "buffer", maxBuffer: 512 * 1024 * 1024 }); | |
| } else { | |
| archive = spawnSync("git", ["archive", "--format=tar", `--prefix=${prefix}/`, "HEAD"], { | |
| cwd: root, | |
| encoding: "buffer", | |
| maxBuffer: 512 * 1024 * 1024, | |
| }); | |
| } | |
| if (archive.status !== 0) throw new Error("Could not package the exact Git candidate source"); | |
| if (!archive.stdout?.length) throw new Error("Exact Git candidate source archive was empty"); | |
| const sourceArchiveSha256 = createHash("sha256").update(archive.stdout).digest("hex"); | |
| const extract = spawnSync("tar", ["-xf", "-", "-C", stage], { input: archive.stdout, stdio: ["pipe", "inherit", "inherit"] }); | |
| if (extract.status !== 0) throw new Error("Could not extract the Git release source for packaging"); | |
| const stagedPackage = resolve(stage, prefix, "package.json"); | |
| if (!existsSync(stagedPackage) || String(JSON.parse(readFileSync(stagedPackage, "utf8")).version || "") !== version) { | |
| throw new Error("Exact Git release source archive is missing its versioned package contract"); | |
| } | |
| const containerDir = join(stage, prefix, "container"); | |
| mkdirSync(containerDir, { recursive: true }); | |
| for (const rel of sealed) { | |
| const src = resolve(root, rel); | |
| if (!existsSync(src)) continue; | |
| copyFileSync(src, join(stage, prefix, rel)); | |
| } | |
| if (candidateIdentity) { | |
| const imageSha256 = String(readFileSync(resolve(root, "container/channel-machine.oci.sha256"), "utf8")).trim(); | |
| if (!/^[a-f0-9]{64}$/.test(imageSha256)) throw new Error("Candidate packaging requires the sealed OCI image SHA-256"); | |
| const identityFile = join(stage, prefix, "resources", "candidate-build.json"); | |
| mkdirSync(dirname(identityFile), { recursive: true }); | |
| writeFileSync(identityFile, `${JSON.stringify({ | |
| ...candidateIdentity, | |
| source_archive_sha256: sourceArchiveSha256, | |
| sealed_oci_sha256: imageSha256, | |
| }, null, 2)}\n`); | |
| sourceArchive = run("git", ["archive", "--format=tar", "HEAD"], { encoding: "buffer", maxBuffer: 512 * 1024 * 1024 }); | |
| } | |
| const archive = sourceArchive; | |
| if (archive.status !== 0 || !archive.stdout?.length) throw new Error("Could not create exact Git source identity archive"); | |
| const listed = run("git", ["ls-files", "-z", "--cached", "--others", "--exclude-standard"], { encoding: "buffer", maxBuffer: 64 * 1024 * 1024 }); | |
| if (listed.status !== 0 || !listed.stdout?.length) throw new Error("Could not list the exact local worktree source identity"); | |
| sourceArchive = spawnSync("tar", ["-cf", "-", "--null", "--files-from=-"], { cwd: root, input: listed.stdout, encoding: "buffer", maxBuffer: 512 * 1024 * 1024 }); |
🤖 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/package-linux-host.mjs` around lines 172 - 179, Validate the result
of run("git", ["ls-files", ...]) in the local-worktree/rollback-fixture branch
before invoking tar. If listed.status is nonzero, fail immediately with the
exact-source-archive error context, and only pipe listed.stdout to tar when the
Git listing succeeds; keep the existing archive validation for tar failures.
| resolve_channel_image | ||
| expected_image_sha="$(sha256sum "$RESOLVED_IMAGE_TAR" | awk '{print $1}')" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the resolve_channel_image result and reuse the verified digest.
Two concerns at these two lines.
First, line 102 calls resolve_channel_image with no failure guard. Every failure path inside the function uses return 1. If the script does not set set -e, execution continues to line 103 with RESOLVED_IMAGE_TAR unset, and the installer proceeds to symlink an unresolved path at line 136. Add an explicit guard so the outcome does not depend on shell options set far away in the file.
Second, line 103 recomputes the digest from the resolved tar. The function already proved that digest against the manifest contract. Recomputing re-reads about 204 MB, and it records the digest of whatever currently sits at that path instead of the contract digest that was verified. Export the verified digest from the function and reuse it.
🛡️ Proposed fix
RESOLVED_IMAGE_TAR="$retained_tar"
RESOLVED_IMAGE_MANIFEST="$retained_manifest"
+ RESOLVED_IMAGE_SHA="$expected_image_sha"
}
-resolve_channel_image
-expected_image_sha="$(sha256sum "$RESOLVED_IMAGE_TAR" | awk '{print $1}')"
+resolve_channel_image || { echo "The sealed channel image could not be resolved." >&2; exit 1; }
+expected_image_sha="$RESOLVED_IMAGE_SHA"The legacy branch at lines 42-44 must also set RESOLVED_IMAGE_SHA="$expected_image_sha" before it returns.
Run the following script to confirm the shell options in effect and every early-return path:
#!/bin/bash
# Description: Confirm strict-mode options and the return paths of resolve_channel_image.
set -euo pipefail
file="$(fd -H -t f 'install-oci-runtime.sh' | head -n 1)"
echo "== file: $file"
# Show the header where shell options would be set.
sed -n '1,30p' "$file"
# Show every return/exit inside the resolver and every use of the resolved variables.
rg -n 'set -[a-z]|return 1|RESOLVED_IMAGE_TAR|RESOLVED_IMAGE_MANIFEST|resolve_channel_image' "$file"🤖 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-oci-runtime.sh` around lines 102 - 103, The install flow
must explicitly handle resolve_channel_image failure and reuse its verified
digest. Update resolve_channel_image so every successful branch, including the
legacy branch, assigns RESOLVED_IMAGE_SHA to the validated manifest digest;
guard the call at the caller before using RESOLVED_IMAGE_TAR, and replace the
sha256sum recomputation with the exported RESOLVED_IMAGE_SHA value.
| curl -fsSL --proto '=https' --tlsv1.2 --retry 3 \ | ||
| "$RELEASE_METADATA_URL" \ | ||
| -o "$TEMP_ROOT/release.json" || fail "The host could not reach the 1Helm release service." |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add explicit timeouts to the metadata request.
Retries do not bound a stalled transfer. A server can accept the connection and leave the updater in the checking state indefinitely.
Proposed fix
-curl -fsSL --proto '=https' --tlsv1.2 --retry 3 \
+curl -fsSL --proto '=https' --tlsv1.2 --retry 3 \
+ --connect-timeout 10 --max-time 60 \
"$RELEASE_METADATA_URL" \📝 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.
| curl -fsSL --proto '=https' --tlsv1.2 --retry 3 \ | |
| "$RELEASE_METADATA_URL" \ | |
| -o "$TEMP_ROOT/release.json" || fail "The host could not reach the 1Helm release service." | |
| curl -fsSL --proto '=https' --tlsv1.2 --retry 3 \ | |
| --connect-timeout 10 --max-time 60 \ | |
| "$RELEASE_METADATA_URL" \ | |
| -o "$TEMP_ROOT/release.json" || fail "The host could not reach the 1Helm release service." |
🤖 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/update-host.sh` around lines 149 - 151, Update the curl
invocation for the release metadata request to include explicit connection and
overall operation timeouts, while preserving the existing retry and failure
handling around RELEASE_METADATA_URL.
1 participant
Outcome
Phase 5 cuts ordinary Linux app-update downloads by separating unchanged OCI machine-image bytes from the app package while retaining complete offline bundles.
Verification
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation