mirror of
https://gitea.com/gitea/act_runner.git
synced 2026-05-08 16:23:23 +02:00
fix: overwrite read-only files when copying action directories (#942)
## Summary
- `CopyCollector.WriteFile` now removes any existing destination file
before writing, handling read-only modes (e.g. git pack files at
`0444`) that cause `EACCES`/`ERROR_ACCESS_DENIED` on macOS and Windows.
- Added `O_TRUNC` to the `OpenFile` flags as a safety net.
## Root cause
When a composite action with a post step runs on a host runner,
`runPostStep` calls `maybeCopyToActionDir`, which re-copies the action
into `miscpath/act/actions/<name>/`. The first copy (main step) writes
`.git/objects/pack/*.idx` at the destination with mode `0444` (as set
by go-git). The second copy (post step) calls
`os.OpenFile(dest, O_CREATE|O_WRONLY, …)` on that existing `0444` file,
which fails immediately:
- macOS: `open <path>: permission denied`
- Windows: `open <path>: Access is denied`
Fixes: https://gitea.com/gitea/runner/issues/941
Fixes: https://gitea.com/gitea/runner/issues/876
---------
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: silverwind <2021+silverwind@noreply.gitea.com>
Reviewed-on: https://gitea.com/gitea/runner/pulls/942
Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com>
Co-authored-by: Nicolas <bircni@icloud.com>
Co-committed-by: Nicolas <bircni@icloud.com>
This commit is contained in:
@@ -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 {
|
if err := os.MkdirAll(filepath.Dir(fdestpath), 0o777); err != nil {
|
||||||
return err
|
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 {
|
if f == nil {
|
||||||
return os.Symlink(linkName, fdestpath)
|
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 {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,7 +8,9 @@ import (
|
|||||||
"archive/tar"
|
"archive/tar"
|
||||||
"context"
|
"context"
|
||||||
"io"
|
"io"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"runtime"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
@@ -20,6 +22,7 @@ import (
|
|||||||
"github.com/go-git/go-git/v5/plumbing/format/index"
|
"github.com/go-git/go-git/v5/plumbing/format/index"
|
||||||
"github.com/go-git/go-git/v5/storage/filesystem"
|
"github.com/go-git/go-git/v5/storage/filesystem"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
type memoryFs struct {
|
type memoryFs struct {
|
||||||
@@ -174,3 +177,47 @@ func TestSymlinks(t *testing.T) {
|
|||||||
assert.Equal(t, ".env", files["test.env"].Linkname)
|
assert.Equal(t, ".env", files["test.env"].Linkname)
|
||||||
assert.ErrorIs(t, err, io.EOF, "tar must be read cleanly to EOF")
|
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)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user