From 0abe8048f10bcf22baffc3fe9c3dfabb0cf0597a Mon Sep 17 00:00:00 2001
From: Peter Stone <thepeterstone@gmail.com>
Date: Sat, 4 Jul 2026 09:43:11 +0000
Subject: fix(sandbox): run GitPush from host to fix local-remote push + harden
 against sandbox escape
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 internal/sandbox/dockersandbox.go | 40 +++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

(limited to 'internal')

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)
 	}
-- 
cgit v1.2.3