fix(cli): harden restore preflight validation

tools/tht/internal/backup/preflight.go

@@ -6,6 +6,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/sha256"
+	"encoding/binary"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
@@ -29,8 +30,25 @@ type PreflightRequest struct {
 	Archive              string
 	Confirm              bool
 	AllowExternalSecrets bool
+	Limits               PreflightLimits
 }
 
+// PreflightLimits bounds hostile archive processing. Zero values select the conservative
+// defaults; callers that need larger installation backups must opt in explicitly.
+type PreflightLimits struct {
+	MaxMembers           int
+	MaxArchiveBytes      uint64
+	MaxUncompressedBytes uint64
+	MaxCompressionRatio  uint64
+}
+
+const (
+	defaultPreflightMaxMembers           = 10_000
+	defaultPreflightMaxArchiveBytes      = 32 << 30
+	defaultPreflightMaxUncompressedBytes = 128 << 30
+	defaultPreflightMaxCompressionRatio  = 100
+)
+
 // PreflightDependencies supplies checks that require knowledge of the current Docker targets.
 // Part B can bind these callbacks to read-only Docker Compose and filesystem inspections before
 // it starts its restore transaction.
@@ -61,6 +79,14 @@ type PreflightResult struct {
 	RequiredBytes uint64
 	Manifest      Manifest
 	Entries       []ArchiveEntryMetadata
+	archive       *verifiedArchive
+}
+
+type verifiedArchive struct {
+	file   *os.File
+	info   os.FileInfo
+	digest string
+	limits PreflightLimits
 }
 
 type inspectedArchiveEntry struct {
@@ -78,21 +104,51 @@ func Preflight(ctx context.Context, installation config.Installation, request Pr
 	if err := contextError(ctx); err != nil {
 		return PreflightResult{}, err
 	}
+	limits, err := normalizePreflightLimits(request.Limits)
+	if err != nil {
+		return PreflightResult{}, err
+	}
 	archivePath, archiveInfo, err := openableArchivePath(request.Archive)
 	if err != nil {
 		return PreflightResult{}, err
 	}
+	if uint64(archiveInfo.Size()) > limits.MaxArchiveBytes {
+		return PreflightResult{}, errors.New("backup archive exceeds the configured archive-size limit")
+	}
 	archiveFile, err := os.Open(archivePath)
 	if err != nil {
 		return PreflightResult{}, fmt.Errorf("open backup archive: %w", err)
 	}
-	defer archiveFile.Close()
+	keepArchiveOpen := false
-	reader, err := zip.NewReader(archiveFile, archiveInfo.Size())
+	defer func() {
+		if !keepArchiveOpen {
+			_ = archiveFile.Close()
+		}
+	}()
+	openedInfo, err := archiveFile.Stat()
+	if err != nil {
+		return PreflightResult{}, fmt.Errorf("inspect opened backup archive: %w", err)
+	}
+	if !openedInfo.Mode().IsRegular() || openedInfo.Size() != archiveInfo.Size() {
+		return PreflightResult{}, errors.New("backup archive changed while opening")
+	}
+	memberCount, err := zipMemberCount(archiveFile, openedInfo.Size())
+	if err != nil {
+		return PreflightResult{}, err
+	}
+	if memberCount > uint64(limits.MaxMembers) {
+		return PreflightResult{}, errors.New("backup archive exceeds the configured member limit")
+	}
+	initialDigest, err := digestArchive(ctx, archiveFile, limits.MaxArchiveBytes)
+	if err != nil {
+		return PreflightResult{}, err
+	}
+	reader, err := zip.NewReader(archiveFile, openedInfo.Size())
 	if err != nil {
 		return PreflightResult{}, fmt.Errorf("read backup archive: %w", err)
 	}
 
-	manifestBytes, entries, err := inspectArchiveMembers(ctx, reader)
+	manifestBytes, entries, err := inspectArchiveMembers(ctx, reader, limits)
 	if err != nil {
 		return PreflightResult{}, err
 	}
@@ -137,12 +193,74 @@ func Preflight(ctx context.Context, installation config.Installation, request Pr
 	if err := dependencies.CheckImageConfigCompatibility(ctx, installation, manifest); err != nil {
 		return PreflightResult{}, fmt.Errorf("validate image/config compatibility: %w", err)
 	}
+	finalDigest, err := digestArchive(ctx, archiveFile, limits.MaxArchiveBytes)
+	if err != nil {
+		return PreflightResult{}, err
+	}
+	if initialDigest != finalDigest {
+		return PreflightResult{}, errors.New("backup archive changed during preflight")
+	}
+	keepArchiveOpen = true
 	return PreflightResult{
-		ArchivePath: archivePath, ArchiveSize: archiveInfo.Size(), RequiredBytes: requiredBytes,
+		ArchivePath: archivePath, ArchiveSize: openedInfo.Size(), RequiredBytes: requiredBytes,
 		Manifest: manifest, Entries: metadata,
+		archive: &verifiedArchive{file: archiveFile, info: openedInfo, digest: finalDigest, limits: limits},
 	}, nil
 }
 
+// RevalidateArchive binds a restore to the bytes inspected by Preflight. A caller must invoke
+// it immediately before a restore transaction and use the returned retained handle, never reopen
+// ArchivePath. It refuses a path replacement or in-place content change.
+func (result PreflightResult) RevalidateArchive() (*os.File, error) {
+	if result.archive == nil || result.archive.file == nil {
+		return nil, errors.New("backup archive has not been retained by preflight")
+	}
+	pathInfo, err := os.Lstat(result.ArchivePath)
+	if err != nil {
+		return nil, errors.New("backup archive changed after preflight")
+	}
+	if !pathInfo.Mode().IsRegular() || !os.SameFile(result.archive.info, pathInfo) {
+		return nil, errors.New("backup archive changed after preflight")
+	}
+	heldInfo, err := result.archive.file.Stat()
+	if err != nil || !os.SameFile(result.archive.info, heldInfo) || heldInfo.Size() != result.ArchiveSize {
+		return nil, errors.New("backup archive changed after preflight")
+	}
+	digest, err := digestArchive(context.Background(), result.archive.file, result.archive.limits.MaxArchiveBytes)
+	if err != nil || digest != result.archive.digest {
+		return nil, errors.New("backup archive changed after preflight")
+	}
+	return result.archive.file, nil
+}
+
+// CloseArchive releases the retained read-only archive handle after the caller finishes the
+// restore transaction or decides not to proceed.
+func (result PreflightResult) CloseArchive() error {
+	if result.archive == nil || result.archive.file == nil {
+		return nil
+	}
+	return result.archive.file.Close()
+}
+
+func normalizePreflightLimits(requested PreflightLimits) (PreflightLimits, error) {
+	if requested.MaxMembers < 0 {
+		return PreflightLimits{}, errors.New("backup preflight member limit must be positive")
+	}
+	if requested.MaxMembers == 0 {
+		requested.MaxMembers = defaultPreflightMaxMembers
+	}
+	if requested.MaxArchiveBytes == 0 {
+		requested.MaxArchiveBytes = defaultPreflightMaxArchiveBytes
+	}
+	if requested.MaxUncompressedBytes == 0 {
+		requested.MaxUncompressedBytes = defaultPreflightMaxUncompressedBytes
+	}
+	if requested.MaxCompressionRatio == 0 {
+		requested.MaxCompressionRatio = defaultPreflightMaxCompressionRatio
+	}
+	return requested, nil
+}
+
 func validateRawManifestPaths(value []byte) error {
 	var raw struct {
 		Entries []struct {
@@ -180,13 +298,20 @@ func openableArchivePath(requested string) (string, os.FileInfo, error) {
 	return archivePath, info, nil
 }
 
-func inspectArchiveMembers(ctx context.Context, reader *zip.Reader) ([]byte, map[string]inspectedArchiveEntry, error) {
+func inspectArchiveMembers(ctx context.Context, reader *zip.Reader, limits PreflightLimits) ([]byte, map[string]inspectedArchiveEntry, error) {
+	if len(reader.File) > limits.MaxMembers {
+		return nil, nil, errors.New("backup archive exceeds the configured member limit")
+	}
 	var manifestBytes []byte
 	entries := make(map[string]inspectedArchiveEntry, len(reader.File))
+	var totalUncompressed uint64
 	for _, member := range reader.File {
 		if err := contextError(ctx); err != nil {
 			return nil, nil, err
 		}
+		if err := checkArchiveMemberLimits(member, limits, &totalUncompressed); err != nil {
+			return nil, nil, err
+		}
 		name, err := validateArchiveMemberPath(member.Name)
 		if err != nil {
 			return nil, nil, err
@@ -216,10 +341,13 @@ func inspectArchiveMembers(ctx context.Context, reader *zip.Reader) ([]byte, map
 			if int64(len(manifestBytes)) > maxPreflightManifestBytes {
 				return nil, nil, errors.New("backup manifest is too large")
 			}
+			if uint64(len(manifestBytes)) != member.UncompressedSize64 {
+				return nil, nil, errors.New("backup manifest size does not match its archive metadata")
+			}
 			continue
 		}
 		hashValue := sha256.New()
-		size, copyErr := io.Copy(hashValue, opened)
+		size, copyErr := io.Copy(hashValue, io.LimitReader(opened, int64(member.UncompressedSize64)+1))
 		closeErr := opened.Close()
 		if copyErr != nil {
 			return nil, nil, fmt.Errorf("read backup archive entry: %w", copyErr)
@@ -230,6 +358,9 @@ func inspectArchiveMembers(ctx context.Context, reader *zip.Reader) ([]byte, map
 		if size < 0 {
 			return nil, nil, errors.New("backup archive entry size overflow")
 		}
+		if uint64(size) != member.UncompressedSize64 {
+			return nil, nil, errors.New("backup archive entry size does not match its archive metadata")
+		}
 		entries[name] = inspectedArchiveEntry{
 			metadata: ArchiveEntryMetadata{Path: name, Size: size, SHA256: digestForHash(hashValue), Mode: uint32(member.Mode().Perm())},
 			member:   member,
@@ -241,6 +372,118 @@ func inspectArchiveMembers(ctx context.Context, reader *zip.Reader) ([]byte, map
 	return manifestBytes, entries, nil
 }
 
+func checkArchiveMemberLimits(member *zip.File, limits PreflightLimits, total *uint64) error {
+	if member.UncompressedSize64 > limits.MaxUncompressedBytes || *total > limits.MaxUncompressedBytes-member.UncompressedSize64 {
+		return errors.New("backup archive exceeds the configured uncompressed-size limit")
+	}
+	*total += member.UncompressedSize64
+	if member.CompressedSize64 == 0 {
+		if member.UncompressedSize64 != 0 {
+			return errors.New("backup archive exceeds the configured compression-ratio limit")
+		}
+		return nil
+	}
+	if limits.MaxCompressionRatio <= ^uint64(0)/member.CompressedSize64 &&
+		member.UncompressedSize64 > limits.MaxCompressionRatio*member.CompressedSize64 {
+		return errors.New("backup archive exceeds the configured compression-ratio limit")
+	}
+	return nil
+}
+
+func digestArchive(ctx context.Context, file *os.File, maximum uint64) (string, error) {
+	if _, err := file.Seek(0, io.SeekStart); err != nil {
+		return "", fmt.Errorf("seek backup archive: %w", err)
+	}
+	digest := sha256.New()
+	buffer := make([]byte, 128*1024)
+	var total uint64
+	for {
+		if err := contextError(ctx); err != nil {
+			return "", err
+		}
+		count, err := file.Read(buffer)
+		if count > 0 {
+			if uint64(count) > maximum-total {
+				return "", errors.New("backup archive exceeds the configured archive-size limit")
+			}
+			total += uint64(count)
+			if _, writeErr := digest.Write(buffer[:count]); writeErr != nil {
+				return "", fmt.Errorf("hash backup archive: %w", writeErr)
+			}
+		}
+		if errors.Is(err, io.EOF) {
+			break
+		}
+		if err != nil {
+			return "", fmt.Errorf("read backup archive: %w", err)
+		}
+	}
+	if _, err := file.Seek(0, io.SeekStart); err != nil {
+		return "", fmt.Errorf("rewind backup archive: %w", err)
+	}
+	return digestForHash(digest), nil
+}
+
+func zipMemberCount(file *os.File, size int64) (uint64, error) {
+	const (
+		endOfCentralDirectorySignature = 0x06054b50
+		zip64LocatorSignature          = 0x07064b50
+		zip64EndSignature              = 0x06064b50
+		endOfCentralDirectorySize      = 22
+		zipCommentMaximum              = 1<<16 - 1
+		zip64LocatorSize               = 20
+		zip64EndMinimumSize            = 56
+	)
+	if size < endOfCentralDirectorySize {
+		return 0, errors.New("read backup archive directory: archive is too small")
+	}
+	tailSize := int64(endOfCentralDirectorySize + zipCommentMaximum)
+	if size < tailSize {
+		tailSize = size
+	}
+	tail := make([]byte, tailSize)
+	if _, err := file.ReadAt(tail, size-tailSize); err != nil {
+		return 0, fmt.Errorf("read backup archive directory: %w", err)
+	}
+	for index := len(tail) - endOfCentralDirectorySize; index >= 0; index-- {
+		if binary.LittleEndian.Uint32(tail[index:index+4]) != endOfCentralDirectorySignature {
+			continue
+		}
+		commentLength := int(binary.LittleEndian.Uint16(tail[index+20 : index+22]))
+		if index+endOfCentralDirectorySize+commentLength != len(tail) {
+			continue
+		}
+		count := uint64(binary.LittleEndian.Uint16(tail[index+10 : index+12]))
+		if count != 0xffff {
+			return count, nil
+		}
+		endOffset := size - tailSize + int64(index)
+		if endOffset < zip64LocatorSize {
+			return 0, errors.New("read backup archive directory: ZIP64 locator is missing")
+		}
+		locator := make([]byte, zip64LocatorSize)
+		if _, err := file.ReadAt(locator, endOffset-zip64LocatorSize); err != nil {
+			return 0, fmt.Errorf("read backup archive directory: %w", err)
+		}
+		if binary.LittleEndian.Uint32(locator[:4]) != zip64LocatorSignature {
+			return 0, errors.New("read backup archive directory: ZIP64 locator is invalid")
+		}
+		zip64Offset := binary.LittleEndian.Uint64(locator[8:16])
+		if size < zip64EndMinimumSize || zip64Offset > uint64(size-zip64EndMinimumSize) {
+			return 0, errors.New("read backup archive directory: ZIP64 record is invalid")
+		}
+		zip64End := make([]byte, zip64EndMinimumSize)
+		if _, err := file.ReadAt(zip64End, int64(zip64Offset)); err != nil {
+			return 0, fmt.Errorf("read backup archive directory: %w", err)
+		}
+		if binary.LittleEndian.Uint32(zip64End[:4]) != zip64EndSignature || binary.LittleEndian.Uint64(zip64End[4:12]) < 44 {
+			return 0, errors.New("read backup archive directory: ZIP64 record is invalid")
+		}
+		return binary.LittleEndian.Uint64(zip64End[32:40]), nil
+	}
+	return 0, errors.New("read backup archive directory: end record is missing")
+}
+
 const maxPreflightManifestBytes = 16 << 20
 
 func validateArchiveMemberPath(value string) (string, error) {
@@ -277,6 +520,9 @@ func reconcileArchiveEntries(ctx context.Context, manifest Manifest, entries map
 		if actual.metadata.Size != entry.Size || actual.metadata.SHA256 != entry.SHA256 {
 			return nil, 0, fmt.Errorf("checksum or size mismatch for backup entry %q", entry.Path)
 		}
+		if actual.metadata.Mode != entry.Mode {
+			return nil, 0, fmt.Errorf("mode mismatch for backup entry %q", entry.Path)
+		}
 		if entry.Kind == EntryVolume {
 			if err := validateVolumeTar(ctx, actual.member); err != nil {
 				return nil, 0, fmt.Errorf("validate volume archive %q: %w", entry.Path, err)

tools/tht/internal/backup/preflight_test.go

@@ -233,6 +233,91 @@ func TestPreflightRejectsTraversalAndSymlinkInsideVolumeTar(t *testing.T) {
 	}
 }
 
+func TestPreflightRejectsArchivesThatExceedConfiguredProcessingLimits(t *testing.T) {
+	installation := preflightTestInstallation(t)
+	tests := []struct {
+		name    string
+		spec    preflightArchiveSpec
+		limits  PreflightLimits
+		wantErr string
+	}{
+		{
+			name:    "member count",
+			spec:    preflightArchiveSpec{entries: []preflightArchiveEntry{{path: "one", body: []byte("one")}}},
+			limits:  PreflightLimits{MaxMembers: 1, MaxUncompressedBytes: 1 << 20, MaxCompressionRatio: 100},
+			wantErr: "member limit",
+		},
+		{
+			name:    "uncompressed bytes",
+			spec:    preflightArchiveSpec{entries: []preflightArchiveEntry{{path: "large", body: []byte("123456789")}}},
+			limits:  PreflightLimits{MaxMembers: 10, MaxUncompressedBytes: 1, MaxCompressionRatio: 100},
+			wantErr: "uncompressed-size limit",
+		},
+		{
+			name: "compression ratio",
+			spec: preflightArchiveSpec{entries: []preflightArchiveEntry{{
+				path: "compressed", body: []byte(strings.Repeat("A", 4096)), method: zip.Deflate,
+			}}},
+			limits:  PreflightLimits{MaxMembers: 10, MaxUncompressedBytes: 1 << 20, MaxCompressionRatio: 2},
+			wantErr: "compression-ratio limit",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			archive := filepath.Join(t.TempDir(), "limited.zip")
+			writePreflightArchive(t, archive, test.spec)
+			_, err := Preflight(context.Background(), installation, PreflightRequest{
+				Archive: archive, Confirm: true, Limits: test.limits,
+			}, permissivePreflightDependencies())
+			if err == nil || !strings.Contains(err.Error(), test.wantErr) {
+				t.Fatalf("Preflight() error = %v, want %q", err, test.wantErr)
+			}
+		})
+	}
+}
+
+func TestPreflightRevalidationRefusesAnArchivePathThatWasReplaced(t *testing.T) {
+	installation := preflightTestInstallation(t)
+	archive := filepath.Join(t.TempDir(), "checked.zip")
+	writePreflightArchive(t, archive, preflightArchiveSpec{
+		entries: []preflightArchiveEntry{{path: "configuration/operator.env", body: []byte("validated")}},
+	})
+
+	result, err := Preflight(context.Background(), installation, PreflightRequest{Archive: archive, Confirm: true}, permissivePreflightDependencies())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer result.CloseArchive()
+
+	replacement := filepath.Join(t.TempDir(), "replacement.zip")
+	writePreflightArchive(t, replacement, preflightArchiveSpec{
+		entries: []preflightArchiveEntry{{path: "configuration/operator.env", body: []byte("unchecked")}},
+	})
+	if err := os.Rename(replacement, archive); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := result.RevalidateArchive(); err == nil || !strings.Contains(err.Error(), "changed") {
+		t.Fatalf("RevalidateArchive() error = %v, want replaced path refusal", err)
+	}
+}
+
+func TestPreflightRejectsArchiveEntryWithModeDifferentFromManifest(t *testing.T) {
+	installation := preflightTestInstallation(t)
+	archive := filepath.Join(t.TempDir(), "mode-mismatch.zip")
+	manifestMode := uint32(0o600)
+	writePreflightArchive(t, archive, preflightArchiveSpec{
+		entries: []preflightArchiveEntry{{
+			path: "configuration/operator.env", body: []byte("safe"), mode: 0o644, manifestMode: &manifestMode,
+		}},
+	})
+
+	_, err := Preflight(context.Background(), installation, PreflightRequest{Archive: archive, Confirm: true}, permissivePreflightDependencies())
+	if err == nil || !strings.Contains(err.Error(), "mode mismatch") {
+		t.Fatalf("Preflight() error = %v, want mode mismatch", err)
+	}
+}
+
 type preflightArchiveSpec struct {
 	installationID string
 	schemaVersion  int
@@ -243,12 +328,15 @@ type preflightArchiveSpec struct {
 }
 
 type preflightArchiveEntry struct {
-	path      string
+	path         string
-	body      []byte
+	body         []byte
-	checksum  string
+	checksum     string
-	kind      string
+	kind         string
-	sensitive bool
+	sensitive    bool
-	symlink   bool
+	symlink      bool
+	mode         os.FileMode
+	manifestMode *uint32
+	method       uint16
 }
 
 type preflightRawArchiveEntry struct {
@@ -302,7 +390,14 @@ func writePreflightArchive(t *testing.T, archivePath string, spec preflightArchi
 			sourcePath = "/protected/secret"
 			owner = "external-secret"
 		}
-		manifest.Entries = append(manifest.Entries, Entry{Path: entry.path, Kind: kind, Owner: owner, SourcePath: sourcePath, SHA256: checksum, Size: int64(len(entry.body)), Archived: true, Sensitive: entry.sensitive})
+		mode := uint32(entry.mode.Perm())
+		if mode == 0 {
+			mode = 0o600
+		}
+		if entry.manifestMode != nil {
+			mode = *entry.manifestMode
+		}
+		manifest.Entries = append(manifest.Entries, Entry{Path: entry.path, Kind: kind, Owner: owner, SourcePath: sourcePath, SHA256: checksum, Size: int64(len(entry.body)), Mode: mode, Archived: true, Sensitive: entry.sensitive})
 	}
 	manifestBytes, err := manifest.JSON()
 	if err != nil {
@@ -322,7 +417,16 @@ func writePreflightArchive(t *testing.T, archivePath string, spec preflightArchi
 	defer file.Close()
 	writer := zip.NewWriter(file)
 	for _, entry := range spec.entries {
-		header := &zip.FileHeader{Name: entry.path, Method: zip.Store}
+		method := entry.method
+		if method == 0 {
+			method = zip.Store
+		}
+		header := &zip.FileHeader{Name: entry.path, Method: method}
+		mode := entry.mode
+		if mode == 0 {
+			mode = 0o600
+		}
+		header.SetMode(mode)
 		if entry.symlink {
 			header.SetMode(os.ModeSymlink | 0o777)
 		}
@@ -343,7 +447,9 @@ func writePreflightArchive(t *testing.T, archivePath string, spec preflightArchi
 			t.Fatal(err)
 		}
 	}
-	created, err := writer.Create(ManifestPath)
+	manifestHeader := &zip.FileHeader{Name: ManifestPath, Method: zip.Store}
+	manifestHeader.SetMode(0o600)
+	created, err := writer.CreateHeader(manifestHeader)
 	if err != nil {
 		t.Fatal(err)
 	}