Add enterprise machine policy foundation

[AviBackToBlack]

Summary

• load a strict versioned machine policy from the fixed Windows or Linux path and fail closed on invalid ownership, syntax, expiry, or version
• enforce mandatory locks, explicit local-image exceptions, and canonical repository-boundary allowlists against configured and resolved lock references before Docker activity
• preflight bulk lock/update, restore, runtime-default, add, expose, diagnostics, self-test, and shim execution paths without allowing optional policy propagation
• expose stable policy error codes, fingerprinted diagnostics, operator documentation, and stricter immutable repository lock validation

Validation

• go test -race ./...
• go vet ./...
• Windows amd64 release-style build with injected v0.0.0-citest version
• built Windows binary smoke test: version output matched v0.0.0-citest
• git diff --check

Roadmap: enterprise-policy foundation in issue #2. Registry signatures and image-signature trust remain intentionally separate follow-up units.

[CherylSnowVeil]

Review — enterprise machine policy foundation

Initial review of 9e50bb6. Overall this is a careful, well-scoped implementation: strict fail-closed parsing, canonical repository normalization, both-sides lock authorization (configured + resolved), preflight-before-mutation ordering in bulk lock/update, stable error codes, and solid test coverage including the "denied before Docker" preflight tests. The stricter immutable repository-digest validation in lockfile.Load is good defense in depth. A few issues below, none structural.

Findings

1. 🟡 [important] Policy file is read before its ownership/permissions are verified
internal/policy/policy.go — loadAt calls os.ReadFile(path) first, then ownership(path). On a mis-deployed, user-writable policy directory — the exact condition verifyOwnership is designed to detect — a user can plant a FIFO (or other blocking special file) at the policy path. os.ReadFile then blocks indefinitely before the ownership check runs, so every cb invocation hangs silently instead of failing closed with [policy.ownership]. Reorder: Lstat/ownership verification first, then read — and consider requiring info.Mode().IsRegular() in verifyOwnership, since a root-owned FIFO would pass the current checks and still block the read.

2. 🟡 [important] Windows ownership verdict is produced by a PATH-resolved powershell
internal/policy/ownership_windows.go — exec.Command("powershell", ...) resolves via PATH, and the ACL verdict is parsed from that process's stdout. A powershell shim earlier in PATH can emit a forged OWNER|S-1-5-18 response and pass verification for a policy file the check should reject. Admittedly narrow — it only pays off where the file grants a non-admin principal write-but-not-delete rights (otherwise deleting the file for unmanaged mode is strictly easier) — but this is the security core of the feature on the primary platform. Separately, a Windows environment where powershell.exe isn't on PATH fails closed with a misleading policy.ownership error on a perfectly valid policy. Note the existing powershell uses in internal/diag are best-effort diagnostics, not security verdicts, so the precedent doesn't cover this. Recommend resolving the absolute %SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe, or querying the ACL natively (also avoids two PowerShell spawns — and their startup latency — on every shim invocation under a managed policy).

3. 🟡 [important] cb add cannot express local-image intent and over-denies under allowlist policies
internal/cli/cli.go — add calls machinePolicy.AuthorizeLockTarget(image, false) unconditionally. Under a managed policy with allowed_repositories set plus allow_local_images = true, cb add TOOL --image mylocal:dev fails with [policy.repository_denied] — even though the policy explicitly permits local images and cb lock --local TOOL would authorize the same profile added via a manual registry edit. There is no --local flag or other way for add to declare intent. Either add a flag or document that local-image tools must be added by editing container-bin.toml under allowlist policies.

4. 💡 [suggestion] statearchive helper image runs outside policy authorization
cb backup --state / cb restore --state run docker run on the compiled-in docker.io/library/alpine@sha256:… helper (internal/statearchive/statearchive.go) with no machinePolicy check. It is digest-pinned, --pull never, read-only and networkless, so treating it as outside the "user-resolved request" scope is defensible — but docs/enterprise-policy.md says authorization occurs before "docker run" without noting the exemption, and an admin using allowed_repositories to fence off Docker Hub would reasonably expect it covered. Worth an explicit decision: authorize it too (it's already an exact immutable reference), or document the carve-out.

5. 🟢 [nit] cb lock --check dropped the presence pass for remaining images
internal/cli/cli.go — the check now returns after the MISSING/DENIED pass, so OK/ABSENT status for the other locked images is never reported when any entry is missing or denied; previously all states were shown in one pass.

6. 🟢 [nit] Single-segment repository rules may surprise
canonicalRule("python") → docker.io/python (a namespace boundary), while python:3.13 → docker.io/library/python — so allowed_repositories = ["python"] does not authorize the official python image. Fail-closed and internally consistent, but a doc note in enterprise-policy.md (e.g. "use library or docker.io/library for official Docker Hub images") would prevent admin confusion.

Notes

• Verified no docker run/pull/inspect path for configured images bypasses lockfile.RuntimeImageForTool / AuthorizeLockTarget / authorizeRegistrySnapshot; bootstrap commands (version, help, config) correctly return before policy I/O.
• SetDefaultVersion only accepts configured DefaultVersion labels, so the cli.Default preflight's candidate.DefaultVersion == version comparison matches the same semantics — no alias bypass.
• No new dependencies; CI was still running at review time.

[AviBackToBlack]

Addressed the review in signed commit

[CherylSnowVeil]

Re-review — 082ab5e (fix: harden enterprise policy enforcement)

Validation of previous findings (from review of 9e50bb6)

1. Policy file read before ownership check — ✅ fixed. loadAt now Lstats first, requires info.Mode().IsRegular() (rejecting FIFOs, symlinks, dirs) with [policy.ownership], verifies ownership, then os.ReadFile. The blocking-read vector is closed. A residual check-then-read swap would require write access to a directory the verifier itself just proved is admin-owned — acceptable.
2. PATH-resolved powershell produces the ACL verdict — ✅ fixed, with one residual gap noted as a new finding below. powerShellExecutable resolves an absolute %WindowsDir%\System32\WindowsPowerShell\v1.0\powershell.exe via GetWindowsDirectoryW and requires a regular file — PATH-forged shims can no longer fabricate the OWNER|S-1-5-18 verdict.
3. cb add could not express local-image intent — ✅ fixed. --local is parsed, threaded into AuthorizeLockTarget, and the follow-up messages point at cb lock --local / cb update --local. Covered by TestAddLocalIntentUsesLocalPolicyAndReportsLocalLockCommand.
4. statearchive helper outside policy — ✅ addressed by documentation. enterprise-policy.md now states the compiled-in digest-pinned helper is outside allowed_repositories, and the claim matches the implementation (--pull never, --network none, --read-only).
5. cb lock --check dropped the presence pass — ✅ fixed. checkLock reports MISSING/DENIED for all configured images, then OK/ABSENT for authorized candidates; denied refs never reach docker image inspect. Covered by TestCheckLockReportsEveryStatusAfterPolicyPreflight.
6. Single-segment rule surprise — ✅ fixed. The doc now spells out that python means the docker.io/python namespace, not docker.io/library/python.

Findings from a complete fresh review of 082ab5e

1. 🔴 [blocking] The documented schema example cannot be parsed

docs/enterprise-policy.md:40-44 shows allowed_repositories as a multi-line array:

allowed_repositories = [
  "docker.io/library",
  ...
]

The policy parser (internal/policy/policy.go parse) is strictly line-oriented: it scans line-by-line and calls toml.ParseStringArray on each line's value. ParseStringArray requires [ and ] on the same line, so allowed_repositories = [ fails with [policy.syntax] line 4 allowed_repositories: expected array of quoted strings, and the continuation lines fail expected key = value. An administrator who provisions the policy by copying the reference example ends up with a fail-closed deployment — every non-bootstrap cb invocation (all shims included) stops at machine policy: [policy.syntax]. Fix the example to a single-line array (or teach the parser multi-line arrays). Note the same multi-line style already appears for registry arrays in README.md (host_mounts) and docs/proxy-airgap.md (env_names), which the same line-oriented ParseStringArray path cannot parse either — worth correcting in the same sweep, but that is pre-existing.

2. 🟡 [important] GetWindowsDirectoryW can return a per-user Windows directory under Terminal Services

internal/policy/ownership_windows.go powerShellExecutable pins powershell.exe under GetWindowsDirectoryW's result. Under Terminal Services application-compatibility mode, GetWindowsDirectoryW returns the calling user's private Windows directory (under their profile, user-writable) rather than the shared system directory — GetSystemWindowsDirectoryW is the API that always returns the shared one. In that environment a user could plant System32\WindowsPowerShell\v1.0\powershell.exe under their private Windows directory and regain the forged-verdict bypass that the PATH fix closed (or get a spurious policy.ownership failure if the private directory exists but has no PowerShell). Narrow — requires TS app-compat and the enterprise-policy scenario makes RDSH/AVD plausible — but GetSystemWindowsDirectoryW is a one-word fix with identical behavior elsewhere.

3. 💡 [suggestion] Two PowerShell spawns per managed invocation

verifyOwnership still shells out twice (parent dir + file) on every cb call under a managed policy, including every shim execution. Resolving the system binary was the right minimal fix, but a native ACL read (or caching the verdict) would remove a few hundred ms of PowerShell startup from each shim invocation. Non-blocking; already flagged last round as the alternative remediation.

Verified during fresh review (no issues)

• Every docker run/pull/inspect path that consumes a configured or resolved image ref is gated: dockerrun.RunTool/EnsureImageLocalForTool → RuntimeImageForTool; expose discovery and shared-file inspection → RuntimeImageForTool; cb lock/cb update preflight all targets via AuthorizeLockTarget before any Docker call; cb restore preflights the archived registry+lock via authorizeRegistrySnapshot before writes; cb lock --check and cb doctor authorize resolved refs before inspection; cb default set authorizes every candidate matching DefaultFamily/DefaultVersion — the same semantics SetDefaultVersion enforces. The statearchive helper and dockervol/docker version calls are the only ungated Docker use, and the former is now an explicit documented exemption.
• lockfile.Load's stricter immutable-digest validation (repo match via matchRepoDigest, digest consistency) is consistent with what ResolveRepositoryImage produces, so existing lockfiles written by cb lock/cb update remain valid.
• CanonicalRepository/canonicalRule normalization is fail-closed on the edges I probed (schemes, .., trailing @, IPv6, docker.io. FQDN, port handling); boundary matching is segment-wise (evil.com.evil2.com does not match evil.com).
• Bootstrap commands (version/config/help, bare cb) still return before policy I/O — the recovery path the docs promise — and loadPolicy now covers both management and shim dispatch.
• CI is green on both platforms (format/vet/test, Windows test+build, govulncheck, CodeQL, release reproducibility).

[devin-ai-integration]

Devin Review found 1 potential issue.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved moderate policy authorization and lock-validation issues remain, along with a documentation nit.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

• Authorize unlocked images before generic lockfile errors · New
• Normalize unqualified repository rules with the docker.io prefix · New

What changed in this PR

Adds administrator-controlled machine-policy enforcement for image authorization, lockfiles, Docker execution, diagnostics, and management operations.

Changes:

• Adds strict policy loading, ownership checks, expiry/version validation, and stable errors.
• Enforces repository allowlists and locking across execution and CLI workflows.
• Strengthens immutable lock validation and documents the policy model.

File	Reviewed changes
README.md	Documents local images and enterprise policy.
main.go	Loads and propagates machine policy.
main_test.go	Tests bootstrap policy-loading behavior.
internal/​policy/​policy.go	Implements policy parsing and authorization. moderate (2 votes): Unqualified rules such as astral-sh/uv are not normalized with the docker.io prefix and cannot authorize the corresponding image.
internal/​policy/​policy_test.go	Tests policy validation and normalization.
internal/​policy/​ownership_windows.go	Validates Windows ownership and ACLs.
internal/​policy/​ownership_windows_test.go	Tests Windows ownership helpers.
internal/​policy/​ownership_unix.go	Validates Unix ownership and permissions.
internal/​lockfile/​lockfile.go	Enforces immutable repository locks and policy checks. moderate (2 votes): Missing image entries bypass authorization and return generic errors instead of stable policy errors. moderate (1 vote): Repository syntax is not strictly validated before accepting matching immutable references.
internal/​lockfile/​lockfile_test.go	Tests lock validation and policy preflight.
internal/​dockerrun/​dockerrun.go	Authorizes images before Docker execution.
internal/​diag/​diag.go	Adds policy-aware diagnostics and self-tests. moderate (1 vote): The missing-entry path skips authorization and can hide allowlist or required-lock failures.
internal/​cli/​cli.go	Applies policy to management operations.
internal/​cli/​cli_test.go	Tests policy-aware CLI behavior.
docs/​security-model.md	Documents the machine-policy trust boundary.
docs/​proxy-airgap.md	Reformats proxy configuration examples.
docs/​enterprise-policy.md	Documents policy schema and operational behavior.
docs/​architecture.md	Documents policy flow and dependencies. nit (1 vote): The RunTool example in docs/shell-contract.md still shows the old two-argument call.

---

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

[AviBackToBlack]

Addressed the complete second-round review in signed commit bfa36ed. The blocking copy-paste failure is fixed: the enterprise policy example now uses the supported single-line array grammar, and the two adjacent pre-existing multiline registry examples in README and docs/proxy-airgap.md were corrected in the same sweep; no published TOML example now opens an array across lines. The Windows trust path now calls GetSystemWindowsDirectoryW, not GetWindowsDirectoryW, so Terminal Services per-user Windows-directory virtualization cannot redirect the PowerShell ACL verdict. Full race tests and vet pass, and the Windows policy tests cross-compile for amd64 and arm64. I am intentionally not adding ACL-verdict caching in this security foundation: correct invalidation across ACL, owner, file, and parent changes needs a separate design, while native ACL loading is a larger performance follow-up rather than a correctness patch.

[Copilot]

Copilot review overview

🟡 Changes recommended

Three critical validation and policy-enforcement issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3

Open (3)

• Reject malformed repository digests with extra @​ or tagged repositories · New
• Deny Windows WriteData permission on policy files · New
• Fix trailing commas in multiline allowed-repositories arrays · New

Resolved since last review (2)

• Normalize unqualified repository rules with the docker.io prefix
• Authorize unlocked images before generic lockfile errors

[Copilot]

Copilot review overview

🔵 Needs a closer look

Two moderate findings leave policy checks incomplete in bulk locking and diagnostics; documentation and diagnostic nits also remain.

Review effort: Lite
Findings: None

Resolved since last review (5)

• Fix trailing commas in multiline allowed-repositories arrays
• Deny Windows WriteData permission on policy files
• Reject malformed repository digests with extra @​ or tagged repositories
• Normalize unqualified repository rules with the docker.io prefix
• Authorize unlocked images before generic lockfile errors

Previously missed (1)

In code that hasn't changed since last review

Authorize missing lockfile images under managed policies

internal/​diag/​diag.go:245

When a lockfile exists but lacks this configured image, this branch skips machine-policy authorization and only increments the generic missing count. Under a managed policy, cb doctor therefore omits the stable [policy.lock_required] or [policy.repository_denied] diagnostic that runtime resolution and the missing-lockfile branch provide. Authorize the image as unlocked before continuing.