diff --git a/README.md b/README.md index 84ed4f6..1fd4408 100644 --- a/README.md +++ b/README.md @@ -139,15 +139,55 @@ env_prefixes = ["TF_", "AWS_", "ARM_"] # only these host vars enter the contain Semantics include `command`, `args_prefix`, `path_next`, `path_equals`, `path_last`, `path_last_if_any`, `env_names`, `env_prefixes`, `env_set`, -`project_markers`, `state_group`, `project_volumes` and `shared_volumes`. -Unknown keys **fail validation** instead of being silently ignored, and a -`schema_version` newer than the binary supports fails closed. Edit the file, -then run `cb install` to reconcile shims. - -One exception applies to every path-forcing key (`path_next`, `path_equals`, -`path_last`): a value whose final element is `...` is never rewritten, because -Windows cannot represent such a directory and rewriting it would silently -change which files the tool acts on. See [Windows path mapping](#windows-path-mapping). +`project_markers`, `state_group`, `project_volumes`, `shared_volumes` and +`host_mounts`. Unknown keys **fail validation** instead of being silently +ignored, and a `schema_version` newer than the binary supports fails closed. +Edit the file, then run `cb install` to reconcile shims. + +### Explicit host bind mounts (`host_mounts`) + +`host_mounts` lets a trusted profile declare fixed host paths that are always +bind-mounted into the container, regardless of whether they appear in the +command line. This is provider-agnostic: it works for `stateless`, `python` +and `stateful` profiles alike. + +```toml +[tools.token-meter] +image = "example/token-meter:latest" +provider = "stateless" +host_mounts = [ + "%USERPROFILE%\\.claude:/root/.claude:ro", + "%USERPROFILE%\\.codex:/root/.codex:ro", +] +``` + +Entries follow the `SOURCE:/CONTAINER_PATH:MODE` shape used by +`project_volumes`/`shared_volumes`, extended with a required third `:MODE` +segment. `ro` and `rw` are accepted; there is **no default** — every mount's +write access must be explicit in the registry line. + +`%USERPROFILE%` is the only host variable container-bin expands; no other +`%...%` token is recognized or guessed. The source may also be a literal +Windows absolute path using a backslash after the drive letter +(e.g. `D:\Video`). The forward-slash drive form (`D:/Video`) embeds the `:/` +source/target delimiter and is rejected as ambiguous, so always use `X:\...`. + +Targets under `/workspace`, `/cb`, `/venv` and `/root/.cache/pip` are reserved +for container-bin's own project workspace and managed state mounts (the last +two are the python provider's fixed venv/pip-cache paths) and cannot be +claimed by `host_mounts`, on any provider. + +> A `host_mounts` entry grants the configured Docker image direct access to the +> named host files or directories. ContainerBin is **not a security sandbox**; +> a `rw` mount exposes those host files to any code running in the container, +> exactly as a `docker run --mount` would. Use `ro` when the tool only needs to +> read, review every `rw` mount, and keep the registry and shim directory +> under your control. +> +> Exposed profiles created by `cb expose` from an npm-shaped source profile do +> **not** inherit that source's `host_mounts`; host access never propagates +> implicitly to an auto-generated shim. A profile that genuinely needs a host +> mount must declare it explicitly. ## Windows path mapping diff --git a/docs/security-model.md b/docs/security-model.md index a2c2257..a1b92da 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -11,8 +11,9 @@ mounted in, and the environment variables your profiles select passed through. Inside the boundary (whoever controls these controls execution): - `container-bin.toml` — names the images, the env allowlists, the volumes, - the path semantics. Arbitrary registry write access ≈ arbitrary code - execution with your Docker privileges. + the `host_mounts`, and the path semantics. Arbitrary registry write access + ≈ arbitrary code execution with your Docker privileges; `host_mounts` with + `rw` access hand the image write access to those host paths. - `container-bin.lock` — pins image digests. Whoever can rewrite it can pin a malicious digest. - The shim directory on `PATH` — whoever can write executables there doesn't @@ -31,7 +32,11 @@ readable, and dangerous to let others edit. - **Narrow mounts.** The project root is mounted; paths outside it get individual narrow bind mounts (for files: the parent directory; for not-yet-existing outputs: nearest existing ancestor). Whole drives are never - mounted because one argument lives on them. + mounted because one argument lives on them. `host_mounts` are the explicit, + mode-required exception: a trusted profile can declare a fixed host source + and container target, but every entry must spell out `ro` or `rw` and is + validated as a `SOURCE:/CONTAINER_PATH:MODE` binding with the same fail-closed + checks as the volume fields. - **Conservative path rewriting.** Arguments are only treated as paths when their shape is unambiguous or the tool profile explicitly declares the semantics. Unknown strings pass through untouched. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 9e0ca65..30e05a4 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -98,6 +98,28 @@ func Trace(reg registry.Registry, args []string) error { fmt.Printf("shared_volume: %s -> %s\n", pathmap.StatefulSharedVolumeID(t.StateGroup, name), dst) } } + for _, spec := range t.HostMounts { + source, target, mode, err := registry.ParseHostMount(spec) + if err != nil { + return err + } + expanded, err := dockerrun.ExpandHostMountSource(source) + if err == nil { + expanded, err = pathmap.CanonicalPath(expanded) + } + switch { + case err != nil: + fmt.Printf("host_mount: %s -> %s (%s) [resolve error: %v]\n", source, target, mode, err) + case strings.HasPrefix(expanded, `\\`): + fmt.Printf("host_mount: %s -> %s (%s) [would fail: resolves to a UNC path, which Docker Desktop cannot share]\n", expanded, target, mode) + default: + if _, statErr := os.Stat(expanded); statErr != nil { + fmt.Printf("host_mount: %s -> %s (%s) [would fail: source does not exist]\n", expanded, target, mode) + } else { + fmt.Printf("host_mount: %s -> %s (%s)\n", expanded, target, mode) + } + } + } return nil } @@ -314,6 +336,13 @@ func Inspect(reg registry.Registry, args []string) error { } fmt.Printf("shared_volume: %s -> %s\n", pathmap.StatefulSharedVolumeID(t.StateGroup, logical), dst) } + for _, spec := range t.HostMounts { + source, target, mode, e := registry.ParseHostMount(spec) + if e != nil { + return e + } + fmt.Printf("host_mount: %s -> %s (%s)\n", source, target, mode) + } if t.Provider == "python" { fmt.Printf("python_env: %s\npip_cache: cb-pip-cache\n", pathmap.PythonEnvID(root, found)) } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index a00397d..ac3951c 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -1,11 +1,14 @@ package cli import ( + "os" "path/filepath" "reflect" + "runtime" "strings" "testing" + "github.com/AviBackToBlack/container-bin/internal/pathmap" "github.com/AviBackToBlack/container-bin/internal/registry" ) @@ -87,3 +90,218 @@ func TestRenderExposedToolSection(t *testing.T) { t.Errorf("command = %v, want %v", got.Command, wantCommand) } } + +// captureStdout redirects os.Stdout to a temp file for the duration of fn, +// then restores it and returns everything fn wrote. Mirrors the helper in +// internal/diag so the new inspect/trace output lines can be asserted without +// changing those functions' signatures just for tests. Safe only because +// none of this package's tests call t.Parallel() — it swaps the process- +// global os.Stdout, so a parallel test using it would interleave captures +// silently; keep every test using it serial (same precondition +// internal/diag's copy documents at its own call site). +func captureStdout(fn func() error) (string, error) { + f, err := os.CreateTemp("", "cb-cli-test-") + if err != nil { + return "", err + } + tmpPath := f.Name() + defer os.Remove(tmpPath) + + real := os.Stdout + os.Stdout = f + defer func() { os.Stdout = real }() + + _ = fn() + + if cerr := f.Close(); cerr != nil { + return "", cerr + } + data, rerr := os.ReadFile(tmpPath) + if rerr != nil { + return "", rerr + } + return string(data), nil +} + +func setTestHome(t *testing.T, dir string) { + t.Helper() + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", dir) + } else { + t.Setenv("HOME", dir) + } +} + +func TestInspectPrintsHostMountRaw(t *testing.T) { + dir := t.TempDir() + old, _ := os.Getwd() + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + defer os.Chdir(old) + if err := os.WriteFile(filepath.Join(dir, ".git"), []byte{}, 0644); err != nil { + t.Fatal(err) + } + + reg, err := registry.ParseTOML(`[tools.demo] +image = "demo:1" +provider = "stateless" +host_mounts = ["%USERPROFILE%\\.claude:/root/.claude:ro"] +`) + if err != nil { + t.Fatalf("parse registry: %v", err) + } + + out, err := captureStdout(func() error { return Inspect(reg, []string{"demo"}) }) + if err != nil { + t.Fatalf("capture: %v", err) + } + if !strings.Contains(out, "host_mount: %USERPROFILE%\\.claude -> /root/.claude (ro)") { + t.Fatalf("inspect output missing raw host_mount line:\n%s", out) + } +} + +func TestTracePrintsHostMountExpanded(t *testing.T) { + dir := t.TempDir() + homeDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(homeDir, ".claude"), 0755); err != nil { + t.Fatal(err) + } + setTestHome(t, homeDir) + + old, _ := os.Getwd() + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + defer os.Chdir(old) + if err := os.WriteFile(filepath.Join(dir, ".git"), []byte{}, 0644); err != nil { + t.Fatal(err) + } + + reg, err := registry.ParseTOML(`[tools.demo] +image = "demo:1" +provider = "stateless" +host_mounts = ["%USERPROFILE%/.claude:/root/.claude:ro"] +`) + if err != nil { + t.Fatalf("parse registry: %v", err) + } + + expectedCanon, err := pathmap.CanonicalPath(homeDir + "/.claude") + if err != nil { + t.Fatal(err) + } + + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + if err != nil { + t.Fatalf("capture: %v", err) + } + want := "host_mount: " + expectedCanon + " -> /root/.claude (ro)" + if !strings.Contains(out, want) { + t.Fatalf("trace output missing expanded host_mount line (want %q):\n%s", want, out) + } +} + +func TestTraceHostMountResolveErrorIsPrintedNotFatal(t *testing.T) { + dir := t.TempDir() + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", "") + } else { + t.Setenv("HOME", "") + } + + old, _ := os.Getwd() + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + defer os.Chdir(old) + if err := os.WriteFile(filepath.Join(dir, ".git"), []byte{}, 0644); err != nil { + t.Fatal(err) + } + + reg, err := registry.ParseTOML(`[tools.demo] +image = "demo:1" +provider = "stateless" +host_mounts = ["%USERPROFILE%/.claude:/root/.claude:ro"] +`) + if err != nil { + t.Fatalf("parse registry: %v", err) + } + + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + if err != nil { + t.Fatalf("capture: %v", err) + } + if !strings.Contains(out, "host_mount: %USERPROFILE%/.claude -> /root/.claude (ro)") { + t.Fatalf("trace output missing host_mount line:\n%s", out) + } + if !strings.Contains(out, "resolve error:") { + t.Fatalf("trace output missing resolve error marker:\n%s", out) + } +} + +func TestTraceHostMountMissingSourceWouldFail(t *testing.T) { + dir := t.TempDir() + homeDir := t.TempDir() + setTestHome(t, homeDir) + + old, _ := os.Getwd() + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + defer os.Chdir(old) + if err := os.WriteFile(filepath.Join(dir, ".git"), []byte{}, 0644); err != nil { + t.Fatal(err) + } + + reg, err := registry.ParseTOML(`[tools.demo] +image = "demo:1" +provider = "stateless" +host_mounts = ["%USERPROFILE%/does-not-exist:/root/missing:ro"] +`) + if err != nil { + t.Fatalf("parse registry: %v", err) + } + + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + if err != nil { + t.Fatalf("capture: %v", err) + } + if !strings.Contains(out, "[would fail: source does not exist]") { + t.Fatalf("trace output missing would-fail annotation:\n%s", out) + } +} + +func TestTraceHostMountUNCWouldFail(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("UNC resolution is only meaningful on Windows") + } + dir := t.TempDir() + t.Setenv("USERPROFILE", `\\server\share\home`) + + old, _ := os.Getwd() + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + defer os.Chdir(old) + if err := os.WriteFile(filepath.Join(dir, ".git"), []byte{}, 0644); err != nil { + t.Fatal(err) + } + + reg, err := registry.ParseTOML(`[tools.demo] +image = "demo:1" +provider = "stateless" +host_mounts = ["%USERPROFILE%/.claude:/root/.claude:ro"] +`) + if err != nil { + t.Fatalf("parse registry: %v", err) + } + + out, err := captureStdout(func() error { return Trace(reg, []string{"demo"}) }) + if err != nil { + t.Fatalf("capture: %v", err) + } + if !strings.Contains(out, "[would fail: resolves to a UNC path, which Docker Desktop cannot share]") { + t.Fatalf("trace output missing UNC would-fail annotation:\n%s", out) + } +} diff --git a/internal/diag/diag.go b/internal/diag/diag.go index c4802a4..db042c9 100644 --- a/internal/diag/diag.go +++ b/internal/diag/diag.go @@ -115,6 +115,24 @@ func reparsePointVerdict(subject, literalPath, resolvedPath string) (status, mes return "warn", fmt.Sprintf("%s does not resolve to itself (literal: %s, resolved: %s); if this is a junction or symlink, that is supported (see docs/windows-paths.md P1), but links inside the tree do not traverse from inside the container (P3) — filepath.EvalSymlinks can also produce a different form for reasons other than a reparse point (e.g. expanding an 8.3 short name), so this may be informational only", subject, literalPath, resolvedPath) } +// hostMountVerdict reports whether a declared host_mounts source resolves to +// a usable path right now. A missing source is a per-machine/per-user state +// fact (the directory may simply not exist yet on this machine), not a +// container-bin defect, so this warns rather than fails -- the same +// warn-not-fail precedent as dockerOSTypeVerdict. +func hostMountVerdict(toolName, source, target, canonicalPath string, exists bool) (status, message string) { + if !exists { + // This is warn, not fail, because a missing source is per-machine/ + // per-user state, not a container-bin defect (dockerOSTypeVerdict's + // warn-not-fail precedent). But RunTool fails closed on the identical + // condition -- cb doctor summarizing "0 failures" must not read as + // "this tool will run"; naming the actual consequence in the message + // closes that gap without changing the warn/fail severity. + return "warn", fmt.Sprintf("%s: host_mounts source %q for %s does not exist: %s (this tool cannot run until it does)", toolName, source, target, canonicalPath) + } + return "ok", fmt.Sprintf("%s: host_mounts source %q for %s exists: %s", toolName, source, target, canonicalPath) +} + func networkStorageVerdict(subject, path, driveType string) (status, message string) { if strings.HasPrefix(path, `\\`) { return "warn", fmt.Sprintf("%s is on a UNC path (%s); Docker Desktop cannot bind-mount a UNC source; see docs/windows-paths.md P8", subject, path) @@ -341,6 +359,56 @@ func Doctor(reg registry.Registry, cfgPath string) error { } } + // Sorted, not map-order: reg.Tools iteration order is otherwise + // nondeterministic, and cb bugreport embeds this output verbatim, which + // would make diffing two reports noisier than necessary for no benefit. + hostMountToolNames := make([]string, 0, len(reg.Tools)) + for name, t := range reg.Tools { + if len(t.HostMounts) > 0 { + hostMountToolNames = append(hostMountToolNames, name) + } + } + sort.Strings(hostMountToolNames) + for _, name := range hostMountToolNames { + t := reg.Tools[name] + for _, spec := range t.HostMounts { + source, target, _, err := registry.ParseHostMount(spec) + if err != nil { + warn("%s: host_mounts entry %q is invalid: %v", name, spec, err) + continue + } + expanded, err := dockerrun.ExpandHostMountSource(source) + if err != nil { + warn("%s: host_mounts source %q could not be resolved: %v", name, source, err) + continue + } + canon, err := pathmap.CanonicalPath(expanded) + if err != nil { + warn("%s: host_mounts source %q could not be canonicalized: %v", name, source, err) + continue + } + _, statErr := os.Stat(canon) + status, msg := hostMountVerdict(name, source, target, canon, statErr == nil) + if status == "ok" { + ok("%s", msg) + } else { + warn("%s", msg) + } + if runtime.GOOS == "windows" { + driveType := "" + if !strings.HasPrefix(canon, `\\`) { + if dt, err := windowsDriveType(filepath.VolumeName(canon)); err == nil { + driveType = dt + } + } + nsStatus, nsMsg := networkStorageVerdict("host_mounts source for "+target, canon, driveType) + if nsStatus != "ok" { + warn("%s", nsMsg) + } + } + } + } + managed, err := dockervol.LabeledManaged() if err != nil { warn("managed volume inspection failed: %v", err) diff --git a/internal/diag/doctor_test.go b/internal/diag/doctor_test.go index 3184647..b3d11e1 100644 --- a/internal/diag/doctor_test.go +++ b/internal/diag/doctor_test.go @@ -388,3 +388,25 @@ func TestNetworkStorageVerdict(t *testing.T) { } } } + +func TestHostMountVerdict(t *testing.T) { + status, msg := hostMountVerdict("demo", "%USERPROFILE%\\.claude", "/root/.claude", "C:\\Users\\x\\.claude", true) + if status != "ok" { + t.Fatalf("expected ok for existing source, got %q", status) + } + for _, want := range []string{"demo", "\"%USERPROFILE%\\\\.claude\"", "/root/.claude", "exists", "C:\\Users\\x\\.claude"} { + if !strings.Contains(msg, want) { + t.Fatalf("message %q does not contain %q", msg, want) + } + } + + status, msg = hostMountVerdict("demo", "%USERPROFILE%\\.claude", "/root/.claude", "C:\\Users\\x\\.claude", false) + if status != "warn" { + t.Fatalf("expected warn for missing source, got %q", status) + } + for _, want := range []string{"demo", "\"%USERPROFILE%\\\\.claude\"", "/root/.claude", "does not exist", "C:\\Users\\x\\.claude", "cannot run"} { + if !strings.Contains(msg, want) { + t.Fatalf("message %q does not contain %q", msg, want) + } + } +} diff --git a/internal/dockerrun/dockerrun.go b/internal/dockerrun/dockerrun.go index 43503a1..0ed61d4 100644 --- a/internal/dockerrun/dockerrun.go +++ b/internal/dockerrun/dockerrun.go @@ -78,6 +78,11 @@ func RunTool(t registry.Tool, userArgs []string) (int, error) { } args = append(args, "--mount", mount) } + hostMountArgs, err := buildHostMountArgs(t.HostMounts) + if err != nil { + return 1, err + } + args = append(args, hostMountArgs...) for _, name := range selectedHostEnv(t) { args = append(args, "-e", name) } @@ -213,7 +218,7 @@ func selectedHostEnv(t registry.Tool) []string { return out } -func MountSpec(kind, src, dst string) (string, error) { +func mountSpecChecked(kind, src, dst string) error { // src is checked before dst deliberately: for the project-root bind // mount, dst (workspaceRoot) is always derived from a substring of src // (root), so this ordering is what makes a comma-named project fail on @@ -222,14 +227,84 @@ func MountSpec(kind, src, dst string) (string, error) { // would not change whether the mount is rejected, but would change which // message the row and test depend on. if strings.Contains(src, ",") { - return "", fmt.Errorf("source path/volume contains a comma and cannot be represented safely in docker --mount syntax (values are comma-separated with no escaping): %s", src) + return fmt.Errorf("source path/volume contains a comma and cannot be represented safely in docker --mount syntax (values are comma-separated with no escaping): %s", src) } if strings.Contains(dst, ",") { - return "", fmt.Errorf("destination path contains a comma and cannot be represented safely in docker --mount syntax (values are comma-separated with no escaping): %s", dst) + return fmt.Errorf("destination path contains a comma and cannot be represented safely in docker --mount syntax (values are comma-separated with no escaping): %s", dst) + } + return nil +} + +func MountSpec(kind, src, dst string) (string, error) { + if err := mountSpecChecked(kind, src, dst); err != nil { + return "", err } return fmt.Sprintf("type=%s,src=%s,dst=%s", kind, src, dst), nil } +// MountSpecMode is MountSpec plus an explicit ro/rw mode. mode must be "ro" or +// "rw" -- registry.ParseHostMount already guarantees this for every +// host_mounts entry, so an unrecognized value here is a programmer error, not +// user input; fail closed rather than silently defaulting. +func MountSpecMode(kind, src, dst, mode string) (string, error) { + if err := mountSpecChecked(kind, src, dst); err != nil { + return "", err + } + switch mode { + case "ro": + return fmt.Sprintf("type=%s,src=%s,dst=%s,readonly", kind, src, dst), nil + case "rw": + return fmt.Sprintf("type=%s,src=%s,dst=%s", kind, src, dst), nil + default: + return "", fmt.Errorf("unsupported mount mode %q", mode) + } +} + +// ExpandHostMountSource resolves the one host variable container-bin +// understands in a host_mounts source. A literal Windows absolute path is +// returned unchanged. registry.ParseHostMount has already rejected any other +// %...% token, so this never needs to guess about anything it hasn't seen. +func ExpandHostMountSource(source string) (string, error) { + if strings.HasPrefix(source, "%USERPROFILE%") { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolve %%USERPROFILE%%: %w", err) + } + return home + strings.TrimPrefix(source, "%USERPROFILE%"), nil + } + return source, nil +} + +func buildHostMountArgs(hostMounts []string) ([]string, error) { + var args []string + for _, spec := range hostMounts { + source, target, mode, err := registry.ParseHostMount(spec) + if err != nil { + return nil, err // unreachable in practice; the registry was already validated at load + } + expanded, err := ExpandHostMountSource(source) + if err != nil { + return nil, err + } + canon, err := pathmap.CanonicalPath(expanded) + if err != nil { + return nil, fmt.Errorf("host_mounts source %q: %w", source, err) + } + if strings.HasPrefix(canon, `\\`) { + return nil, fmt.Errorf("host_mounts source %q resolves to a UNC path, which Docker Desktop cannot share", source) + } + if _, statErr := os.Stat(canon); statErr != nil { + return nil, fmt.Errorf("host_mounts source %q does not exist: %s", source, canon) + } + mount, err := MountSpecMode("bind", canon, target, mode) + if err != nil { + return nil, err + } + args = append(args, "--mount", mount) + } + return args, nil +} + func EnsureImageLocalForTool(t registry.Tool) error { ref, err := lockfile.RuntimeImageForTool(t) if err != nil { diff --git a/internal/dockerrun/dockerrun_test.go b/internal/dockerrun/dockerrun_test.go new file mode 100644 index 0000000..9767739 --- /dev/null +++ b/internal/dockerrun/dockerrun_test.go @@ -0,0 +1,208 @@ +package dockerrun + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/AviBackToBlack/container-bin/internal/registry" +) + +func setTestHome(t *testing.T, dir string) { + t.Helper() + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", dir) + } else { + t.Setenv("HOME", dir) + } +} + +func TestMountSpecMode(t *testing.T) { + got, err := MountSpecMode("bind", "/some/src", "/some/dst", "ro") + if err != nil { + t.Fatalf("MountSpecMode ro returned error: %v", err) + } + want := "type=bind,src=/some/src,dst=/some/dst,readonly" + if got != want { + t.Fatalf("MountSpecMode ro = %q, want %q", got, want) + } + + got, err = MountSpecMode("bind", "/some/src", "/some/dst", "rw") + if err != nil { + t.Fatalf("MountSpecMode rw returned error: %v", err) + } + want = "type=bind,src=/some/src,dst=/some/dst" + if got != want { + t.Fatalf("MountSpecMode rw = %q, want %q", got, want) + } + + _, err = MountSpecMode("bind", "/some/src", "/some/dst", "unsupported") + if err == nil { + t.Fatal("expected error for unsupported mode") + } +} + +func TestMountSpecModeCommaRejection(t *testing.T) { + _, err := MountSpecMode("bind", "/bad, src", "/clean/dst", "ro") + if err == nil { + t.Fatal("expected comma in src to be rejected") + } + if !strings.Contains(err.Error(), "source") { + t.Fatalf("error should name the source side: %v", err) + } + + _, err = MountSpecMode("bind", "/clean/src", "/bad, dst", "ro") + if err == nil { + t.Fatal("expected comma in dst to be rejected") + } + if !strings.Contains(err.Error(), "destination") { + t.Fatalf("error should name the destination side: %v", err) + } +} + +func TestExpandHostMountSource(t *testing.T) { + homeDir := t.TempDir() + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", homeDir) + } else { + t.Setenv("HOME", homeDir) + } + + src := "%USERPROFILE%/.config" + got, err := ExpandHostMountSource(src) + if err != nil { + t.Fatalf("ExpandHostMountSource(%q) error: %v", src, err) + } + want := homeDir + "/.config" + if got != want { + t.Fatalf("ExpandHostMountSource(%q) = %q, want %q", src, got, want) + } + + literal := `C:\Users\x\.claude` + got, err = ExpandHostMountSource(literal) + if err != nil { + t.Fatalf("ExpandHostMountSource(%q) error: %v", literal, err) + } + if got != literal { + t.Fatalf("ExpandHostMountSource(%q) = %q, want %q", literal, got, literal) + } +} + +func TestBuildHostMountArgs(t *testing.T) { + homeDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(homeDir, ".claude"), 0755); err != nil { + t.Fatal(err) + } + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", homeDir) + } else { + t.Setenv("HOME", homeDir) + } + + reg, err := registry.ParseTOML(`[tools.x] +image = "x:1" +provider = "stateless" +host_mounts = ["%USERPROFILE%/.claude:/root/.claude:ro"] +`) + if err != nil { + t.Fatalf("parse registry: %v", err) + } + tool := reg.Tools["x"] + + args, err := buildHostMountArgs(tool.HostMounts) + if err != nil { + t.Fatalf("buildHostMountArgs error: %v", err) + } + if len(args) != 2 || args[0] != "--mount" { + t.Fatalf("unexpected args: %#v", args) + } + mount := args[1] + if !strings.HasPrefix(mount, "type=bind,src=") { + t.Fatalf("unexpected mount string: %q", mount) + } + if !strings.Contains(mount, ",dst=/root/.claude,readonly") { + t.Fatalf("mount string missing readonly dst: %q", mount) + } +} + +func TestBuildHostMountArgsRejectsMissingSource(t *testing.T) { + homeDir := t.TempDir() + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", homeDir) + } else { + t.Setenv("HOME", homeDir) + } + + reg, err := registry.ParseTOML(`[tools.x] +image = "x:1" +provider = "stateless" +host_mounts = ["%USERPROFILE%/does-not-exist:/root/missing:ro"] +`) + if err != nil { + t.Fatalf("parse registry: %v", err) + } + + _, err = buildHostMountArgs(reg.Tools["x"].HostMounts) + if err == nil { + t.Fatal("expected error for missing host_mount source") + } + if !strings.Contains(err.Error(), "does not exist") { + t.Fatalf("error does not mention missing source: %v", err) + } +} + +func TestBuildHostMountArgsRejectsUNCSource(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("UNC path detection is only meaningful on Windows") + } + uncHome := `\\server\share\home` + t.Setenv("USERPROFILE", uncHome) + + reg, err := registry.ParseTOML(`[tools.x] +image = "x:1" +provider = "stateless" +host_mounts = ["%USERPROFILE%/.claude:/root/.claude:ro"] +`) + if err != nil { + t.Fatalf("parse registry: %v", err) + } + + _, err = buildHostMountArgs(reg.Tools["x"].HostMounts) + if err == nil { + t.Fatal("expected error for UNC host_mount source") + } + if !strings.Contains(err.Error(), "UNC") { + t.Fatalf("error does not mention UNC: %v", err) + } +} + +func TestBuildHostMountArgsCanonicalizesBeforeStat(t *testing.T) { + homeDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(homeDir, ".claude"), 0755); err != nil { + t.Fatal(err) + } + setTestHome(t, homeDir) + + reg, err := registry.ParseTOML(`[tools.x] +image = "x:1" +provider = "stateless" +host_mounts = ["%USERPROFILE%/.claude/.:/root/.claude:ro"] +`) + if err != nil { + t.Fatalf("parse registry: %v", err) + } + + args, err := buildHostMountArgs(reg.Tools["x"].HostMounts) + if err != nil { + t.Fatalf("buildHostMountArgs error: %v", err) + } + mount := args[1] + if strings.Contains(mount, "/./") || strings.HasSuffix(mount, "/.") { + t.Fatalf("mount string should be canonicalized, got %q", mount) + } + if !strings.Contains(mount, ",dst=/root/.claude,readonly") { + t.Fatalf("mount string missing readonly dst: %q", mount) + } +} diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 2d645c7..183f28e 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -7,6 +7,7 @@ import ( "bufio" "errors" "fmt" + "path" "sort" "strconv" "strings" @@ -35,6 +36,7 @@ type Tool struct { StateGroup string // stable namespace shared by related stateful tools ProjectVolumes []string // NAME:CONTAINER_PATH, project-scoped named volumes SharedVolumes []string // NAME:CONTAINER_PATH, shared named volumes + HostMounts []string // SOURCE:/CONTAINER_PATH:ro|rw, explicit host bind mounts } type Registry struct { @@ -61,6 +63,7 @@ schema_version = 1 # state_group => namespace shared by related stateful shims # project_volumes => ["name:/container/path"] scoped by project root # shared_volumes => ["name:/container/path"] shared across projects +# host_mounts => ["%USERPROFILE%\\.claude:/root/.claude:ro"] explicit host bind mounts [tools.python] image = "python:3.13-slim" @@ -400,6 +403,12 @@ func ParseTOML(s string) (Registry, error) { return reg, fmt.Errorf("line %d shared_volumes: %w", lineNo, err) } t.SharedVolumes = v + case "host_mounts": + v, err := toml.ParseStringArray(value) + if err != nil { + return reg, fmt.Errorf("line %d host_mounts: %w", lineNo, err) + } + t.HostMounts = v default: return reg, fmt.Errorf("line %d: unsupported key %q", lineNo, key) } @@ -436,6 +445,9 @@ func ParseTOML(s string) (Registry, error) { default: return reg, fmt.Errorf("tool %q: unsupported provider %q", name, t.Provider) } + if err := validateHostMounts(t); err != nil { + return reg, fmt.Errorf("tool %q: %w", name, err) + } } return reg, nil } @@ -502,6 +514,182 @@ func ParseVolumeBinding(spec string) (string, string, error) { return name, dst, nil } +// ParseHostMount splits a "SOURCE:/container/path:MODE" host_mounts entry. +// The source/target delimiter reuses ParseVolumeBinding's ":/" convention: a +// Windows source path's drive-letter colon is always followed by a backslash, +// never a forward slash, so the first ":/" in the string unambiguously marks +// the boundary before the container path even when the source itself (e.g. +// "D:\Video") contains a colon of its own. +func ParseHostMount(spec string) (source, target, mode string, err error) { + i := strings.Index(spec, ":/") + if i <= 0 { + return "", "", "", errors.New("expected SOURCE:/absolute/container/path:ro or SOURCE:/absolute/container/path:rw") + } + source = spec[:i] + rest := spec[i+1:] // "/container/path:MODE" + j := strings.LastIndex(rest, ":") + if j < 0 { + return "", "", "", errors.New("host_mounts entry is missing a :ro or :rw mode suffix") + } + target = rest[:j] + mode = rest[j+1:] + if mode != "ro" && mode != "rw" { + return "", "", "", fmt.Errorf("host_mounts mode must be \"ro\" or \"rw\", got %q", mode) + } + if !strings.HasPrefix(target, "/") { + return "", "", "", errors.New("host_mounts container path must be absolute") + } + if strings.Contains(target, ",") { + // Mirrors the source comma check below: dockerrun.MountSpecMode would + // reject this too, but only at RunTool time, once the tool is + // actually invoked. Catching it here moves the failure to registry + // load, matching this task's two-stage validation split (structural + // checks at load, environment-dependent checks at run). + return "", "", "", errors.New("host_mounts target contains a comma and cannot be represented safely in docker --mount syntax (values are comma-separated with no escaping)") + } + if strings.Contains(target, ":") { + // The mode suffix is found by taking the LAST ":" in the remainder + // (see below), so an entry with an extra colon inside the target, + // e.g. "D:\V:/root/a:b:ro", parses without error today: the LAST + // colon is still correctly found as the mode delimiter, but the + // target itself is left containing one. Docker's --mount tolerates a + // colon in dst=, so this was never a mis-mount risk, but it's an + // unintended shape the documented SOURCE:/CONTAINER_PATH:MODE grammar + // doesn't describe. Rejecting it keeps the grammar as tight as every + // other part of this validation. + return "", "", "", errors.New("host_mounts target must not contain \":\"") + } + // Reject ".." path segments outright, before cleaning: path.Clean collapses + // a traversal like "/workspace/.." to "/", which is not itself in the + // reserved-namespace list validateHostMounts checks against, so a naive + // "clean, then compare against reserved prefixes" order would let a + // declared target ESCAPE the very check it's meant to satisfy. There is no + // legitimate reason for a host_mounts target to contain "..": it should + // always be a direct absolute container path the profile author wrote + // intentionally. Failing this loudly, before any normalization, keeps the + // reserved-namespace check meaningful regardless of what "." segments + // (which ARE legitimate, e.g. for collision-detection equivalence) get + // cleaned away afterward. + for _, seg := range strings.Split(target, "/") { + if seg == ".." { + return "", "", "", fmt.Errorf("host_mounts container path %q must not contain \"..\"", target) + } + } + target = path.Clean(target) + // Defense in depth: no legal traversal-free target should ever clean to + // bare "/" (the shortest reserved path is a single segment below root), + // but reject it explicitly rather than relying on that reasoning holding + // forever as the reserved-namespace list evolves. A host_mounts entry at + // "/" would shadow every container-bin-managed mount. + if target == "/" { + return "", "", "", errors.New("host_mounts container path must not be the filesystem root") + } + if source == "" { + return "", "", "", errors.New("host_mounts source must not be empty") + } + if strings.Contains(source, ",") { + return "", "", "", errors.New("host_mounts source contains a comma and cannot be represented safely in docker --mount syntax (values are comma-separated with no escaping)") + } + if err := validHostMountSource(source); err != nil { + return "", "", "", err + } + return source, target, mode, nil +} + +func validHostMountSource(source string) error { + if strings.Contains(source, "%") { + if !strings.HasPrefix(source, "%USERPROFILE%") { + return errors.New("host_mounts source contains an unsupported %-variable; only %USERPROFILE% is recognized") + } + rest := source[len("%USERPROFILE%"):] + if strings.Contains(rest, "%") { + return errors.New("host_mounts source contains an unsupported %-variable after %USERPROFILE%") + } + // %USERPROFILE% must be the whole source, or immediately followed by a + // path separator. ExpandHostMountSource does plain string + // concatenation of os.UserHomeDir() with whatever follows the token, + // so without this, "%USERPROFILE%foo" would resolve to a *sibling* of + // the home directory (e.g. C:\Users\foo) rather than a child of + // it -- silently not the path the registry line visually suggests. + if rest != "" && rest[0] != '\\' && rest[0] != '/' { + return errors.New("host_mounts source must be exactly %USERPROFILE% or followed immediately by \\ or /") + } + return nil + } + if isWindowsAbsPath(source) { + return nil + } + return errors.New("host_mounts source must be %USERPROFILE%\\... or an absolute Windows path (X:\\...)") +} + +func isWindowsAbsPath(s string) bool { + if len(s) < 3 { + return false + } + c := s[0] + isLetter := (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') + return isLetter && s[1] == ':' && (s[2] == '\\' || s[2] == '/') +} + +// pathContainsOrEquals reports whether target is parent itself, or a strict +// descendant of it. Both arguments must already be path.Clean'd absolute +// POSIX container paths. Used only for the fixed, cb-owned reserved +// namespaces (/workspace, /cb, /venv, /root/.cache/pip) -- not for arbitrary +// project_volumes/shared_volumes destinations, where a host_mounts entry +// nested under a user-declared volume is a normal, valid Docker +// configuration (the more specific mount just shadows part of the outer +// one), not a defect to reject. +func pathContainsOrEquals(parent, target string) bool { + return target == parent || strings.HasPrefix(target, parent+"/") +} + +func validateHostMounts(t Tool) error { + // project_volumes and shared_volumes are both checked here, not just + // shared_volumes: ParseVolumeBinding places no requirement that a + // project_volumes destination live under /workspace (that is only true + // by convention for this repo's built-in profiles), so a custom profile + // declaring e.g. project_volumes = ["foo:/root/.foo"] could otherwise + // collide with a host_mounts target at that same literal path with + // nothing catching it. + volumeDst := map[string]bool{} + for _, spec := range append(append([]string{}, t.ProjectVolumes...), t.SharedVolumes...) { + _, dst, err := ParseVolumeBinding(spec) + if err != nil { + return err // already validated earlier in the same loop; defensive only + } + volumeDst[path.Clean(dst)] = true + } + seen := map[string]bool{} + for _, spec := range t.HostMounts { + _, target, _, err := ParseHostMount(spec) + if err != nil { + return err + } + // Prefix-based, not exact-match: /venv and /root/.cache/pip are fixed, + // semantically load-bearing paths the python provider's bootstrap + // script depends on having their full structure intact (it checks + // /venv/bin/python specifically), not just their top-level directory. + // An exact-match-only reservation would let a target like /venv/bin + // validate cleanly and only break the tool at run time with no + // explanatory error -- the same shape of gap /workspace and /cb were + // already guarded against with HasPrefix, extended here to all four + // reserved namespaces uniformly via pathContainsOrEquals. + for _, reserved := range []string{"/workspace", "/cb", "/venv", "/root/.cache/pip"} { + if pathContainsOrEquals(reserved, target) { + return fmt.Errorf("host_mounts target %q is reserved for container-bin's own workspace/state mounts", target) + } + } + if seen[target] { + return fmt.Errorf("host_mounts target %q is declared more than once", target) + } + seen[target] = true + if volumeDst[target] { + return fmt.Errorf("host_mounts target %q collides with a project_volumes/shared_volumes destination", target) + } + } + return nil +} + func ListTools(reg Registry, cfgPath string) { names := make([]string, 0, len(reg.Tools)) for n := range reg.Tools { diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index cfecd6f..afd773d 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -4,6 +4,8 @@ import ( "reflect" "strings" "testing" + + "github.com/AviBackToBlack/container-bin/internal/toml" ) // containsString mirrors the tiny helper these profile assertions used while @@ -418,3 +420,245 @@ func TestNode22ProfilesParseAndVolumes(t *testing.T) { } } } + +func TestParseHostMount(t *testing.T) { + cases := []struct { + name string + spec string + wantErr bool + wantSrc string + wantDst string + wantMode string + errSubstr string + }{ + { + name: "ro_ok", + spec: "D:\\Video:/root/videos:ro", + wantSrc: "D:\\Video", + wantDst: "/root/videos", + wantMode: "ro", + }, + { + name: "rw_ok", + spec: "D:\\Video:/root/videos:rw", + wantSrc: "D:\\Video", + wantDst: "/root/videos", + wantMode: "rw", + }, + { + name: "userprofile_ok", + spec: "%USERPROFILE%\\.claude:/root/.claude:ro", + wantSrc: "%USERPROFILE%\\.claude", + wantDst: "/root/.claude", + wantMode: "ro", + }, + { + name: "missing_mode", + spec: "D:\\Video:/root/videos", + wantErr: true, + errSubstr: "missing a :ro or :rw mode suffix", + }, + { + name: "invalid_mode", + spec: "D:\\Video:/root/videos:rx", + wantErr: true, + errSubstr: "mode must be", + }, + { + name: "non_absolute_target", + spec: "D:\\Video:relative:ro", + wantErr: true, + errSubstr: "absolute", + }, + { + name: "empty_source", + spec: ":/root/videos:ro", + wantErr: true, + errSubstr: "expected SOURCE:/absolute/container/path", + }, + { + name: "comma_in_source", + spec: "D:\\Video, here:/root/videos:ro", + wantErr: true, + errSubstr: "comma", + }, + { + name: "comma_in_target", + spec: "D:\\Video:/root/videos, here:ro", + wantErr: true, + errSubstr: "comma", + }, + { + // The mode suffix is found via the LAST ":" in the remainder, so + // this would otherwise parse as target "/root/a:b", mode "ro" + // without error -- an unintended shape outside the documented + // SOURCE:/CONTAINER_PATH:MODE grammar. + name: "colon_in_target", + spec: "D:\\Video:/root/a:b:ro", + wantErr: true, + errSubstr: "must not contain", + }, + { + name: "userprofile_no_separator_concatenates_into_sibling", + spec: "%USERPROFILE%foo:/root/foo:ro", + wantErr: true, + errSubstr: "followed immediately by", + }, + { + name: "userprofile_bare_token_ok", + spec: "%USERPROFILE%:/root/home:ro", + wantSrc: "%USERPROFILE%", + wantDst: "/root/home", + wantMode: "ro", + }, + { + name: "other_variable_rejected", + spec: "%APPDATA%\\x:/root/x:ro", + wantErr: true, + errSubstr: "unsupported", + }, + { + name: "other_variable_after_userprofile", + spec: "%USERPROFILE%\\%APPDATA%\\x:/root/x:ro", + wantErr: true, + errSubstr: "unsupported", + }, + { + name: "userprofile_not_at_start", + spec: "C:\\%USERPROFILE%\\.claude:/root/.claude:ro", + wantErr: true, + errSubstr: "unsupported", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + src, dst, mode, err := ParseHostMount(c.spec) + if c.wantErr { + if err == nil { + t.Fatalf("ParseHostMount(%q) expected error, got src=%q dst=%q mode=%q", c.spec, src, dst, mode) + } + if c.errSubstr != "" && !strings.Contains(err.Error(), c.errSubstr) { + t.Fatalf("ParseHostMount(%q) error %q does not contain %q", c.spec, err.Error(), c.errSubstr) + } + return + } + if err != nil { + t.Fatalf("ParseHostMount(%q) unexpected error: %v", c.spec, err) + } + if src != c.wantSrc || dst != c.wantDst || mode != c.wantMode { + t.Fatalf("ParseHostMount(%q) = src=%q dst=%q mode=%q, want src=%q dst=%q mode=%q", c.spec, src, dst, mode, c.wantSrc, c.wantDst, c.wantMode) + } + }) + } +} + +func TestParseHostMountWindowsLiteralWithForwardSlashDrive(t *testing.T) { + // The "X:\" form is unambiguous with the ":/" source/target delimiter. + src, dst, mode, err := ParseHostMount("D:/Video:/root/videos:ro") + if err == nil { + t.Fatalf("expected ambiguity to be rejected, got src=%q dst=%q mode=%q", src, dst, mode) + } +} + +func TestValidateHostMounts(t *testing.T) { + base := `[tools.x] +image = "x:1" +provider = "stateless" +` + + mustFail := func(label, extra string) { + t.Helper() + _, err := ParseTOML(base + extra) + if err == nil { + t.Fatalf("%s: expected error", label) + } + } + mustPass := func(label, extra string) { + t.Helper() + _, err := ParseTOML(base + extra) + if err != nil { + t.Fatalf("%s: unexpected error: %v", label, err) + } + } + + mustFail("workspace_root_reserved", "host_mounts = "+toml.Array([]string{"C:\\\\Video:/workspace:ro"})+"\n") + mustFail("workspace_child_reserved", "host_mounts = "+toml.Array([]string{"C:\\\\Video:/workspace/foo:ro"})+"\n") + mustFail("cb_root_reserved", "host_mounts = "+toml.Array([]string{"C:\\\\Video:/cb:ro"})+"\n") + mustFail("cb_child_reserved", "host_mounts = "+toml.Array([]string{"C:\\\\Video:/cb/global:ro"})+"\n") + mustFail("venv_reserved", "host_mounts = "+toml.Array([]string{"C:\\\\Video:/venv:ro"})+"\n") + mustFail("pip_cache_reserved", "host_mounts = "+toml.Array([]string{"C:\\\\Video:/root/.cache/pip:ro"})+"\n") + // /venv and /root/.cache/pip must be reserved by prefix, not exact match + // only: the python provider's bootstrap script depends on their full + // structure (it checks /venv/bin/python specifically), so a sub-path + // target like /venv/bin would otherwise validate cleanly and only break + // the tool at run time. + mustFail("venv_subpath_reserved", "host_mounts = "+toml.Array([]string{"C:\\\\Video:/venv/bin:ro"})+"\n") + mustFail("pip_cache_subpath_reserved", "host_mounts = "+toml.Array([]string{"C:\\\\Video:/root/.cache/pip/http:ro"})+"\n") + mustFail("duplicate_target", "host_mounts = "+toml.Array([]string{"C:\\\\A:/root/.x:ro", "C:\\\\B:/root/.x:rw"})+"\n") + mustFail("duplicate_target_equivalent", "host_mounts = "+toml.Array([]string{"C:\\\\A:/root/./.x:ro", "C:\\\\B:/root/.x:rw"})+"\n") + mustFail("shared_volume_collision", "shared_volumes = "+toml.Array([]string{"cache:/root/.cache"})+"\nhost_mounts = "+toml.Array([]string{"C:\\\\Video:/root/.cache:ro"})+"\n") + mustFail("shared_volume_collision_equivalent", "shared_volumes = "+toml.Array([]string{"cache:/root/./.cache"})+"\nhost_mounts = "+toml.Array([]string{"C:\\\\Video:/root/.cache:ro"})+"\n") + + // path.Clean("/workspace/..") == "/", which is not itself in the reserved + // list -- a naive clean-then-compare order would let a ".."-traversal + // target escape the reserved-namespace check entirely and mount at the + // filesystem root, shadowing every container-bin-managed mount. These + // must be rejected outright, before any normalization collapses them. + mustFail("workspace_traversal_to_root", "host_mounts = "+toml.Array([]string{"C:\\\\data:/workspace/..:ro"})+"\n") + mustFail("cb_traversal_to_root", "host_mounts = "+toml.Array([]string{"C:\\\\data:/cb/..:ro"})+"\n") + mustFail("bare_traversal_segment", "host_mounts = "+toml.Array([]string{"C:\\\\data:/root/../etc:ro"})+"\n") + // No ".." segment is present in either of these, so they reach path.Clean + // unrejected by the traversal loop and must be caught by the separate + // bare-"/" defense-in-depth check instead -- locking in that branch so a + // future refactor that assumes the ".." loop alone is sufficient breaks a + // test rather than silently reopening the root-mount hole. + mustFail("bare_root_target", "host_mounts = "+toml.Array([]string{"C:\\\\data:/:ro"})+"\n") + mustFail("dot_only_target_to_root", "host_mounts = "+toml.Array([]string{"C:\\\\data:/./.:ro"})+"\n") + + // ParseVolumeBinding places no requirement that a project_volumes + // destination live under /workspace -- that's only true by convention + // for this repo's built-in profiles, not enforced by the schema -- so a + // stateful profile's project_volumes destination must be checked for + // host_mounts collisions too, not just shared_volumes. + statefulBase := `[tools.y] +image = "y:1" +provider = "stateful" +state_group = "ygroup" +` + _, pvErr := ParseTOML(statefulBase + "project_volumes = " + toml.Array([]string{"data:/root/.ydata"}) + "\nhost_mounts = " + toml.Array([]string{"C:\\\\Video:/root/.ydata:ro"}) + "\n") + if pvErr == nil { + t.Fatal("project_volume_collision: expected error") + } + + mustPass("valid_multi_entry", "host_mounts = "+toml.Array([]string{ + "%USERPROFILE%\\\\.claude:/root/.claude:ro", + "%USERPROFILE%\\\\.codex:/root/.codex:ro", + })+"\n") + + // stateless provider is allowed to use host_mounts + _, err := ParseTOML(base + "host_mounts = " + toml.Array([]string{"%USERPROFILE%\\\\.claude:/root/.claude:ro"}) + "\n") + if err != nil { + t.Fatalf("stateless tool with host_mounts should parse: %v", err) + } + + // Ensure the parsed value is retained. + reg, err := ParseTOML(base + "host_mounts = " + toml.Array([]string{"%USERPROFILE%\\\\.claude:/root/.claude:ro"}) + "\n") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(reg.Tools["x"].HostMounts) != 1 { + t.Fatalf("expected 1 host_mount, got %#v", reg.Tools["x"].HostMounts) + } +} + +func TestHostMountDefaultTOMLComment(t *testing.T) { + // The default TOML must still parse and the host_mounts comment must not + // create an extra tool or confuse the parser. + reg, err := ParseTOML(DefaultTOML) + if err != nil { + t.Fatal(err) + } + if len(reg.Tools) != 16 { + t.Fatalf("expected 16 tools, got %d", len(reg.Tools)) + } +}