Preserve staging I/O failure causes (T-615)

core/installer/durability.go

@@ -39,14 +39,14 @@ func effectiveDurability(fence durabilityFence) durabilityFence {
 
 func syncFileWithFence(fence durabilityFence, file *os.File, description string) error {
 	if err := effectiveDurability(fence).syncFile(file); err != nil {
-		return fmt.Errorf("%w: sync %s: %v", ErrDurability, description, err)
+		return fmt.Errorf("%w: sync %s: %w", ErrDurability, description, err)
 	}
 	return nil
 }
 
 func syncDirectoryWithFence(fence durabilityFence, path, description string) error {
 	if err := effectiveDurability(fence).syncDirectory(path); err != nil {
-		return fmt.Errorf("%w: sync %s: %v", ErrDurability, description, err)
+		return fmt.Errorf("%w: sync %s: %w", ErrDurability, description, err)
 	}
 	return nil
 }

core/installer/extractor.go

@@ -33,12 +33,15 @@ var (
 	ErrAppManifestMissing       = errors.New("package app.json is missing")
 	ErrDestinationExists        = errors.New("staging destination already exists")
 	ErrArchiveCorrupt           = errors.New("ZIP archive data is corrupt")
+	ErrStagingOutput            = errors.New("staging output failed")
+	ErrStagingCleanup           = errors.New("staging cleanup failed")
 )
 
 // Extractor writes only payload/ contents from a pre-verified package ZIP.
 type Extractor struct {
 	limits     Limits
 	durability durabilityFence
+	files      stagingFileOperations
 }
 
 type ExtractResult struct {
@@ -87,7 +90,11 @@ func NewExtractor(limits Limits) (Extractor, error) {
 	if err := limits.validate(); err != nil {
 		return Extractor{}, err
 	}
-	return Extractor{limits: limits, durability: defaultDurability()}, nil
+	return Extractor{
+		limits:     limits,
+		durability: defaultDurability(),
+		files:      defaultStagingFileOperations(),
+	}, nil
 }
 
 // ExtractFile requires expectedPackageSize from the verified Catalog package.
@@ -136,6 +143,7 @@ func (extractor Extractor) extractPlan(
 	plan []plannedEntry,
 ) (result ExtractResult, err error) {
 	fence := effectiveDurability(extractor.durability)
+	files := effectiveStagingFileOperations(extractor.files)
 	destinationRoot, entrypointPath, err := planOutputPaths(
 		destination,
 		entrypoint,
@@ -145,19 +153,21 @@ func (extractor Extractor) extractPlan(
 		return ExtractResult{}, err
 	}
 
-	if err := os.MkdirAll(filepath.Dir(destinationRoot), 0o700); err != nil {
-		return ExtractResult{}, fmt.Errorf("create staging parent: %w", err)
+	if err := files.mkdirAll(filepath.Dir(destinationRoot), 0o700); err != nil {
+		return ExtractResult{}, stagingOutputError("create staging parent", err)
 	}
-	if err := os.Mkdir(destinationRoot, 0o700); err != nil {
+	if err := files.mkdir(destinationRoot, 0o700); err != nil {
 		if os.IsExist(err) {
 			return ExtractResult{}, ErrDestinationExists
 		}
-		return ExtractResult{}, fmt.Errorf("create staging destination: %w", err)
+		return ExtractResult{}, stagingOutputError("create staging destination", err)
 	}
 	complete := false
 	defer func() {
 		if !complete {
-			_ = os.RemoveAll(destinationRoot)
+			if cleanupErr := files.removeAll(destinationRoot); cleanupErr != nil {
+				err = errors.Join(err, stagingCleanupError(cleanupErr))
+			}
 		}
 	}()
 
@@ -167,31 +177,37 @@ func (extractor Extractor) extractPlan(
 			if entry.outputPath == "" {
 				continue
 			}
-			if err := os.MkdirAll(entry.targetPath, 0o700); err != nil {
-				return ExtractResult{}, fmt.Errorf("create staging directory: %w", err)
+			if err := files.mkdirAll(entry.targetPath, 0o700); err != nil {
+				return ExtractResult{}, stagingOutputError("create staging directory", err)
 			}
 			continue
 		}
-		if err := os.MkdirAll(filepath.Dir(entry.targetPath), 0o700); err != nil {
-			return ExtractResult{}, fmt.Errorf("create staging file parent: %w", err)
+		if err := files.mkdirAll(filepath.Dir(entry.targetPath), 0o700); err != nil {
+			return ExtractResult{}, stagingOutputError("create staging file parent", err)
 		}
 
 		source, err := entry.file.Open()
 		if err != nil {
-			return ExtractResult{}, fmt.Errorf("%w: open %s: %v", ErrArchiveCorrupt, entry.archivePath, err)
+			return ExtractResult{}, archiveInputError("open", entry.archivePath, err)
 		}
 		mode := os.FileMode(0o600)
 		if entry.file.Mode().Perm()&0o111 != 0 {
 			mode = 0o700
 		}
-		output, err := os.OpenFile(
+		output, err := files.openFile(
 			entry.targetPath,
 			os.O_CREATE|os.O_EXCL|os.O_WRONLY,
 			mode,
 		)
 		if err != nil {
-			source.Close()
-			return ExtractResult{}, fmt.Errorf("create staging file: %w", err)
+			outputErr := stagingOutputError("create staging file", err)
+			if closeErr := source.Close(); closeErr != nil {
+				return ExtractResult{}, errors.Join(
+					outputErr,
+					archiveInputError("close", entry.archivePath, closeErr),
+				)
+			}
+			return ExtractResult{}, outputErr
 		}
 
 		remaining := extractor.limits.MaxUncompressedBytes - written
@@ -200,39 +216,45 @@ func (extractor Extractor) extractPlan(
 			readLimit++
 		}
 		digest := sha256.New()
+		writer := stagingWriter{writer: output}
 		copied, copyErr := io.Copy(
-			io.MultiWriter(output, digest),
+			io.MultiWriter(&writer, digest),
 			io.LimitReader(source, readLimit),
 		)
 		closeSourceErr := source.Close()
 		if copyErr != nil {
-			_ = output.Close()
-			return ExtractResult{}, fmt.Errorf("%w: read %s: %v", ErrArchiveCorrupt, entry.archivePath, copyErr)
+			primary := archiveInputError("read", entry.archivePath, copyErr)
+			if writer.err != nil {
+				primary = stagingOutputError("write staging file", writer.err)
+			}
+			return ExtractResult{}, joinStagingCloseError(primary, output)
 		}
 		if closeSourceErr != nil {
-			_ = output.Close()
-			return ExtractResult{}, fmt.Errorf("%w: close %s: %v", ErrArchiveCorrupt, entry.archivePath, closeSourceErr)
+			return ExtractResult{}, joinStagingCloseError(
+				archiveInputError("close", entry.archivePath, closeSourceErr),
+				output,
+			)
 		}
 		if copied > remaining {
-			_ = output.Close()
-			return ExtractResult{}, ErrExpandedTooLarge
+			return ExtractResult{}, joinStagingCloseError(ErrExpandedTooLarge, output)
 		}
 		if uint64(copied) != entry.file.UncompressedSize64 {
-			_ = output.Close()
-			return ExtractResult{}, fmt.Errorf(
+			return ExtractResult{}, joinStagingCloseError(fmt.Errorf(
 				"%w: %s expanded to %d bytes, header declares %d",
 				ErrArchiveCorrupt,
 				entry.archivePath,
 				copied,
 				entry.file.UncompressedSize64,
+			), output)
+		}
+		if err := syncFileWithFence(fence, output.osFile(), "staging payload"); err != nil {
+			return ExtractResult{}, joinStagingCloseError(
+				stagingOutputError("sync staging file", err),
+				output,
 			)
 		}
-		if err := syncFileWithFence(fence, output, "staging payload"); err != nil {
-			_ = output.Close()
-			return ExtractResult{}, err
-		}
 		if err := output.Close(); err != nil {
-			return ExtractResult{}, fmt.Errorf("close staging file: %w", err)
+			return ExtractResult{}, stagingOutputError("close staging file", err)
 		}
 		written += copied
 		result.Files++
@@ -243,7 +265,7 @@ func (extractor Extractor) extractPlan(
 		})
 	}
 	if err := syncStagingTree(fence, destinationRoot); err != nil {
-		return ExtractResult{}, err
+		return ExtractResult{}, stagingOutputError("sync staging tree", err)
 	}
 
 	result.Bytes = written
@@ -252,6 +274,40 @@ func (extractor Extractor) extractPlan(
 	return result, nil
 }
 
+type stagingWriter struct {
+	writer io.Writer
+	err    error
+}
+
+func (writer *stagingWriter) Write(data []byte) (int, error) {
+	written, err := writer.writer.Write(data)
+	if err != nil {
+		writer.err = err
+	} else if written != len(data) {
+		writer.err = io.ErrShortWrite
+	}
+	return written, err
+}
+
+func archiveInputError(operation, path string, cause error) error {
+	return fmt.Errorf("%w: %s %s: %w", ErrArchiveCorrupt, operation, path, cause)
+}
+
+func stagingOutputError(operation string, cause error) error {
+	return fmt.Errorf("%w: %s: %w", ErrStagingOutput, operation, cause)
+}
+
+func stagingCleanupError(cause error) error {
+	return fmt.Errorf("%w: remove staging: %w", ErrStagingCleanup, cause)
+}
+
+func joinStagingCloseError(primary error, output stagingOutputFile) error {
+	if closeErr := output.Close(); closeErr != nil {
+		return errors.Join(primary, stagingOutputError("close staging file", closeErr))
+	}
+	return primary
+}
+
 func planOutputPaths(
 	destination string,
 	entrypoint string,

core/installer/extractor_io_test.go

@@ -0,0 +1,238 @@
+package installer
+
+import (
+	"archive/zip"
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestExtractorKeepsStagingOutputFailuresDistinctFromZIPInput(t *testing.T) {
+	archivePath := writeTestZIP(t, []testZIPEntry{
+		{name: "app.json", body: []byte(`{}`)},
+		{name: "payload/App.exe", body: []byte("executable")},
+	})
+	errDiskFull := errors.New("injected disk full")
+	errOutput := errors.New("injected output failure")
+
+	tests := []struct {
+		name      string
+		failure   error
+		configure func(*Extractor, error)
+	}{
+		{
+			name:    "write disk full",
+			failure: errDiskFull,
+			configure: func(extractor *Extractor, failure error) {
+				extractor.files = outputFailureOperations{
+					stagingFileOperations: defaultStagingFileOperations(),
+					writeErr:              failure,
+				}
+			},
+		},
+		{
+			name:    "sync disk full",
+			failure: errDiskFull,
+			configure: func(extractor *Extractor, failure error) {
+				extractor.durability = fileSyncFailureFence{err: failure}
+			},
+		},
+		{
+			name:    "close disk full",
+			failure: errDiskFull,
+			configure: func(extractor *Extractor, failure error) {
+				extractor.files = outputFailureOperations{
+					stagingFileOperations: defaultStagingFileOperations(),
+					closeErr:              failure,
+				}
+			},
+		},
+		{
+			name:    "generic output I O",
+			failure: errOutput,
+			configure: func(extractor *Extractor, failure error) {
+				extractor.files = outputFailureOperations{
+					stagingFileOperations: defaultStagingFileOperations(),
+					writeErr:              failure,
+				}
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			extractor := mustExtractor(t, testLimits())
+			test.configure(&extractor, test.failure)
+			destination := filepath.Join(t.TempDir(), "staging")
+
+			_, err := extractor.ExtractFile(
+				archivePath,
+				destination,
+				"App.exe",
+				archiveSize(t, archivePath),
+			)
+			if !errors.Is(err, ErrStagingOutput) {
+				t.Fatalf("ExtractFile() error = %v, want %v", err, ErrStagingOutput)
+			}
+			if errors.Is(err, ErrArchiveCorrupt) {
+				t.Fatalf("ExtractFile() error = %v, must not be ZIP corruption", err)
+			}
+			if !errors.Is(err, test.failure) {
+				t.Fatalf("ExtractFile() error = %v, want preserved %v", err, test.failure)
+			}
+			if _, statErr := os.Stat(destination); !os.IsNotExist(statErr) {
+				t.Fatalf("output failure left staging, stat error = %v", statErr)
+			}
+		})
+	}
+}
+
+func TestExtractorReportsCleanupFailureAndRecoveryRemovesOnlyManagedStaging(t *testing.T) {
+	archivePath := writeTestZIP(t, []testZIPEntry{
+		{name: "app.json", body: []byte(`{}`)},
+		{name: "payload/App.exe", body: []byte("verified bytes"), method: zip.Store},
+	})
+	corruptZIPEntryData(t, archivePath, "payload/App.exe")
+
+	root := t.TempDir()
+	currentPath := filepath.Join(root, "current", "App.exe")
+	if err := os.MkdirAll(filepath.Dir(currentPath), 0o700); err != nil {
+		t.Fatalf("create current: %v", err)
+	}
+	if err := os.WriteFile(currentPath, []byte("old executable"), 0o600); err != nil {
+		t.Fatalf("write current: %v", err)
+	}
+	recordPath := filepath.Join(root, "installed-app.json")
+	oldRecord := []byte(`{"version":"1.0.0"}`)
+	if err := os.WriteFile(recordPath, oldRecord, 0o600); err != nil {
+		t.Fatalf("write record: %v", err)
+	}
+	for _, preserved := range []string{"data", "licenses"} {
+		path := filepath.Join(root, preserved, "keep.txt")
+		if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
+			t.Fatalf("create %s: %v", preserved, err)
+		}
+		if err := os.WriteFile(path, []byte(preserved), 0o600); err != nil {
+			t.Fatalf("write %s: %v", preserved, err)
+		}
+	}
+
+	errCleanup := errors.New("injected cleanup failure")
+	extractor := mustExtractor(t, testLimits())
+	extractor.files = outputFailureOperations{
+		stagingFileOperations: defaultStagingFileOperations(),
+		cleanupErr:            errCleanup,
+	}
+	destination := filepath.Join(root, "staging")
+	_, err := extractor.ExtractFile(
+		archivePath,
+		destination,
+		"App.exe",
+		archiveSize(t, archivePath),
+	)
+	if !errors.Is(err, ErrArchiveCorrupt) || !errors.Is(err, ErrStagingCleanup) || !errors.Is(err, errCleanup) {
+		t.Fatalf("ExtractFile() error = %v, want ZIP and cleanup causes", err)
+	}
+	if _, statErr := os.Stat(destination); statErr != nil {
+		t.Fatalf("cleanup failure removed staging, stat error = %v", statErr)
+	}
+
+	result, err := Recover(root)
+	if err != nil {
+		t.Fatalf("Recover() error = %v", err)
+	}
+	if result.Action != RecoveryAborted {
+		t.Fatalf("recovery action = %q, want %q", result.Action, RecoveryAborted)
+	}
+	if got := mustReadTestFile(t, currentPath); got != "old executable" {
+		t.Fatalf("current after recovery = %q", got)
+	}
+	if record, readErr := os.ReadFile(recordPath); readErr != nil || string(record) != string(oldRecord) {
+		t.Fatalf("record after recovery = %q error = %v", record, readErr)
+	}
+	for _, preserved := range []string{"data", "licenses"} {
+		if got := mustReadTestFile(t, filepath.Join(root, preserved, "keep.txt")); got != preserved {
+			t.Fatalf("%s after recovery = %q", preserved, got)
+		}
+	}
+	if _, statErr := os.Stat(destination); !os.IsNotExist(statErr) {
+		t.Fatalf("Recover() left staging, stat error = %v", statErr)
+	}
+}
+
+type outputFailureOperations struct {
+	stagingFileOperations
+	writeErr   error
+	closeErr   error
+	cleanupErr error
+}
+
+func (operations outputFailureOperations) openFile(
+	name string,
+	flag int,
+	perm os.FileMode,
+) (stagingOutputFile, error) {
+	file, err := operations.stagingFileOperations.openFile(name, flag, perm)
+	if err != nil {
+		return nil, err
+	}
+	return &outputFailureFile{
+		stagingOutputFile: file,
+		writeErr:          operations.writeErr,
+		closeErr:          operations.closeErr,
+	}, nil
+}
+
+func (operations outputFailureOperations) removeAll(path string) error {
+	if operations.cleanupErr != nil {
+		return operations.cleanupErr
+	}
+	return operations.stagingFileOperations.removeAll(path)
+}
+
+type outputFailureFile struct {
+	stagingOutputFile
+	writeErr error
+	closeErr error
+}
+
+func (file *outputFailureFile) Write(data []byte) (int, error) {
+	if file.writeErr != nil {
+		return 0, file.writeErr
+	}
+	return file.stagingOutputFile.Write(data)
+}
+
+func (file *outputFailureFile) Close() error {
+	closeErr := file.stagingOutputFile.Close()
+	if file.closeErr != nil {
+		return file.closeErr
+	}
+	return closeErr
+}
+
+func (file *outputFailureFile) osFile() *os.File {
+	return file.stagingOutputFile.osFile()
+}
+
+type fileSyncFailureFence struct {
+	err error
+}
+
+func (fence fileSyncFailureFence) syncFile(*os.File) error {
+	return fence.err
+}
+
+func (fence fileSyncFailureFence) syncDirectory(string) error {
+	return nil
+}
+
+func mustReadTestFile(t *testing.T, path string) string {
+	t.Helper()
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("read %s: %v", path, err)
+	}
+	return string(data)
+}

core/installer/extractor_test.go

@@ -401,6 +401,9 @@ func TestExtractorRemovesDestinationAfterCopyFailure(t *testing.T) {
 	if !errors.Is(err, ErrArchiveCorrupt) {
 		t.Fatalf("ExtractFile() error = %v, want %v", err, ErrArchiveCorrupt)
 	}
+	if errors.Is(err, ErrStagingOutput) {
+		t.Fatalf("ExtractFile() error = %v, must not be a staging output failure", err)
+	}
 	if _, statErr := os.Stat(destination); !os.IsNotExist(statErr) {
 		t.Fatalf("copy failure left staging, stat error = %v", statErr)
 	}

core/installer/recovery.go

@@ -47,6 +47,12 @@ func recoverWithFence(root string, fence durabilityFence) (RecoveryResult, error
 				ErrRecoveryInconsistent,
 			)
 		}
+		if state.staging {
+			if err := removeManagedDirectoryWithFence(layout, layout.staging, fence); err != nil {
+				return RecoveryResult{}, err
+			}
+			return RecoveryResult{Action: RecoveryAborted}, nil
+		}
 		return RecoveryResult{Action: RecoveryNone}, nil
 	}
 

core/installer/staging_output.go

@@ -0,0 +1,72 @@
+package installer
+
+import "os"
+
+// stagingOutputFile is the narrow output seam used by extraction. Keeping it
+// internal lets tests inject write and close failures without exposing a file
+// abstraction to package consumers.
+type stagingOutputFile interface {
+	Write([]byte) (int, error)
+	Close() error
+	osFile() *os.File
+}
+
+type stagingFileOperations interface {
+	mkdirAll(path string, perm os.FileMode) error
+	mkdir(path string, perm os.FileMode) error
+	openFile(name string, flag int, perm os.FileMode) (stagingOutputFile, error)
+	removeAll(path string) error
+}
+
+type filesystemStagingFileOperations struct{}
+
+type filesystemStagingOutputFile struct {
+	file *os.File
+}
+
+func (operations filesystemStagingFileOperations) mkdirAll(path string, perm os.FileMode) error {
+	return os.MkdirAll(path, perm)
+}
+
+func (operations filesystemStagingFileOperations) mkdir(path string, perm os.FileMode) error {
+	return os.Mkdir(path, perm)
+}
+
+func (operations filesystemStagingFileOperations) openFile(
+	name string,
+	flag int,
+	perm os.FileMode,
+) (stagingOutputFile, error) {
+	file, err := os.OpenFile(name, flag, perm)
+	if err != nil {
+		return nil, err
+	}
+	return filesystemStagingOutputFile{file: file}, nil
+}
+
+func (operations filesystemStagingFileOperations) removeAll(path string) error {
+	return os.RemoveAll(path)
+}
+
+func (file filesystemStagingOutputFile) Write(data []byte) (int, error) {
+	return file.file.Write(data)
+}
+
+func (file filesystemStagingOutputFile) Close() error {
+	return file.file.Close()
+}
+
+func (file filesystemStagingOutputFile) osFile() *os.File {
+	return file.file
+}
+
+func defaultStagingFileOperations() stagingFileOperations {
+	return filesystemStagingFileOperations{}
+}
+
+func effectiveStagingFileOperations(operations stagingFileOperations) stagingFileOperations {
+	if operations == nil {
+		return defaultStagingFileOperations()
+	}
+	return operations
+}