Never write fetch's temp file into an existing file (closes #115)
check / check (push) Successful in 53s
check / check (push) Successful in 53s
downloadFile opened the temp file with os.Create, which opens and empties a file already at that name. If that file was a hard link to a file outside the destination directory, fetch overwrote the outside file. It now removes whatever is at the temp name, which removes only that name, and creates the temp file with O_EXCL, so the create fails if anything is still or again there. A leftover temp file from an interrupted run is still replaced. The new test puts a hard link at the temp name and checks that fetch succeeds and the outside file is unchanged. The command name is now the constant cmdFetch, like the other command names, because lint requires it once a third test uses it. Model: opus-5-5
This commit was merged in pull request #118.
This commit is contained in:
@@ -478,7 +478,7 @@ func TestFetchRefusesSymlinks(t *testing.T) {
|
||||
require.NoError(t, os.MkdirAll(filepath.Dir(tt.link), 0o750))
|
||||
require.NoError(t, os.Symlink(filepath.Join(outside, tt.target), tt.link))
|
||||
|
||||
opts := testOpts([]string{testApp, "fetch", "-q", server.URL}, afero.NewOsFs())
|
||||
opts := testOpts([]string{testApp, cmdFetch, "-q", server.URL}, afero.NewOsFs())
|
||||
assert.Equal(t, 1, runCLI(opts))
|
||||
assert.Contains(t, testStderr(t, opts), "failed to download "+tt.entry+
|
||||
": symlink in path not allowed: "+tt.link)
|
||||
@@ -489,3 +489,37 @@ func TestFetchRefusesSymlinks(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestFetchReplacesHardLinkAtTempName runs fetch into a destination
|
||||
// directory that holds, at the temp file's name, a hard link to a file
|
||||
// outside it. To fetch that is an ordinary leftover from an interrupted
|
||||
// earlier run: it must replace it and succeed, and the outside file must
|
||||
// not change.
|
||||
//
|
||||
//nolint:paralleltest // changes the process-global working directory
|
||||
func TestFetchReplacesHardLinkAtTempName(t *testing.T) {
|
||||
content := []byte("fetched")
|
||||
sourceFs := afero.NewMemMapFs()
|
||||
require.NoError(t, afero.WriteFile(sourceFs, "/"+testFileTxt, content, 0o644))
|
||||
|
||||
server := httptest.NewServer(fetchTestHandler(
|
||||
scanToManifest(t, sourceFs), map[string][]byte{testFileTxt: content}))
|
||||
defer server.Close()
|
||||
|
||||
outsideFile := filepath.Join(t.TempDir(), "secret.txt")
|
||||
require.NoError(t, os.WriteFile(outsideFile, []byte("outside"), 0o600))
|
||||
|
||||
chdirTemp(t)
|
||||
require.NoError(t, os.Link(outsideFile, ".file.txt.tmp"))
|
||||
|
||||
opts := testOpts([]string{testApp, cmdFetch, "-q", server.URL}, afero.NewOsFs())
|
||||
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
|
||||
|
||||
fetched, err := os.ReadFile(testFileTxt)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, content, fetched)
|
||||
|
||||
outside, err := os.ReadFile(outsideFile) //nolint:gosec // test-controlled path
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "outside", string(outside), "fetch wrote outside the destination")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user