Add fail-closed WSL host runtime boundary - #77
Conversation
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Review summary
This is a clean, well-scoped boundary slice. I verified the current head (80f2817) directly:
- Enforcement placement is correct.
requireHostFrontend()runs inmain()afterhandleBootstrapCommand(socb version/help/configand bare-cbusage remain bootstrap-safe, matchingdocs/wsl.md) and beforeloadRegistry()— covering both management and shimargv[0]dispatches through a single gate. Noinit()functions or package-level vars perform host work that could bypass it. - Fail-closed direction is sound. Kernel markers — not env vars — drive Linux classification (the "standalone linux ignores env alone" test proves env can't promote a host), probe failure on
/proc/sys/kernel/osreleaserefuses, and on native WindowsCurrent()can never error, so the supported path can't be broken by the gate. - Docs are accurate. I checked the
docs/architecture.mdimport edges against the actual package imports (including the drive-bystatearchivecorrection) — they match.docs/security-model.mdand the README platform table are consistent with the code. - Tests are reasonable. Pure-function classification/enforcement tables, the boundary-precedes-registry ordering test, the bootstrap-doesn't-enforce test, and a real subprocess exit-120 check on non-Windows. CI passes on Linux and Windows.
Two minor findings, neither blocking:
Findings
[nit] Early WSL2 kernels are misclassified as WSL1 — internal/hostenv/hostenv.go, classify()
Microsoft shipped WSL2 kernels as 4.19.x-microsoft-standard (e.g. 4.19.84-microsoft-standard, 4.19.128-microsoft-standard, ~2019–2020) before the -WSL2 suffix was standardized. These contain microsoft but not wsl2, so a real (if very old) WSL2 host gets WSL1Native and the "WSL1 is unsupported" message instead of the WSL2-gated message. The host is still correctly refused, so the only consequence is a misleading diagnostic on legacy kernels. If desired, -microsoft-standard without wsl2 could map to a distinct "unrecognized Microsoft kernel" label, or the doc simplification in docs/wsl.md ("A Microsoft WSL kernel without the WSL2 marker is classified as WSL1") can simply stand as the acknowledged trade-off.
[suggestion] Interop rejection doesn't name the triggering marker — internal/hostenv/hostenv.go, requireFrontend() WindowsWSLInterop case
WSL_DISTRO_NAME and WSL_INTEROP propagate to Windows processes spawned through WSL interop and are inherited by their children, so a native-Windows process tree descending from an interop-launched ancestor (e.g. a terminal spawned via wt.exe from WSL) or a stray WSL_DISTRO_NAME set on the Windows host will refuse all non-bootstrap commands with "Windows cb.exe launched through WSL interoperability is unsupported". The message doesn't say which marker tripped the check, so a user on genuinely native Windows with an inherited/stray variable has no diagnostic handle. Including the detected marker name(s) in the error (and/or naming the invoked binary rather than hardcoding cb.exe, since shims hit the same path) would make the fail-closed failure self-diagnosing.
Minor test suggestion
On non-Windows, TestSubprocessExitCodes now only asserts cb doctor → 120. Asserting cb version → 0 in the same subprocess pass would prove bootstrap-safety end-to-end (currently only covered indirectly by the seam-stub unit test).
No blocking or important issues found — the slice does exactly what it claims and nothing more.
|
Addressed all three review notes in signed commit |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation is cohesive and well-tested, with only a minor error-message clarity nit identified.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR introduces a fail-closed host-runtime boundary so ContainerBin only proceeds with non-bootstrap work on supported native Windows hosts, while explicitly detecting (but still gating) WSL1/WSL2 and other non-Windows environments.
Changes:
- Add
internal/hostenvto conservatively classify host runtime (Windows native, Windows↔WSL interop, WSL1/WSL2, Linux, unsupported) and enforce a fail-closed frontend gate. - Enforce the host boundary early in
main()(after bootstrap-safe commands, before registry load) and add tests to lock in that ordering. - Document current WSL posture and the remaining work before WSL execution can be enabled.
| File | Description |
|---|---|
| README.md | Documents WSL2 as not yet supported and clarifies current limitations. |
| main.go | Enforces the new host boundary before registry/Docker work via hostenv.RequireFrontend. |
| main_test.go | Adds coverage ensuring the host boundary runs before registry load and bootstrap commands bypass it. |
| internal/hostenv/hostenv.go | Implements host runtime classification and fail-closed frontend enforcement. |
| internal/hostenv/hostenv_test.go | Adds unit tests for classification, enforcement messages, and bounded kernel-release reads. |
| exitcode_test.go | Adds non-Windows subprocess expectations (bootstrap version OK; doctor fails). |
| docs/wsl.md | New doc describing the WSL2 boundary and the gating rationale/work remaining. |
| docs/security-model.md | Adds fail-closed host boundary to the documented security model. |
| docs/architecture.md | Updates the architecture overview/import edges to include hostenv and related package notes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…e-boundary # Conflicts: # README.md # docs/architecture.md # docs/security-model.md # main.go # main_test.go
…e-boundary # Conflicts: # docs/architecture.md # main.go # main_test.go
## Summary - define fixed native-WSL user-local paths for the managed binary, management/tool shims, registry, lockfile, and private state - reject ambiguous or Windows-filesystem home paths and noncanonical distribution or machine identities - derive a stable opaque Docker-state namespace from the exact WSL distribution, canonical machine ID, and Linux UID - document the ownership/symlink contract and exact namespace requirements for later volume lifecycle wiring - keep the WSL frontend gated; this PR publishes no Linux artifact and makes no WSL support claim This slice starts from current `main` after #77 and is independent of the open project-overlay and self-update PRs. ## Validation - `gofmt` / `git diff --check` - `go vet ./...` - `go test -race ./...` - `python -m unittest -v internal/registry/pipx_wrapper_test.py internal/cli/pipx_discovery_test.py` - `go list -deps ./...` - Windows/amd64 release-style build with injected `v0.0.0-citest` version smoke Roadmap: #2 (WSL2 native config/shim/state layout foundation). Remaining WSL filesystem enforcement, namespace wiring, Docker Desktop integration, path/signal work, and real E2E qualification stay open. <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/avibacktoblack/container-bin/pull/83" target="_blank"><picture><source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-devin-review-dark.svg?v=4"><img src="https://static.devin.ai/assets/gh-devin-review-light.svg?v=4" alt="Devin Review"></picture></a> <!-- devin-review-badge-end -->

Summary
This is the first WSL2 foundation slice. It intentionally does not publish a Linux artifact or claim WSL support; later slices remain blocked on their own implementation and real WSL2 + Docker Desktop qualification.
Validation
go test -race ./...go vet ./...go list