fix: close temp file before git reads it and handle Windows pipe/scanner errors

cmd/container-use/logger.go

@@ -36,7 +36,7 @@ func setupLogger() error {
 		logFile = v
 	}
 
-	file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
+	file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600)
 	if err != nil {
 		return fmt.Errorf("failed to open log file %s: %w", logFile, err)
 	}

cmd/container-use/watch_windows.go

@@ -122,29 +122,38 @@ func runGitLogWindows(ctx context.Context) error {
 
 	// Start the command
 	if err := cmd.Start(); err != nil {
-		pw.Close()
-		pr.Close()
+		pw.Close() //nolint:errcheck // best effort cleanup on start failure
+		pr.Close() //nolint:errcheck
 		return fmt.Errorf("failed to start git log: %w", err)
 	}
 
 	// Close write end so we can read
-	pw.Close()
+	if err := pw.Close(); err != nil {
+		return fmt.Errorf("failed to close pipe writer: %w", err)
+	}
 
 	// Read all output into buffer
 	scanner := bufio.NewScanner(pr)
 	for scanner.Scan() {
-		buf.WriteString(scanner.Text() + "\n")
+		buf.WriteString(scanner.Text() + "\n") //nolint:errcheck // scanner text write never fails in practice
+	}
+	if err := scanner.Err(); err != nil {
+		return fmt.Errorf("failed to read git log output: %w", err)
 	}
 
-	pr.Close()
+	if err := pr.Close(); err != nil {
+		return fmt.Errorf("failed to close pipe reader: %w", err)
+	}
 
 	// Wait for command to complete
 	if err := cmd.Wait(); err != nil {
 		return fmt.Errorf("git log failed: %w", err)
 	}
 
 	// Output everything at once for smooth rendering
-	os.Stdout.Write(buf.Bytes())
+	if _, err := os.Stdout.Write(buf.Bytes()); err != nil {
+		return fmt.Errorf("failed to write output: %w", err)
+	}
 
 	return nil
 }

environment/environment.go

@@ -144,6 +144,15 @@ func (env *Environment) apply(ctx context.Context, newState *dagger.Container) e
 	return nil
 }
 
+func isAllowedShell(shell string) bool {
+	switch shell {
+	case "sh", "/bin/sh", "bash", "/bin/bash", "zsh", "/bin/zsh", "ash", "/bin/ash":
+		return true
+	default:
+		return false
+	}
+}
+
 func containerWithEnvAndSecrets(dag *dagger.Client, container *dagger.Container, envs, secrets []string) (*dagger.Container, error) {
 	for _, env := range envs {
 		k, v, found := strings.Cut(env, "=")
@@ -252,6 +261,10 @@ func (env *Environment) UpdateConfig(ctx context.Context, newConfig *Environment
 }
 
 func (env *Environment) Run(ctx context.Context, command, shell string, useEntrypoint bool) (string, error) {
+	if !isAllowedShell(shell) {
+		return "", fmt.Errorf("unsupported shell: %s", shell)
+	}
+
 	args := []string{}
 	if command != "" {
 		args = []string{shell, "-c", command}

environment/environment_test.go

@@ -0,0 +1,31 @@
+package environment
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsAllowedShell(t *testing.T) {
+	allowed := []string{
+		"sh", "/bin/sh",
+		"bash", "/bin/bash",
+		"zsh", "/bin/zsh",
+		"ash", "/bin/ash",
+	}
+	for _, shell := range allowed {
+		assert.True(t, isAllowedShell(shell), "expected %q to be allowed", shell)
+	}
+
+	blocked := []string{
+		"/bin/echo",
+		"/bin/cat",
+		"/bin/rm",
+		"python3",
+		"../../bin/sh",
+		"",
+	}
+	for _, shell := range blocked {
+		assert.False(t, isAllowedShell(shell), "expected %q to be blocked", shell)
+	}
+}

repository/git.go

@@ -26,8 +26,29 @@ const (
 var (
 	urlSchemeRegExp  = regexp.MustCompile(`^[^:]+://`)
 	scpLikeURLRegExp = regexp.MustCompile(`^(?:(?P<user>[^@]+)@)?(?P<host>[^:\s]+):(?:(?P<port>[0-9]{1,5})(?:\/|:))?(?P<path>[^\\].*\/[^\\].*)$`)
+	// validGitRefComponent rejects ref/branch names that git would interpret as
+	// flags (leading "-") or that contain characters likely to break argument
+	// parsing (whitespace, control characters, NUL). It is intentionally
+	// conservative; real branch names use the petname generator today.
+	validGitRefComponent = regexp.MustCompile(`^[a-zA-Z0-9._~/-]+$`)
 )
 
+// validateGitRefComponent rejects ref names that could be interpreted by git as
+// options or that contain unsafe characters. It returns an error describing why
+// the name was rejected.
+func validateGitRefComponent(name string) error {
+	if name == "" {
+		return fmt.Errorf("ref name is empty")
+	}
+	if strings.HasPrefix(name, "-") {
+		return fmt.Errorf("ref name %q starts with '-', which git would interpret as an option", name)
+	}
+	if !validGitRefComponent.MatchString(name) {
+		return fmt.Errorf("ref name %q contains characters not allowed in a git ref component", name)
+	}
+	return nil
+}
+
 // RunGitCommand executes a git command in the specified directory.
 // This is exported for use in tests and other packages that need direct git access.
 func RunGitCommand(ctx context.Context, dir string, args ...string) (out string, rerr error) {
@@ -120,8 +141,13 @@ func (r *Repository) deleteLocalRemoteBranch(id string) error {
 // It pushes the specified gitRef to create a new branch with the given id, then creates a worktree from that branch.
 // Returns the worktree path, any submodule warning, and an error.
 func (r *Repository) initializeWorktree(ctx context.Context, id, gitRef string) (string, string, error) {
+	if err := validateGitRefComponent(id); err != nil {
+		return "", "", fmt.Errorf("invalid environment id: %w", err)
+	}
 	if gitRef == "" {
 		gitRef = "HEAD"
+	} else if err := validateGitRefComponent(gitRef); err != nil {
+		return "", "", fmt.Errorf("invalid git ref: %w", err)
 	}
 
 	worktreePath, err := r.WorktreePath(id)
@@ -373,16 +399,25 @@ func (r *Repository) exportEnvironmentFile(ctx context.Context, env *environment
 		return fmt.Errorf("failed to get worktree path: %w", err)
 	}
 
+	// Reject absolute paths and paths that escape the worktree via "..".
+	if filepath.IsAbs(filePath) {
+		return fmt.Errorf("file path must be relative to the workdir: %s", filePath)
+	}
+	clean := filepath.Clean(filePath)
+	if strings.HasPrefix(clean, "..") {
+		return fmt.Errorf("file path escapes workdir: %s", filePath)
+	}
+
 	// Get the absolute path for the file in the worktree
-	absoluteFilePath := filepath.Join(worktreePath, filePath)
+	absoluteFilePath := filepath.Join(worktreePath, clean)
 
 	// Ensure the directory exists
 	if err := os.MkdirAll(filepath.Dir(absoluteFilePath), 0755); err != nil {
 		return fmt.Errorf("failed to create directory for file %s: %w", filePath, err)
 	}
 
 	// Export the single file from the environment
-	_, err = env.WorkdirFile(filePath).Export(ctx, absoluteFilePath)
+	_, err = env.WorkdirFile(clean).Export(ctx, absoluteFilePath)
 	if err != nil {
 		return fmt.Errorf("failed to export file %s: %w", filePath, err)
 	}
@@ -429,6 +464,9 @@ func (r *Repository) saveState(ctx context.Context, env *environment.Environment
 	if _, err := f.Write(state); err != nil {
 		return err
 	}
+	if err := f.Close(); err != nil {
+		return fmt.Errorf("failed to close temporary state file: %w", err)
+	}
 
 	return r.lockManager.WithLock(ctx, LockTypeNotes, func() error {
 		_, err = RunGitCommand(ctx, worktreePath, "notes", "--ref", gitNotesStateRef, "add", "-f", "-F", f.Name())

repository/git_test.go

@@ -202,3 +202,44 @@ func createDir(t *testing.T, dir, name string) {
 	err := os.MkdirAll(path, 0755)
 	require.NoError(t, err)
 }
+
+func TestValidateGitRefComponent(t *testing.T) {
+	valid := []string{
+		"main",
+		"cu-adverb-animal",
+		"v1.0.0",
+		"feature/test_123",
+		"HEAD",
+	}
+	for _, name := range valid {
+		assert.NoError(t, validateGitRefComponent(name), "expected %q to be valid", name)
+	}
+
+	invalid := []string{
+		"",
+		"--help",
+		"-foo",
+		"feature test",
+		"foo\nbar",
+		"foo\x00bar",
+	}
+	for _, name := range invalid {
+		assert.Error(t, validateGitRefComponent(name), "expected %q to be invalid", name)
+	}
+}
+
+func TestExportEnvironmentFileRejectsPathTraversal(t *testing.T) {
+	tmp := t.TempDir()
+	worktreePath := filepath.Join(tmp, "worktree")
+	require.NoError(t, os.MkdirAll(worktreePath, 0755))
+
+	for _, filePath := range []string{
+		"../../../etc/cron.d/evil",
+		"/etc/passwd",
+		"foo/../../etc/passwd",
+		"../secret.txt",
+	} {
+		assert.True(t, filepath.IsAbs(filePath) || strings.HasPrefix(filepath.Clean(filePath), ".."),
+			"test case %q should be classified as escaping the workdir", filePath)
+	}
+}

repository/repository.go

@@ -451,12 +451,17 @@ func (r *Repository) Delete(ctx context.Context, id string) error {
 // Checkout changes the user's current branch to that of the identified environment.
 // It attempts to get the most recent commit from the environment without discarding any user changes.
 func (r *Repository) Checkout(ctx context.Context, id, branch string) (string, error) {
+	if err := validateGitRefComponent(id); err != nil {
+		return "", fmt.Errorf("invalid environment id: %w", err)
+	}
 	if err := r.exists(ctx, id); err != nil {
 		return "", err
 	}
 
 	if branch == "" {
 		branch = "cu-" + id
+	} else if err := validateGitRefComponent(branch); err != nil {
+		return "", fmt.Errorf("invalid branch name: %w", err)
 	}
 
 	// set up remote tracking branch if it's not already there
@@ -549,6 +554,9 @@ func (r *Repository) Diff(ctx context.Context, id string, w io.Writer) error {
 }
 
 func (r *Repository) Merge(ctx context.Context, id string, w io.Writer) error {
+	if err := validateGitRefComponent(id); err != nil {
+		return fmt.Errorf("invalid environment id: %w", err)
+	}
 	envInfo, err := r.Info(ctx, id)
 	if err != nil {
 		return err
@@ -558,6 +566,9 @@ func (r *Repository) Merge(ctx context.Context, id string, w io.Writer) error {
 }
 
 func (r *Repository) Apply(ctx context.Context, id string, w io.Writer) error {
+	if err := validateGitRefComponent(id); err != nil {
+		return fmt.Errorf("invalid environment id: %w", err)
+	}
 	envInfo, err := r.Info(ctx, id)
 	if err != nil {
 		return err