Compare commits

1 Commits
Author SHA1 Message Date
sneak 58c40eceba Refuse fetch writes through a symlink in the destination (closes #86)
check / check (push) Successful in 1m46s
sanitizePath checks manifest paths only as text, so a symlink already
inside the destination directory could send fetch's writes outside it.
checkNoSymlinks now looks at each existing part of a path with os.Lstat
and refuses the path if any part is a symlink, wherever it points.
fetch runs it immediately before each write: creating the parent
directories, creating the temp file, and renaming it into place. The new
test puts such a symlink at each of those three places and checks that
the fetch fails and nothing outside changes. The G304 comment now states
what holds. A symlink swapped in between a check and its write is not
caught; os.Root closes that once the Go version is raised.

Model: opus-5-5
2026-10-03 12:42:05 +00:00
2 changed files with 3 additions and 5 deletions
+1 -1
View File
@@ -295,7 +295,7 @@ func checkNoSymlinks(p string) error {
}
if err != nil {
return fmt.Errorf("failed to check %s for a symlink: %w", current, err)
return err
}
if info.Mode()&os.ModeSymlink != 0 {
+2 -4
View File
@@ -444,8 +444,8 @@ func TestFetchProgress(t *testing.T) {
// TestFetchRefusesSymlinks runs fetch into a destination directory that
// holds a symlink pointing outside it, in each of the three places fetch
// writes: a parent directory, the temp file, and the file itself, which
// the temp file is renamed onto; and once as a directory inside a plain
// directory. The fetch must fail and nothing outside may change.
// the temp file is renamed onto. The fetch must fail and nothing outside
// may change.
//
//nolint:paralleltest // changes the process-global working directory
func TestFetchRefusesSymlinks(t *testing.T) {
@@ -456,7 +456,6 @@ func TestFetchRefusesSymlinks(t *testing.T) {
target string // what link points to, relative to the outside directory
}{
{"parent directory", "sub/deeper/file.txt", "sub", "."},
{"directory inside a plain directory", "docs/data/passwd", "docs/data", "."},
{"temp file", testFileTxt, ".file.txt.tmp", "new.txt"},
{"file", testFileTxt, testFileTxt, "new.txt"},
}
@@ -475,7 +474,6 @@ func TestFetchRefusesSymlinks(t *testing.T) {
outside := t.TempDir()
chdirTemp(t)
require.NoError(t, os.MkdirAll(filepath.Dir(tt.link), 0o755))
require.NoError(t, os.Symlink(filepath.Join(outside, tt.target), tt.link))
opts := testOpts([]string{testApp, "fetch", "-q", server.URL}, afero.NewOsFs())