Validate downloads before installing workspace files

This commit is contained in:
2026-09-02 00:37:00 +00:00
parent ba858cc997
commit 39b2665d8b
13 changed files with 204 additions and 665 deletions

View File

@@ -5,7 +5,6 @@ import (
"encoding/hex"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strings"
@@ -325,232 +324,3 @@ func validateOpenedConfinedFile(parent *os.Root, name string, file *os.File) (*o
}
return file, nil
}
// DownloadedTempFile is a completed, destination-confined temporary file. It
// keeps its parent directory open until the caller installs or cleans it up.
type DownloadedTempFile struct {
parent *os.Root
name string
}
// DownloadToSiblingTemp writes a remote object into a sibling temporary file.
// The caller supplies transport behavior through download and retains object
// identity and validation policy. The destination parent must already exist.
func DownloadToSiblingTemp(destination string, download func(io.Writer) error) (*DownloadedTempFile, error) {
if download == nil {
return nil, fmt.Errorf("download function is required")
}
parent, name, err := openConfinedParent(destination, false, 0)
if err != nil {
return nil, err
}
file, tempName, err := createSiblingTemp(parent, name)
if err != nil {
_ = parent.Close()
return nil, fmt.Errorf("create sibling temporary file: %w", err)
}
if err := download(file); err != nil {
_ = file.Close()
_ = parent.Remove(tempName)
_ = parent.Close()
return nil, err
}
if err := file.Sync(); err != nil {
_ = file.Close()
_ = parent.Remove(tempName)
_ = parent.Close()
return nil, fmt.Errorf("sync downloaded temporary file: %w", err)
}
if err := file.Close(); err != nil {
_ = parent.Remove(tempName)
_ = parent.Close()
return nil, fmt.Errorf("close downloaded temporary file: %w", err)
}
return &DownloadedTempFile{parent: parent, name: tempName}, nil
}
// Open returns a read handle for validation before installation.
func (f *DownloadedTempFile) Open() (*os.File, error) {
if f == nil || f.parent == nil || f.name == "" {
return nil, fmt.Errorf("downloaded temporary file is unavailable")
}
return f.parent.Open(f.name)
}
// Install atomically replaces destinationName in the opened parent directory.
func (f *DownloadedTempFile) Install(destinationName string, mode os.FileMode) error {
if f == nil || f.parent == nil || f.name == "" {
return fmt.Errorf("downloaded temporary file is unavailable")
}
if filepath.Base(destinationName) != destinationName || destinationName == "." || destinationName == "" {
return fmt.Errorf("destination file name is required")
}
file, err := f.parent.Open(f.name)
if err != nil {
return fmt.Errorf("open downloaded temporary file: %w", err)
}
if err := file.Chmod(mode); err != nil {
_ = file.Close()
return fmt.Errorf("set downloaded temporary file permissions: %w", err)
}
if err := file.Sync(); err != nil {
_ = file.Close()
return fmt.Errorf("sync downloaded temporary file: %w", err)
}
if err := file.Close(); err != nil {
return fmt.Errorf("close downloaded temporary file: %w", err)
}
if err := f.parent.Rename(f.name, destinationName); err != nil {
return fmt.Errorf("install downloaded temporary file: %w", err)
}
f.name = ""
if err := syncOpenedDirectory(f.parent); err != nil {
return fmt.Errorf("sync destination directory after downloaded-file replacement: %w", err)
}
return f.closeParent()
}
// Cleanup removes an uninstalled temporary file. It is safe to call repeatedly.
func (f *DownloadedTempFile) Cleanup() error {
if f == nil || f.parent == nil {
return nil
}
var err error
if f.name != "" {
err = f.parent.Remove(f.name)
if errors.Is(err, os.ErrNotExist) {
err = nil
}
f.name = ""
}
closeErr := f.closeParent()
if err != nil && closeErr != nil {
return errors.Join(err, closeErr)
}
if err != nil {
return err
}
return closeErr
}
func (f *DownloadedTempFile) closeParent() error {
if f.parent == nil {
return nil
}
err := f.parent.Close()
f.parent = nil
return err
}
// DownloadAndInstall obtains remote bytes through a confined sibling temporary
// file and installs them with the caller-selected final mode.
func DownloadAndInstall(destination string, mode os.FileMode, download func(io.Writer) error) error {
temporary, err := DownloadToSiblingTemp(destination, download)
if err != nil {
return err
}
defer func() { _ = temporary.Cleanup() }()
return temporary.Install(filepath.Base(destination), mode)
}
func replaceFileFromReaderConfined(dst string, source io.Reader, options ReplaceFileOptions) (resultErr error) {
parent, name, err := openConfinedParent(dst, false, 0)
if err != nil {
return err
}
defer func() { _ = parent.Close() }()
temporary, tempName, err := createSiblingTemp(parent, name)
if err != nil {
return fmt.Errorf("create temp file: %w", err)
}
removeTemp := true
defer func() {
if removeTemp {
if err := parent.Remove(tempName); err != nil && !errors.Is(err, os.ErrNotExist) {
resultErr = errors.Join(resultErr, fmt.Errorf("remove temporary file: %w", err))
}
}
}()
if _, err := io.Copy(temporary, source); err != nil {
return closeTemporaryFileAfterError(temporary, fmt.Errorf("write temp file: %w", err))
}
if err := temporary.Chmod(options.Mode); err != nil {
return closeTemporaryFileAfterError(temporary, fmt.Errorf("set temp file permissions: %w", err))
}
if err := temporary.Sync(); err != nil {
return closeTemporaryFileAfterError(temporary, fmt.Errorf("sync temp file: %w", err))
}
if err := temporary.Close(); err != nil {
return fmt.Errorf("close temp file: %w", err)
}
if options.BeforeRename != nil {
if err := options.BeforeRename(); err != nil {
return fmt.Errorf("check before file replacement: %w", err)
}
}
if err := parent.Rename(tempName, name); err != nil {
return fmt.Errorf("install temp file: %w", err)
}
removeTemp = false
if err := syncOpenedDirectory(parent); err != nil {
return fmt.Errorf("sync destination directory after replacement: %w", err)
}
return nil
}
func installDownloadedTempFileConfined(tmpPath, dst string, mode os.FileMode) error {
if filepath.Clean(filepath.Dir(tmpPath)) != filepath.Clean(filepath.Dir(dst)) {
source, err := os.Open(tmpPath)
if err != nil {
return fmt.Errorf("open downloaded temporary file: %w", err)
}
err = replaceFileFromReaderConfined(dst, source, ReplaceFileOptions{Mode: mode})
closeErr := source.Close()
if err != nil {
return fmt.Errorf("install downloaded file: %w", err)
}
if closeErr != nil {
return fmt.Errorf("close downloaded temporary file: %w", closeErr)
}
if err := os.Remove(tmpPath); err != nil {
return fmt.Errorf("remove installed temporary file: %w", err)
}
return nil
}
parent, destinationName, err := openConfinedParent(dst, false, 0)
if err != nil {
return err
}
defer func() { _ = parent.Close() }()
temporaryName := filepath.Base(tmpPath)
info, err := parent.Lstat(temporaryName)
if err != nil {
return fmt.Errorf("inspect downloaded temporary file: %w", err)
}
if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() {
return fmt.Errorf("downloaded temporary file is not a regular file")
}
file, err := parent.Open(temporaryName)
if err != nil {
return fmt.Errorf("open downloaded temporary file: %w", err)
}
if err := file.Chmod(mode); err != nil {
_ = file.Close()
return fmt.Errorf("set temp file permissions: %w", err)
}
if err := file.Sync(); err != nil {
_ = file.Close()
return fmt.Errorf("sync downloaded temp file: %w", err)
}
if err := file.Close(); err != nil {
return fmt.Errorf("close downloaded temp file: %w", err)
}
if err := parent.Rename(temporaryName, destinationName); err != nil {
return fmt.Errorf("install downloaded file: %w", err)
}
if err := syncOpenedDirectory(parent); err != nil {
return fmt.Errorf("sync destination directory after downloaded-file replacement: %w", err)
}
return nil
}

View File

@@ -12,8 +12,8 @@ import (
"strings"
)
// ErrDirectorySyncUnsupported remains for legacy confined migration paths.
// The retained replacement entry points do not perform directory syncing.
// ErrDirectorySyncUnsupported is used only by the retained directory-promotion
// implementation. Single-file replacement does not perform directory syncing.
var ErrDirectorySyncUnsupported = errors.New("directory sync is unsupported")
// ReplaceFileOptions describes the caller-owned parts of a byte-file
@@ -131,147 +131,21 @@ func replaceFileFromReader(dst string, source io.Reader, options ReplaceFileOpti
return nil
}
// InstallDownloadedTempFile installs a fully downloaded sibling temporary file
// at dst. On failure before the rename, the caller retains ownership of tmpPath
// and is responsible for cleanup.
func InstallDownloadedTempFile(tmpPath, dst string, perm os.FileMode) error {
// InstallSiblingTempFile installs a fully written and validated sibling
// temporary file at dst. The caller retains ownership of tmpPath if installation
// fails and is responsible for removing it.
func InstallSiblingTempFile(tmpPath, dst string, perm os.FileMode) error {
if strings.TrimSpace(tmpPath) == "" || strings.TrimSpace(dst) == "" {
return fmt.Errorf("temp and destination paths are required")
}
if err := EnsureWorkspaceDirectory(filepath.Dir(dst)); err != nil {
return fmt.Errorf("create destination directory: %w", err)
if filepath.Clean(filepath.Dir(tmpPath)) != filepath.Clean(filepath.Dir(dst)) {
return fmt.Errorf("temporary file must be a sibling of the destination")
}
return installDownloadedTempFileConfined(tmpPath, dst, perm)
}
type temporaryFile interface {
io.Writer
Name() string
Sync() error
Close() error
}
type atomicReplacementOperations struct {
createTemp func(string, string) (temporaryFile, error)
chmod func(string, os.FileMode) error
rename func(string, string) error
remove func(string) error
syncDirectory func(string) error
syncFile func(string) error
}
var systemAtomicReplacementOperations = atomicReplacementOperations{
createTemp: func(dir, pattern string) (temporaryFile, error) {
return os.CreateTemp(dir, pattern)
},
chmod: os.Chmod,
rename: os.Rename,
remove: os.Remove,
syncDirectory: syncDirectory,
syncFile: syncFile,
}
func replaceFileAtomicWithOperations(
dst string,
data []byte,
options ReplaceFileOptions,
ops atomicReplacementOperations,
) error {
return replaceFileFromReaderWithOperations(dst, bytes.NewReader(data), options, ops)
}
func replaceFileFromReaderWithOperations(
dst string,
source io.Reader,
options ReplaceFileOptions,
ops atomicReplacementOperations,
) (resultErr error) {
if strings.TrimSpace(dst) == "" {
return fmt.Errorf("destination path is required")
}
dir := filepath.Dir(dst)
tmp, err := ops.createTemp(dir, "."+filepath.Base(dst)+".tmp-*")
if err != nil {
return fmt.Errorf("create temp file: %w", err)
}
tmpPath := tmp.Name()
removeTmp := true
defer func() {
if removeTmp {
if err := ops.remove(tmpPath); err != nil {
cleanupErr := fmt.Errorf("remove temporary file: %w", err)
if resultErr == nil {
resultErr = cleanupErr
} else {
resultErr = errors.Join(resultErr, cleanupErr)
}
}
}
}()
if _, err := io.Copy(tmp, source); err != nil {
return closeTemporaryFileAfterError(tmp, fmt.Errorf("write temp file: %w", err))
}
if err := ops.chmod(tmpPath, options.Mode); err != nil {
return closeTemporaryFileAfterError(tmp, fmt.Errorf("set temp file permissions: %w", err))
}
if err := tmp.Sync(); err != nil {
return closeTemporaryFileAfterError(tmp, fmt.Errorf("sync temp file: %w", err))
}
if err := tmp.Close(); err != nil {
return fmt.Errorf("close temp file: %w", err)
}
if options.BeforeRename != nil {
if err := options.BeforeRename(); err != nil {
return fmt.Errorf("check before file replacement: %w", err)
}
}
if err := ops.rename(tmpPath, dst); err != nil {
return fmt.Errorf("install temp file: %w", err)
}
removeTmp = false
if err := ops.syncDirectory(dir); err != nil {
return fmt.Errorf("sync destination directory after replacement: %w", err)
}
return nil
}
func closeTemporaryFileAfterError(tmp temporaryFile, operationErr error) error {
if err := tmp.Close(); err != nil {
return errors.Join(operationErr, fmt.Errorf("close temporary file: %w", err))
}
return operationErr
}
func installDownloadedTempFileWithOperations(
tmpPath, dst string,
perm os.FileMode,
ops atomicReplacementOperations,
) error {
if err := ops.chmod(tmpPath, perm); err != nil {
if err := os.Chmod(tmpPath, perm); err != nil {
return fmt.Errorf("set temp file permissions: %w", err)
}
if err := ops.syncFile(tmpPath); err != nil {
return fmt.Errorf("sync downloaded temp file: %w", err)
}
if err := ops.rename(tmpPath, dst); err != nil {
if err := os.Rename(tmpPath, dst); err != nil {
return fmt.Errorf("install downloaded file: %w", err)
}
if err := ops.syncDirectory(filepath.Dir(dst)); err != nil {
return fmt.Errorf("sync destination directory after downloaded-file replacement: %w", err)
}
return nil
}
func syncFile(path string) error {
file, err := os.Open(path)
if err != nil {
return err
}
if err := file.Sync(); err != nil {
_ = file.Close()
return err
}
return file.Close()
}

View File

@@ -3,16 +3,12 @@ package fileops
import (
"crypto/sha256"
"encoding/hex"
"errors"
"io"
"os"
"path/filepath"
"strings"
"testing"
)
var errAtomicReplacementTest = errors.New("atomic replacement test failure")
func TestWriteFileAtomicOverwritesAndLeavesNoTempFile(t *testing.T) {
root := t.TempDir()
dst := filepath.Join(root, "out", "value.txt")
@@ -116,16 +112,16 @@ func TestCopyFileAtomicCleansTempFileOnInstallFailure(t *testing.T) {
assertNoMatchingTempFiles(t, root, ".blocked.tmp-")
}
func TestInstallDownloadedTempFileSetsPermissions(t *testing.T) {
func TestInstallSiblingTempFileSetsPermissions(t *testing.T) {
root := t.TempDir()
dst := filepath.Join(root, "payload.json")
tmpPath := filepath.Join(root, ".payload.tmp")
dst := filepath.Join(root, "out", "payload.json")
if err := os.WriteFile(tmpPath, []byte("{\"ok\":true}\n"), 0o600); err != nil {
t.Fatalf("WriteFile(temp) error = %v", err)
}
if err := InstallDownloadedTempFile(tmpPath, dst, 0o644); err != nil {
t.Fatalf("InstallDownloadedTempFile() error = %v", err)
if err := InstallSiblingTempFile(tmpPath, dst, 0o644); err != nil {
t.Fatalf("InstallSiblingTempFile() error = %v", err)
}
if _, err := os.Stat(tmpPath); !os.IsNotExist(err) {
t.Fatalf("temp file still exists: stat err = %v", err)
@@ -186,253 +182,6 @@ func TestWriteFileAtomicReplacesLeafSymlinkWithoutFollowingIt(t *testing.T) {
}
}
func TestDownloadAndInstallRejectsSymlinkedDestinationAncestor(t *testing.T) {
root := t.TempDir()
outside := t.TempDir()
if err := os.Symlink(outside, filepath.Join(root, "redirect")); err != nil {
t.Skipf("Symlink unavailable: %v", err)
}
err := DownloadAndInstall(filepath.Join(root, "redirect", "output.txt"), 0o640, func(io.Writer) error {
return nil
})
if err == nil {
t.Fatal("DownloadAndInstall() error = nil, want symlink ancestor rejection")
}
entries, err := os.ReadDir(outside)
if err != nil {
t.Fatalf("ReadDir(outside) error = %v", err)
}
if len(entries) != 0 {
t.Fatalf("outside destination received entries: %v", entries)
}
}
func TestReplaceFileAtomicOrdersDurableOperations(t *testing.T) {
events := make([]string, 0, 8)
tmp := &recordingTemporaryFile{name: "/work/.result.tmp-1", events: &events}
ops := recordingAtomicReplacementOperations(&events, tmp)
err := replaceFileAtomicWithOperations(
"/work/result",
[]byte("replacement"),
ReplaceFileOptions{
Mode: 0o640,
BeforeRename: func() error {
events = append(events, "check")
return nil
},
},
ops,
)
if err != nil {
t.Fatalf("replaceFileAtomicWithOperations() error = %v", err)
}
want := []string{"create", "write", "chmod:rw-r-----", "sync", "close", "check", "rename", "sync-directory"}
if !equalStrings(events, want) {
t.Fatalf("operation order = %v, want %v", events, want)
}
}
func TestReplaceFileAtomicFailuresCleanUninstalledTemporaryFile(t *testing.T) {
tests := []struct {
name string
configure func(*recordingTemporaryFile, *atomicReplacementOperations)
wantErr error
wantRemove bool
}{
{
name: "short write",
configure: func(tmp *recordingTemporaryFile, _ *atomicReplacementOperations) {
tmp.shortWrite = true
},
wantErr: io.ErrShortWrite,
wantRemove: true,
},
{
name: "write",
configure: func(tmp *recordingTemporaryFile, _ *atomicReplacementOperations) {
tmp.writeErr = errAtomicReplacementTest
},
wantErr: errAtomicReplacementTest,
wantRemove: true,
},
{
name: "sync",
configure: func(tmp *recordingTemporaryFile, _ *atomicReplacementOperations) {
tmp.syncErr = errAtomicReplacementTest
},
wantErr: errAtomicReplacementTest,
wantRemove: true,
},
{
name: "rename",
configure: func(_ *recordingTemporaryFile, ops *atomicReplacementOperations) {
ops.rename = func(string, string) error { return errAtomicReplacementTest }
},
wantErr: errAtomicReplacementTest,
wantRemove: true,
},
{
name: "directory sync",
configure: func(_ *recordingTemporaryFile, ops *atomicReplacementOperations) {
ops.syncDirectory = func(string) error { return ErrDirectorySyncUnsupported }
},
wantErr: ErrDirectorySyncUnsupported,
wantRemove: false,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
events := make([]string, 0, 8)
tmp := &recordingTemporaryFile{name: "/work/.result.tmp-1", events: &events}
ops := recordingAtomicReplacementOperations(&events, tmp)
test.configure(tmp, &ops)
err := replaceFileAtomicWithOperations("/work/result", []byte("replacement"), ReplaceFileOptions{Mode: 0o640}, ops)
if !errors.Is(err, test.wantErr) {
t.Fatalf("replaceFileAtomicWithOperations() error = %v, want %v", err, test.wantErr)
}
gotRemove := containsString(events, "remove")
if gotRemove != test.wantRemove {
t.Fatalf("remove temporary file = %t, want %t; operations = %v", gotRemove, test.wantRemove, events)
}
})
}
}
func TestReplaceFileAtomicReportsCleanupFailureWithPrimaryFailure(t *testing.T) {
events := make([]string, 0, 5)
tmp := &recordingTemporaryFile{
name: "/work/.result.tmp-1",
events: &events,
writeErr: errAtomicReplacementTest,
}
ops := recordingAtomicReplacementOperations(&events, tmp)
cleanupErr := errors.New("temporary cleanup failed")
ops.remove = func(string) error {
events = append(events, "remove")
return cleanupErr
}
err := replaceFileAtomicWithOperations("/work/result", []byte("replacement"), ReplaceFileOptions{Mode: 0o640}, ops)
if !errors.Is(err, errAtomicReplacementTest) {
t.Fatalf("replacement error = %v, want write failure", err)
}
if !errors.Is(err, cleanupErr) {
t.Fatalf("replacement error = %v, want cleanup failure", err)
}
}
func TestInstallDownloadedTempFileOrdersDurableOperations(t *testing.T) {
events := make([]string, 0, 4)
ops := atomicReplacementOperations{
chmod: func(string, os.FileMode) error {
events = append(events, "chmod")
return nil
},
syncFile: func(string) error {
events = append(events, "sync-file")
return nil
},
rename: func(string, string) error {
events = append(events, "rename")
return nil
},
syncDirectory: func(string) error {
events = append(events, "sync-directory")
return nil
},
}
if err := installDownloadedTempFileWithOperations("/work/.download.tmp", "/work/result", 0o640, ops); err != nil {
t.Fatalf("installDownloadedTempFileWithOperations() error = %v", err)
}
want := []string{"chmod", "sync-file", "rename", "sync-directory"}
if !equalStrings(events, want) {
t.Fatalf("operation order = %v, want %v", events, want)
}
}
type recordingTemporaryFile struct {
name string
events *[]string
shortWrite bool
writeErr error
syncErr error
}
func (f *recordingTemporaryFile) Write(data []byte) (int, error) {
*f.events = append(*f.events, "write")
if f.writeErr != nil {
return 0, f.writeErr
}
if f.shortWrite {
return len(data) - 1, nil
}
return len(data), nil
}
func (f *recordingTemporaryFile) Name() string { return f.name }
func (f *recordingTemporaryFile) Sync() error {
*f.events = append(*f.events, "sync")
return f.syncErr
}
func (f *recordingTemporaryFile) Close() error {
*f.events = append(*f.events, "close")
return nil
}
func recordingAtomicReplacementOperations(events *[]string, tmp temporaryFile) atomicReplacementOperations {
return atomicReplacementOperations{
createTemp: func(string, string) (temporaryFile, error) {
*events = append(*events, "create")
return tmp, nil
},
chmod: func(_ string, mode os.FileMode) error {
*events = append(*events, "chmod:"+mode.Perm().String()[1:])
return nil
},
rename: func(string, string) error {
*events = append(*events, "rename")
return nil
},
remove: func(string) error {
*events = append(*events, "remove")
return nil
},
syncDirectory: func(string) error {
*events = append(*events, "sync-directory")
return nil
},
}
}
func containsString(values []string, target string) bool {
for _, value := range values {
if value == target {
return true
}
}
return false
}
func equalStrings(got, want []string) bool {
if len(got) != len(want) {
return false
}
for i := range got {
if got[i] != want[i] {
return false
}
}
return true
}
func assertNoMatchingTempFiles(t *testing.T, dir, prefix string) {
t.Helper()
entries, err := os.ReadDir(dir)