Skip to content

fix(claude): preserve source file mode in marketplace tar#363

Merged
dpup merged 6 commits into
majorcontext:mainfrom
abezzub-dr:fix-tar-marketplace-perms
May 31, 2026
Merged

fix(claude): preserve source file mode in marketplace tar#363
dpup merged 6 commits into
majorcontext:mainfrom
abezzub-dr:fix-tar-marketplace-perms

Conversation

@abezzub-dr
Copy link
Copy Markdown
Contributor

Summary

  • CollectMarketplaceTar was writing every file's tar header with a hardcoded Mode: 0644, so files extracted in the container always landed as -rw-r--r-- regardless of the upstream git mode. Executable hook scripts (e.g. bin/aw-hook, scripts/on-prompt-submit.sh) then failed at runtime with Permission denied.
  • Now both the file and directory branches derive Mode from info.Mode().Perm() (the info value was already in scope from d.Info()).
  • Adds TestCollectMarketplaceTarPreservesFileMode covering an executable and a non-executable file through the tar round-trip.

Fixes #350.

Test plan

  • go test ./internal/providers/claude/ — all marketplace tests pass, including the new mode-preservation test.
  • make lint — clean.
  • Manual: rebuild a container with a marketplace that ships executable hooks (e.g. warpdotdev/claude-code-warp) and confirm ls -la shows +x and UserPromptSubmit no longer errors with Permission denied.

abezzub-dr added a commit to abezzub-dr/moat that referenced this pull request May 28, 2026
abezzub-dr added a commit to abezzub-dr/moat that referenced this pull request May 28, 2026
Two follow-ups from review of majorcontext#363:

Skip non-regular files (symlinks, devices, sockets) during the walk.
Previously, os.ReadFile followed committed symlinks and copied the
target's content into the tar under the symlink's name with the
symlink's 0777 mode bits — a malicious marketplace could ship
"bin/foo -> /etc/passwd" and end up with a world-writable, executable
copy of the target inside the container.

Skip the "." root entry. The destination directory is created by
"mkdir -p" in the Dockerfile before "tar xf", so emitting a "./"
header only chmod'd the destination to the host temp-clone-dir mode
(0700 from os.MkdirTemp) on extraction.
The host-side marketplace tar in CollectMarketplaceTar wrote every file
header with a hardcoded Mode: 0644, so executable hook scripts (e.g.
bin/aw-hook, scripts/on-prompt-submit.sh) extracted at 0644 in the
container and failed with "Permission denied" on UserPromptSubmit
hooks. The fix derives Mode from info.Mode().Perm(), which is already
in scope. Directory entries follow the same pattern for consistency.

Fixes majorcontext#350.
Preserving info.Mode().Perm() literally narrows the safety margin against
restrictive host umasks: a host with umask 0077 lays an upstream 100755
script down as 0700, the tar carries 0700, and BuildKit extracts it as
root inside the build — leaving moatuser unable to read or execute the
file. Previously the hardcoded 0644/0755 protected against this by
accident. Mirroring owner bits to group and other preserves the
executable preservation intent while keeping files usable for the
non-root container user.

Also asserts the bin/ directory is traversable in the existing
mode-preservation test.
The Dockerfile sets USER ${containerUser} before COPY + tar xf
(dockerfile.go:187, 207-208), so the tar is extracted by the agent
user — not by root as the previous comment claimed. Owner bits alone
are sufficient; mirroring them into group and other only widened the
mode (0o755 → 0o777, 0o644 → 0o666) without unlocking any real access.

Reverts to plain int64(info.Mode().Perm()) and notes that .Perm()
intentionally drops setuid/setgid/sticky so marketplaces cannot smuggle
those bits into the image. Removes the umask test that asserted a
problem that does not exist in this code path.
Two follow-ups from review of majorcontext#363:

Skip non-regular files (symlinks, devices, sockets) during the walk.
Previously, os.ReadFile followed committed symlinks and copied the
target's content into the tar under the symlink's name with the
symlink's 0777 mode bits — a malicious marketplace could ship
"bin/foo -> /etc/passwd" and end up with a world-writable, executable
copy of the target inside the container.

Skip the "." root entry. The destination directory is created by
"mkdir -p" in the Dockerfile before "tar xf", so emitting a "./"
header only chmod'd the destination to the host temp-clone-dir mode
(0700 from os.MkdirTemp) on extraction.
@abezzub-dr abezzub-dr force-pushed the fix-tar-marketplace-perms branch from 17b9074 to ef177d3 Compare May 29, 2026 06:56
@dpup dpup merged commit 4c689f3 into majorcontext:main May 31, 2026
4 checks passed
@abezzub-dr abezzub-dr deleted the fix-tar-marketplace-perms branch May 31, 2026 17:23
@dpup dpup mentioned this pull request Jun 1, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

claude: host-side marketplace tar strips executable bit from plugin hook scripts (regression from #240)

2 participants