Authenticate enterprise-managed registries - #84
Conversation
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Review: Authenticate enterprise-managed registries (initial review)
The design is sound and implemented carefully. The detached-signature scheme binds the exact container-bin.toml bytes before TOML parsing (registry.loadAt), the verified bytes are exactly the bytes parsed, the .sig envelope is strict (version, algorithm, canonical base64, key-id charset, bounded regular non-symlink file), key rotation/revocation/validity windows are handled, and every registry-mutating command (add, default set, expose incl. --shared-file, unexpose, uninstall, restore --apply) is denied before any side effect under require_registry_signature. install/setup correctly skip registry creation/upgrade in signed mode while still reconciling shims from an authenticated registry. Docs and tests are consistent with the code. All CI checks are green at review time.
Findings
[important] cb restore preview and cb backup are unreachable in signed mode precisely when they are needed — a missing or unauthenticated live registry
main() calls loadRegistry(machinePolicy.AuthenticateRegistry) unconditionally before dispatching (main.go:62). Neither cli.Backup nor cli.Restore consumes the loaded Registry — both take only cfgPath (which registry.Path()/bootstrapRegistryPath() can produce without reading the file) and re-read/authenticate the bytes themselves.
Consequence: on a require_registry_signature machine whose container-bin.toml is missing, corrupt, or fails authentication — the primary situation in which restore is needed — cb restore backup.zip exits at startup with registry: [policy.registry_signature_missing] (or _invalid) before ever opening the archive. The documented "verify and preview that archive" workflow is therefore unavailable exactly when an administrator would want to check which backup contains an authentic registry before re-provisioning. In unmanaged mode a missing registry falls back to Default() and cb restore still works, so this is a new signed-mode limitation.
Preview performs no mutation and writes nothing; gating it on live-registry validity buys no security. Suggestion: route cb restore/cb backup through a path that only resolves cfgPath (like the existing bootstrap commands), or tolerate loadRegistry failure for these registry-independent commands. If the current behavior is deliberate, docs/enterprise-policy.md should state that signed-mode cb restore preview requires an intact live registry.
[nit] .sig hygiene asymmetry in cli.Restore --apply (unmanaged mode)
atomicio.WriteFile(cfgPath+".sig", b, 0644)(internal/cli/cli.go:1357) writes the archive's envelope without the 16 KiB / regular-file bound thatLoadRegistrySignatureenforces everywhere else. A restored oversized.sigthen makes every subsequentcb backupfail withpolicy.registry_signature_invalid— restore can create a state backup cannot handle.- When the archive contains no
container-bin.toml.sig, an existing.sigon disk is left in place, unlike the lockfile which is deleted when absent (cli.go:1367-1369). A stale.sigpaired with the restored bytes then gets archived verbatim by future unmanagedcb backupruns and fails closed (loudly, which is the safe direction) if signing is later enabled. Mirroring the lockfile's delete-when-absent branch for.sigwould make restore reproduce the archive state faithfully.
[suggestion] Rollback window worth one doc sentence
Any previously valid container-bin.toml + .sig pair remains acceptable until its key is revoked or expires — there is no freshness/serial binding, which is inherent to a detached-file scheme. cb backup archives now conveniently ship complete signed pairs. For anyone able to write the registry directory, restoring an older signed pair (e.g., re-adding a tool that was removed) is possible while the key is active. docs/enterprise-policy.md could note that revocation is the only remedy for superseded signed content.
[suggestion] Test coverage gaps
Signed-mode cli.Backup (verified envelope captured into the archive) and cli.Restore --apply .sig write/removal are not exercised; current tests cover the unsigned-preservation path and signed-mode denial paths only.
Verified
- All production registry loads pass
machinePolicy.AuthenticateRegistry;loadAtrejects a nil authenticator. - Missing signed registry →
registry_signature_missingbefore.bakrecovery orDefault()fallback; recovered.bakbytes would also be re-authenticated. - Backup archives the verified
(toml, sig)snapshot; restore authenticates the archive snapshot beforeParseTOMLand denies--applyunder signed mode. - Policy parsing rejects signature fields under schema 1, requires ≥1 active non-revoked key when signing is required, rejects duplicate key IDs/public keys, and enforces canonical UTC timestamps.
No blocking issues found.
## Summary - add an unexposed Linux-only preflight for the fixed native WSL layout - require current-user ownership, strict managed modes, real directories/files, safe symlinks, and a home on the distribution root filesystem device - roll back only directories created by a failed preparation attempt - refresh WSL architecture and live roadmap status after merged PRs #80-#84 and #87 The WSL frontend remains gated; this PR does not publish a Linux artifact or claim WSL support. ## Validation - `gofmt -l .` - `go vet ./...` - `go test -race ./...` - `python3 -m unittest -v internal/registry/pipx_wrapper_test.py internal/cli/pipx_discovery_test.py` - Windows amd64 and arm64 compile checks for `./internal/wslfs` Roadmap: #2 <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/avibacktoblack/container-bin/pull/88" 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
container-bin.tomlbytescontainer-bin.toml.sigenvelope before registry parsing, built-in fallback, backup recovery, shim reconciliation, or Docker workcbmutation commands while allowingsetup/installto reconcile shims without creating or upgrading the registryThis starts from current
mainafter the enterprise-policy foundation merged in #76. It is independent of open project-overlay, self-update, and WSL work.Validation
gofmt/git diff --checkgo vet ./...go test -race ./...python -m unittest -v internal/registry/pipx_wrapper_test.py internal/cli/pipx_discovery_test.pygo list -deps ./...v0.0.0-citestinternal/policytest binaryRoadmap: #2 (registry-signature enterprise policy). The roadmap remains authoritative and this item is not complete until the PR is merged.