From 41857964bfe0c051476896dddf404ea3217cac46 Mon Sep 17 00:00:00 2001 From: Andrii Bezzub Date: Thu, 21 May 2026 14:50:17 +0000 Subject: [PATCH 1/3] fix(container): set tmpfs mode to 1777 for non-root container users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tmpfs mounts created via the Docker SDK without explicit TmpfsOptions inherit runc's default mode of 755 owned by root. Non-root container users (e.g., the `moatuser` we run agents as) cannot write to such mounts, so `mounts.exclude` directories like `/workspace/node_modules` return EACCES on any write attempt: pnpm: EACCES: permission denied, mkdir '/workspace/.pnpm-store/v11' Set mode 1777 (sticky world-writable) explicitly. This matches the default that Docker's `--tmpfs` CLI flag uses, so behavior now lines up with what users coming from `docker run --tmpfs /foo` expect. The mount-building logic in CreateContainer was extracted to a `buildContainerMounts` helper so the behavior is unit-testable without a live Docker daemon. Apple Containers (apple.go) likely has the same bug but is left for a follow-up — the `container run --tmpfs` mode syntax needs verification on a Mac host. --- internal/container/docker.go | 42 +++++++++++------ internal/container/docker_test.go | 76 +++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 15 deletions(-) diff --git a/internal/container/docker.go b/internal/container/docker.go index 2a19aa02..4384b38f 100644 --- a/internal/container/docker.go +++ b/internal/container/docker.go @@ -161,19 +161,10 @@ func (r *DockerRuntime) Ping(ctx context.Context) error { return nil } -// CreateContainer creates a new Docker container. -func (r *DockerRuntime) CreateContainer(ctx context.Context, cfg Config) (string, error) { - // Verify gVisor is still available if we're configured to use it - if r.ociRuntime == "runsc" && !r.gvisorAvailable() { - return "", fmt.Errorf("gVisor was available at startup but is no longer configured - did Docker daemon configuration change? %w", ErrGVisorNotAvailable) - } - - // Pull image if not present - if err := r.ensureImage(ctx, cfg.Image); err != nil { - return "", err - } - - // Convert mounts +// buildContainerMounts converts moat's MountConfig and TmpfsMount entries into +// Docker SDK mount.Mount structs. Tmpfs mounts overlay subdirectories of +// bind-mounted paths and are appended after bind mounts so they apply on top. +func buildContainerMounts(cfg Config) []mount.Mount { mounts := make([]mount.Mount, 0, len(cfg.Mounts)+len(cfg.TmpfsMounts)) for _, m := range cfg.Mounts { mounts = append(mounts, mount.Mount{ @@ -183,14 +174,35 @@ func (r *DockerRuntime) CreateContainer(ctx context.Context, cfg Config) (string ReadOnly: m.ReadOnly, }) } - // Tmpfs mounts — overlays for excluded directories. - // Appended after bind mounts so tmpfs overlays subdirectories of bind-mounted paths. for _, tm := range cfg.TmpfsMounts { + // Mode 1777 (sticky world-writable) matches Docker CLI's `--tmpfs` + // default. Without it, runc defaults to mode 755 owned by root, + // causing EACCES for non-root container users writing into the tmpfs + // (e.g., pnpm install creating its store inside an excluded path). mounts = append(mounts, mount.Mount{ Type: mount.TypeTmpfs, Target: tm.Target, + TmpfsOptions: &mount.TmpfsOptions{ + Mode: 0o1777, + }, }) } + return mounts +} + +// CreateContainer creates a new Docker container. +func (r *DockerRuntime) CreateContainer(ctx context.Context, cfg Config) (string, error) { + // Verify gVisor is still available if we're configured to use it + if r.ociRuntime == "runsc" && !r.gvisorAvailable() { + return "", fmt.Errorf("gVisor was available at startup but is no longer configured - did Docker daemon configuration change? %w", ErrGVisorNotAvailable) + } + + // Pull image if not present + if err := r.ensureImage(ctx, cfg.Image); err != nil { + return "", err + } + + mounts := buildContainerMounts(cfg) // Default to bridge network if not specified networkMode := container.NetworkMode(cfg.NetworkMode) diff --git a/internal/container/docker_test.go b/internal/container/docker_test.go index 15393940..712272f8 100644 --- a/internal/container/docker_test.go +++ b/internal/container/docker_test.go @@ -3,16 +3,92 @@ package container import ( "context" "os" + "reflect" "strconv" "strings" "sync" "testing" "time" + "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" ) +// TestBuildContainerMounts_TmpfsWorldWritable asserts that tmpfs mounts are +// created with mode 1777 so non-root container users can write to them. +// Without an explicit mode, runc creates the tmpfs as mode 755 owned by root, +// causing EACCES for non-root processes (e.g., pnpm install as moatuser). +func TestBuildContainerMounts_TmpfsWorldWritable(t *testing.T) { + cfg := Config{ + TmpfsMounts: []TmpfsMount{ + {Target: "/workspace/node_modules"}, + }, + } + + got := buildContainerMounts(cfg) + + if len(got) != 1 { + t.Fatalf("got %d mounts, want 1", len(got)) + } + if got[0].Type != mount.TypeTmpfs { + t.Errorf("Type = %v, want %v", got[0].Type, mount.TypeTmpfs) + } + if got[0].TmpfsOptions == nil { + t.Fatal("TmpfsOptions is nil; expected non-nil with Mode set to 0o1777") + } + if got[0].TmpfsOptions.Mode != 0o1777 { + t.Errorf("TmpfsOptions.Mode = %o, want %o (sticky world-writable)", got[0].TmpfsOptions.Mode, 0o1777) + } +} + +// TestBuildContainerMounts_BindMounts asserts that bind mounts are produced +// with the right type, source, target, and read-only flag. +func TestBuildContainerMounts_BindMounts(t *testing.T) { + cfg := Config{ + Mounts: []MountConfig{ + {Source: "/host/project", Target: "/workspace", ReadOnly: false}, + {Source: "/host/secret", Target: "/etc/secret", ReadOnly: true}, + }, + } + + got := buildContainerMounts(cfg) + + want := []mount.Mount{ + {Type: mount.TypeBind, Source: "/host/project", Target: "/workspace", ReadOnly: false}, + {Type: mount.TypeBind, Source: "/host/secret", Target: "/etc/secret", ReadOnly: true}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("buildContainerMounts() = %#v, want %#v", got, want) + } +} + +// TestBuildContainerMounts_TmpfsAfterBind verifies tmpfs mounts come after bind +// mounts in the slice. Docker applies mounts in order; tmpfs overlays of paths +// inside a bind mount must follow the bind to take effect. +func TestBuildContainerMounts_TmpfsAfterBind(t *testing.T) { + cfg := Config{ + Mounts: []MountConfig{ + {Source: "/host/project", Target: "/workspace"}, + }, + TmpfsMounts: []TmpfsMount{ + {Target: "/workspace/node_modules"}, + }, + } + + got := buildContainerMounts(cfg) + + if len(got) != 2 { + t.Fatalf("got %d mounts, want 2", len(got)) + } + if got[0].Type != mount.TypeBind { + t.Errorf("mounts[0].Type = %v, want bind", got[0].Type) + } + if got[1].Type != mount.TypeTmpfs { + t.Errorf("mounts[1].Type = %v, want tmpfs", got[1].Type) + } +} + func TestConfig_GroupAdd(t *testing.T) { // Verify that GroupAdd field can be set on Config struct // and is properly typed as []string From e0ea46914bad153f21f73fd51c7a05f19b55e3b0 Mon Sep 17 00:00:00 2001 From: Andrii Bezzub Date: Thu, 21 May 2026 15:30:31 +0000 Subject: [PATCH 2/3] fix(container): add exec option to tmpfs and set mode on Apple containers Docker: add "exec" to TmpfsOptions to override runc's default "noexec" flag on tmpfs mounts. Without this, native binaries in excluded paths (e.g., turbo, esbuild in node_modules) fail with EACCES when spawned. Apple: switch from --tmpfs to --mount type=tmpfs,destination=...,mode=1777 so the mode is set explicitly. Apple's --tmpfs flag passes empty options and does not support mode syntax. --- internal/container/apple.go | 8 ++++++-- internal/container/apple_test.go | 4 ++-- internal/container/docker.go | 13 ++++++++++--- internal/container/docker_test.go | 17 +++++++++++------ 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/internal/container/apple.go b/internal/container/apple.go index 5c9b7cac..fc7bc05b 100644 --- a/internal/container/apple.go +++ b/internal/container/apple.go @@ -302,9 +302,13 @@ func (r *AppleRuntime) buildCreateArgs(cfg Config) ([]string, error) { args = append(args, "--volume", mountStr) } - // Tmpfs mounts (overlays for excluded directories) + // Tmpfs mounts (overlays for excluded directories). + // Use --mount instead of --tmpfs so we can set mode=1777 (sticky + // world-writable). Without an explicit mode the runtime may default to + // 755, causing EACCES for non-root container users — the same issue + // fixed for Docker in PR #355. for _, tm := range cfg.TmpfsMounts { - args = append(args, "--tmpfs", tm.Target) + args = append(args, "--mount", fmt.Sprintf("type=tmpfs,destination=%s,mode=1777", tm.Target)) } // Image diff --git a/internal/container/apple_test.go b/internal/container/apple_test.go index a582cfc1..44b62441 100644 --- a/internal/container/apple_test.go +++ b/internal/container/apple_test.go @@ -174,7 +174,7 @@ func TestBuildCreateArgs(t *testing.T) { {Target: "/workspace/node_modules"}, }, }, - want: []string{"create", "--memory", "4096MB", "--dns", "8.8.8.8", "--dns", "8.8.4.4", "--tmpfs", "/workspace/node_modules", "ubuntu:22.04"}, + want: []string{"create", "--memory", "4096MB", "--dns", "8.8.8.8", "--dns", "8.8.4.4", "--mount", "type=tmpfs,destination=/workspace/node_modules,mode=1777", "ubuntu:22.04"}, }, { name: "with volume and tmpfs mounts", @@ -188,7 +188,7 @@ func TestBuildCreateArgs(t *testing.T) { {Target: "/workspace/.venv"}, }, }, - want: []string{"create", "--memory", "4096MB", "--dns", "8.8.8.8", "--dns", "8.8.4.4", "--volume", "/home/user/project:/workspace", "--tmpfs", "/workspace/node_modules", "--tmpfs", "/workspace/.venv", "ubuntu:22.04"}, + want: []string{"create", "--memory", "4096MB", "--dns", "8.8.8.8", "--dns", "8.8.4.4", "--volume", "/home/user/project:/workspace", "--mount", "type=tmpfs,destination=/workspace/node_modules,mode=1777", "--mount", "type=tmpfs,destination=/workspace/.venv,mode=1777", "ubuntu:22.04"}, }, { // Apple's container CLI has no --add-host equivalent. ExtraHosts must diff --git a/internal/container/docker.go b/internal/container/docker.go index 4384b38f..f6316551 100644 --- a/internal/container/docker.go +++ b/internal/container/docker.go @@ -175,15 +175,22 @@ func buildContainerMounts(cfg Config) []mount.Mount { }) } for _, tm := range cfg.TmpfsMounts { + // Mode 1777 + exec: two fixes for tmpfs mounts used by mounts.exclude. + // // Mode 1777 (sticky world-writable) matches Docker CLI's `--tmpfs` // default. Without it, runc defaults to mode 755 owned by root, - // causing EACCES for non-root container users writing into the tmpfs - // (e.g., pnpm install creating its store inside an excluded path). + // causing EACCES for non-root container users writing into the tmpfs. + // + // "exec" overrides runc's default "noexec" flag. Excluded paths like + // node_modules contain native binaries (e.g., turbo, esbuild) that + // must be executable. Without "exec", any spawn from the tmpfs fails + // with EACCES regardless of file permissions. mounts = append(mounts, mount.Mount{ Type: mount.TypeTmpfs, Target: tm.Target, TmpfsOptions: &mount.TmpfsOptions{ - Mode: 0o1777, + Mode: 0o1777, + Options: [][]string{{"exec"}}, }, }) } diff --git a/internal/container/docker_test.go b/internal/container/docker_test.go index 712272f8..d33434d5 100644 --- a/internal/container/docker_test.go +++ b/internal/container/docker_test.go @@ -15,11 +15,12 @@ import ( "github.com/docker/docker/client" ) -// TestBuildContainerMounts_TmpfsWorldWritable asserts that tmpfs mounts are -// created with mode 1777 so non-root container users can write to them. -// Without an explicit mode, runc creates the tmpfs as mode 755 owned by root, -// causing EACCES for non-root processes (e.g., pnpm install as moatuser). -func TestBuildContainerMounts_TmpfsWorldWritable(t *testing.T) { +// TestBuildContainerMounts_TmpfsWritableAndExec asserts that tmpfs mounts are +// created with mode 1777 and the exec option. Mode 1777 lets non-root users +// write (runc defaults to 755). The exec option overrides runc's default +// noexec flag so native binaries in excluded paths (e.g., turbo in +// node_modules) can be executed. +func TestBuildContainerMounts_TmpfsWritableAndExec(t *testing.T) { cfg := Config{ TmpfsMounts: []TmpfsMount{ {Target: "/workspace/node_modules"}, @@ -35,11 +36,15 @@ func TestBuildContainerMounts_TmpfsWorldWritable(t *testing.T) { t.Errorf("Type = %v, want %v", got[0].Type, mount.TypeTmpfs) } if got[0].TmpfsOptions == nil { - t.Fatal("TmpfsOptions is nil; expected non-nil with Mode set to 0o1777") + t.Fatal("TmpfsOptions is nil; expected non-nil with Mode and Options set") } if got[0].TmpfsOptions.Mode != 0o1777 { t.Errorf("TmpfsOptions.Mode = %o, want %o (sticky world-writable)", got[0].TmpfsOptions.Mode, 0o1777) } + wantOpts := [][]string{{"exec"}} + if !reflect.DeepEqual(got[0].TmpfsOptions.Options, wantOpts) { + t.Errorf("TmpfsOptions.Options = %v, want %v (exec required for native binaries)", got[0].TmpfsOptions.Options, wantOpts) + } } // TestBuildContainerMounts_BindMounts asserts that bind mounts are produced From 5f7a2414e7d8fb86beb2ca25052d9014bada33e0 Mon Sep 17 00:00:00 2001 From: Andrii Bezzub Date: Fri, 22 May 2026 10:26:13 +0200 Subject: [PATCH 3/3] refactor(container): extract tmpfsMode constant and narrow buildContainerMounts signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Lift mode 1777 to a single package-level constant in runtime.go so docker and apple runtimes can't drift. - Narrow buildContainerMounts to take ([]MountConfig, []TmpfsMount) instead of the full Config — function was only reading two fields. - Reuse buildContainerMounts in dockerSidecarManager.StartSidecar, which had an identical bind-mount loop. - Switch 0o1777 to 01777 to match the rest of the codebase's octal style. - Trim the 9-line tmpfs comment to one line covering the non-obvious WHY (the exec option for native binaries); drop the "PR #355" task reference in apple.go. - Delete narrative WHAT-only doc comments from the new tests; test names already state what's asserted. --- internal/container/apple.go | 8 ++--- internal/container/docker.go | 39 +++++++----------------- internal/container/docker_test.go | 50 +++++++++---------------------- internal/container/runtime.go | 5 ++++ 4 files changed, 32 insertions(+), 70 deletions(-) diff --git a/internal/container/apple.go b/internal/container/apple.go index fc7bc05b..8f607038 100644 --- a/internal/container/apple.go +++ b/internal/container/apple.go @@ -302,13 +302,9 @@ func (r *AppleRuntime) buildCreateArgs(cfg Config) ([]string, error) { args = append(args, "--volume", mountStr) } - // Tmpfs mounts (overlays for excluded directories). - // Use --mount instead of --tmpfs so we can set mode=1777 (sticky - // world-writable). Without an explicit mode the runtime may default to - // 755, causing EACCES for non-root container users — the same issue - // fixed for Docker in PR #355. + // --mount (not --tmpfs) so we can set the mode; --tmpfs accepts no options. for _, tm := range cfg.TmpfsMounts { - args = append(args, "--mount", fmt.Sprintf("type=tmpfs,destination=%s,mode=1777", tm.Target)) + args = append(args, "--mount", fmt.Sprintf("type=tmpfs,destination=%s,mode=%o", tm.Target, tmpfsMode)) } // Image diff --git a/internal/container/docker.go b/internal/container/docker.go index f6316551..110472c5 100644 --- a/internal/container/docker.go +++ b/internal/container/docker.go @@ -162,11 +162,11 @@ func (r *DockerRuntime) Ping(ctx context.Context) error { } // buildContainerMounts converts moat's MountConfig and TmpfsMount entries into -// Docker SDK mount.Mount structs. Tmpfs mounts overlay subdirectories of -// bind-mounted paths and are appended after bind mounts so they apply on top. -func buildContainerMounts(cfg Config) []mount.Mount { - mounts := make([]mount.Mount, 0, len(cfg.Mounts)+len(cfg.TmpfsMounts)) - for _, m := range cfg.Mounts { +// Docker SDK mount.Mount structs. Tmpfs mounts follow bind mounts so overlays +// of paths inside a bind take effect. +func buildContainerMounts(binds []MountConfig, tmpfs []TmpfsMount) []mount.Mount { + mounts := make([]mount.Mount, 0, len(binds)+len(tmpfs)) + for _, m := range binds { mounts = append(mounts, mount.Mount{ Type: mount.TypeBind, Source: m.Source, @@ -174,22 +174,14 @@ func buildContainerMounts(cfg Config) []mount.Mount { ReadOnly: m.ReadOnly, }) } - for _, tm := range cfg.TmpfsMounts { - // Mode 1777 + exec: two fixes for tmpfs mounts used by mounts.exclude. - // - // Mode 1777 (sticky world-writable) matches Docker CLI's `--tmpfs` - // default. Without it, runc defaults to mode 755 owned by root, - // causing EACCES for non-root container users writing into the tmpfs. - // - // "exec" overrides runc's default "noexec" flag. Excluded paths like - // node_modules contain native binaries (e.g., turbo, esbuild) that - // must be executable. Without "exec", any spawn from the tmpfs fails - // with EACCES regardless of file permissions. + for _, tm := range tmpfs { + // "exec" overrides runc's default noexec — excluded paths like + // node_modules contain native binaries (turbo, esbuild) that must spawn. mounts = append(mounts, mount.Mount{ Type: mount.TypeTmpfs, Target: tm.Target, TmpfsOptions: &mount.TmpfsOptions{ - Mode: 0o1777, + Mode: tmpfsMode, Options: [][]string{{"exec"}}, }, }) @@ -209,7 +201,7 @@ func (r *DockerRuntime) CreateContainer(ctx context.Context, cfg Config) (string return "", err } - mounts := buildContainerMounts(cfg) + mounts := buildContainerMounts(cfg.Mounts, cfg.TmpfsMounts) // Default to bridge network if not specified networkMode := container.NetworkMode(cfg.NetworkMode) @@ -1018,16 +1010,7 @@ func (m *dockerSidecarManager) StartSidecar(ctx context.Context, cfg SidecarConf return "", fmt.Errorf("pulling sidecar image: %w", err) } - // Prepare mounts - mounts := make([]mount.Mount, 0, len(cfg.Mounts)) - for _, mt := range cfg.Mounts { - mounts = append(mounts, mount.Mount{ - Type: mount.TypeBind, - Source: mt.Source, - Target: mt.Target, - ReadOnly: mt.ReadOnly, - }) - } + mounts := buildContainerMounts(cfg.Mounts, nil) // Create container with labels for orphan cleanup labels := make(map[string]string) diff --git a/internal/container/docker_test.go b/internal/container/docker_test.go index d33434d5..57d91028 100644 --- a/internal/container/docker_test.go +++ b/internal/container/docker_test.go @@ -15,19 +15,8 @@ import ( "github.com/docker/docker/client" ) -// TestBuildContainerMounts_TmpfsWritableAndExec asserts that tmpfs mounts are -// created with mode 1777 and the exec option. Mode 1777 lets non-root users -// write (runc defaults to 755). The exec option overrides runc's default -// noexec flag so native binaries in excluded paths (e.g., turbo in -// node_modules) can be executed. func TestBuildContainerMounts_TmpfsWritableAndExec(t *testing.T) { - cfg := Config{ - TmpfsMounts: []TmpfsMount{ - {Target: "/workspace/node_modules"}, - }, - } - - got := buildContainerMounts(cfg) + got := buildContainerMounts(nil, []TmpfsMount{{Target: "/workspace/node_modules"}}) if len(got) != 1 { t.Fatalf("got %d mounts, want 1", len(got)) @@ -36,28 +25,24 @@ func TestBuildContainerMounts_TmpfsWritableAndExec(t *testing.T) { t.Errorf("Type = %v, want %v", got[0].Type, mount.TypeTmpfs) } if got[0].TmpfsOptions == nil { - t.Fatal("TmpfsOptions is nil; expected non-nil with Mode and Options set") + t.Fatal("TmpfsOptions is nil") } - if got[0].TmpfsOptions.Mode != 0o1777 { - t.Errorf("TmpfsOptions.Mode = %o, want %o (sticky world-writable)", got[0].TmpfsOptions.Mode, 0o1777) + if got[0].TmpfsOptions.Mode != tmpfsMode { + t.Errorf("Mode = %o, want %o", got[0].TmpfsOptions.Mode, tmpfsMode) } wantOpts := [][]string{{"exec"}} if !reflect.DeepEqual(got[0].TmpfsOptions.Options, wantOpts) { - t.Errorf("TmpfsOptions.Options = %v, want %v (exec required for native binaries)", got[0].TmpfsOptions.Options, wantOpts) + t.Errorf("Options = %v, want %v", got[0].TmpfsOptions.Options, wantOpts) } } -// TestBuildContainerMounts_BindMounts asserts that bind mounts are produced -// with the right type, source, target, and read-only flag. func TestBuildContainerMounts_BindMounts(t *testing.T) { - cfg := Config{ - Mounts: []MountConfig{ - {Source: "/host/project", Target: "/workspace", ReadOnly: false}, - {Source: "/host/secret", Target: "/etc/secret", ReadOnly: true}, - }, + binds := []MountConfig{ + {Source: "/host/project", Target: "/workspace", ReadOnly: false}, + {Source: "/host/secret", Target: "/etc/secret", ReadOnly: true}, } - got := buildContainerMounts(cfg) + got := buildContainerMounts(binds, nil) want := []mount.Mount{ {Type: mount.TypeBind, Source: "/host/project", Target: "/workspace", ReadOnly: false}, @@ -68,20 +53,13 @@ func TestBuildContainerMounts_BindMounts(t *testing.T) { } } -// TestBuildContainerMounts_TmpfsAfterBind verifies tmpfs mounts come after bind -// mounts in the slice. Docker applies mounts in order; tmpfs overlays of paths -// inside a bind mount must follow the bind to take effect. +// Tmpfs must follow binds in the output slice so overlays of paths inside a +// bind take effect on the daemon side. func TestBuildContainerMounts_TmpfsAfterBind(t *testing.T) { - cfg := Config{ - Mounts: []MountConfig{ - {Source: "/host/project", Target: "/workspace"}, - }, - TmpfsMounts: []TmpfsMount{ - {Target: "/workspace/node_modules"}, - }, - } + binds := []MountConfig{{Source: "/host/project", Target: "/workspace"}} + tmpfs := []TmpfsMount{{Target: "/workspace/node_modules"}} - got := buildContainerMounts(cfg) + got := buildContainerMounts(binds, tmpfs) if len(got) != 2 { t.Fatalf("got %d mounts, want 2", len(got)) diff --git a/internal/container/runtime.go b/internal/container/runtime.go index c0317dc1..9636c52e 100644 --- a/internal/container/runtime.go +++ b/internal/container/runtime.go @@ -368,6 +368,11 @@ type TmpfsMount struct { Target string // absolute container path } +// tmpfsMode is the permission mode for tmpfs overlays. Sticky world-writable +// (01777) so non-root container users can write — runc otherwise defaults +// tmpfs to 0755 owned by root. +const tmpfsMode = 01777 + // ImageInfo contains information about a container image. type ImageInfo struct { ID string `json:"id"`