From 9e50bb6eb22bb5a212f7f3a8606151a88c665581 Mon Sep 17 00:00:00 2001 From: AviBackToBlack <54722547+AviBackToBlack@users.noreply.github.com> Date: Sat, 19 Sep 2026 19:12:53 +0100 Subject: [PATCH 1/5] feat: add enterprise machine policy foundation --- README.md | 14 + docs/architecture.md | 30 +- docs/enterprise-policy.md | 99 ++++++ docs/security-model.md | 11 + internal/cli/cli.go | 150 ++++++++-- internal/cli/cli_test.go | 120 ++++++-- internal/diag/diag.go | 58 ++-- internal/dockerrun/dockerrun.go | 9 +- internal/lockfile/lockfile.go | 30 +- internal/lockfile/lockfile_test.go | 32 ++ internal/policy/ownership_unix.go | 33 ++ internal/policy/ownership_windows.go | 61 ++++ internal/policy/ownership_windows_test.go | 27 ++ internal/policy/policy.go | 349 ++++++++++++++++++++++ internal/policy/policy_test.go | 132 ++++++++ main.go | 38 ++- main_test.go | 6 + 17 files changed, 1097 insertions(+), 102 deletions(-) create mode 100644 docs/enterprise-policy.md create mode 100644 internal/policy/ownership_unix.go create mode 100644 internal/policy/ownership_windows.go create mode 100644 internal/policy/ownership_windows_test.go create mode 100644 internal/policy/policy.go create mode 100644 internal/policy/policy_test.go diff --git a/README.md b/README.md index 9550c6d..c7b1056 100644 --- a/README.md +++ b/README.md @@ -614,6 +614,20 @@ An image-ID lock is deliberately host-local: it makes execution immutable on that Docker daemon, but it does not make the image portable or pullable. Keep the Dockerfile/build inputs or export the image separately for recovery. +## Enterprise machine policy + +Administrators can constrain resolved user configuration through a fixed, +machine-owned policy at `C:\ProgramData\ContainerBin\policy.toml` (Windows) or +`/etc/container-bin/policy.toml` (native Linux/WSL). A missing policy preserves +unmanaged behavior. A present but unreadable, invalid, expired, unsupported or +insufficiently protected policy fails closed before non-bootstrap work. + +Schema 1 can require an exact image lock, reject local image-ID locks unless +explicitly allowed, and allowlist canonical registry/repository boundaries. +Lower-precedence registry or command-line choices cannot weaken it. See +[enterprise machine policy](docs/enterprise-policy.md) for the schema, +ownership rules, normalization behavior and stable diagnostic codes. + ## State inspection and garbage collection ```powershell diff --git a/docs/architecture.md b/docs/architecture.md index 2080610..6572200 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -9,12 +9,14 @@ is configuration (`container-bin.toml`), a generated lockfile ``` NAME.exe (hardlink to cb.exe) → argv[0] dispatch main() inspects its own invocation name + → machine policy load fixed admin path, ownership/version validated → registry profile lookup container-bin.toml, schema-validated, fail-closed → argv normalization repair PowerShell-split "-opt=" "value" pairs → path mapping conservative Windows→container translation → host_mounts resolution explicit registry-declared bind mounts, provider-agnostic → provider assembly stateless | python | stateful volume/env setup → image lock resolution container-bin.lock digest, fail-closed + → policy authorization lock/local-origin/repository constraints → docker run --rm ... stdio passthrough, exit code preserved ``` @@ -166,6 +168,21 @@ a fresh matching RepoDigest, while local locks only re-inspect the configured tag and record its current image ID. A missing local tag is an error, not an implicit switch to a registry image. `cb update --local TOOL` and `cb update --registry TOOL` are the explicit mode-switch operations. +Repository entries are accepted only as an immutable, valid SHA-256 RepoDigest +whose repository matches the configured reference after Docker Hub alias +normalization; a mutable tag or foreign repository in a hand-edited lockfile is +invalid. + +## Enterprise policy + +`internal/policy` is deliberately independent of registry parsing. `main` +loads it from the fixed machine path before the user registry, then passes the +immutable result to request resolution and diagnostics. The zero value means +unmanaged operation. Managed policy authorizes the final configured image plus +its lock identity; repository-mode lock creation is authorized before pull. +This preserves the precedence boundary: user/project/CLI layers may choose a +request, but only the machine layer can authorize it. Full schema and ownership +rules are in [enterprise-policy.md](enterprise-policy.md). ## Atomic writes @@ -229,6 +246,7 @@ internal/state cb state, cb gc ↓ internal/dockervol docker volume primitives (leaf) internal/lockfile container-bin.lock, digest resolution +internal/policy fixed machine policy, ownership and authorization ↓ internal/pathmap Windows path classification and mapping, project roots, volume naming @@ -245,14 +263,16 @@ The exact import edges, from `go list -f '{{.ImportPath}} {{.Imports}}' ./...`, project-internal imports only: ``` -main -> cli, diag, dockerrun, mutationlock, registry, state -cli -> atomicio, diag, dockerrun, lockfile, pathmap, registry, toml -diag -> dockerrun, dockervol, lockfile, pathmap, registry -dockerrun -> dockervol, lockfile, pathmap, registry +main -> cli, diag, dockerrun, mutationlock, policy, registry, state +cli -> atomicio, diag, dockerrun, lockfile, pathmap, policy, registry, statearchive, toml +diag -> dockerrun, dockervol, lockfile, pathmap, policy, registry +dockerrun -> dockervol, lockfile, pathmap, policy, registry state -> dockervol, pathmap, registry -lockfile -> atomicio, registry, toml +statearchive -> dockervol, pathmap +lockfile -> atomicio, policy, registry, toml pathmap -> registry registry -> atomicio, toml +policy -> toml atomicio, dockervol, mutationlock, toml -> (leaves) ``` diff --git a/docs/enterprise-policy.md b/docs/enterprise-policy.md new file mode 100644 index 0000000..7d98e69 --- /dev/null +++ b/docs/enterprise-policy.md @@ -0,0 +1,99 @@ +# Enterprise machine policy + +ContainerBin can apply a fixed, administrator-owned authorization policy after +the user registry, lockfile and command line have resolved a request. The +policy is a constraint layer, not another registry: it cannot add profiles, +change their settings or be weakened by `container-bin.toml`. + +## Location and protection + +The location is compiled in and has no environment-variable or command-line +override: + +- Windows: `C:\ProgramData\ContainerBin\policy.toml` +- native Linux and WSL: `/etc/container-bin/policy.toml` + +A missing file means unmanaged operation and preserves the existing behavior. +A present file must be readable, valid and sufficiently protected. Otherwise +every non-bootstrap invocation fails before registry mutation or Docker use. +`cb version`, `cb config` and help remain available for recovery. + +On Windows, neither the policy nor its parent directory may be a reparse point. +Both must be owned by `SYSTEM` or the built-in Administrators group. The policy +file may not grant write, modify, delete, ownership or permission-changing +rights to another principal. The parent may allow users to create entries, but +may not let them delete or replace protected children or change ownership or +permissions. A user-created lookalike still fails the file-owner check. + +On Linux/WSL, both the file and parent directory must be real paths owned by +root and may not be group- or world-writable. + +ContainerBin never creates or edits this file. Provision it and its ACL/mode +with the machine's normal administrator configuration-management mechanism. + +## Schema 1 + +```toml +policy_version = 1 +require_lock = true +allow_local_images = false +allowed_repositories = [ + "docker.io/library", + "ghcr.io/acme/developer-tools", + "registry.example.com:5443/platform" +] +expires_at = "2027-01-01T00:00:00Z" +``` + +Unknown or duplicate keys, sections, malformed values, unsupported versions +and expired policies are errors. `expires_at` is optional and, when present, +must be an RFC 3339 timestamp. At least one actual constraint +(`require_lock` or a non-empty repository allowlist) is required. + +`require_lock = true` rejects every unlocked or stale request, including shim +execution, expose discovery, self-test tool execution and diagnostics that +would inspect an image. It does not prevent `cb lock` or `cb update` from +creating the required entry. + +Local image-ID locks have no registry origin and are rejected by default under +a managed policy. `allow_local_images = true` is the explicit exception. It +does not make an unlocked local tag acceptable when `require_lock = true`. + +Repository rules are canonical namespace boundaries: + +- `python:3.13` and `docker.io/python:3.13` normalize to + `docker.io/library/python`; +- `index.docker.io` and `registry-1.docker.io` normalize to `docker.io`; +- tags and digests do not affect origin authorization; +- a host-only rule allows that registry; a longer rule allows that repository + and descendants; +- `ghcr.io/acme` does **not** allow `ghcr.io/acme-tools`. + +Authorization occurs before a repository-mode `docker pull`, local-image +inspection, expose discovery or `docker run`. A managed-policy restore is +preflighted against the complete archived registry and lock before any state or +configuration is changed. Switching a runtime default likewise requires every +target profile to be authorized first. + +## Diagnostics and error contract + +`cb doctor`, `cb inspect TOOL`, `cb trace TOOL ...` and `cb bugreport` report +whether operation is managed, the schema, effective switches, rule count, +expiry, fixed source path and SHA-256 fingerprint. They do not print the policy +file contents. `cb inspect` and `cb trace` also show the selected tool's +authorization result. + +Policy failures have a stable bracketed code suitable for log processing: + +- `policy.unreadable` +- `policy.ownership` +- `policy.syntax` +- `policy.version` +- `policy.expired` +- `policy.lock_required` +- `policy.local_image_denied` +- `policy.repository_denied` + +The fingerprint hashes the exact policy bytes. It is an audit correlation +value, not a signature. Registry-signature and image-signature policy are +separate roadmap stages and are not implied by schema 1. diff --git a/docs/security-model.md b/docs/security-model.md index 6e7074c..ce39975 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -20,6 +20,12 @@ Inside the boundary (whoever controls these controls execution): need ContainerBin to attack you. - Docker Desktop itself, and every image you configure or `docker pull`. +An optional administrator-owned machine policy sits above this user-controlled +boundary. Its fixed path, owner and permissions are validated before use. It +can require locking and restrict image origins, but schema 1 does not constrain +mounts, environment allowlists or commands and does not authenticate registry +or image signatures. See [enterprise machine policy](enterprise-policy.md). + Treat the registry and lockfile like your PowerShell `$PROFILE`: yours, readable, and dangerous to let others edit. @@ -43,6 +49,11 @@ readable, and dangerous to let others edit. - **Fail-closed configuration.** Unknown registry keys, duplicate tool sections, newer schema versions, incomplete lock entries, and registry-image-not-in-lock all refuse to run rather than guess. +- **Machine policy cannot be redirected or weakened.** A present enterprise + policy is loaded only from the fixed OS path, requires administrator/root + ownership and restrictive permissions, and authorizes the already-resolved + image request before pulls, image inspection or execution. Missing means + unmanaged; unreadable, malformed, expired or unsupported means stop. - **Reserved shim names.** Tool names that would collide with `cb` itself or Windows device names (`con`, `nul`, `com1`, …) are rejected at validation, as are versioned management-binary names beginning with `cb-v` plus a digit. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 3732944..b826623 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -23,12 +23,13 @@ import ( "github.com/AviBackToBlack/container-bin/internal/dockerrun" "github.com/AviBackToBlack/container-bin/internal/lockfile" "github.com/AviBackToBlack/container-bin/internal/pathmap" + "github.com/AviBackToBlack/container-bin/internal/policy" "github.com/AviBackToBlack/container-bin/internal/registry" "github.com/AviBackToBlack/container-bin/internal/statearchive" "github.com/AviBackToBlack/container-bin/internal/toml" ) -func Setup(cfgPath, version string) error { +func Setup(cfgPath, version string, machinePolicy policy.Policy) error { if err := registry.EnsureFile(cfgPath); err != nil { return err } @@ -43,7 +44,7 @@ func Setup(cfgPath, version string) error { return err } fmt.Println("\nRunning doctor after setup...") - return diag.Doctor(reg, cfgPath) + return diag.Doctor(reg, cfgPath, machinePolicy) } func renderAddedToolSection(name, image string) string { @@ -53,11 +54,11 @@ func renderAddedToolSection(name, image string) string { // Add appends the smallest useful profile: a stateless tool that runs its // image entrypoint. More privileged behavior (environment, state, mounts and // path rules) remains an explicit registry edit rather than inferred defaults. -func Add(reg registry.Registry, cfgPath string, args []string) error { - return add(reg, cfgPath, args, registry.InstallShims) +func Add(reg registry.Registry, cfgPath string, args []string, machinePolicy policy.Policy) error { + return add(reg, cfgPath, args, registry.InstallShims, machinePolicy) } -func add(reg registry.Registry, cfgPath string, args []string, install func(registry.Registry) error) error { +func add(reg registry.Registry, cfgPath string, args []string, install func(registry.Registry) error, machinePolicy policy.Policy) error { if len(args) != 3 || args[1] != "--image" || args[0] == "" || args[2] == "" { return errors.New("usage: cb add TOOL --image IMAGE") } @@ -78,6 +79,9 @@ func add(reg registry.Registry, cfgPath string, args []string, install func(regi if strings.HasPrefix(image, "-") { return errors.New("image reference must not start with '-'") } + if err := machinePolicy.AuthorizeLockTarget(image, false); err != nil { + return err + } lockExists := false if _, err := os.Stat(lockfile.PathFor(cfgPath)); err == nil { lockExists = true @@ -113,7 +117,7 @@ func add(reg registry.Registry, cfgPath string, args []string, install func(regi return nil } -func Trace(reg registry.Registry, args []string) error { +func Trace(reg registry.Registry, args []string, machinePolicy policy.Policy) error { if len(args) == 0 { return errors.New("usage: cb trace TOOL [ARGS...]") } @@ -158,6 +162,12 @@ func Trace(reg registry.Registry, args []string) error { } fmt.Printf("image: %s\n", t.Image) fmt.Printf("provider: %s\n", t.Provider) + fmt.Printf("policy: %s\n", machinePolicy.Summary()) + if image, err := lockfile.RuntimeImageForTool(t, machinePolicy); err != nil { + fmt.Printf("authorization: WOULD FAIL (%v)\n", err) + } else { + fmt.Printf("authorization: ALLOWED (%s)\n", image) + } fmt.Printf("cwd: %s\n", cwd) if t.CwdMode == "isolated" { fmt.Printf("cwd_mode: isolated\n") @@ -243,7 +253,7 @@ func Env(reg registry.Registry) error { return nil } -func Default(reg registry.Registry, cfgPath string, args []string) error { +func Default(reg registry.Registry, cfgPath string, args []string, machinePolicy policy.Policy) error { if reg.SchemaVersion < 2 { return errors.New("runtime defaults require registry schema 2; run `cb install` to upgrade") } @@ -266,6 +276,15 @@ func Default(reg registry.Registry, cfgPath string, args []string) error { return errors.New("usage: cb default | cb default set FAMILY VERSION") } family, version := strings.ToLower(args[1]), strings.ToLower(args[2]) + if machinePolicy.Managed() { + for _, candidate := range reg.Tools { + if candidate.DefaultFamily == family && candidate.DefaultVersion == version { + if _, err := lockfile.RuntimeImageForTool(candidate, machinePolicy); err != nil { + return fmt.Errorf("default target %s is not authorized: %w", candidate.Name, err) + } + } + } + } if err := registry.SetDefaultVersion(cfgPath, family, version); err != nil { return err } @@ -416,8 +435,8 @@ func exposeSharedFileFor(t registry.Tool, logicalName, command string) (exposeSt return *store, exposedBin{name: name, command: command}, nil } -func discoverGlobalBins(t registry.Tool, store exposeStore) ([]exposedBin, error) { - image, err := lockfile.RuntimeImageForTool(t) +func discoverGlobalBins(t registry.Tool, store exposeStore, machinePolicy policy.Policy) ([]exposedBin, error) { + image, err := lockfile.RuntimeImageForTool(t, machinePolicy) if err != nil { return nil, err } @@ -486,8 +505,8 @@ func inspectExposeImage(sourceName, image string) error { const sharedFileInspectScript = `if [ -L "$1" ]; then printf 'is a symbolic link'; exit 1; fi; if ! cd -P "$2" 2>/dev/null; then printf 'declared volume mount does not exist'; exit 1; fi; resolved_mount=$(pwd -P) || { printf 'cannot resolve declared volume mount'; exit 1; }; parent=${1%/*}; if [ "$parent" = "$1" ]; then parent=/; fi; if ! cd -P "$parent" 2>/dev/null; then printf 'parent directory does not exist'; exit 1; fi; resolved_parent=$(pwd -P) || { printf 'cannot resolve parent directory'; exit 1; }; case "$resolved_parent" in "$resolved_mount"|"$resolved_mount"/*) ;; *) printf 'parent directory resolves outside the declared volume'; exit 1;; esac; if [ ! -e "$1" ]; then printf 'does not exist'; elif [ ! -f "$1" ]; then printf 'is not a regular file'; elif [ ! -x "$1" ]; then printf 'is not executable'; else exit 0; fi; exit 1` -func inspectSharedVolumeFile(t registry.Tool, store exposeStore, command string) error { - image, err := lockfile.RuntimeImageForTool(t) +func inspectSharedVolumeFile(t registry.Tool, store exposeStore, command string, machinePolicy policy.Policy) error { + image, err := lockfile.RuntimeImageForTool(t, machinePolicy) if err != nil { return err } @@ -589,7 +608,7 @@ func renderExposedToolSectionWithComment(comment string, source registry.Tool, n ) } -func exposeSharedVolumeFile(reg registry.Registry, cfgPath string, args []string) error { +func exposeSharedVolumeFile(reg registry.Registry, cfgPath string, args []string, machinePolicy policy.Policy) error { if len(args) != 3 { return errors.New("usage: cb expose --shared-file TOOL VOLUME /absolute/container/file") } @@ -609,7 +628,7 @@ func exposeSharedVolumeFile(reg registry.Registry, cfgPath string, args []string if _, _, exists := reg.Resolve(bin.name); exists { return fmt.Errorf("tool %q already exists; unexpose/uninstall it or choose a file with a different basename", bin.name) } - if err := inspectSharedVolumeFile(source, store, bin.command); err != nil { + if err := inspectSharedVolumeFile(source, store, bin.command, machinePolicy); err != nil { return err } data, err := os.ReadFile(cfgPath) @@ -635,10 +654,10 @@ func exposeSharedVolumeFile(reg registry.Registry, cfgPath string, args []string return nil } -func Expose(reg registry.Registry, cfgPath string, args []string) error { +func Expose(reg registry.Registry, cfgPath string, args []string, machinePolicy policy.Policy) error { const usage = "usage: cb expose TOOL [BINARY ...] | cb expose --shared-file TOOL VOLUME /absolute/container/file" if len(args) > 0 && args[0] == "--shared-file" { - return exposeSharedVolumeFile(reg, cfgPath, args[1:]) + return exposeSharedVolumeFile(reg, cfgPath, args[1:], machinePolicy) } if len(args) == 0 || strings.HasPrefix(args[0], "-") { return errors.New(usage) @@ -660,7 +679,7 @@ func Expose(reg registry.Registry, cfgPath string, args []string) error { if err != nil { return err } - bins, err := discoverGlobalBins(source, store) + bins, err := discoverGlobalBins(source, store, machinePolicy) if err != nil { return err } @@ -751,7 +770,7 @@ func isManagedExposedTool(name string, t registry.Tool) bool { return false } -func Inspect(reg registry.Registry, args []string) error { +func Inspect(reg registry.Registry, args []string, machinePolicy policy.Policy) error { if len(args) != 1 { return errors.New("usage: cb inspect TOOL") } @@ -788,6 +807,7 @@ func Inspect(reg registry.Registry, args []string) error { fmt.Printf("resolved: %s\n", resolved) } fmt.Printf("image: %s\nprovider: %s\n", t.Image, t.Provider) + fmt.Printf("policy: %s\n", machinePolicy.Summary()) lock, lockPath, lerr := lockfile.LoadForRegistry() if lerr != nil { fmt.Printf("lock: ERROR (%v)\n", lerr) @@ -798,6 +818,11 @@ func Inspect(reg registry.Registry, args []string) error { } else { fmt.Printf("lock: STALE/UNLOCKED (no matching entry for configured image)\n") } + if _, err := lockfile.RuntimeImageForTool(t, machinePolicy); err != nil { + fmt.Printf("authorization: REJECTED (%v)\n", err) + } else { + fmt.Printf("authorization: ALLOWED\n") + } if t.Role != "" { fmt.Printf("role: %s\n", t.Role) } @@ -1053,7 +1078,7 @@ func parseBackupArgs(args []string) (path string, state []string, err error) { return path, state, nil } -func Restore(cfgPath string, args []string) error { +func Restore(cfgPath string, args []string, machinePolicy policy.Policy) error { backupPath, apply, restoreState, err := parseRestoreArgs(args) if err != nil { return err @@ -1086,9 +1111,11 @@ func Restore(cfgPath string, args []string) error { if !ok { return errors.New("backup does not contain container-bin.toml") } - if _, err := registry.ParseTOML(string(cfg)); err != nil { + restoredRegistry, err := registry.ParseTOML(string(cfg)) + if err != nil { return fmt.Errorf("backup registry invalid: %w", err) } + var restoredLock *lockfile.LockFile if lock, ok := files["container-bin.lock"]; ok { tmp, err := os.CreateTemp("", "cb-lock-*.tmp") if err != nil { @@ -1100,10 +1127,14 @@ func Restore(cfgPath string, args []string) error { if err := os.WriteFile(name, lock, 0600); err != nil { return err } - if _, err := lockfile.Load(name); err != nil { + restoredLock, err = lockfile.Load(name) + if err != nil { return fmt.Errorf("backup lock invalid: %w", err) } } + if err := authorizeRegistrySnapshot(restoredRegistry, restoredLock, machinePolicy); err != nil { + return fmt.Errorf("backup violates machine policy: %w", err) + } var stateBackup *statearchive.Archive var statePlan []statearchive.Plan if restoreState { @@ -1165,6 +1196,33 @@ func Restore(cfgPath string, args []string) error { return nil } +func authorizeRegistrySnapshot(reg registry.Registry, lf *lockfile.LockFile, machinePolicy policy.Policy) error { + if !machinePolicy.Managed() { + return nil + } + for _, image := range lockfile.ConfiguredImages(reg) { + locked, local := false, false + resolved := "" + if lf != nil { + if entry, ok := lf.Images[image]; ok && entry.Configured == image { + locked = true + local = lockfile.IsLocalResolved(entry.Resolved) + resolved = entry.Resolved + } + } + var err error + if locked { + err = machinePolicy.AuthorizeResolvedImage(image, resolved, local) + } else { + err = machinePolicy.AuthorizeImage(image, false, false) + } + if err != nil { + return fmt.Errorf("image %q: %w", image, err) + } + } + return nil +} + func parseRestoreArgs(args []string) (path string, apply, state bool, err error) { if len(args) == 0 || strings.HasPrefix(args[0], "-") { return "", false, false, errors.New("usage: cb restore BACKUP.zip [--state] [--apply]") @@ -1189,7 +1247,7 @@ func parseRestoreArgs(args []string) (path string, apply, state bool, err error) return path, apply, state, nil } -func Lock(reg registry.Registry, cfgPath string, args []string) error { +func Lock(reg registry.Registry, cfgPath string, args []string, machinePolicy policy.Policy) error { path := lockfile.PathFor(cfgPath) check, localTools, err := parseLockArgs(args) if err != nil { @@ -1203,14 +1261,26 @@ func Lock(reg registry.Registry, cfgPath string, args []string) error { if lf == nil { return fmt.Errorf("lockfile missing: %s (run `cb lock`)", path) } + images := lockfile.ConfiguredImages(reg) missing := 0 - for _, image := range lockfile.ConfiguredImages(reg) { + for _, image := range images { e, ok := lf.Images[image] if !ok || e.Configured != image { fmt.Printf("MISSING %s\n", image) missing++ continue } + if err := machinePolicy.AuthorizeResolvedImage(image, e.Resolved, lockfile.IsLocalResolved(e.Resolved)); err != nil { + fmt.Printf("DENIED %s (%v)\n", image, err) + missing++ + continue + } + } + if missing > 0 { + return fmt.Errorf("lock check failed: %d image(s) missing/unlocked/denied", missing) + } + for _, image := range images { + e := lf.Images[image] cmd := exec.Command("docker", "image", "inspect", e.Resolved) if err := cmd.Run(); err != nil { fmt.Printf("ABSENT %s -> %s\n", image, e.Resolved) @@ -1220,7 +1290,7 @@ func Lock(reg registry.Registry, cfgPath string, args []string) error { } } if missing > 0 { - return fmt.Errorf("lock check failed: %d image(s) missing/unlocked", missing) + return fmt.Errorf("lock check failed: %d image(s) unavailable", missing) } fmt.Printf("lock OK: %s\n", path) return nil @@ -1233,14 +1303,20 @@ func Lock(reg registry.Registry, cfgPath string, args []string) error { } localImages[t.Image] = true } + images := lockfile.ConfiguredImages(reg) + for _, image := range images { + if err := machinePolicy.AuthorizeLockTarget(image, localImages[image]); err != nil { + return err + } + } lf := &lockfile.LockFile{Version: 1, Images: map[string]lockfile.LockEntry{}} - for _, image := range lockfile.ConfiguredImages(reg) { + for _, image := range images { fmt.Printf("locking %s\n", image) var e lockfile.LockEntry if localImages[image] { - e, err = lockfile.ResolveLocalImage(image) + e, err = lockfile.ResolveLocalImage(image, machinePolicy) } else { - e, err = lockfile.ResolveRepositoryImage(image) + e, err = lockfile.ResolveRepositoryImage(image, machinePolicy) } if err != nil { return err @@ -1275,7 +1351,7 @@ func parseLockArgs(args []string) (bool, map[string]bool, error) { return false, localTools, nil } -func Update(reg registry.Registry, cfgPath string, args []string) error { +func Update(reg registry.Registry, cfgPath string, args []string, machinePolicy policy.Policy) error { target, mode, err := parseUpdateArgs(args) if err != nil { return err @@ -1300,18 +1376,28 @@ func Update(reg registry.Registry, cfgPath string, args []string) error { images = []string{t.Image} } seen := map[string]bool{} + uniqueImages := make([]string, 0, len(images)) for _, image := range images { - if seen[image] { - continue + if !seen[image] { + seen[image] = true + uniqueImages = append(uniqueImages, image) } - seen[image] = true + } + for _, image := range uniqueImages { + old := lf.Images[image] + local := mode == "local" || (mode == "" && lockfile.IsLocalResolved(old.Resolved)) + if err := machinePolicy.AuthorizeLockTarget(image, local); err != nil { + return err + } + } + for _, image := range uniqueImages { old := lf.Images[image] fmt.Printf("updating %s\n", image) var e lockfile.LockEntry if mode == "local" || (mode == "" && lockfile.IsLocalResolved(old.Resolved)) { - e, err = lockfile.ResolveLocalImage(image) + e, err = lockfile.ResolveLocalImage(image, machinePolicy) } else { - e, err = lockfile.ResolveRepositoryImage(image) + e, err = lockfile.ResolveRepositoryImage(image, machinePolicy) } if err != nil { return err diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 2e5138f..d782b84 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -10,7 +10,9 @@ import ( "strings" "testing" + "github.com/AviBackToBlack/container-bin/internal/lockfile" "github.com/AviBackToBlack/container-bin/internal/pathmap" + "github.com/AviBackToBlack/container-bin/internal/policy" "github.com/AviBackToBlack/container-bin/internal/registry" ) @@ -27,7 +29,7 @@ func TestAddRequiresExactShape(t *testing.T) { err := add(registry.Default(), filepath.Join(t.TempDir(), "container-bin.toml"), args, func(registry.Registry) error { t.Fatal("installer called for invalid arguments") return nil - }) + }, policy.Policy{}) if err == nil || !strings.Contains(err.Error(), "usage: cb add TOOL --image IMAGE") { t.Fatalf("Add(%v) error = %v, want usage error", args, err) } @@ -51,7 +53,7 @@ func TestAddRejectsUnsafeOrExistingNames(t *testing.T) { err := add(registry.Default(), path, tt.args, func(registry.Registry) error { t.Fatal("installer called for rejected profile") return nil - }) + }, policy.Policy{}) if err == nil || !strings.Contains(err.Error(), tt.want) { t.Fatalf("Add(%v) error = %v, want substring %q", tt.args, err, tt.want) } @@ -69,7 +71,7 @@ func TestAddCreatesMinimalProfileAndInstalls(t *testing.T) { return add(registry.Default(), path, []string{"Demo_Tool", "--image", "registry.example/dev/demo:1.2.3"}, func(got registry.Registry) error { installed = got return nil - }) + }, policy.Policy{}) }) if captureErr != nil { t.Fatal(captureErr) @@ -109,6 +111,78 @@ func TestAddCreatesMinimalProfileAndInstalls(t *testing.T) { } } +func TestAddPolicyDenialDoesNotMutateRegistry(t *testing.T) { + path := filepath.Join(t.TempDir(), "container-bin.toml") + p := policy.Policy{SchemaVersion: 1, AllowedRepositories: []string{"ghcr.io/acme"}} + err := add(registry.Default(), path, []string{"demo", "--image", "ghcr.io/other/demo:1"}, func(registry.Registry) error { + t.Fatal("installer called for policy-denied profile") + return nil + }, p) + if err == nil || !strings.Contains(err.Error(), "[policy.repository_denied]") { + t.Fatalf("add policy error = %v", err) + } + if _, statErr := os.Stat(path); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("policy-denied add mutated registry: %v", statErr) + } +} + +func TestAuthorizeRegistrySnapshot(t *testing.T) { + reg, err := registry.ParseTOML("schema_version = 1\n[tools.demo]\nimage = \"ghcr.io/acme/demo:1\"\nprovider = \"stateless\"\n") + if err != nil { + t.Fatal(err) + } + p := policy.Policy{SchemaVersion: 1, RequireLock: true, AllowedRepositories: []string{"ghcr.io/acme"}} + if err := authorizeRegistrySnapshot(reg, nil, p); err == nil || !strings.Contains(err.Error(), "[policy.lock_required]") { + t.Fatalf("missing restored lock error = %v", err) + } + lf := &lockfile.LockFile{Version: 1, Images: map[string]lockfile.LockEntry{ + "ghcr.io/acme/demo:1": { + Configured: "ghcr.io/acme/demo:1", + Resolved: "ghcr.io/acme/demo@sha256:" + strings.Repeat("a", 64), + Digest: "sha256:" + strings.Repeat("a", 64), + }, + }} + if err := authorizeRegistrySnapshot(reg, lf, p); err != nil { + t.Fatalf("authorized restored snapshot rejected: %v", err) + } + entry := lf.Images["ghcr.io/acme/demo:1"] + entry.Resolved = "evil.example/demo@sha256:" + strings.Repeat("b", 64) + lf.Images["ghcr.io/acme/demo:1"] = entry + if err := authorizeRegistrySnapshot(reg, lf, p); err == nil || !strings.Contains(err.Error(), "resolved lock reference") { + t.Fatalf("foreign resolved repository error = %v", err) + } + entry.Resolved = "ghcr.io/acme/demo@sha256:" + strings.Repeat("a", 64) + lf.Images["ghcr.io/acme/demo:1"] = entry + p.AllowedRepositories = []string{"ghcr.io/other"} + if err := authorizeRegistrySnapshot(reg, lf, p); err == nil || !strings.Contains(err.Error(), "[policy.repository_denied]") { + t.Fatalf("disallowed restored repository error = %v", err) + } +} + +func TestBulkLockOperationsPreflightAllPolicyTargetsBeforeDocker(t *testing.T) { + reg, err := registry.ParseTOML("schema_version = 1\n[tools.allowed]\nimage = \"ghcr.io/acme/allowed:1\"\nprovider = \"stateless\"\n[tools.denied]\nimage = \"ghcr.io/zzz/denied:1\"\nprovider = \"stateless\"\n") + if err != nil { + t.Fatal(err) + } + // If either operation reaches Docker for the alphabetically first allowed + // image, the empty PATH produces an executable-not-found error instead of + // the expected policy denial for the later target. + t.Setenv("PATH", t.TempDir()) + p := policy.Policy{SchemaVersion: 1, AllowedRepositories: []string{"ghcr.io/acme"}} + cfgPath := filepath.Join(t.TempDir(), "container-bin.toml") + for name, run := range map[string]func() error{ + "lock": func() error { return Lock(reg, cfgPath, nil, p) }, + "update_all": func() error { return Update(reg, cfgPath, []string{"--all"}, p) }, + } { + t.Run(name, func(t *testing.T) { + err := run() + if err == nil || !strings.Contains(err.Error(), "[policy.repository_denied]") { + t.Fatalf("bulk operation error = %v, want policy denial before Docker", err) + } + }) + } +} + func TestAddReportsIncompleteExistingLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "container-bin.toml") @@ -119,7 +193,7 @@ func TestAddReportsIncompleteExistingLock(t *testing.T) { t.Fatal(err) } out, captureErr := captureStdout(func() error { - return add(registry.Default(), path, []string{"demo", "--image", "example/demo:1"}, func(registry.Registry) error { return nil }) + return add(registry.Default(), path, []string{"demo", "--image", "example/demo:1"}, func(registry.Registry) error { return nil }, policy.Policy{}) }) if captureErr != nil { t.Fatal(captureErr) @@ -133,7 +207,7 @@ func TestAddInstallerFailureLeavesValidRegistry(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "container-bin.toml") wantErr := errors.New("shim directory denied") - err := add(registry.Default(), path, []string{"demo", "--image", "example/demo:1"}, func(registry.Registry) error { return wantErr }) + err := add(registry.Default(), path, []string{"demo", "--image", "example/demo:1"}, func(registry.Registry) error { return wantErr }, policy.Policy{}) if !errors.Is(err, wantErr) || !strings.Contains(err.Error(), "profile \"demo\" was added") || !strings.Contains(err.Error(), "cb install") { t.Fatalf("installer error = %v", err) } @@ -152,7 +226,7 @@ func TestAddInstallerFailureLeavesValidRegistry(t *testing.T) { func TestDefaultAliasesAreVisibleInManagementCommands(t *testing.T) { reg := registry.Default() - out, err := captureStdout(func() error { return Trace(reg, []string{"node", "--version"}) }) + out, err := captureStdout(func() error { return Trace(reg, []string{"node", "--version"}, policy.Policy{}) }) if err != nil { t.Fatal(err) } @@ -161,7 +235,7 @@ func TestDefaultAliasesAreVisibleInManagementCommands(t *testing.T) { t.Fatalf("trace output missing %q:\n%s", want, out) } } - out, err = captureStdout(func() error { return Default(reg, "unused", nil) }) + out, err = captureStdout(func() error { return Default(reg, "unused", nil, policy.Policy{}) }) if err != nil { t.Fatal(err) } @@ -282,7 +356,7 @@ func TestParseUpdateArgs(t *testing.T) { func TestExposeRequiresSourceTool(t *testing.T) { reg := registry.Default() - if err := Expose(reg, filepath.Join(t.TempDir(), "container-bin.toml"), nil); err == nil { + if err := Expose(reg, filepath.Join(t.TempDir(), "container-bin.toml"), nil, policy.Policy{}); err == nil { t.Fatal("expected usage error for empty args") } else if !strings.Contains(err.Error(), "usage: cb expose TOOL") { t.Fatalf("unexpected error message: %v", err) @@ -296,7 +370,7 @@ func TestExposeRejectsFlagShapedArgumentsBeforeDocker(t *testing.T) { {"go", "--shared-file", "gobin", "/go/bin/stringer"}, {"go", "-h"}, } { - if err := Expose(reg, filepath.Join(t.TempDir(), "container-bin.toml"), args); err == nil || !strings.Contains(err.Error(), "usage: cb expose TOOL") { + if err := Expose(reg, filepath.Join(t.TempDir(), "container-bin.toml"), args, policy.Policy{}); err == nil || !strings.Contains(err.Error(), "usage: cb expose TOOL") { t.Fatalf("Expose(%v) error = %v, want usage", args, err) } } @@ -304,7 +378,7 @@ func TestExposeRejectsFlagShapedArgumentsBeforeDocker(t *testing.T) { func TestExposeRejectsUnknownSource(t *testing.T) { reg := registry.Default() - if err := Expose(reg, filepath.Join(t.TempDir(), "container-bin.toml"), []string{"notarealtool"}); err == nil { + if err := Expose(reg, filepath.Join(t.TempDir(), "container-bin.toml"), []string{"notarealtool"}, policy.Policy{}); err == nil { t.Fatal("expected not-found error") } else if !strings.Contains(err.Error(), `tool "notarealtool" not found`) { t.Fatalf("unexpected error message: %v", err) @@ -317,7 +391,7 @@ func TestExposeRejectsUnknownSource(t *testing.T) { // than an empty string. func TestExposeRejectsStatelessTool(t *testing.T) { reg := registry.Default() - err := Expose(reg, filepath.Join(t.TempDir(), "container-bin.toml"), []string{"terraform"}) + err := Expose(reg, filepath.Join(t.TempDir(), "container-bin.toml"), []string{"terraform"}, policy.Policy{}) if err == nil { t.Fatal("expected error for stateless source tool") } @@ -344,13 +418,13 @@ provider = "stateless" t.Fatal(err) } cfgPath := filepath.Join(t.TempDir(), "container-bin.toml") - if err := Expose(reg, cfgPath, []string{"--shared-file", "acme"}); err == nil || !strings.Contains(err.Error(), "usage: cb expose --shared-file") { + if err := Expose(reg, cfgPath, []string{"--shared-file", "acme"}, policy.Policy{}); err == nil || !strings.Contains(err.Error(), "usage: cb expose --shared-file") { t.Fatalf("short shared-file args error = %v", err) } - if err := Expose(reg, cfgPath, []string{"--shared-file", "acme-lint", "tools", "/opt/acme/tool"}); err == nil || !strings.Contains(err.Error(), "not a stateful profile") { + if err := Expose(reg, cfgPath, []string{"--shared-file", "acme-lint", "tools", "/opt/acme/tool"}, policy.Policy{}); err == nil || !strings.Contains(err.Error(), "not a stateful profile") { t.Fatalf("stateless shared-file source error = %v", err) } - if err := Expose(reg, cfgPath, []string{"--shared-file", "acme", "tools", "/opt/acme/bin/acme-lint"}); err == nil || !strings.Contains(err.Error(), "already exists") { + if err := Expose(reg, cfgPath, []string{"--shared-file", "acme", "tools", "/opt/acme/bin/acme-lint"}, policy.Policy{}); err == nil || !strings.Contains(err.Error(), "already exists") { t.Fatalf("shared-file collision error = %v", err) } } @@ -984,7 +1058,7 @@ host_mounts = ["%USERPROFILE%\\.claude:/root/.claude:ro"] t.Fatalf("parse registry: %v", err) } - out, err := captureStdout(func() error { return Inspect(reg, []string{"demo"}) }) + out, err := captureStdout(func() error { return Inspect(reg, []string{"demo"}, policy.Policy{}) }) if err != nil { t.Fatalf("capture: %v", err) } @@ -1024,7 +1098,7 @@ host_mounts = ["%USERPROFILE%/.claude:/root/.claude:ro"] t.Fatal(err) } - out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}, policy.Policy{}) }) if err != nil { t.Fatalf("capture: %v", err) } @@ -1060,7 +1134,7 @@ host_mounts = ["%USERPROFILE%/.claude:/root/.claude:ro"] t.Fatalf("parse registry: %v", err) } - out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}, policy.Policy{}) }) if err != nil { t.Fatalf("capture: %v", err) } @@ -1095,7 +1169,7 @@ host_mounts = ["%USERPROFILE%/does-not-exist:/root/missing:ro"] t.Fatalf("parse registry: %v", err) } - out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}, policy.Policy{}) }) if err != nil { t.Fatalf("capture: %v", err) } @@ -1120,7 +1194,7 @@ provider = "stateless" t.Fatalf("parse registry: %v", err) } - out, err := captureStdout(func() error { return Inspect(reg, []string{"demo"}) }) + out, err := captureStdout(func() error { return Inspect(reg, []string{"demo"}, policy.Policy{}) }) if err != nil { t.Fatalf("capture: %v", err) } @@ -1148,7 +1222,7 @@ shared_volumes = ["cache:/root/.cache"] t.Fatalf("parse registry: %v", err) } - out, err := captureStdout(func() error { return Inspect(reg, []string{"demo"}) }) + out, err := captureStdout(func() error { return Inspect(reg, []string{"demo"}, policy.Policy{}) }) if err != nil { t.Fatalf("capture: %v", err) } @@ -1188,7 +1262,7 @@ provider = "stateless" t.Fatalf("parse registry: %v", err) } - out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}, policy.Policy{}) }) if err != nil { t.Fatalf("capture: %v", err) } @@ -1222,7 +1296,7 @@ shared_volumes = ["cache:/root/.cache"] t.Fatalf("parse registry: %v", err) } - out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}, policy.Policy{}) }) if err != nil { t.Fatalf("capture: %v", err) } @@ -1268,7 +1342,7 @@ host_mounts = ["%USERPROFILE%/.claude:/root/.claude:ro"] t.Fatalf("parse registry: %v", err) } - out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}, policy.Policy{}) }) if err != nil { t.Fatalf("capture: %v", err) } diff --git a/internal/diag/diag.go b/internal/diag/diag.go index 7bb35b2..e68dde1 100644 --- a/internal/diag/diag.go +++ b/internal/diag/diag.go @@ -20,6 +20,7 @@ import ( "github.com/AviBackToBlack/container-bin/internal/dockervol" "github.com/AviBackToBlack/container-bin/internal/lockfile" "github.com/AviBackToBlack/container-bin/internal/pathmap" + "github.com/AviBackToBlack/container-bin/internal/policy" "github.com/AviBackToBlack/container-bin/internal/registry" ) @@ -168,12 +169,13 @@ func registrySchemaVerdict(version int) (status, message string) { return "ok", fmt.Sprintf("registry schema %d", version) } -func Doctor(reg registry.Registry, cfgPath string) error { +func Doctor(reg registry.Registry, cfgPath string, machinePolicy policy.Policy) error { failures := 0 warnings := 0 ok := func(format string, args ...any) { fmt.Printf("OK "+format+"\n", args...) } warn := func(format string, args ...any) { warnings++; fmt.Printf("WARN "+format+"\n", args...) } fail := func(format string, args ...any) { failures++; fmt.Printf("FAIL "+format+"\n", args...) } + ok("machine policy: %s", machinePolicy.Summary()) dockerPath, err := exec.LookPath("docker") if err != nil { @@ -214,7 +216,20 @@ func Doctor(reg registry.Registry, cfgPath string) error { if err != nil { fail("lockfile invalid: %v", err) } else if lf == nil { - warn("lockfile missing: %s (runtime is UNLOCKED)", lockPath) + if machinePolicy.RequireLock { + fail("lockfile missing: %s ([policy.lock_required] runtime is not authorized)", lockPath) + } else { + denied := 0 + for _, image := range lockfile.ConfiguredImages(reg) { + if err := machinePolicy.AuthorizeImage(image, false, false); err != nil { + fail("image %s is denied: %v", image, err) + denied++ + } + } + if denied == 0 { + warn("lockfile missing: %s (runtime is UNLOCKED)", lockPath) + } + } } else { missing := 0 for _, image := range lockfile.ConfiguredImages(reg) { @@ -223,6 +238,11 @@ func Doctor(reg registry.Registry, cfgPath string) error { missing++ continue } + if err := machinePolicy.AuthorizeResolvedImage(image, e.Resolved, lockfile.IsLocalResolved(e.Resolved)); err != nil { + fmt.Printf("FAIL image %s is denied: %v\n", image, err) + missing++ + continue + } if exec.Command("docker", "image", "inspect", e.Resolved).Run() != nil { missing++ } @@ -492,7 +512,7 @@ func redactSecrets(text string) string { // when the report is successfully assembled and printed, even if Doctor() // found failures — that signal is in the captured text itself. Only genuine // capture or assembly errors are returned as Bugreport's own error. -func Bugreport(reg registry.Registry, cfgPath, version string) error { +func Bugreport(reg registry.Registry, cfgPath, version string, machinePolicy policy.Policy) error { var b strings.Builder b.WriteString("container-bin bugreport\n") b.WriteString(fmt.Sprintf("generated: %s\n", time.Now().UTC().Format(time.RFC3339))) @@ -528,7 +548,7 @@ func Bugreport(reg registry.Registry, cfgPath, version string) error { b.WriteString("\nRegistry:\n") b.WriteString(registryText) - doctorText, err := captureStdout(func() error { return Doctor(reg, cfgPath) }) + doctorText, err := captureStdout(func() error { return Doctor(reg, cfgPath, machinePolicy) }) if err != nil { return fmt.Errorf("capture doctor: %w", err) } @@ -865,7 +885,7 @@ func buildEnvironmentChecks(dockerCheck selfTestCheck, cwd string) []selfTestChe return env } -func SelfTest(reg registry.Registry, jsonOut, release bool, version string) error { +func SelfTest(reg registry.Registry, jsonOut, release bool, version string, machinePolicy policy.Policy) error { tmp, err := os.MkdirTemp("", "cb-selftest-") if err != nil { return err @@ -894,7 +914,7 @@ func SelfTest(reg registry.Registry, jsonOut, release bool, version string) erro return err } - report, err := runSelfTestChecksAndCleanup(reg, project, external, jsonOut, release, old, version) + report, err := runSelfTestChecksAndCleanup(reg, project, external, jsonOut, release, old, version, machinePolicy) if err != nil { return err } @@ -934,7 +954,7 @@ func SelfTest(reg registry.Registry, jsonOut, release bool, version string) erro return nil } -func runSelfTestChecksAndCleanup(reg registry.Registry, project, external string, jsonOut, release bool, cwd, version string) (selfTestReport, error) { +func runSelfTestChecksAndCleanup(reg registry.Registry, project, external string, jsonOut, release bool, cwd, version string, machinePolicy policy.Policy) (selfTestReport, error) { // Redirect process-level stdout/stderr around the check-and-cleanup phase so // that tools whose containers write to stdout (jq, terraform) and the // docker volume rm cleanup output cannot corrupt a --json report. cb runs @@ -960,7 +980,7 @@ func runSelfTestChecksAndCleanup(reg registry.Registry, project, external string defer dockervol.RemoveQuiet(volumeID) } - return runSelfTestChecks(reg, project, external, release, cwd, version) + return runSelfTestChecks(reg, project, external, release, cwd, version, machinePolicy) } func selfTestProjectVolumeIDs(reg registry.Registry, root string) []string { @@ -986,7 +1006,7 @@ func selfTestProjectVolumeIDs(reg registry.Registry, root string) []string { return ids } -func runSelfTestChecks(reg registry.Registry, project, external string, release bool, cwd, version string) (selfTestReport, error) { +func runSelfTestChecks(reg registry.Registry, project, external string, release bool, cwd, version string, machinePolicy policy.Policy) (selfTestReport, error) { dockerCheck := selfTestCheck{ID: "docker"} dockerAvailable := false out, err := exec.Command("docker", "version", "--format", "{{.Server.Version}}").Output() @@ -1003,7 +1023,7 @@ func runSelfTestChecks(reg registry.Registry, project, external string, release if dockerAvailable { for _, name := range []string{"python", "node", "node22", "jq", "terraform"} { if t, _, ok := reg.Resolve(name); ok { - toolOutcomes[name] = runSelfTestTool(t, name, project, external) + toolOutcomes[name] = runSelfTestTool(t, name, project, external, machinePolicy) } } } @@ -1016,51 +1036,51 @@ func runSelfTestChecks(reg registry.Registry, project, external string, release return buildSelfTestReport(version, time.Now(), dockerCheck, toolOutcomes, env), nil } -func runSelfTestTool(t registry.Tool, name, project, external string) toolSelfTestOutcome { +func runSelfTestTool(t registry.Tool, name, project, external string, machinePolicy policy.Policy) toolSelfTestOutcome { o := toolSelfTestOutcome{} - if err := dockerrun.EnsureImageLocalForTool(t); err != nil { + if err := dockerrun.EnsureImageLocalForTool(t, machinePolicy); err != nil { s := err.Error() o.ImageLocalErr = &s return o } switch name { case "python": - code, err := dockerrun.RunTool(t, []string{"-c", "open('/venv/.cb-selftest','w').write('ok')"}) + code, err := dockerrun.RunTool(t, []string{"-c", "open('/venv/.cb-selftest','w').write('ok')"}, machinePolicy) if err != nil || code != 0 { s := selfTestRunError(name, err, code) o.PersistWriteErr = &s } else { - code, err = dockerrun.RunTool(t, []string{"-c", "assert open('/venv/.cb-selftest').read()=='ok'"}) + code, err = dockerrun.RunTool(t, []string{"-c", "assert open('/venv/.cb-selftest').read()=='ok'"}, machinePolicy) if err != nil || code != 0 { s := selfTestRunError(name, err, code) o.PersistReadErr = &s } } - code, err = dockerrun.RunTool(t, []string{filepath.Join(external, "outside.py")}) + code, err = dockerrun.RunTool(t, []string{filepath.Join(external, "outside.py")}, machinePolicy) if err != nil || code != 0 { s := selfTestRunError(name, err, code) o.ExternalPathErr = &s } case "node", "node22": - code, err := dockerrun.RunTool(t, []string{"-e", "require('fs').mkdirSync('node_modules',{recursive:true}); require('fs').writeFileSync('node_modules/.cb-selftest','ok')"}) + code, err := dockerrun.RunTool(t, []string{"-e", "require('fs').mkdirSync('node_modules',{recursive:true}); require('fs').writeFileSync('node_modules/.cb-selftest','ok')"}, machinePolicy) if err != nil || code != 0 { s := selfTestRunError(name, err, code) o.ModulesWriteErr = &s } else { - code, err = dockerrun.RunTool(t, []string{"-e", "if(require('fs').readFileSync('node_modules/.cb-selftest','utf8')!=='ok')process.exit(9)"}) + code, err = dockerrun.RunTool(t, []string{"-e", "if(require('fs').readFileSync('node_modules/.cb-selftest','utf8')!=='ok')process.exit(9)"}, machinePolicy) if err != nil || code != 0 { s := selfTestRunError(name, err, code) o.ModulesReadErr = &s } } case "jq": - code, err := dockerrun.RunTool(t, []string{".", `.\data.json`}) + code, err := dockerrun.RunTool(t, []string{".", `.\data.json`}, machinePolicy) if err != nil || code != 0 { s := selfTestRunError(name, err, code) o.RelativePathErr = &s } case "terraform": - code, err := dockerrun.RunTool(t, []string{`-chdir=.\tf`, "validate"}) + code, err := dockerrun.RunTool(t, []string{`-chdir=.\tf`, "validate"}, machinePolicy) if err != nil || code != 0 { s := selfTestRunError(name, err, code) o.ChdirErr = &s diff --git a/internal/dockerrun/dockerrun.go b/internal/dockerrun/dockerrun.go index 47e2ab4..a5533e1 100644 --- a/internal/dockerrun/dockerrun.go +++ b/internal/dockerrun/dockerrun.go @@ -18,6 +18,7 @@ import ( "github.com/AviBackToBlack/container-bin/internal/dockervol" "github.com/AviBackToBlack/container-bin/internal/lockfile" "github.com/AviBackToBlack/container-bin/internal/pathmap" + "github.com/AviBackToBlack/container-bin/internal/policy" "github.com/AviBackToBlack/container-bin/internal/registry" ) @@ -49,7 +50,7 @@ func interactiveTerminal() bool { return err == nil && out.Mode()&os.ModeCharDevice != 0 } -func RunTool(t registry.Tool, userArgs []string) (int, error) { +func RunTool(t registry.Tool, userArgs []string, machinePolicy policy.Policy) (int, error) { cwd, err := os.Getwd() if err != nil { return 1, err @@ -64,7 +65,7 @@ func RunTool(t registry.Tool, userArgs []string) (int, error) { return 1, err } - imageRef, err := lockfile.RuntimeImageForTool(t) + imageRef, err := lockfile.RuntimeImageForTool(t, machinePolicy) if err != nil { return 1, err } @@ -475,8 +476,8 @@ func buildHostMountArgs(hostMounts []string) ([]string, error) { return args, nil } -func EnsureImageLocalForTool(t registry.Tool) error { - ref, err := lockfile.RuntimeImageForTool(t) +func EnsureImageLocalForTool(t registry.Tool, machinePolicy policy.Policy) error { + ref, err := lockfile.RuntimeImageForTool(t, machinePolicy) if err != nil { return err } diff --git a/internal/lockfile/lockfile.go b/internal/lockfile/lockfile.go index df5db52..c389c50 100644 --- a/internal/lockfile/lockfile.go +++ b/internal/lockfile/lockfile.go @@ -17,6 +17,7 @@ import ( "strings" "github.com/AviBackToBlack/container-bin/internal/atomicio" + "github.com/AviBackToBlack/container-bin/internal/policy" "github.com/AviBackToBlack/container-bin/internal/registry" "github.com/AviBackToBlack/container-bin/internal/toml" ) @@ -88,6 +89,17 @@ func Load(path string) (*LockFile, error) { if cur.Digest != cur.Resolved { return fmt.Errorf("lock entry %q local image digest must match resolved ID", curKey) } + } else { + i := strings.LastIndex(cur.Resolved, "@") + if i <= 0 || !validImageID(cur.Resolved[i+1:]) { + return fmt.Errorf("lock entry %q has invalid immutable repository digest %q", curKey, cur.Resolved) + } + if cur.Digest != cur.Resolved[i+1:] { + return fmt.Errorf("lock entry %q digest does not match resolved repository digest", curKey) + } + if matched, ok := matchRepoDigest(cur.Configured, []string{cur.Resolved}); !ok || matched != cur.Resolved { + return fmt.Errorf("lock entry %q resolved repository does not match configured image %q", curKey, cur.Configured) + } } lf.Images[cur.Configured] = *cur return nil @@ -296,7 +308,10 @@ func localLockEntry(configured string, inspected imageInspection) (LockEntry, er // ResolveRepositoryImage refreshes a registry-backed lock entry. Repository // and local identity are deliberately selected by the CLI, never inferred // from Docker metadata: current engines can report RepoDigests for both. -func ResolveRepositoryImage(configured string) (LockEntry, error) { +func ResolveRepositoryImage(configured string, machinePolicy policy.Policy) (LockEntry, error) { + if err := machinePolicy.AuthorizeLockTarget(configured, false); err != nil { + return LockEntry{}, err + } cmd := exec.Command("docker", "pull", configured) cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr if err := cmd.Run(); err != nil { @@ -312,7 +327,10 @@ func ResolveRepositoryImage(configured string) (LockEntry, error) { // ResolveLocalImage refreshes a local-image lock by inspecting the configured // tag only. It never pulls or silently switches an existing local lock to a // repository identity. -func ResolveLocalImage(configured string) (LockEntry, error) { +func ResolveLocalImage(configured string, machinePolicy policy.Policy) (LockEntry, error) { + if err := machinePolicy.AuthorizeLockTarget(configured, true); err != nil { + return LockEntry{}, err + } inspected, err := inspectImage(configured) if err != nil { return LockEntry{}, fmt.Errorf("local image %s is not available (build or load it before locking): %w", configured, err) @@ -320,18 +338,24 @@ func ResolveLocalImage(configured string) (LockEntry, error) { return localLockEntry(configured, inspected) } -func RuntimeImageForTool(t registry.Tool) (string, error) { +func RuntimeImageForTool(t registry.Tool, machinePolicy policy.Policy) (string, error) { lf, path, err := LoadForRegistry() if err != nil { return "", fmt.Errorf("lockfile: %w", err) } if lf == nil { + if err := machinePolicy.AuthorizeImage(t.Image, false, false); err != nil { + return "", err + } return t.Image, nil } e, ok := lf.Images[t.Image] if !ok || e.Configured != t.Image { return "", fmt.Errorf("image %q is not locked in %s; run `cb update %s` or `cb lock`", t.Image, path, t.Name) } + if err := machinePolicy.AuthorizeResolvedImage(t.Image, e.Resolved, IsLocalResolved(e.Resolved)); err != nil { + return "", err + } return e.Resolved, nil } diff --git a/internal/lockfile/lockfile_test.go b/internal/lockfile/lockfile_test.go index d550d8b..101f6a0 100644 --- a/internal/lockfile/lockfile_test.go +++ b/internal/lockfile/lockfile_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/AviBackToBlack/container-bin/internal/policy" "github.com/AviBackToBlack/container-bin/internal/registry" ) @@ -80,6 +81,27 @@ digest = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa } } +func TestLockFileRejectsMutableOrForeignRepositoryResolution(t *testing.T) { + configured := "ghcr.io/acme/tool:1" + id := entryID(configured) + cases := []string{ + "ghcr.io/acme/tool:latest", + "evil.example/tool@sha256:" + strings.Repeat("a", 64), + "ghcr.io/acme/tool@sha256:short", + } + for _, resolved := range cases { + digest := "sha256:" + strings.Repeat("a", 64) + contents := fmt.Sprintf("lock_version = 1\n[images.%s]\nconfigured = %q\nresolved = %q\ndigest = %q\n", id, configured, resolved, digest) + path := filepath.Join(t.TempDir(), "container-bin.lock") + if err := os.WriteFile(path, []byte(contents), 0644); err != nil { + t.Fatal(err) + } + if _, err := Load(path); err == nil { + t.Errorf("Load accepted unsafe resolved reference %q", resolved) + } + } +} + func TestConfiguredImagesIncludesNode22(t *testing.T) { reg := registry.Default() got := ConfiguredImages(reg) @@ -277,6 +299,16 @@ func TestRepositoryLockEntryDoesNotTreatForeignDigestAsLocal(t *testing.T) { } } +func TestResolveImagePolicyDenialHappensBeforeDocker(t *testing.T) { + p := policy.Policy{SchemaVersion: 1, AllowedRepositories: []string{"ghcr.io/acme"}} + if _, err := ResolveRepositoryImage("ghcr.io/other/tool:1", p); err == nil || !strings.Contains(err.Error(), "[policy.repository_denied]") { + t.Fatalf("repository resolution error = %v", err) + } + if _, err := ResolveLocalImage("local/tool:dev", p); err == nil || !strings.Contains(err.Error(), "[policy.local_image_denied]") { + t.Fatalf("local resolution error = %v", err) + } +} + func TestLoadLockFile_RecoversFromBackup(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "container-bin.lock") diff --git a/internal/policy/ownership_unix.go b/internal/policy/ownership_unix.go new file mode 100644 index 0000000..1b83013 --- /dev/null +++ b/internal/policy/ownership_unix.go @@ -0,0 +1,33 @@ +//go:build !windows + +package policy + +import ( + "fmt" + "os" + "path/filepath" + "syscall" +) + +func verifyOwnership(path string) error { + for _, candidate := range []string{filepath.Dir(path), path} { + info, err := os.Lstat(candidate) + if err != nil { + return err + } + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("%s is a symbolic link", candidate) + } + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("cannot determine owner of %s", candidate) + } + if stat.Uid != 0 { + return fmt.Errorf("%s is not owned by root", candidate) + } + if info.Mode().Perm()&0o022 != 0 { + return fmt.Errorf("%s is writable by group or other", candidate) + } + } + return nil +} diff --git a/internal/policy/ownership_windows.go b/internal/policy/ownership_windows.go new file mode 100644 index 0000000..15670a6 --- /dev/null +++ b/internal/policy/ownership_windows.go @@ -0,0 +1,61 @@ +//go:build windows + +package policy + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" +) + +func verifyOwnership(path string) error { + for _, candidate := range []string{filepath.Dir(path), path} { + cmd := exec.Command("powershell", "-NoProfile", "-NonInteractive", "-Command", `$item = Get-Item -LiteralPath $env:CB_POLICY_ACL_PATH -Force; if (($item.Attributes -band [IO.FileAttributes]::ReparsePoint) -ne 0) { throw 'reparse point' }; $acl = Get-Acl -LiteralPath $env:CB_POLICY_ACL_PATH; $owner = $acl.Owner; try { $owner = ([System.Security.Principal.NTAccount]$acl.Owner).Translate([System.Security.Principal.SecurityIdentifier]).Value } catch {}; "OWNER|$owner"; $acl.Access | ForEach-Object { $sid = $_.IdentityReference.Value; try { $sid = $_.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]).Value } catch {}; "$sid|$($_.AccessControlType)|$($_.FileSystemRights)" }`) + cmd.Env = append(os.Environ(), "CB_POLICY_ACL_PATH="+candidate) + out, err := cmd.Output() + if err != nil { + return fmt.Errorf("inspect %s permissions: %w", candidate, err) + } + if err := secureWindowsACLVerdict(string(out), candidate == path); err != nil { + return fmt.Errorf("%s: %w", candidate, err) + } + } + return nil +} + +func secureWindowsACLVerdict(raw string, policyFile bool) error { + lines := strings.Split(strings.ReplaceAll(raw, "\r", ""), "\n") + if len(lines) == 0 || !strings.HasPrefix(lines[0], "OWNER|") { + return fmt.Errorf("owner could not be determined") + } + owner := strings.TrimPrefix(lines[0], "OWNER|") + if owner != "S-1-5-18" && owner != "S-1-5-32-544" { + return fmt.Errorf("owner %q is not SYSTEM or Administrators", owner) + } + for _, line := range lines[1:] { + line = strings.TrimSpace(line) + if line == "" { + continue + } + parts := strings.Split(line, "|") + if len(parts) != 3 || parts[0] == "" || (parts[1] != "Allow" && parts[1] != "Deny") { + return fmt.Errorf("permission entry could not be interpreted") + } + if parts[1] != "Allow" || parts[0] == "S-1-5-18" || parts[0] == "S-1-5-32-544" { + continue + } + rights := parts[2] + blocked := []string{"FullControl", "Modify", "Delete", "DeleteSubdirectoriesAndFiles", "TakeOwnership", "ChangePermissions"} + if policyFile { + blocked = append(blocked, "Write", "CreateFiles", "AppendData") + } + for _, right := range blocked { + if strings.Contains(rights, right) { + return fmt.Errorf("untrusted principal %s has %s", parts[0], right) + } + } + } + return nil +} diff --git a/internal/policy/ownership_windows_test.go b/internal/policy/ownership_windows_test.go new file mode 100644 index 0000000..8e3de50 --- /dev/null +++ b/internal/policy/ownership_windows_test.go @@ -0,0 +1,27 @@ +//go:build windows + +package policy + +import "testing" + +func TestSecureWindowsACLVerdict(t *testing.T) { + secure := "OWNER|S-1-5-32-544\nS-1-5-18|Allow|FullControl\nS-1-5-32-544|Allow|FullControl\nS-1-5-32-545|Allow|ReadAndExecute, Synchronize" + if err := secureWindowsACLVerdict(secure, true); err != nil { + t.Fatalf("secure ACL rejected: %v", err) + } + if err := secureWindowsACLVerdict("OWNER|S-1-5-21-1\n", true); err == nil { + t.Fatal("untrusted owner accepted") + } + if err := secureWindowsACLVerdict("OWNER|S-1-5-18\nS-1-1-0|Allow|Write", true); err == nil { + t.Fatal("untrusted file writer accepted") + } + if err := secureWindowsACLVerdict("OWNER|S-1-5-18\nS-1-1-0|Allow|CreateFiles", false); err != nil { + t.Fatalf("safe parent create right rejected: %v", err) + } + if err := secureWindowsACLVerdict("OWNER|S-1-5-18\nS-1-1-0|Allow|DeleteSubdirectoriesAndFiles", false); err == nil { + t.Fatal("untrusted parent delete right accepted") + } + if err := secureWindowsACLVerdict("OWNER|S-1-5-18\nmalformed", true); err == nil { + t.Fatal("malformed permission entry accepted") + } +} diff --git a/internal/policy/policy.go b/internal/policy/policy.go new file mode 100644 index 0000000..d5ce1ec --- /dev/null +++ b/internal/policy/policy.go @@ -0,0 +1,349 @@ +// Package policy loads and enforces the administrator-owned machine policy. +// Policy is an authorization layer over the user's resolved configuration; it +// never supplies defaults or merges into the registry. +package policy + +import ( + "bufio" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "os" + "runtime" + "sort" + "strconv" + "strings" + "time" + + "github.com/AviBackToBlack/container-bin/internal/toml" +) + +const SchemaVersion = 1 + +type Error struct { + Code string + Err error +} + +func (e *Error) Error() string { return fmt.Sprintf("[policy.%s] %v", e.Code, e.Err) } +func (e *Error) Unwrap() error { return e.Err } + +func policyError(code, format string, args ...any) error { + return &Error{Code: code, Err: fmt.Errorf(format, args...)} +} + +type Policy struct { + Path string + SchemaVersion int + RequireLock bool + AllowLocalImages bool + AllowedRepositories []string + ExpiresAt *time.Time + Fingerprint string +} + +func Path() string { + if runtime.GOOS == "windows" { + return `C:\ProgramData\ContainerBin\policy.toml` + } + return "/etc/container-bin/policy.toml" +} + +func (p Policy) Managed() bool { return p.SchemaVersion != 0 } + +func (p Policy) Summary() string { + if !p.Managed() { + return "unmanaged (machine policy absent)" + } + expires := "none" + if p.ExpiresAt != nil { + expires = p.ExpiresAt.UTC().Format(time.RFC3339) + } + return fmt.Sprintf("managed schema=%d require_lock=%t allow_local_images=%t allowed_repositories=%d expires=%s fingerprint=sha256:%s source=%s", + p.SchemaVersion, p.RequireLock, p.AllowLocalImages, len(p.AllowedRepositories), expires, p.Fingerprint, p.Path) +} + +func Load() (Policy, error) { + return loadAt(Path(), verifyOwnership, time.Now()) +} + +func loadAt(path string, ownership func(string) error, now time.Time) (Policy, error) { + b, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return Policy{}, nil + } + if err != nil { + return Policy{}, policyError("unreadable", "read %s: %v", path, err) + } + if err := ownership(path); err != nil { + return Policy{}, policyError("ownership", "%s: %v", path, err) + } + p, err := parse(path, b, now) + if err != nil { + return Policy{}, err + } + return p, nil +} + +func parse(path string, b []byte, now time.Time) (Policy, error) { + p := Policy{Path: path} + seen := map[string]bool{} + sc := bufio.NewScanner(strings.NewReader(string(b))) + for lineNo := 1; sc.Scan(); lineNo++ { + line := strings.TrimSpace(toml.StripComment(sc.Text())) + if line == "" { + continue + } + if section, ok, err := toml.ParseSectionHeader(line); err != nil { + return Policy{}, policyError("syntax", "line %d: %v", lineNo, err) + } else if ok { + return Policy{}, policyError("syntax", "line %d: unsupported section %q", lineNo, section) + } + kv := strings.SplitN(line, "=", 2) + if len(kv) != 2 { + return Policy{}, policyError("syntax", "line %d: expected key = value", lineNo) + } + key, raw := strings.TrimSpace(kv[0]), strings.TrimSpace(kv[1]) + if seen[key] { + return Policy{}, policyError("syntax", "line %d: duplicate key %q", lineNo, key) + } + seen[key] = true + switch key { + case "policy_version": + v, err := strconv.Atoi(raw) + if err != nil { + return Policy{}, policyError("version", "line %d: policy_version must be an integer", lineNo) + } + p.SchemaVersion = v + case "require_lock": + v, err := toml.ParseBool(raw) + if err != nil { + return Policy{}, policyError("syntax", "line %d require_lock: %v", lineNo, err) + } + p.RequireLock = v + case "allow_local_images": + v, err := toml.ParseBool(raw) + if err != nil { + return Policy{}, policyError("syntax", "line %d allow_local_images: %v", lineNo, err) + } + p.AllowLocalImages = v + case "allowed_repositories": + values, err := toml.ParseStringArray(raw) + if err != nil { + return Policy{}, policyError("syntax", "line %d allowed_repositories: %v", lineNo, err) + } + p.AllowedRepositories = values + case "expires_at": + value, err := toml.ParseQuoted(raw) + if err != nil { + return Policy{}, policyError("syntax", "line %d expires_at: %v", lineNo, err) + } + expires, err := time.Parse(time.RFC3339, value) + if err != nil { + return Policy{}, policyError("syntax", "line %d expires_at must be RFC3339: %v", lineNo, err) + } + p.ExpiresAt = &expires + default: + return Policy{}, policyError("syntax", "line %d: unsupported key %q", lineNo, key) + } + } + if err := sc.Err(); err != nil { + return Policy{}, policyError("unreadable", "scan %s: %v", path, err) + } + if p.SchemaVersion != SchemaVersion { + return Policy{}, policyError("version", "unsupported policy_version %d (supported: %d)", p.SchemaVersion, SchemaVersion) + } + if !p.RequireLock && len(p.AllowedRepositories) == 0 { + return Policy{}, policyError("syntax", "policy has no authorization controls") + } + if p.ExpiresAt != nil && !now.Before(*p.ExpiresAt) { + return Policy{}, policyError("expired", "policy expired at %s", p.ExpiresAt.UTC().Format(time.RFC3339)) + } + + canonical := make([]string, 0, len(p.AllowedRepositories)) + set := map[string]bool{} + for _, rule := range p.AllowedRepositories { + rule, err := canonicalRule(rule) + if err != nil { + return Policy{}, policyError("syntax", "invalid allowed_repositories entry %q: %v", rule, err) + } + if !set[rule] { + set[rule] = true + canonical = append(canonical, rule) + } + } + sort.Strings(canonical) + p.AllowedRepositories = canonical + sum := sha256.Sum256(b) + p.Fingerprint = hex.EncodeToString(sum[:]) + return p, nil +} + +func (p Policy) AuthorizeImage(configured string, locked, local bool) error { + if !p.Managed() { + return nil + } + if p.RequireLock && !locked { + return policyError("lock_required", "image %q is not covered by an exact lock entry", configured) + } + if local { + if !p.AllowLocalImages { + return policyError("local_image_denied", "local image %q has no authorized registry origin", configured) + } + return nil + } + if len(p.AllowedRepositories) == 0 { + return nil + } + _, err := p.authorizeRepository(configured) + if err != nil { + return policyError("repository_denied", "image %q: %v", configured, err) + } + return nil +} + +// AuthorizeResolvedImage checks both sides of a lock entry. The configured +// reference alone is not sufficient proof of origin because a hand-edited +// lockfile could point its resolved digest at another repository. +func (p Policy) AuthorizeResolvedImage(configured, resolved string, local bool) error { + if err := p.AuthorizeImage(configured, true, local); err != nil { + return err + } + if local || !p.Managed() || len(p.AllowedRepositories) == 0 { + return nil + } + if _, err := p.authorizeRepository(resolved); err != nil { + return policyError("repository_denied", "resolved lock reference %q is not authorized: %v", resolved, err) + } + return nil +} + +func (p Policy) authorizeRepository(ref string) (string, error) { + repo, err := CanonicalRepository(ref) + if err != nil { + return "", err + } + for _, allowed := range p.AllowedRepositories { + if repo == allowed || strings.HasPrefix(repo, allowed+"/") { + return repo, nil + } + } + return repo, fmt.Errorf("repository %q is outside the allowed repository boundaries", repo) +} + +// AuthorizeLockTarget checks whether policy permits creating or refreshing a +// lock entry. It intentionally omits the runtime require_lock check because the +// operation itself is what establishes that lock. +func (p Policy) AuthorizeLockTarget(configured string, local bool) error { + if !p.Managed() { + return nil + } + copy := p + copy.RequireLock = false + return copy.AuthorizeImage(configured, true, local) +} + +// CanonicalRepository returns the registry-qualified repository without a tag +// or digest. Docker Hub aliases and implicit references are normalized. +func CanonicalRepository(ref string) (string, error) { + ref = strings.TrimSpace(strings.ToLower(ref)) + if strings.HasPrefix(ref, "sha256:") { + return "", errors.New("local image IDs do not have a registry repository") + } + if ref == "" || strings.ContainsAny(ref, "\\ \r\n") || strings.Contains(ref, "://") || strings.HasPrefix(ref, "/") { + return "", errors.New("invalid image reference") + } + if strings.Count(ref, "@") > 1 || strings.HasPrefix(ref, "@") || strings.HasSuffix(ref, "@") { + return "", errors.New("invalid image digest") + } + if i := strings.IndexByte(ref, '@'); i >= 0 { + ref = ref[:i] + } + lastSlash := strings.LastIndexByte(ref, '/') + if colon := strings.LastIndexByte(ref, ':'); colon > lastSlash { + ref = ref[:colon] + } + parts := strings.Split(ref, "/") + for i, part := range parts { + if part == "" || part == "." || part == ".." { + return "", errors.New("invalid repository path") + } + if i > 0 && strings.Contains(part, ":") { + return "", errors.New("tag separator is only valid at the end of an image reference") + } + } + first := parts[0] + qualified := strings.Contains(first, ".") || strings.Contains(first, ":") || first == "localhost" + if qualified { + if err := validateRegistryHost(first); err != nil { + return "", err + } + } + if !qualified { + if len(parts) == 1 { + parts = []string{"docker.io", "library", first} + } else { + parts = append([]string{"docker.io"}, parts...) + } + } else { + switch first { + case "index.docker.io", "registry-1.docker.io": + parts[0] = "docker.io" + } + if parts[0] == "docker.io" && len(parts) == 2 { + parts = []string{"docker.io", "library", parts[1]} + } + } + if len(parts) < 2 { + return "", errors.New("repository name is required") + } + return strings.Join(parts, "/"), nil +} + +func canonicalRule(rule string) (string, error) { + rule = strings.TrimSpace(strings.ToLower(rule)) + if rule == "" || strings.ContainsAny(rule, "@\\ \r\n") || strings.Contains(rule, "://") || strings.HasPrefix(rule, "/") || strings.HasSuffix(rule, "/") { + return "", errors.New("invalid repository boundary") + } + parts := strings.Split(rule, "/") + for i, part := range parts { + if part == "" || part == "." || part == ".." { + return "", errors.New("invalid repository boundary") + } + if i > 0 && strings.Contains(part, ":") { + return "", errors.New("repository boundaries cannot contain tags") + } + } + if strings.Contains(parts[0], ".") || strings.Contains(parts[0], ":") || parts[0] == "localhost" { + if err := validateRegistryHost(parts[0]); err != nil { + return "", err + } + } + switch parts[0] { + case "index.docker.io", "registry-1.docker.io": + parts[0] = "docker.io" + } + if len(parts) == 1 && !strings.Contains(parts[0], ".") && !strings.Contains(parts[0], ":") && parts[0] != "localhost" { + parts = []string{"docker.io", parts[0]} + } + return strings.Join(parts, "/"), nil +} + +func validateRegistryHost(host string) error { + if strings.ContainsAny(host, "[]") { + return errors.New("IPv6 registry hosts are not supported in policy schema 1") + } + if i := strings.LastIndexByte(host, ':'); i >= 0 { + name, port := host[:i], host[i+1:] + if name == "" || port == "" { + return errors.New("invalid registry host or port") + } + for _, r := range port { + if r < '0' || r > '9' { + return errors.New("registry port must be numeric") + } + } + } + return nil +} diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go new file mode 100644 index 0000000..7d85055 --- /dev/null +++ b/internal/policy/policy_test.go @@ -0,0 +1,132 @@ +package policy + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestLoadAtMissingIsUnmanaged(t *testing.T) { + p, err := loadAt(filepath.Join(t.TempDir(), "missing.toml"), func(string) error { return nil }, time.Now()) + if err != nil || p.Managed() { + t.Fatalf("loadAt missing = (%+v, %v), want unmanaged nil", p, err) + } +} + +func TestLoadAtStrictAndCanonical(t *testing.T) { + path := filepath.Join(t.TempDir(), "policy.toml") + contents := "policy_version = 1\nrequire_lock = true\nallow_local_images = true\nallowed_repositories = [\"registry-1.docker.io/library\", \"GHCR.IO/Astral-SH\", \"ghcr.io/astral-sh\"]\nexpires_at = \"2030-01-02T03:04:05Z\"\n" + if err := os.WriteFile(path, []byte(contents), 0600); err != nil { + t.Fatal(err) + } + p, err := loadAt(path, func(got string) error { + if got != path { + t.Fatalf("ownership path = %q, want %q", got, path) + } + return nil + }, time.Date(2029, 1, 1, 0, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatal(err) + } + if !p.Managed() || !p.RequireLock || !p.AllowLocalImages || p.Fingerprint == "" { + t.Fatalf("unexpected parsed policy: %+v", p) + } + want := []string{"docker.io/library", "ghcr.io/astral-sh"} + if strings.Join(p.AllowedRepositories, ",") != strings.Join(want, ",") { + t.Fatalf("allowed repositories = %v, want %v", p.AllowedRepositories, want) + } + if !strings.Contains(p.Summary(), "fingerprint=sha256:") || !strings.Contains(p.Summary(), path) { + t.Fatalf("summary missing provenance: %q", p.Summary()) + } +} + +func TestLoadAtOwnershipFailureIsCoded(t *testing.T) { + path := filepath.Join(t.TempDir(), "policy.toml") + if err := os.WriteFile(path, []byte("policy_version = 1\nrequire_lock = true\n"), 0600); err != nil { + t.Fatal(err) + } + _, err := loadAt(path, func(string) error { return errors.New("writable") }, time.Now()) + assertPolicyCode(t, err, "ownership") +} + +func TestParseRejectsInvalidPolicies(t *testing.T) { + now := time.Date(2029, 1, 1, 0, 0, 0, 0, time.UTC) + cases := []struct { + name, contents, code string + }{ + {"missing version", "require_lock = true\n", "version"}, + {"unknown version", "policy_version = 2\nrequire_lock = true\n", "version"}, + {"duplicate", "policy_version = 1\nrequire_lock = true\nrequire_lock = false\n", "syntax"}, + {"unknown key", "policy_version = 1\nrequire_lock = true\nsurprise = true\n", "syntax"}, + {"section", "policy_version = 1\nrequire_lock = true\n[extra]\n", "syntax"}, + {"no controls", "policy_version = 1\nallow_local_images = true\n", "syntax"}, + {"expired", "policy_version = 1\nrequire_lock = true\nexpires_at = \"2028-01-01T00:00:00Z\"\n", "expired"}, + {"bad rule", "policy_version = 1\nallowed_repositories = [\"ghcr.io//team\"]\n", "syntax"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := parse("policy.toml", []byte(tc.contents), now) + assertPolicyCode(t, err, tc.code) + }) + } +} + +func TestCanonicalRepository(t *testing.T) { + cases := map[string]string{ + "python:3.13": "docker.io/library/python", + "docker.io/python@sha256:aaaa": "docker.io/library/python", + "index.docker.io/library/python:latest": "docker.io/library/python", + "registry-1.docker.io/team/tool@sha256:bbbb": "docker.io/team/tool", + "astral-sh/uv:0.8": "docker.io/astral-sh/uv", + "GHCR.IO/AviBackToBlack/Container-Bin:v1": "ghcr.io/avibacktoblack/container-bin", + "localhost:5000/team/tool:dev": "localhost:5000/team/tool", + "example.com:5443/nested/team/tool@sha256:deadbeef": "example.com:5443/nested/team/tool", + } + for input, want := range cases { + got, err := CanonicalRepository(input) + if err != nil || got != want { + t.Errorf("CanonicalRepository(%q) = (%q, %v), want %q", input, got, err, want) + } + } + for _, bad := range []string{"", "https://ghcr.io/team/tool", `ghcr.io\team\tool`, "ghcr.io//team", "../tool", "ghcr.io/team/../tool", "sha256:" + strings.Repeat("a", 64), "python@", "ghcr.io/team/tool:bad:tag", "localhost:abc/team/tool"} { + if got, err := CanonicalRepository(bad); err == nil { + t.Errorf("CanonicalRepository(%q) = %q, want error", bad, got) + } + } +} + +func TestAuthorizeImage(t *testing.T) { + p := Policy{SchemaVersion: 1, RequireLock: true, AllowedRepositories: []string{"docker.io/library", "ghcr.io/team"}} + if err := p.AuthorizeImage("python:3.13", true, false); err != nil { + t.Fatal(err) + } + if err := p.AuthorizeImage("ghcr.io/team/sub/tool:1", true, false); err != nil { + t.Fatal(err) + } + assertPolicyCode(t, p.AuthorizeImage("python:3.13", false, false), "lock_required") + assertPolicyCode(t, p.AuthorizeImage("ghcr.io/teamster/tool:1", true, false), "repository_denied") + assertPolicyCode(t, p.AuthorizeImage("sha256:"+strings.Repeat("a", 64), true, true), "local_image_denied") + + p.AllowLocalImages = true + if err := p.AuthorizeImage("local-tool:dev", true, true); err != nil { + t.Fatalf("explicit local exception rejected: %v", err) + } + if err := (Policy{}).AuthorizeImage("anything", false, true); err != nil { + t.Fatalf("unmanaged policy changed behavior: %v", err) + } + p = Policy{SchemaVersion: 1, AllowedRepositories: []string{"ghcr.io/team"}} + if err := p.AuthorizeResolvedImage("ghcr.io/team/tool:1", "evil.example/tool@sha256:"+strings.Repeat("a", 64), false); err == nil { + t.Fatal("resolved lock repository bypassed allowlist") + } +} + +func assertPolicyCode(t *testing.T, err error, want string) { + t.Helper() + var pe *Error + if !errors.As(err, &pe) || pe.Code != want { + t.Fatalf("error = %v, want policy code %q", err, want) + } +} diff --git a/main.go b/main.go index d9df3cf..95b2fa9 100644 --- a/main.go +++ b/main.go @@ -11,6 +11,7 @@ import ( "github.com/AviBackToBlack/container-bin/internal/diag" "github.com/AviBackToBlack/container-bin/internal/dockerrun" "github.com/AviBackToBlack/container-bin/internal/mutationlock" + "github.com/AviBackToBlack/container-bin/internal/policy" "github.com/AviBackToBlack/container-bin/internal/registry" "github.com/AviBackToBlack/container-bin/internal/state" ) @@ -22,15 +23,20 @@ import ( // Local/dev builds report "dev". var version = "dev" -// loadRegistry is a test seam for proving bootstrap commands return before -// registry I/O. Production always uses registry.Load. +// These test seams prove bootstrap commands return before policy or registry +// I/O. Production always uses the corresponding package loaders. var loadRegistry = registry.Load +var loadPolicy = policy.Load func main() { invoked := invokedName(os.Args[0]) if isManagementInvocation(invoked) && handleBootstrapCommand(os.Args[1:]) { return } + machinePolicy, err := loadPolicy() + if err != nil { + fatalf("machine policy: %v", err) + } reg, cfgPath, err := loadRegistry() if err != nil { @@ -42,7 +48,7 @@ func main() { if !ok { fatalf("no tool profile for %q (registry: %s)", invoked, cfgPath) } - code, err := dockerrun.RunTool(tool, os.Args[1:]) + code, err := dockerrun.RunTool(tool, os.Args[1:], machinePolicy) if err != nil { fatalf("%v", err) } @@ -62,22 +68,22 @@ func main() { if err != nil { return err } - return cli.Add(reg, cfgPath, os.Args[2:]) + return cli.Add(reg, cfgPath, os.Args[2:], machinePolicy) }); err != nil { fatalf("add: %v", err) } case "setup": if err := withMutationLock(cfgPath, func() error { - return cli.Setup(cfgPath, version) + return cli.Setup(cfgPath, version, machinePolicy) }); err != nil { fatalf("setup: %v", err) } case "doctor": - if err := diag.Doctor(reg, cfgPath); err != nil { + if err := diag.Doctor(reg, cfgPath, machinePolicy); err != nil { fatalf("doctor: %v", err) } case "bugreport": - if err := diag.Bugreport(reg, cfgPath, version); err != nil { + if err := diag.Bugreport(reg, cfgPath, version, machinePolicy); err != nil { fatalf("bugreport: %v", err) } case "backup": @@ -88,7 +94,7 @@ func main() { } case "restore": if err := withMutationLock(cfgPath, func() error { - return cli.Restore(cfgPath, os.Args[2:]) + return cli.Restore(cfgPath, os.Args[2:], machinePolicy) }); err != nil { fatalf("restore: %v", err) } @@ -97,7 +103,7 @@ func main() { if err != nil { fatalf("self-test: %v", err) } - if err := diag.SelfTest(reg, jsonOut, release, version); err != nil { + if err := diag.SelfTest(reg, jsonOut, release, version, machinePolicy); err != nil { fatalf("self-test: %v", err) } case "list": @@ -109,15 +115,15 @@ func main() { if err != nil { return err } - return cli.Default(fresh, cfgPath, os.Args[2:]) + return cli.Default(fresh, cfgPath, os.Args[2:], machinePolicy) }); err != nil { fatalf("default: %v", err) } - } else if err := cli.Default(reg, cfgPath, os.Args[2:]); err != nil { + } else if err := cli.Default(reg, cfgPath, os.Args[2:], machinePolicy); err != nil { fatalf("default: %v", err) } case "trace": - if err := cli.Trace(reg, os.Args[2:]); err != nil { + if err := cli.Trace(reg, os.Args[2:], machinePolicy); err != nil { fatalf("trace: %v", err) } case "env": @@ -129,7 +135,7 @@ func main() { fatalf("state: %v", err) } case "inspect": - if err := cli.Inspect(reg, os.Args[2:]); err != nil { + if err := cli.Inspect(reg, os.Args[2:], machinePolicy); err != nil { fatalf("inspect: %v", err) } case "gc": @@ -142,7 +148,7 @@ func main() { if err != nil { return err } - return cli.Expose(reg, cfgPath, os.Args[2:]) + return cli.Expose(reg, cfgPath, os.Args[2:], machinePolicy) }); err != nil { fatalf("expose: %v", err) } @@ -172,7 +178,7 @@ func main() { if err != nil { return err } - return cli.Lock(reg, cfgPath, os.Args[2:]) + return cli.Lock(reg, cfgPath, os.Args[2:], machinePolicy) }); err != nil { fatalf("lock: %v", err) } @@ -182,7 +188,7 @@ func main() { if err != nil { return err } - return cli.Update(reg, cfgPath, os.Args[2:]) + return cli.Update(reg, cfgPath, os.Args[2:], machinePolicy) }); err != nil { fatalf("update: %v", err) } diff --git a/main_test.go b/main_test.go index 8a6161c..9f554dc 100644 --- a/main_test.go +++ b/main_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/AviBackToBlack/container-bin/internal/policy" "github.com/AviBackToBlack/container-bin/internal/registry" ) @@ -36,14 +37,19 @@ func TestInvokedNameIsCaseInsensitive(t *testing.T) { func TestBootstrapCommandsDoNotLoadRegistry(t *testing.T) { oldArgs := os.Args oldLoadRegistry := loadRegistry + oldLoadPolicy := loadPolicy defer func() { os.Args = oldArgs loadRegistry = oldLoadRegistry + loadPolicy = oldLoadPolicy }() loadRegistry = func() (registry.Registry, string, error) { panic("bootstrap command attempted to load the registry") } + loadPolicy = func() (policy.Policy, error) { + panic("bootstrap command attempted to load machine policy") + } tests := []struct { name string From 082ab5e82f9f5c106472dee9356f80a311371be8 Mon Sep 17 00:00:00 2001 From: AviBackToBlack <54722547+AviBackToBlack@users.noreply.github.com> Date: Sat, 19 Sep 2026 20:08:46 +0100 Subject: [PATCH 2/5] fix: harden enterprise policy enforcement --- README.md | 7 +- docs/enterprise-policy.md | 13 +++ internal/cli/cli.go | 107 +++++++++++++--------- internal/cli/cli_test.go | 79 ++++++++++++++++ internal/policy/ownership_windows.go | 37 +++++++- internal/policy/ownership_windows_test.go | 33 ++++++- internal/policy/policy.go | 11 ++- internal/policy/policy_test.go | 16 ++++ main.go | 2 +- 9 files changed, 254 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index c7b1056..fa2ac13 100644 --- a/README.md +++ b/README.md @@ -172,10 +172,15 @@ the image: ```powershell cb add jq-corp --image registry.corp.example/devtools/jq:1.8.1 +# For an intentionally local image, declare that identity explicitly: +cb add jq-local --image jq-local:dev --local ``` If a lockfile exists, it becomes intentionally incomplete until you run -`cb update jq-corp` or `cb lock`; execution fails closed in the meantime. +`cb update jq-corp` or `cb lock`; execution fails closed in the meantime. A +profile added with `--local` must be locked explicitly with +`cb update --local jq-local` or `cb lock --local jq-local`; the flag never +infers local identity from Docker metadata. State, environment allowlists, path rules, command overrides, and mounts still require an explicit reviewed registry edit followed by `cb install`. diff --git a/docs/enterprise-policy.md b/docs/enterprise-policy.md index 7d98e69..1aa3b10 100644 --- a/docs/enterprise-policy.md +++ b/docs/enterprise-policy.md @@ -58,6 +58,10 @@ creating the required entry. Local image-ID locks have no registry origin and are rejected by default under a managed policy. `allow_local_images = true` is the explicit exception. It does not make an unlocked local tag acceptable when `require_lock = true`. +Use `cb add TOOL --image IMAGE --local` when creating a profile for such an +image, then follow the reported explicit `cb lock --local TOOL` or +`cb update --local TOOL` command. ContainerBin never guesses local intent from +the daemon's current image metadata. Repository rules are canonical namespace boundaries: @@ -67,6 +71,9 @@ Repository rules are canonical namespace boundaries: - tags and digests do not affect origin authorization; - a host-only rule allows that registry; a longer rule allows that repository and descendants; +- a single-segment rule such as `python` means the Docker Hub namespace + `docker.io/python`; it does not match the official image repository + `docker.io/library/python`; - `ghcr.io/acme` does **not** allow `ghcr.io/acme-tools`. Authorization occurs before a repository-mode `docker pull`, local-image @@ -75,6 +82,12 @@ preflighted against the complete archived registry and lock before any state or configuration is changed. Switching a runtime default likewise requires every target profile to be authorized first. +The compiled-in state backup/restore helper is not a user-selected registry +profile and is outside `allowed_repositories`. It is an exact Alpine digest, +is never pulled implicitly, and runs with networking disabled and a read-only +container root. Docker daemon policy may still reject it, and state operations +fail if that exact helper image is not already available. + ## Diagnostics and error contract `cb doctor`, `cb inspect TOOL`, `cb trace TOOL ...` and `cb bugreport` report diff --git a/internal/cli/cli.go b/internal/cli/cli.go index b826623..e3d7d23 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -59,8 +59,9 @@ func Add(reg registry.Registry, cfgPath string, args []string, machinePolicy pol } func add(reg registry.Registry, cfgPath string, args []string, install func(registry.Registry) error, machinePolicy policy.Policy) error { - if len(args) != 3 || args[1] != "--image" || args[0] == "" || args[2] == "" { - return errors.New("usage: cb add TOOL --image IMAGE") + local := len(args) == 4 && args[3] == "--local" + if (len(args) != 3 && !local) || args[1] != "--image" || args[0] == "" || args[2] == "" { + return errors.New("usage: cb add TOOL --image IMAGE [--local]") } name := strings.ToLower(args[0]) image := args[2] @@ -79,7 +80,7 @@ func add(reg registry.Registry, cfgPath string, args []string, install func(regi if strings.HasPrefix(image, "-") { return errors.New("image reference must not start with '-'") } - if err := machinePolicy.AuthorizeLockTarget(image, false); err != nil { + if err := machinePolicy.AuthorizeLockTarget(image, local); err != nil { return err } lockExists := false @@ -110,9 +111,17 @@ func add(reg registry.Registry, cfgPath string, args []string, install func(regi fmt.Printf("added %s -> %s (stateless)\n", name, image) if lockExists { - fmt.Printf("lockfile is now incomplete; run `cb update %s` or `cb lock` before using the shim\n", name) + if local { + fmt.Printf("lockfile is now incomplete; run `cb update --local %s` or `cb lock --local %s` before using the shim\n", name, name) + } else { + fmt.Printf("lockfile is now incomplete; run `cb update %s` or `cb lock` before using the shim\n", name) + } } else { - fmt.Println("run `cb lock` to pin configured images before relying on the shim") + if local { + fmt.Printf("run `cb lock --local %s` to pin the local image ID before relying on the shim\n", name) + } else { + fmt.Println("run `cb lock` to pin configured images before relying on the shim") + } } return nil } @@ -1254,46 +1263,9 @@ func Lock(reg registry.Registry, cfgPath string, args []string, machinePolicy po return err } if check { - lf, err := lockfile.Load(path) - if err != nil { - return err - } - if lf == nil { - return fmt.Errorf("lockfile missing: %s (run `cb lock`)", path) - } - images := lockfile.ConfiguredImages(reg) - missing := 0 - for _, image := range images { - e, ok := lf.Images[image] - if !ok || e.Configured != image { - fmt.Printf("MISSING %s\n", image) - missing++ - continue - } - if err := machinePolicy.AuthorizeResolvedImage(image, e.Resolved, lockfile.IsLocalResolved(e.Resolved)); err != nil { - fmt.Printf("DENIED %s (%v)\n", image, err) - missing++ - continue - } - } - if missing > 0 { - return fmt.Errorf("lock check failed: %d image(s) missing/unlocked/denied", missing) - } - for _, image := range images { - e := lf.Images[image] - cmd := exec.Command("docker", "image", "inspect", e.Resolved) - if err := cmd.Run(); err != nil { - fmt.Printf("ABSENT %s -> %s\n", image, e.Resolved) - missing++ - } else { - fmt.Printf("OK %s -> %s\n", image, e.Resolved) - } - } - if missing > 0 { - return fmt.Errorf("lock check failed: %d image(s) unavailable", missing) - } - fmt.Printf("lock OK: %s\n", path) - return nil + return checkLock(reg, path, machinePolicy, func(resolved string) error { + return exec.Command("docker", "image", "inspect", resolved).Run() + }) } localImages := map[string]bool{} for name := range localTools { @@ -1331,6 +1303,51 @@ func Lock(reg registry.Registry, cfgPath string, args []string, machinePolicy po return nil } +func checkLock(reg registry.Registry, path string, machinePolicy policy.Policy, inspect func(string) error) error { + lf, err := lockfile.Load(path) + if err != nil { + return err + } + if lf == nil { + return fmt.Errorf("lockfile missing: %s (run `cb lock`)", path) + } + type candidate struct { + image string + entry lockfile.LockEntry + } + var candidates []candidate + failures := 0 + for _, image := range lockfile.ConfiguredImages(reg) { + e, ok := lf.Images[image] + if !ok || e.Configured != image { + fmt.Printf("MISSING %s\n", image) + failures++ + continue + } + if err := machinePolicy.AuthorizeResolvedImage(image, e.Resolved, lockfile.IsLocalResolved(e.Resolved)); err != nil { + fmt.Printf("DENIED %s (%v)\n", image, err) + failures++ + continue + } + candidates = append(candidates, candidate{image: image, entry: e}) + } + // Authorization for every configured image is complete before any Docker + // inspection. Denied images are never handed to Docker. + for _, candidate := range candidates { + if err := inspect(candidate.entry.Resolved); err != nil { + fmt.Printf("ABSENT %s -> %s\n", candidate.image, candidate.entry.Resolved) + failures++ + } else { + fmt.Printf("OK %s -> %s\n", candidate.image, candidate.entry.Resolved) + } + } + if failures > 0 { + return fmt.Errorf("lock check failed: %d image(s) missing/unlocked/denied/unavailable", failures) + } + fmt.Printf("lock OK: %s\n", path) + return nil +} + func parseLockArgs(args []string) (bool, map[string]bool, error) { localTools := map[string]bool{} if len(args) == 0 { diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index d782b84..7018a2a 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -25,6 +25,7 @@ func TestAddRequiresExactShape(t *testing.T) { {"--image", "example/demo:1", "demo"}, {"demo", "--provider", "stateless"}, {"demo", "--image", "example/demo:1", "extra"}, + {"demo", "--image", "example/demo:1", "--local", "extra"}, } { err := add(registry.Default(), filepath.Join(t.TempDir(), "container-bin.toml"), args, func(registry.Registry) error { t.Fatal("installer called for invalid arguments") @@ -126,6 +127,33 @@ func TestAddPolicyDenialDoesNotMutateRegistry(t *testing.T) { } } +func TestAddLocalIntentUsesLocalPolicyAndReportsLocalLockCommand(t *testing.T) { + path := filepath.Join(t.TempDir(), "container-bin.toml") + p := policy.Policy{SchemaVersion: 1, AllowLocalImages: true, AllowedRepositories: []string{"ghcr.io/acme"}} + out, err := captureStdout(func() error { + return add(registry.Default(), path, []string{"demo", "--image", "local/demo:dev", "--local"}, func(registry.Registry) error { return nil }, p) + }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "run `cb lock --local demo`") { + t.Fatalf("local add did not report explicit local lock command:\n%s", out) + } + + deniedPath := filepath.Join(t.TempDir(), "container-bin.toml") + p.AllowLocalImages = false + err = add(registry.Default(), deniedPath, []string{"demo", "--image", "ghcr.io/acme/demo:dev", "--local"}, func(registry.Registry) error { + t.Fatal("installer called for policy-denied local profile") + return nil + }, p) + if err == nil || !strings.Contains(err.Error(), "[policy.local_image_denied]") { + t.Fatalf("local add policy error = %v", err) + } + if _, statErr := os.Stat(deniedPath); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("policy-denied local add mutated registry: %v", statErr) + } +} + func TestAuthorizeRegistrySnapshot(t *testing.T) { reg, err := registry.ParseTOML("schema_version = 1\n[tools.demo]\nimage = \"ghcr.io/acme/demo:1\"\nprovider = \"stateless\"\n") if err != nil { @@ -324,6 +352,57 @@ func TestParseLockArgs(t *testing.T) { } } +func TestCheckLockReportsEveryStatusAfterPolicyPreflight(t *testing.T) { + reg := registry.Registry{Tools: map[string]registry.Tool{ + "present": {Image: "ghcr.io/acme/present:1"}, + "absent": {Image: "ghcr.io/acme/absent:1"}, + "denied": {Image: "ghcr.io/other/denied:1"}, + "missing": {Image: "ghcr.io/acme/missing:1"}, + }} + digest := "sha256:" + strings.Repeat("a", 64) + path := filepath.Join(t.TempDir(), "container-bin.lock") + lf := &lockfile.LockFile{Version: 1, Images: map[string]lockfile.LockEntry{ + "ghcr.io/acme/present:1": {Configured: "ghcr.io/acme/present:1", Resolved: "ghcr.io/acme/present@" + digest, Digest: digest}, + "ghcr.io/acme/absent:1": {Configured: "ghcr.io/acme/absent:1", Resolved: "ghcr.io/acme/absent@" + digest, Digest: digest}, + "ghcr.io/other/denied:1": {Configured: "ghcr.io/other/denied:1", Resolved: "ghcr.io/other/denied@" + digest, Digest: digest}, + }} + if err := lockfile.Write(path, lf); err != nil { + t.Fatal(err) + } + var inspected []string + p := policy.Policy{SchemaVersion: 1, AllowedRepositories: []string{"ghcr.io/acme"}} + var checkErr error + out, captureErr := captureStdout(func() error { + checkErr = checkLock(reg, path, p, func(resolved string) error { + inspected = append(inspected, resolved) + if strings.Contains(resolved, "/absent@") { + return errors.New("not present") + } + return nil + }) + return nil + }) + if captureErr != nil { + t.Fatal(captureErr) + } + if checkErr == nil || !strings.Contains(checkErr.Error(), "3 image(s)") { + t.Fatalf("checkLock error = %v, want three failures", checkErr) + } + for _, want := range []string{"ABSENT ghcr.io/acme/absent:1", "MISSING ghcr.io/acme/missing:1", "OK ghcr.io/acme/present:1", "DENIED ghcr.io/other/denied:1"} { + if !strings.Contains(out, want) { + t.Errorf("check output missing %q:\n%s", want, out) + } + } + if len(inspected) != 2 { + t.Fatalf("inspected refs = %v, want only two authorized refs", inspected) + } + for _, ref := range inspected { + if strings.Contains(ref, "/denied@") { + t.Fatalf("policy-denied ref reached Docker inspection: %s", ref) + } + } +} + func TestParseUpdateArgs(t *testing.T) { for _, tt := range []struct { args []string diff --git a/internal/policy/ownership_windows.go b/internal/policy/ownership_windows.go index 15670a6..4a403b4 100644 --- a/internal/policy/ownership_windows.go +++ b/internal/policy/ownership_windows.go @@ -8,11 +8,17 @@ import ( "os/exec" "path/filepath" "strings" + "syscall" + "unsafe" ) func verifyOwnership(path string) error { + powerShell, err := powerShellExecutable() + if err != nil { + return err + } for _, candidate := range []string{filepath.Dir(path), path} { - cmd := exec.Command("powershell", "-NoProfile", "-NonInteractive", "-Command", `$item = Get-Item -LiteralPath $env:CB_POLICY_ACL_PATH -Force; if (($item.Attributes -band [IO.FileAttributes]::ReparsePoint) -ne 0) { throw 'reparse point' }; $acl = Get-Acl -LiteralPath $env:CB_POLICY_ACL_PATH; $owner = $acl.Owner; try { $owner = ([System.Security.Principal.NTAccount]$acl.Owner).Translate([System.Security.Principal.SecurityIdentifier]).Value } catch {}; "OWNER|$owner"; $acl.Access | ForEach-Object { $sid = $_.IdentityReference.Value; try { $sid = $_.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]).Value } catch {}; "$sid|$($_.AccessControlType)|$($_.FileSystemRights)" }`) + cmd := exec.Command(powerShell, "-NoProfile", "-NonInteractive", "-Command", `$item = Get-Item -LiteralPath $env:CB_POLICY_ACL_PATH -Force; if (($item.Attributes -band [IO.FileAttributes]::ReparsePoint) -ne 0) { throw 'reparse point' }; $acl = Get-Acl -LiteralPath $env:CB_POLICY_ACL_PATH; $owner = $acl.Owner; try { $owner = ([System.Security.Principal.NTAccount]$acl.Owner).Translate([System.Security.Principal.SecurityIdentifier]).Value } catch {}; "OWNER|$owner"; $acl.Access | ForEach-Object { $sid = $_.IdentityReference.Value; try { $sid = $_.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]).Value } catch {}; "$sid|$($_.AccessControlType)|$($_.FileSystemRights)" }`) cmd.Env = append(os.Environ(), "CB_POLICY_ACL_PATH="+candidate) out, err := cmd.Output() if err != nil { @@ -25,6 +31,35 @@ func verifyOwnership(path string) error { return nil } +var getWindowsDirectoryW = syscall.NewLazyDLL("kernel32.dll").NewProc("GetWindowsDirectoryW") + +func powerShellExecutable() (string, error) { + buffer := make([]uint16, 32768) + n, _, callErr := getWindowsDirectoryW.Call(uintptr(unsafe.Pointer(&buffer[0])), uintptr(len(buffer))) + if n == 0 { + return "", fmt.Errorf("resolve Windows directory: %w", callErr) + } + if n >= uintptr(len(buffer)) { + return "", fmt.Errorf("resolve Windows directory: returned path is too long") + } + return powerShellExecutableAt(syscall.UTF16ToString(buffer[:n])) +} + +func powerShellExecutableAt(windowsDirectory string) (string, error) { + if !filepath.IsAbs(windowsDirectory) { + return "", fmt.Errorf("resolve PowerShell: Windows directory %q is not absolute", windowsDirectory) + } + path := filepath.Join(windowsDirectory, "System32", "WindowsPowerShell", "v1.0", "powershell.exe") + info, err := os.Lstat(path) + if err != nil { + return "", fmt.Errorf("resolve PowerShell %s: %w", path, err) + } + if !info.Mode().IsRegular() { + return "", fmt.Errorf("resolve PowerShell %s: executable must be a regular file", path) + } + return path, nil +} + func secureWindowsACLVerdict(raw string, policyFile bool) error { lines := strings.Split(strings.ReplaceAll(raw, "\r", ""), "\n") if len(lines) == 0 || !strings.HasPrefix(lines[0], "OWNER|") { diff --git a/internal/policy/ownership_windows_test.go b/internal/policy/ownership_windows_test.go index 8e3de50..b7a329c 100644 --- a/internal/policy/ownership_windows_test.go +++ b/internal/policy/ownership_windows_test.go @@ -2,7 +2,38 @@ package policy -import "testing" +import ( + "os" + "path/filepath" + "testing" +) + +func TestPowerShellExecutableAtRequiresAbsoluteRegularSystemBinary(t *testing.T) { + root := t.TempDir() + executable := filepath.Join(root, "System32", "WindowsPowerShell", "v1.0", "powershell.exe") + if err := os.MkdirAll(filepath.Dir(executable), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(executable, []byte("fixture"), 0600); err != nil { + t.Fatal(err) + } + got, err := powerShellExecutableAt(root) + if err != nil || got != executable { + t.Fatalf("powerShellExecutableAt(%q) = (%q, %v), want %q", root, got, err, executable) + } + if _, err := powerShellExecutableAt("relative-windows"); err == nil { + t.Fatal("relative Windows directory accepted") + } + if err := os.Remove(executable); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(executable, 0700); err != nil { + t.Fatal(err) + } + if _, err := powerShellExecutableAt(root); err == nil { + t.Fatal("non-regular PowerShell executable accepted") + } +} func TestSecureWindowsACLVerdict(t *testing.T) { secure := "OWNER|S-1-5-32-544\nS-1-5-18|Allow|FullControl\nS-1-5-32-544|Allow|FullControl\nS-1-5-32-545|Allow|ReadAndExecute, Synchronize" diff --git a/internal/policy/policy.go b/internal/policy/policy.go index d5ce1ec..0cf8a3d 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -69,16 +69,23 @@ func Load() (Policy, error) { } func loadAt(path string, ownership func(string) error, now time.Time) (Policy, error) { - b, err := os.ReadFile(path) + info, err := os.Lstat(path) if errors.Is(err, os.ErrNotExist) { return Policy{}, nil } if err != nil { - return Policy{}, policyError("unreadable", "read %s: %v", path, err) + return Policy{}, policyError("unreadable", "inspect %s: %v", path, err) + } + if !info.Mode().IsRegular() { + return Policy{}, policyError("ownership", "%s: policy must be a regular file", path) } if err := ownership(path); err != nil { return Policy{}, policyError("ownership", "%s: %v", path, err) } + b, err := os.ReadFile(path) + if err != nil { + return Policy{}, policyError("unreadable", "read %s: %v", path, err) + } p, err := parse(path, b, now) if err != nil { return Policy{}, err diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index 7d85055..63687b6 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -52,6 +52,22 @@ func TestLoadAtOwnershipFailureIsCoded(t *testing.T) { assertPolicyCode(t, err, "ownership") } +func TestLoadAtRejectsNonRegularPolicyBeforeOwnershipOrRead(t *testing.T) { + path := filepath.Join(t.TempDir(), "policy.toml") + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + ownershipCalled := false + _, err := loadAt(path, func(string) error { + ownershipCalled = true + return nil + }, time.Now()) + assertPolicyCode(t, err, "ownership") + if ownershipCalled { + t.Fatal("ownership verifier called for non-regular policy object") + } +} + func TestParseRejectsInvalidPolicies(t *testing.T) { now := time.Date(2029, 1, 1, 0, 0, 0, 0, time.UTC) cases := []struct { diff --git a/main.go b/main.go index 95b2fa9..d0e4ac0 100644 --- a/main.go +++ b/main.go @@ -246,7 +246,7 @@ func usage(cfg string) { Commands: cb setup initialize/upgrade registry, install shims, then run doctor cb install create/update shims from the tool registry - cb add add a minimal stateless tool profile and install its shim + cb add add a minimal stateless tool profile; --local declares local intent cb doctor validate Docker, PATH, shims, registry, lock and managed volumes cb bugreport assemble a paste-ready diagnostic report with best-effort redaction cb backup back up registry + lock; --state adds explicitly named volumes From bfa36edb6996807890194def36a994039f6f2674 Mon Sep 17 00:00:00 2001 From: AviBackToBlack <54722547+AviBackToBlack@users.noreply.github.com> Date: Tue, 22 Sep 2026 01:33:30 +0100 Subject: [PATCH 3/5] fix: align policy docs and system path --- README.md | 5 +---- docs/enterprise-policy.md | 6 +----- docs/proxy-airgap.md | 5 +---- internal/policy/ownership_windows.go | 8 ++++---- 4 files changed, 7 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index fa2ac13..d9e4d28 100644 --- a/README.md +++ b/README.md @@ -204,10 +204,7 @@ and `stateful` profiles alike. [tools.token-meter] image = "example/token-meter:latest" provider = "stateless" -host_mounts = [ - "%USERPROFILE%\\.claude:/root/.claude:ro", - "%USERPROFILE%\\.codex:/root/.codex:ro", -] +host_mounts = ["%USERPROFILE%\\.claude:/root/.claude:ro", "%USERPROFILE%\\.codex:/root/.codex:ro"] ``` Entries follow the `SOURCE:/CONTAINER_PATH:MODE` shape used by diff --git a/docs/enterprise-policy.md b/docs/enterprise-policy.md index 1aa3b10..3f59360 100644 --- a/docs/enterprise-policy.md +++ b/docs/enterprise-policy.md @@ -37,11 +37,7 @@ with the machine's normal administrator configuration-management mechanism. policy_version = 1 require_lock = true allow_local_images = false -allowed_repositories = [ - "docker.io/library", - "ghcr.io/acme/developer-tools", - "registry.example.com:5443/platform" -] +allowed_repositories = ["docker.io/library", "ghcr.io/acme/developer-tools", "registry.example.com:5443/platform"] expires_at = "2027-01-01T00:00:00Z" ``` diff --git a/docs/proxy-airgap.md b/docs/proxy-airgap.md index 1705776..300c42b 100644 --- a/docs/proxy-airgap.md +++ b/docs/proxy-airgap.md @@ -27,10 +27,7 @@ For an explicit ContainerBin profile, prefer an allowlist such as: image = "registry.corp.example/devtools/terraform:1.13.3" provider = "stateless" path_equals = ["-chdir"] -env_names = [ - "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", - "http_proxy", "https_proxy", "no_proxy", -] +env_names = ["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy"] ``` The profile passes only variables already present in the host environment. Do diff --git a/internal/policy/ownership_windows.go b/internal/policy/ownership_windows.go index 4a403b4..94deafb 100644 --- a/internal/policy/ownership_windows.go +++ b/internal/policy/ownership_windows.go @@ -31,16 +31,16 @@ func verifyOwnership(path string) error { return nil } -var getWindowsDirectoryW = syscall.NewLazyDLL("kernel32.dll").NewProc("GetWindowsDirectoryW") +var getSystemWindowsDirectoryW = syscall.NewLazyDLL("kernel32.dll").NewProc("GetSystemWindowsDirectoryW") func powerShellExecutable() (string, error) { buffer := make([]uint16, 32768) - n, _, callErr := getWindowsDirectoryW.Call(uintptr(unsafe.Pointer(&buffer[0])), uintptr(len(buffer))) + n, _, callErr := getSystemWindowsDirectoryW.Call(uintptr(unsafe.Pointer(&buffer[0])), uintptr(len(buffer))) if n == 0 { - return "", fmt.Errorf("resolve Windows directory: %w", callErr) + return "", fmt.Errorf("resolve system Windows directory: %w", callErr) } if n >= uintptr(len(buffer)) { - return "", fmt.Errorf("resolve Windows directory: returned path is too long") + return "", fmt.Errorf("resolve system Windows directory: returned path is too long") } return powerShellExecutableAt(syscall.UTF16ToString(buffer[:n])) } From 26c1ba0e72e1fb260192b35e4750f2131ed9c073 Mon Sep 17 00:00:00 2001 From: AviBackToBlack <54722547+AviBackToBlack@users.noreply.github.com> Date: Wed, 23 Sep 2026 19:33:03 +0100 Subject: [PATCH 4/5] fix(policy): close authorization edge cases --- docs/enterprise-policy.md | 8 +++++- internal/lockfile/lockfile.go | 7 +++++ internal/lockfile/lockfile_test.go | 20 ++++++++++++++ internal/policy/policy.go | 44 +++++++++++++++++++++++++++--- internal/policy/policy_test.go | 23 ++++++++++++++++ 5 files changed, 97 insertions(+), 5 deletions(-) diff --git a/docs/enterprise-policy.md b/docs/enterprise-policy.md index 3f59360..f890d15 100644 --- a/docs/enterprise-policy.md +++ b/docs/enterprise-policy.md @@ -37,7 +37,11 @@ with the machine's normal administrator configuration-management mechanism. policy_version = 1 require_lock = true allow_local_images = false -allowed_repositories = ["docker.io/library", "ghcr.io/acme/developer-tools", "registry.example.com:5443/platform"] +allowed_repositories = [ + "docker.io/library", + "ghcr.io/acme/developer-tools", + "registry.example.com:5443/platform", +] expires_at = "2027-01-01T00:00:00Z" ``` @@ -70,6 +74,8 @@ Repository rules are canonical namespace boundaries: - a single-segment rule such as `python` means the Docker Hub namespace `docker.io/python`; it does not match the official image repository `docker.io/library/python`; +- an unqualified multi-segment rule such as `astral-sh/uv` means + `docker.io/astral-sh/uv`; - `ghcr.io/acme` does **not** allow `ghcr.io/acme-tools`. Authorization occurs before a repository-mode `docker pull`, local-image diff --git a/internal/lockfile/lockfile.go b/internal/lockfile/lockfile.go index c389c50..1e1d4b0 100644 --- a/internal/lockfile/lockfile.go +++ b/internal/lockfile/lockfile.go @@ -343,6 +343,10 @@ func RuntimeImageForTool(t registry.Tool, machinePolicy policy.Policy) (string, if err != nil { return "", fmt.Errorf("lockfile: %w", err) } + return runtimeImageForTool(t, machinePolicy, lf, path) +} + +func runtimeImageForTool(t registry.Tool, machinePolicy policy.Policy, lf *LockFile, path string) (string, error) { if lf == nil { if err := machinePolicy.AuthorizeImage(t.Image, false, false); err != nil { return "", err @@ -351,6 +355,9 @@ func RuntimeImageForTool(t registry.Tool, machinePolicy policy.Policy) (string, } e, ok := lf.Images[t.Image] if !ok || e.Configured != t.Image { + if err := machinePolicy.AuthorizeImage(t.Image, false, false); err != nil { + return "", err + } return "", fmt.Errorf("image %q is not locked in %s; run `cb update %s` or `cb lock`", t.Image, path, t.Name) } if err := machinePolicy.AuthorizeResolvedImage(t.Image, e.Resolved, IsLocalResolved(e.Resolved)); err != nil { diff --git a/internal/lockfile/lockfile_test.go b/internal/lockfile/lockfile_test.go index 101f6a0..bc3d686 100644 --- a/internal/lockfile/lockfile_test.go +++ b/internal/lockfile/lockfile_test.go @@ -309,6 +309,26 @@ func TestResolveImagePolicyDenialHappensBeforeDocker(t *testing.T) { } } +func TestRuntimeImageForToolReportsPolicyBeforeStaleLock(t *testing.T) { + tool := registry.Tool{Name: "python", Image: "python:3.13"} + stale := &LockFile{Version: 1, Images: map[string]LockEntry{}} + + requireLock := policy.Policy{SchemaVersion: 1, RequireLock: true} + if _, err := runtimeImageForTool(tool, requireLock, stale, "container-bin.lock"); err == nil || !strings.Contains(err.Error(), "[policy.lock_required]") { + t.Fatalf("require-lock stale entry error = %v", err) + } + + denyRepository := policy.Policy{SchemaVersion: 1, AllowedRepositories: []string{"ghcr.io/acme"}} + if _, err := runtimeImageForTool(tool, denyRepository, stale, "container-bin.lock"); err == nil || !strings.Contains(err.Error(), "[policy.repository_denied]") { + t.Fatalf("repository-denied stale entry error = %v", err) + } + + allowRepository := policy.Policy{SchemaVersion: 1, AllowedRepositories: []string{"docker.io/library"}} + if _, err := runtimeImageForTool(tool, allowRepository, stale, "container-bin.lock"); err == nil || !strings.Contains(err.Error(), "is not locked") { + t.Fatalf("authorized stale entry error = %v, want generic stale-lock error", err) + } +} + func TestLoadLockFile_RecoversFromBackup(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "container-bin.lock") diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 0cf8a3d..10054f1 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -97,7 +97,9 @@ func parse(path string, b []byte, now time.Time) (Policy, error) { p := Policy{Path: path} seen := map[string]bool{} sc := bufio.NewScanner(strings.NewReader(string(b))) - for lineNo := 1; sc.Scan(); lineNo++ { + lineNo := 0 + for sc.Scan() { + lineNo++ line := strings.TrimSpace(toml.StripComment(sc.Text())) if line == "" { continue @@ -136,9 +138,20 @@ func parse(path string, b []byte, now time.Time) (Policy, error) { } p.AllowLocalImages = v case "allowed_repositories": + startLine := lineNo + for strings.HasPrefix(strings.TrimSpace(raw), "[") && !arrayValueComplete(raw) { + if !sc.Scan() { + if err := sc.Err(); err != nil { + return Policy{}, policyError("unreadable", "scan %s: %v", path, err) + } + return Policy{}, policyError("syntax", "line %d allowed_repositories: unterminated array", startLine) + } + lineNo++ + raw += "\n" + strings.TrimSpace(toml.StripComment(sc.Text())) + } values, err := toml.ParseStringArray(raw) if err != nil { - return Policy{}, policyError("syntax", "line %d allowed_repositories: %v", lineNo, err) + return Policy{}, policyError("syntax", "line %d allowed_repositories: %v", startLine, err) } p.AllowedRepositories = values case "expires_at": @@ -187,6 +200,29 @@ func parse(path string, b []byte, now time.Time) (Policy, error) { return p, nil } +func arrayValueComplete(raw string) bool { + inQuote := false + escaped := false + for _, r := range raw { + if escaped { + escaped = false + continue + } + if r == '\\' && inQuote { + escaped = true + continue + } + if r == '"' { + inQuote = !inQuote + continue + } + if r == ']' && !inQuote { + return true + } + } + return false +} + func (p Policy) AuthorizeImage(configured string, locked, local bool) error { if !p.Managed() { return nil @@ -331,8 +367,8 @@ func canonicalRule(rule string) (string, error) { case "index.docker.io", "registry-1.docker.io": parts[0] = "docker.io" } - if len(parts) == 1 && !strings.Contains(parts[0], ".") && !strings.Contains(parts[0], ":") && parts[0] != "localhost" { - parts = []string{"docker.io", parts[0]} + if !strings.Contains(parts[0], ".") && !strings.Contains(parts[0], ":") && parts[0] != "localhost" { + parts = append([]string{"docker.io"}, parts...) } return strings.Join(parts, "/"), nil } diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index 63687b6..4b97409 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -43,6 +43,28 @@ func TestLoadAtStrictAndCanonical(t *testing.T) { } } +func TestLoadAtAcceptsMultilineAllowedRepositories(t *testing.T) { + path := filepath.Join(t.TempDir(), "policy.toml") + contents := `policy_version = 1 +allowed_repositories = [ + "docker.io/library", # official images + "astral-sh/uv", + "ghcr.io/acme/developer-tools", +] +` + if err := os.WriteFile(path, []byte(contents), 0600); err != nil { + t.Fatal(err) + } + p, err := loadAt(path, func(string) error { return nil }, time.Now()) + if err != nil { + t.Fatal(err) + } + want := []string{"docker.io/astral-sh/uv", "docker.io/library", "ghcr.io/acme/developer-tools"} + if strings.Join(p.AllowedRepositories, ",") != strings.Join(want, ",") { + t.Fatalf("allowed repositories = %v, want %v", p.AllowedRepositories, want) + } +} + func TestLoadAtOwnershipFailureIsCoded(t *testing.T) { path := filepath.Join(t.TempDir(), "policy.toml") if err := os.WriteFile(path, []byte("policy_version = 1\nrequire_lock = true\n"), 0600); err != nil { @@ -81,6 +103,7 @@ func TestParseRejectsInvalidPolicies(t *testing.T) { {"no controls", "policy_version = 1\nallow_local_images = true\n", "syntax"}, {"expired", "policy_version = 1\nrequire_lock = true\nexpires_at = \"2028-01-01T00:00:00Z\"\n", "expired"}, {"bad rule", "policy_version = 1\nallowed_repositories = [\"ghcr.io//team\"]\n", "syntax"}, + {"unterminated array", "policy_version = 1\nallowed_repositories = [\n \"ghcr.io/team\",\n", "syntax"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { From 1c75596af564b68f91701d97979d3bd939499493 Mon Sep 17 00:00:00 2001 From: AviBackToBlack <54722547+AviBackToBlack@users.noreply.github.com> Date: Wed, 23 Sep 2026 19:58:11 +0100 Subject: [PATCH 5/5] fix(policy): harden lock and ACL validation --- internal/lockfile/lockfile.go | 37 ++++++++++++++++++----- internal/lockfile/lockfile_test.go | 2 ++ internal/policy/ownership_windows.go | 2 +- internal/policy/ownership_windows_test.go | 3 ++ 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/internal/lockfile/lockfile.go b/internal/lockfile/lockfile.go index 1e1d4b0..59f1657 100644 --- a/internal/lockfile/lockfile.go +++ b/internal/lockfile/lockfile.go @@ -90,11 +90,11 @@ func Load(path string) (*LockFile, error) { return fmt.Errorf("lock entry %q local image digest must match resolved ID", curKey) } } else { - i := strings.LastIndex(cur.Resolved, "@") - if i <= 0 || !validImageID(cur.Resolved[i+1:]) { + _, resolvedDigest, ok := splitImmutableRepositoryDigest(cur.Resolved) + if !ok { return fmt.Errorf("lock entry %q has invalid immutable repository digest %q", curKey, cur.Resolved) } - if cur.Digest != cur.Resolved[i+1:] { + if cur.Digest != resolvedDigest { return fmt.Errorf("lock entry %q digest does not match resolved repository digest", curKey) } if matched, ok := matchRepoDigest(cur.Configured, []string{cur.Resolved}); !ok || matched != cur.Resolved { @@ -240,17 +240,35 @@ func canonicalRepository(repo string) string { func matchRepoDigest(configured string, repoDigests []string) (string, bool) { want := canonicalRepository(imageRepository(configured)) for _, rd := range repoDigests { - i := strings.LastIndex(rd, "@") - if i < 0 || !strings.HasPrefix(rd[i+1:], "sha256:") { + repo, _, ok := splitImmutableRepositoryDigest(rd) + if !ok { continue } - if canonicalRepository(rd[:i]) == want { + if canonicalRepository(repo) == want { return rd, true } } return "", false } +func splitImmutableRepositoryDigest(ref string) (string, string, bool) { + if strings.Count(ref, "@") != 1 { + return "", "", false + } + repo, digest, _ := strings.Cut(ref, "@") + if repo == "" || !validImageID(digest) { + return "", "", false + } + lastSlash := strings.LastIndexByte(repo, '/') + if strings.LastIndexByte(repo, ':') > lastSlash { + return "", "", false + } + if _, err := policy.CanonicalRepository(repo); err != nil { + return "", "", false + } + return repo, digest, true +} + type imageInspection struct { id string repoDigests []string @@ -294,8 +312,11 @@ func repositoryLockEntry(configured string, inspected imageInspection) (LockEntr // configured reference never had. return LockEntry{}, fmt.Errorf("image %s has no RepoDigest for repository %q (locally tagged image?); pull it from its registry before locking", configured, imageRepository(configured)) } - i := strings.LastIndex(resolved, "@") - return LockEntry{Configured: configured, Resolved: resolved, Digest: resolved[i+1:]}, nil + _, digest, ok := splitImmutableRepositoryDigest(resolved) + if !ok { + return LockEntry{}, fmt.Errorf("image %s returned malformed RepoDigest %q", configured, resolved) + } + return LockEntry{Configured: configured, Resolved: resolved, Digest: digest}, nil } func localLockEntry(configured string, inspected imageInspection) (LockEntry, error) { diff --git a/internal/lockfile/lockfile_test.go b/internal/lockfile/lockfile_test.go index bc3d686..3b91b47 100644 --- a/internal/lockfile/lockfile_test.go +++ b/internal/lockfile/lockfile_test.go @@ -86,6 +86,8 @@ func TestLockFileRejectsMutableOrForeignRepositoryResolution(t *testing.T) { id := entryID(configured) cases := []string{ "ghcr.io/acme/tool:latest", + "ghcr.io/acme/tool:latest@sha256:" + strings.Repeat("a", 64), + "ghcr.io/acme/tool@bad@sha256:" + strings.Repeat("a", 64), "evil.example/tool@sha256:" + strings.Repeat("a", 64), "ghcr.io/acme/tool@sha256:short", } diff --git a/internal/policy/ownership_windows.go b/internal/policy/ownership_windows.go index 94deafb..698a809 100644 --- a/internal/policy/ownership_windows.go +++ b/internal/policy/ownership_windows.go @@ -84,7 +84,7 @@ func secureWindowsACLVerdict(raw string, policyFile bool) error { rights := parts[2] blocked := []string{"FullControl", "Modify", "Delete", "DeleteSubdirectoriesAndFiles", "TakeOwnership", "ChangePermissions"} if policyFile { - blocked = append(blocked, "Write", "CreateFiles", "AppendData") + blocked = append(blocked, "Write", "WriteData", "CreateFiles", "AppendData") } for _, right := range blocked { if strings.Contains(rights, right) { diff --git a/internal/policy/ownership_windows_test.go b/internal/policy/ownership_windows_test.go index b7a329c..8d824a8 100644 --- a/internal/policy/ownership_windows_test.go +++ b/internal/policy/ownership_windows_test.go @@ -46,6 +46,9 @@ func TestSecureWindowsACLVerdict(t *testing.T) { if err := secureWindowsACLVerdict("OWNER|S-1-5-18\nS-1-1-0|Allow|Write", true); err == nil { t.Fatal("untrusted file writer accepted") } + if err := secureWindowsACLVerdict("OWNER|S-1-5-18\nS-1-1-0|Allow|WriteData", true); err == nil { + t.Fatal("untrusted file WriteData grant accepted") + } if err := secureWindowsACLVerdict("OWNER|S-1-5-18\nS-1-1-0|Allow|CreateFiles", false); err != nil { t.Fatalf("safe parent create right rejected: %v", err) }