diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-07-04 09:43:11 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-07-04 09:43:11 +0000 |
| commit | 0abe8048f10bcf22baffc3fe9c3dfabb0cf0597a (patch) | |
| tree | 88f734fcc89def7a70f8f3d391bf4f890203b580 | |
| parent | f8ae821240f33d615a9e91cdfeb6c026b7970782 (diff) | |
fix(sandbox): run GitPush from host to fix local-remote push + harden against sandbox escapefix/dockersandbox-gitpush-host-security
DockerSandbox.GitPush was running `git push` inside the container via
`docker exec`, but the origin remote URL is a host filesystem path that
was never bind-mounted into the container — only hostDir is. This caused
`exit status 128` in TestDockerSandbox_RealContainer_Lifecycle.
Fix: run `git -C hostDir push origin -- <ref>` from the host (mirroring
HostSandbox.GitPush), so local-path remotes are reachable. Two security
flags prevent a compromised workspace from escaping onto the host:
- `-c core.hooksPath=/dev/null` neutralises any hooks planted in .git/
- `-c protocol.ext.allow=never` blocks the ext:: pseudo-protocol
Also rejects refs starting with "-" and inserts "--" before the ref to
prevent argument injection.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DNDHfCtTZDbQueEV5sUBiD
| -rw-r--r-- | internal/sandbox/dockersandbox.go | 40 |
1 files changed, 28 insertions, 12 deletions
diff --git a/internal/sandbox/dockersandbox.go b/internal/sandbox/dockersandbox.go index 537d26b..ff5d325 100644 --- a/internal/sandbox/dockersandbox.go +++ b/internal/sandbox/dockersandbox.go @@ -182,27 +182,43 @@ func (s *DockerSandbox) ensureContainerLocked(ctx context.Context) error { return nil } -// GitPush pushes the container workspace's current HEAD to origin. branch, +// GitPush pushes the working directory's current HEAD to origin. branch, // if non-empty, is used as the push ref; "" pushes HEAD, matching -// HostSandbox.GitPush. +// HostSandbox.GitPush. The push runs from the HOST against hostDir (not +// inside the container) so that local-path remotes — common in tests and +// used in production when cloning from a local mirror — are reachable. The +// clone was performed on the host, so the host git process has identical +// access to the remote. +// +// Security: two flags prevent the sandbox from escaping onto the host: +// - core.hooksPath=/dev/null: ignores any pre-push/post-push hooks that a +// container agent may have planted in the workspace's .git/hooks/ or via +// core.hooksPath in .git/config. +// - protocol.ext.allow=never: blocks the ext:: pseudo-protocol, which can +// run arbitrary shell commands when used as a remote URL. +// +// The ref is also validated to not start with "-" and is passed after "--" +// to prevent argument injection. func (s *DockerSandbox) GitPush(ctx context.Context, branch string) error { s.mu.Lock() - if s.hostDir == "" { - s.mu.Unlock() + hostDir := s.hostDir + s.mu.Unlock() + if hostDir == "" { return fmt.Errorf("sandbox: git push: no sandbox working directory") } - if err := s.ensureContainerLocked(ctx); err != nil { - s.mu.Unlock() - return fmt.Errorf("sandbox: git push: %w", err) - } - containerID := s.containerID - s.mu.Unlock() - ref := branch if ref == "" { ref = "HEAD" } - out, err := s.cmd(ctx, "docker", "exec", containerID, "git", "push", "origin", ref).CombinedOutput() + if strings.HasPrefix(ref, "-") { + return fmt.Errorf("sandbox: git push: invalid ref %q", ref) + } + out, err := s.cmd(ctx, "git", + "-C", hostDir, + "-c", "core.hooksPath=/dev/null", + "-c", "protocol.ext.allow=never", + "push", "origin", "--", ref, + ).CombinedOutput() if err != nil { return fmt.Errorf("sandbox: git push: %w\n%s", err, out) } |
