diff --git a/act/filecollector/file_collector.go b/act/filecollector/file_collector.go index 6b3e6813..084c0804 100644 --- a/act/filecollector/file_collector.go +++ b/act/filecollector/file_collector.go @@ -73,10 +73,16 @@ func (cc *CopyCollector) WriteFile(fpath string, fi fs.FileInfo, linkName string if err := os.MkdirAll(filepath.Dir(fdestpath), 0o777); err != nil { return err } + // Remove any existing destination so we can overwrite read-only files + // (e.g. git pack files at mode 0444 trip EACCES on macOS and "Access is + // denied" on Windows when reopened with O_WRONLY) and so os.Symlink does + // not fail with EEXIST. os.Remove clears the Windows read-only attribute + // internally; on Unix unlink only needs write permission on the parent. + _ = os.Remove(fdestpath) if f == nil { return os.Symlink(linkName, fdestpath) } - df, err := os.OpenFile(fdestpath, os.O_CREATE|os.O_WRONLY, fi.Mode()) + df, err := os.OpenFile(fdestpath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, fi.Mode()) if err != nil { return err } diff --git a/act/filecollector/file_collector_test.go b/act/filecollector/file_collector_test.go index 1ee0015a..9ab462de 100644 --- a/act/filecollector/file_collector_test.go +++ b/act/filecollector/file_collector_test.go @@ -8,7 +8,9 @@ import ( "archive/tar" "context" "io" + "os" "path/filepath" + "runtime" "strings" "testing" @@ -20,6 +22,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/format/index" "github.com/go-git/go-git/v5/storage/filesystem" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type memoryFs struct { @@ -174,3 +177,47 @@ func TestSymlinks(t *testing.T) { assert.Equal(t, ".env", files["test.env"].Linkname) assert.ErrorIs(t, err, io.EOF, "tar must be read cleanly to EOF") } + +// Regression for https://gitea.com/gitea/runner/issues/876 and /941: +// re-copying an action directory must overwrite a pre-existing read-only +// file (e.g. a git pack .idx at mode 0444) instead of failing with EACCES +// on macOS or "Access is denied" on Windows. +func TestCopyCollectorWriteFileOverwritesReadOnlyFile(t *testing.T) { + dst := t.TempDir() + target := filepath.Join(dst, "sub", "pack.idx") + require.NoError(t, os.MkdirAll(filepath.Dir(target), 0o755)) + require.NoError(t, os.WriteFile(target, []byte("old"), 0o444)) + + src := filepath.Join(t.TempDir(), "pack.idx") + require.NoError(t, os.WriteFile(src, []byte("new"), 0o444)) + fi, err := os.Stat(src) + require.NoError(t, err) + + cc := &CopyCollector{DstDir: dst} + require.NoError(t, cc.WriteFile("sub/pack.idx", fi, "", strings.NewReader("new"))) + + got, err := os.ReadFile(target) + require.NoError(t, err) + assert.Equal(t, "new", string(got)) +} + +// Without the destination removal, os.Symlink fails with EEXIST when the +// path already holds a regular file from an earlier copy of the action. +func TestCopyCollectorWriteFileOverwritesFileWithSymlink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("creating symlinks requires elevated privileges on Windows") + } + dst := t.TempDir() + target := filepath.Join(dst, "link") + require.NoError(t, os.WriteFile(target, []byte("stale"), 0o644)) + + fi, err := os.Lstat(target) + require.NoError(t, err) + + cc := &CopyCollector{DstDir: dst} + require.NoError(t, cc.WriteFile("link", fi, "target", nil)) + + resolved, err := os.Readlink(target) + require.NoError(t, err) + assert.Equal(t, "target", resolved) +}