fix(container): set tmpfs mode to 1777 for non-root container users#355
Open
abezzub-dr wants to merge 3 commits into
Open
fix(container): set tmpfs mode to 1777 for non-root container users#355abezzub-dr wants to merge 3 commits into
abezzub-dr wants to merge 3 commits into
Conversation
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.
…ners 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.
…inerMounts signature - 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 majorcontext#355" task reference in apple.go. - Delete narrative WHAT-only doc comments from the new tests; test names already state what's asserted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TmpfsOptions{Mode: 0o1777}on tmpfs mounts produced by the Docker runtime. Without this, runc creates tmpfs as mode755owned byroot, and any non-root container user (e.g.,moatuser) getsEACCESwriting into directories declared viamounts.exclude.execoption to Docker tmpfs mounts to override runc defaultnoexecflag. Without this, native binaries in excluded paths (e.g., turbo, esbuild innode_modules) fail withEACCESwhen spawned.DockerRuntime.CreateContainerinto abuildContainerMountshelper so the behavior can be unit-tested without a live Docker daemon.--tmpfsto--mount type=tmpfs,destination=...,mode=1777so the mode is set explicitly. Apple--tmpfsflag passes empty options and does not support mode syntax. Apple containers do not needexec— the kernel defaults to allowing exec on user-supplied tmpfs mounts.BuildCreateArgstests for the new--mountsyntax.Why
A user hit this with a pnpm install inside a moat container:
Their
moat.yamlhadmounts.exclude: [node_modules, .pnpm-store, ...]. The exclude correctly produced tmpfs mounts at the right paths, but the tmpfs landed asroot:root 755— unwritable for the non-root container user, so pnpm could not even create its store directory.Mode
1777matches what Docker CLI--tmpfsflag uses by default, so the fix brings the SDK path in line with user expectations.Additionally, even after fixing the mode,
pnpm buildfailed because runc defaults tmpfs tonoexec:The
execoption fixes this — excluded paths likenode_modulescontain native binaries that must be executable.Test plan
go test -short -race ./...— all packages greenmake lint— 0 issuesTestBuildContainerMounts_TmpfsWritableAndExecverifies mode 1777 and exec optionTestBuildCreateArgstests updated for--mountsyntaxmounts.exclude: [node_modules]and confirm a non-root user canmkdirand execute binaries inside/workspace/node_modulesOut of scope
1777hardcoded for now. If anyone wants per-mount mode control, amodefield on theexcludeconfig can be added later without changing the default.