Merge pull request #5 from thepeterstone/fix/dockersandbox-gitpush-host-security

fix(sandbox): run GitPush from host to fix local-remote push + harden against sandbox escape

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)
 	}