Restore files at 0600 and make the blob hash check unskippable (closes #163)

Regular files are now created with O_EXCL at mode 0600 and given their stored mode only after the content is written and closed, so a file whose stored mode is restrictive is never briefly readable by other local users mid-restore. A file whose write or close fails is removed rather than left partial, and a chmod failure is a user-visible warning instead of a debug line.

hashVerifyReader.Close now errors when closed before EOF, so a short read or early close can never obtain a blob whose hash was not verified; downloadBlobToCache drops the cache entry on any such failure.

verifyFile (--verify) now rejects a restored file with bytes past its last chunk. Tests cover each behaviour under umask 022.

Model: opus-4-8

internal/vaultik/restore.go

@@ -37,6 +37,8 @@ var (
 	errShortChunkRead           = errors.New("short read")
 	errRestorePathEscapesTarget = errors.New(
 		"refusing to restore path outside the target directory")
+	errTrailingRestoreData = errors.New(
+		"restored file has trailing data after its last chunk")
 )
 
 // restoreDirMode is the permission mode for directories created while
@@ -44,6 +46,13 @@ var (
 // directories themselves get their stored mode).
 const restoreDirMode = 0o755
 
+// restoreFileMode is the restrictive mode a regular file is created with
+// during restore. Content is written while the file holds this mode; the
+// stored mode is applied only after the file is fully written and closed,
+// so a file whose stored mode is restrictive is never briefly readable by
+// other local users while its content is being written.
+const restoreFileMode = 0o600
+
 // sweepIntervalDivisor sets the sweeper threshold to one N-th of the
 // configured blob size limit.
 const sweepIntervalDivisor = 100
@@ -889,6 +898,13 @@ func (s *restoreSession) restoreDirectory(
 		return fmt.Errorf("creating directory: %w", err)
 	}
 
+	// MkdirAll applies the process umask, so chmod to the exact stored
+	// mode. A failure here is non-fatal.
+	err = s.v.Fs.Chmod(targetPath, os.FileMode(file.Mode))
+	if err != nil {
+		log.Debug("Failed to set permissions", "path", targetPath, "error", err)
+	}
+
 	s.applyFileMetadata(file, targetPath)
 
 	s.result.FilesRestored++
@@ -896,25 +912,22 @@ func (s *restoreSession) restoreDirectory(
 	return nil
 }
 
-// applyFileMetadata applies stored permissions, ownership (when running
-// as root on a real filesystem), and mtime to a restored path. Failures
-// are logged at debug level and do not abort the restore.
+// applyFileMetadata applies ownership (when running as root on a real
+// filesystem) and mtime to a restored path. Permission mode is applied
+// separately by each caller, with different failure handling, so it is
+// not touched here. Failures are logged at debug level and do not abort
+// the restore.
 func (s *restoreSession) applyFileMetadata(file *database.File, targetPath string) {
-	err := s.v.Fs.Chmod(targetPath, os.FileMode(file.Mode))
-	if err != nil {
-		log.Debug("Failed to set permissions", "path", targetPath, "error", err)
-	}
-
 	if s.runningAsRoot {
 		if _, ok := s.v.Fs.(*afero.OsFs); ok {
-			err = os.Chown(targetPath, int(file.UID), int(file.GID))
+			err := os.Chown(targetPath, int(file.UID), int(file.GID))
 			if err != nil {
 				log.Debug("Failed to set ownership", "path", targetPath, "error", err)
 			}
 		}
 	}
 
-	err = s.v.Fs.Chtimes(targetPath, file.MTime, file.MTime)
+	err := s.v.Fs.Chtimes(targetPath, file.MTime, file.MTime)
 	if err != nil {
 		log.Debug("Failed to set mtime", "path", targetPath, "error", err)
 	}
@@ -948,17 +961,30 @@ func (s *restoreSession) restoreRegularFile(
 
 	t0 = time.Now()
 
-	outFile, err := s.v.Fs.Create(targetPath)
+	// Remove any existing entry, then create the file with a restrictive
+	// mode via O_EXCL. The stored mode is applied only after the content
+	// is written and the file closed, so a file whose stored mode is
+	// restrictive is never briefly readable by other local users while
+	// its content is written. Removing first (rather than failing on a
+	// leftover file) matches the documented behaviour that re-running
+	// restore overwrites partial output.
+	_ = s.v.Fs.Remove(targetPath)
+
+	outFile, err := s.v.Fs.OpenFile(
+		targetPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, restoreFileMode)
 	createDur := time.Since(t0)
 
 	if err != nil {
 		return fmt.Errorf("creating output file: %w", err)
 	}
 
-	defer func() { _ = outFile.Close() }()
-
 	bytesWritten, timings, err := s.writeFileChunks(outFile, fileChunks)
 	if err != nil {
+		// Do not leave a partial file behind.
+		_ = outFile.Close()
+
+		s.removePartialRestore(targetPath)
+
 		return err
 	}
 
@@ -976,9 +1002,12 @@ func (s *restoreSession) restoreRegularFile(
 
 	err = outFile.Close()
 	if err != nil {
+		s.removePartialRestore(targetPath)
+
 		return fmt.Errorf("closing output file: %w", err)
 	}
 
+	s.applyRestoredFileMode(file, targetPath)
 	s.applyFileMetadata(file, targetPath)
 
 	s.result.FilesRestored++
@@ -989,6 +1018,31 @@ func (s *restoreSession) restoreRegularFile(
 	return nil
 }
 
+// applyRestoredFileMode applies the stored permission bits to a
+// just-written regular file (created with restoreFileMode). A failure is
+// a user-visible warning, not a fatal error: the file's content is
+// intact and it remains at the restrictive create-time mode, so the
+// restore is not aborted or discarded over it.
+func (s *restoreSession) applyRestoredFileMode(
+	file *database.File, targetPath string,
+) {
+	err := s.v.Fs.Chmod(targetPath, os.FileMode(file.Mode))
+	if err != nil {
+		s.v.UI.Warningf("Failed to set mode %s on %s: %v",
+			os.FileMode(file.Mode).Perm(), s.v.UI.Path(targetPath), err)
+	}
+}
+
+// removePartialRestore deletes a restore output file whose write did not
+// complete, so a failed restore never leaves a partial file behind.
+func (s *restoreSession) removePartialRestore(targetPath string) {
+	err := s.v.Fs.Remove(targetPath)
+	if err != nil {
+		log.Debug("Failed to remove partial restore file",
+			"path", targetPath, "error", err)
+	}
+}
+
 // writeFileChunks streams each of the file's chunks from the blob disk
 // cache into outFile, crediting restored bytes to the sweeper as it
 // goes. Returns the bytes written plus per-phase timing accumulators.
@@ -1070,11 +1124,19 @@ func (s *restoreSession) downloadBlobToCache(
 	streamDur := time.Since(t0)
 	closeErr := rc.Close()
 
+	// closeErr carries the blob's hash-verification result (a mismatch,
+	// or the stream not being fully read). On any failure, drop the
+	// cache entry so a blob that failed verification is never read back
+	// as if it were valid.
 	if copyErr != nil {
+		s.blobCache.Delete(blobHash)
+
 		return copyErr
 	}
 
 	if closeErr != nil {
+		s.blobCache.Delete(blobHash)
+
 		return closeErr
 	}
 
@@ -1242,6 +1304,17 @@ func (v *Vaultik) verifyFile(
 		bytesVerified += int64(n)
 	}
 
+	// The stored chunks account for the whole file, so the reader must
+	// be at EOF now. Trailing bytes past the last chunk are corruption
+	// the per-chunk loop cannot see.
+	extra := make([]byte, 1)
+
+	n, err := f.Read(extra)
+	if n != 0 || !errors.Is(err, io.EOF) {
+		return bytesVerified, fmt.Errorf("%w: file longer than its %d chunk(s)",
+			errTrailingRestoreData, len(fileChunks))
+	}
+
 	log.Debug("File verified",
 		"path", file.Path, "bytes", bytesVerified, "chunks", len(fileChunks))