From e5a06e0099d8692e34e2c2ef566846103ffc519b Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Fri, 24 Jul 2026 19:34:23 -0500 Subject: [PATCH 1/5] fix(repository): close temp state file before git notes reads it Explicitly close and check the temporary state file after writing but before passing its path to git notes. The previous deferred close happened after the git command ran, which can fail on Windows (open handle prevents git from reading the file) and masks write/close errors. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- repository/git.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/repository/git.go b/repository/git.go index 55dacfbb..5ca11d3d 100644 --- a/repository/git.go +++ b/repository/git.go @@ -429,6 +429,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()) From cc1ac1780c1feea869ff9364b2a4748513fda44c Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Fri, 24 Jul 2026 19:34:28 -0500 Subject: [PATCH 2/5] fix(watch): handle pipe and scanner errors on Windows git log Check errors from scanner.Err() and os.Stdout.Write(), and close pipe ends cleanly after reading. Previously these errors were silently discarded, which could truncate output or leave pipe handles open on Windows without reporting failure to the caller. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- cmd/container-use/watch_windows.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/cmd/container-use/watch_windows.go b/cmd/container-use/watch_windows.go index 108cef7e..15cecd96 100644 --- a/cmd/container-use/watch_windows.go +++ b/cmd/container-use/watch_windows.go @@ -122,21 +122,28 @@ 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 { @@ -144,7 +151,9 @@ func runGitLogWindows(ctx context.Context) error { } // 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 } From 968a27ef4e80c286cd8ba07da46b43607fc44c13 Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sat, 25 Jul 2026 07:27:59 -0500 Subject: [PATCH 3/5] fix(repository): validate git refs and worktree-relative file paths Add validateGitRefComponent to reject ref/branch names that start with '-' or contain unsafe characters before passing them to git commands. Apply validation in Checkout, Merge, Apply, and initializeWorktree to prevent argument injection. Also reject absolute paths and '..' traversal in exportEnvironmentFile so that agent-provided target_file cannot escape the environment worktree on the host filesystem. Regression tests included for ref validation and path traversal classification. --- repository/git.go | 39 ++++++++++++++++++++++++++++++++++++-- repository/git_test.go | 41 ++++++++++++++++++++++++++++++++++++++++ repository/repository.go | 11 +++++++++++ 3 files changed, 89 insertions(+), 2 deletions(-) diff --git a/repository/git.go b/repository/git.go index 5ca11d3d..9145f49d 100644 --- a/repository/git.go +++ b/repository/git.go @@ -26,8 +26,29 @@ const ( var ( urlSchemeRegExp = regexp.MustCompile(`^[^:]+://`) scpLikeURLRegExp = regexp.MustCompile(`^(?:(?P[^@]+)@)?(?P[^:\s]+):(?:(?P[0-9]{1,5})(?:\/|:))?(?P[^\\].*\/[^\\].*)$`) + // 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,8 +399,17 @@ 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 { @@ -382,7 +417,7 @@ func (r *Repository) exportEnvironmentFile(ctx context.Context, env *environment } // 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) } diff --git a/repository/git_test.go b/repository/git_test.go index 702f0874..8727ca55 100644 --- a/repository/git_test.go +++ b/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) + } +} diff --git a/repository/repository.go b/repository/repository.go index dac0cfa2..361dcebf 100644 --- a/repository/repository.go +++ b/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 From 9511a0f9cd0e34bb18ee8cf00c431fad77c6a003 Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sat, 25 Jul 2026 07:28:18 -0500 Subject: [PATCH 4/5] fix(environment): allowlist shell parameter in Run Reject unsupported shell strings before building container exec args. This prevents a caller from passing an arbitrary executable path as the shell interpreter (e.g. '/bin/echo') in environment_run_cmd. Regression test included. --- environment/environment.go | 13 +++++++++++++ environment/environment_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 environment/environment_test.go diff --git a/environment/environment.go b/environment/environment.go index b421fc81..3331356c 100644 --- a/environment/environment.go +++ b/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} diff --git a/environment/environment_test.go b/environment/environment_test.go new file mode 100644 index 00000000..f5981e5d --- /dev/null +++ b/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) + } +} From 9d494e02b2c7c2bab0cb95c9a67b8e311bd37fcf Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sat, 25 Jul 2026 07:28:30 -0500 Subject: [PATCH 5/5] fix(logger): restrict debug log file permissions to owner-only Change the default debug stderr log file mode from 0644 to 0600 so that other users on the same host cannot read potentially sensitive command output or environment metadata. --- cmd/container-use/logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/container-use/logger.go b/cmd/container-use/logger.go index cea1df56..2c7d676c 100644 --- a/cmd/container-use/logger.go +++ b/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) }