Skip to content

Ship deterministic native WSL release artifact - #129

Open
AviBackToBlack wants to merge 3 commits into
mainfrom
codex/wsl-release-artifact
Open

AviBackToBlack wants to merge 3 commits into
mainfrom
codex/wsl-release-artifact

Conversation

@AviBackToBlack

@AviBackToBlack AviBackToBlack commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • publish a deterministic linux-amd64 tarball containing only cb, LICENSE, and README.md
  • bind the WSL archive in a separate one-entry SHA256SUMS-WSL manifest and include it in same-run plus independent-run byte-for-byte reproduction
  • keep the historical three-entry Windows SHA256SUMS contract unchanged so released v1.x clients can self-update directly to v2 on amd64 and ARM64
  • include the WSL archive in draft-release validation, provenance attestation, and release publication
  • document a verified WSL bootstrap flow without expanding support to standalone Linux or adding native WSL self-update behavior

Validation

  • gofmt -l .
  • go vet ./...
  • go test ./...
  • go test -race ./...
  • python -m unittest -v internal/registry/pipx_wrapper_test.py internal/cli/pipx_discovery_test.py
  • Windows amd64 release-style build with injected v2.0.0-rc.compat version
  • workflow YAML parsed successfully
  • isolated Docker build reproduced the WSL archive and both checksum manifests byte-for-byte across two runs
  • exact archive member allowlist and strict three-entry Windows / one-entry WSL manifest shapes verified
  • v1.1-to-v2 plan, staging, and verification validation exercised for Windows amd64 and ARM64
  • git diff --check

The locally reproduced WSL archive SHA-256 was 4444f72d410d4c9781ade0bcedcd2078f1ec071b551246e20043aa1a8c467f49 in both runs.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Devin Review

Comment thread internal/selfupdate/selfupdate.go Outdated
Comment thread .github/workflows/release.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The build reproducibility step always fails, and v2 self-update apply rejects the newly generated checksum layout.

Review effort: Balanced
Findings: 2 High severity · 1 Low severity

Open (3)
What changed in this PR

Adds a deterministic Linux/amd64 bootstrap artifact for the native WSL2 frontend and integrates it into the v2 release and verification pipeline.

Changes:

  • Builds, reproduces, checksums, attests, and publishes the WSL tarball.
  • Requires four-entry checksum manifests for v2 releases.
  • Documents WSL bootstrap and release qualification.
File Description
.github/​workflows/​release.yml Builds and publishes the WSL artifact.
README.md Documents the artifact and checksum layout.
docs/​wsl.md Adds the verified bootstrap procedure.
docs/​release-matrix.md Adds WSL release qualification guidance.
docs/​roadmap-decisions.md Updates WSL delivery status.
docs/​roadmap-implementation-requirements.md Records artifact implementation progress.
internal/​selfupdate/​selfupdate.go Recognizes v2 WSL release layouts.
internal/​selfupdate/​selfupdate_test.go Tests v2 release planning.
internal/​selfupdate/​verify.go Verifies four-entry manifests.
internal/​selfupdate/​verify_test.go Tests v2 manifest requirements.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/release.yml Outdated
Comment thread internal/selfupdate/selfupdate.go Outdated
Comment thread docs/release-matrix.md

@CherylSnowVeil CherylSnowVeil left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial review of the current head (8a7a780). The Build release bundle check on this revision has already failed — details in finding 1.

Summary

The PR adds a deterministic linux-amd64 tar.gz WSL bootstrap artifact to the release workflow (fixed owner/mtime ustar + gzip -n, exact member allowlist, executed-version assertion, byte-for-byte reproduction gate, provenance + checksum-manifest binding) and extends the self-update verifier with checksumLayoutV2WSL, requiring a four-entry manifest for v2 targets while preserving the legacy two-entry and dual-Windows three-entry layouts. The overall design and the verifier-side changes are coherent, and the new plan/verify tests cover the new layout. However, two blocking defects slipped through — one breaks the workflow itself on every run, the other breaks self-update --apply for every v2 release.

Findings

🔴 [blocking] .github/workflows/release.yml — the in-job "Verify reproducibility" step was not updated and now always fails

The build job's Verify reproducibility step (lines ~127–170) still rebuilds $RUNNER_TEMP/repro/SHA256SUMS with only cb.exe and the two Windows ZIPs (lines 160–165), then cmps it against dist/SHA256SUMS — which now contains a fourth line for container-bin-*-linux-amd64.tar.gz. The cmp therefore exits non-zero under set -euo pipefail on every PR, main-push, and tag run.

This is confirmed, not speculative: the check on this head already failed with cmp: EOF on /home/runner/work/_temp/repro/SHA256SUMS after byte 317, line 3 (run 37245381529), which also skips the downstream Reproduce release bundle independently job. On a tag run this blocks the draft release entirely.

The step also doesn't rebuild or compare the WSL artifact at all. The natural fix mirrors the dedicated reproducibility job: build the linux binary in the repro dir, create the tarball with the same flags, and add "${WSL_AMD64_ARCHIVE}" to the manifest and the compare loop.

🔴 [blocking] internal/selfupdate/stage.go (lines 235–245) — cb self-update --apply to any v2 release fails at staging

validateRelease now returns checksumLayoutV2WSL for every v2 target, but validateStagingPlan still implements the old two-layout rule:

wantLayout := checksumLayoutLegacyAMD64
if plan.Arch == "arm64" {
    ...
    wantLayout = checksumLayoutDualArch
} else if plan.checksumLayout == checksumLayoutDualArch {
    wantLayout = checksumLayoutDualArch
}
if plan.checksumLayout != wantLayout {
    return errors.New("self-update staging plan has an unexpected checksum layout")
}

For a v2 plan wantLayout is checksumLayoutLegacyAMD64 (amd64) or checksumLayoutDualArch (arm64) while plan.checksumLayout is checksumLayoutV2WSL, so staging always errors out. updateCommand.run calls c.stage before c.verify, so --apply to a v2 release is dead on both architectures — --check is the only path that works, because it returns before staging. The new tests exercise Plan and Verify but nothing stages a v2 plan, which is why this wasn't caught.

Remediation: validateStagingPlan already computes target (line 209); it should use the same validChecksumLayout(target, plan.Arch, plan.checksumLayout) helper this PR added in verify.go, and a validateStagingPlan test for a V2WSL plan on both arches would lock it in.

🟡 [important] No previously shipped binary can self-update to the first v2 release

The strict-manifest verifier in every already-released build ignores the WSL asset (it isn't in the old wanted map), plans a v2 update as checksumLayoutDualArch, stages successfully, then rejects the real four-entry SHA256SUMS with checksum manifest contains unexpected asset "container-bin-vX.Y.Z-linux-amd64.tar.gz" in verifyChecksumManifest. The fail-closed behavior is the right security property — but as a rollout consequence, self-update --apply v1.x → v2 fails for all deployed installs. If that break is intentional, it's worth stating explicitly (release notes / docs: updating to v2 requires first installing a v1 build carrying this verifier change, or a manual reinstall); if not, a v1 bridge release needs to ship before v2 is tagged.

🟢 [nit] docs/roadmap-decisions.md (lines ~208–209)

The self-update paragraph still says the verifier "requires the canonical three-entry layout for dual-architecture releases". V2 releases are dual-Windows-architecture releases and require four entries; a small consistency update alongside the README change would avoid confusion.

Notes

CI at review time: Build release bundle failed (finding 1); Test and build (Windows) / Windows ARM64 were still running; all other completed checks pass, including Format, vet, test (Linux), govulncheck, zizmor, and dependency review. The docs/wsl.md bootstrap sequence (verify → extract on the distribution-local filesystem → ./cb wsl prepare --apply → ./cb wsl install --apply) is consistent with wslinstall using os.Executable as the install source and the ~/.local/bin/cb management-shim layout.

@CherylSnowVeil CherylSnowVeil left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of the current head (a1cf819). The design changed since my initial review: the four-entry manifest and checksumLayoutV2WSL were dropped in favor of a separate one-entry SHA256SUMS-WSL, so all selfupdate code is now unchanged except for a new regression test.

Validation of previous findings

  1. 🔴 Verify reproducibility step always failing — fixed. The step now rebuilds the linux binary, packs the tarball with identical tar --format=ustar --owner=0 --group=0 --numeric-owner --mtime='@0' | gzip -n flags, generates SHA256SUMS-WSL, and the compare loop covers all six artifacts. Confirmed by the now-green Build release bundle and Reproduce release bundle independently checks on this head.
  2. 🔴 validateStagingPlan rejecting v2 plans — obsolete (code removed). checksumLayoutV2WSL no longer exists; v2 plans keep checksumLayoutDualArch, which staging and verification already accept. The new TestPlanKeepsV1ToV2WindowsSelfUpdateCompatibleWithSeparateWSLAssets locks this in: it plans v1.1.0 → v2.0.0 with the two extra assets present and calls validateStagingPlan and validateVerificationPlan on both arches.
  3. 🟡 v1 → v2 self-update broken — fixed. SHA256SUMS remains the strict three-entry Windows manifest (wc -l -eq 3 asserted in both the build step and draft-release validation), and released verifiers skip the two new assets via the wanted-map continue in validateRelease (selfupdate.go:412). The extra assets never reach verifyChecksumManifest, and the three required names still match checksumManifestNames exactly. Direct v1.x → v2 self-update is preserved, as the PR description claims.
  4. 🟢 Stale "three-entry layout" text in docs/roadmap-decisions.md — fixed; it now says "dual-Windows-architecture" and documents the separate WSL manifest, matching README and docs/release-matrix.md.

Fresh review of the current diff

I re-reviewed the full workflow and docs from scratch: tag/non-tag VERSION derivation, executed-version assertion on the linux binary (cb version prints container-bin <version> from the injected main.version, matching line 82's test), deterministic member order and the exact-member allowlist check, both manifest shape assertions, the subject-path attestation glob covering *.tar.gz, artifact upload globs, and the gh release create asset list. I also verified the docs/wsl.md bootstrap sequence against wslfs/wslinstall (cb wsl prepare --apply, cb wsl install --apply, ~/.local/bin/cb shim layout, os.Executable install source) and confirmed the published-release qualification script does not enumerate release assets, so the two new assets don't perturb it.

🟢 [nit] docs/wsl.md — bootstrap omits the gh prerequisite

The bootstrap block runs gh attestation verify, but a fresh WSL distribution has no gh installed, and attestation verification requires authentication (gh auth login/GH_TOKEN) even for public repositories. The command fails with a clear error, so this is self-diagnosing, but a one-line prerequisite note (install gh and authenticate before running the block) would keep the documented sequence copy-pasteable end to end.

💡 [suggestion] Tar member modes inherit the runner umask

cp LICENSE README.md leaves modes as source&~umask and tar records them (the cb binary is 0755 from go build, so the executable bit is fine either way). If a future ubuntu-24.04 image ever changed the default umask, the byte-for-byte gates would catch it — the pipeline fails closed, so nothing ships wrong — but pinning modes explicitly (--mode= on tar, or a chmod) would remove the last ambient-input dependency in the otherwise fully normalized archive. Optional.

Summary

No blocking or important findings at a1cf819. The revised separate-manifest approach is simpler than the original four-entry design, preserves the released v1 verification contract verbatim, and the new regression test covers the exact compatibility property that mattered. All CI is green. Nothing further needed before merge beyond the optional doc nit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants