diff --git a/repository/git.go b/repository/git.go index f028606b..82325cf2 100644 --- a/repository/git.go +++ b/repository/git.go @@ -398,25 +398,45 @@ func (r *Repository) exportEnvironment(ctx context.Context, env *environment.Env return nil } -// exportEnvironmentFile exports a single file from the environment to the worktree +// validateExportFilePath validates that a file path intended for export from an +// environment stays within the given worktree root. It rejects absolute paths +// and paths that resolve outside the worktree. Based on the bug report in +// https://github.com/dagger/container-use/issues/337. +func validateExportFilePath(worktreePath, filePath string) (clean string, absoluteFilePath string, err error) { + // 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 and verify it does not + // escape the worktree root. + absoluteFilePath = filepath.Join(worktreePath, clean) + rel, err := filepath.Rel(worktreePath, absoluteFilePath) + if err != nil || strings.HasPrefix(rel, "..") { + return "", "", fmt.Errorf("file path escapes worktree: %s", filePath) + } + + return clean, absoluteFilePath, nil +} + +// exportEnvironmentFile exports a single file from the environment to the worktree. +// Prevents directory traversal by ensuring the resolved file path stays inside the +// worktree. Based on the bug report in https://github.com/dagger/container-use/issues/337. func (r *Repository) exportEnvironmentFile(ctx context.Context, env *environment.Environment, filePath string) error { worktreePath, err := r.WorktreePath(env.ID) if err != nil { 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) + clean, absoluteFilePath, err := validateExportFilePath(worktreePath, filePath) + if err != nil { + return err } - // Get the absolute path for the file in the worktree - 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) diff --git a/repository/git_test.go b/repository/git_test.go index 3e4e68b5..ee8b78e9 100644 --- a/repository/git_test.go +++ b/repository/git_test.go @@ -228,20 +228,42 @@ func TestValidateGitRefComponent(t *testing.T) { } } -func TestExportEnvironmentFileRejectsPathTraversal(t *testing.T) { +func TestValidateExportFilePath(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) - } + t.Run("rejects_absolute_paths", func(t *testing.T) { + _, _, err := validateExportFilePath(worktreePath, "/etc/passwd") + require.Error(t, err) + assert.Contains(t, err.Error(), "must be relative to the workdir") + }) + + t.Run("rejects_paths_escaping_worktree", func(t *testing.T) { + for _, filePath := range []string{ + "../../../etc/cron.d/evil", + "foo/../../etc/passwd", + "../secret.txt", + } { + _, _, err := validateExportFilePath(worktreePath, filePath) + require.Error(t, err, "expected %q to be rejected", filePath) + assert.Contains(t, err.Error(), "escapes workdir", filePath) + } + }) + + t.Run("accepts_valid_relative_paths", func(t *testing.T) { + clean, abs, err := validateExportFilePath(worktreePath, "foo/bar.txt") + require.NoError(t, err) + assert.Equal(t, "foo/bar.txt", clean) + assert.Equal(t, filepath.Join(worktreePath, "foo/bar.txt"), abs) + }) + + t.Run("normalizes_relative_paths", func(t *testing.T) { + clean, abs, err := validateExportFilePath(worktreePath, "./foo/../baz.txt") + require.NoError(t, err) + assert.Equal(t, "baz.txt", clean) + assert.Equal(t, filepath.Join(worktreePath, "baz.txt"), abs) + }) } func TestNormalizeGitURL(t *testing.T) {