fix workspace output and cleanup uncertainty

tools/thothctl/internal/safeio/files.go

@@ -13,6 +13,15 @@ import (
 
 var ErrUnsafeFile = errors.New("unsafe file")
 
+// ErrIndeterminateFile means publication cleanup could not establish whether a
+// private candidate is still named. Callers must reconcile the destination and
+// private stages before retrying; it is never a blind-retry-safe failure.
+var ErrIndeterminateFile = errors.New("indeterminate file state")
+
+// beforeBoundedRead is an internal test seam used to deterministically suspend
+// a read between opening the file and resolving its final pathname.
+var beforeBoundedRead func()
+
 // ValidateCanonicalPath rejects relative or lexically non-canonical paths before they are opened.
 func ValidateCanonicalPath(path string) error {
 	if !filepath.IsAbs(path) || filepath.Clean(path) != path || strings.Contains(path, string(filepath.Separator)+".."+string(filepath.Separator)) {
@@ -32,6 +41,9 @@ func readBoundedRegularFile(file *os.File, maximum int64) ([]byte, error) {
 	if err != nil || !info.Mode().IsRegular() {
 		return nil, ErrUnsafeFile
 	}
+	if beforeBoundedRead != nil {
+		beforeBoundedRead()
+	}
 	contents, err := io.ReadAll(io.LimitReader(file, maximum+1))
 	if err != nil || int64(len(contents)) > maximum {
 		return nil, ErrUnsafeFile

tools/thothctl/internal/safeio/files_darwin.go

@@ -1,4 +1,4 @@
-//go:build !windows && !linux
+//go:build darwin
 
 package safeio
 
@@ -85,6 +85,36 @@ func closeUnixDescriptors(descriptors []int) {
 // (including stage hard-link/replacement races and ancestor replacement) is outside
 // this mode's threat model. Under that precondition renameatx_np(RENAME_EXCL) is the
 // final fallible no-replace commit and removes the stage atomically.
+var darwinUnlinkat = unix.Unlinkat
+var darwinFstatat = unix.Fstatat
+var darwinCloseStage = func(file *os.File) error { return file.Close() }
+
+func cleanupDarwinStage(dir int, stage string, staged *unix.Stat_t, closeStage func() error) error {
+	var current unix.Stat_t
+	statErr := darwinFstatat(dir, stage, &current, unix.AT_SYMLINK_NOFOLLOW)
+	if statErr == unix.ENOENT {
+		_ = closeStage()
+		return ErrUnsafeFile
+	}
+	if statErr != nil || staged == nil || current.Ino != staged.Ino || current.Dev != staged.Dev {
+		_ = closeStage()
+		return ErrIndeterminateFile
+	}
+	unlinkErr := darwinUnlinkat(dir, stage, 0)
+	closeErr := closeStage()
+	if unlinkErr == nil {
+		// Successful unlink proves that no named candidate bytes remain; close
+		// errors cannot make the unlinked inode reachable by pathname.
+		return ErrUnsafeFile
+	}
+	var after unix.Stat_t
+	if verifyErr := darwinFstatat(dir, stage, &after, unix.AT_SYMLINK_NOFOLLOW); verifyErr == unix.ENOENT {
+		return ErrUnsafeFile
+	}
+	_ = closeErr
+	return ErrIndeterminateFile
+}
+
 func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) error {
 	if err := ValidateCanonicalPath(path); err != nil || len(contents) > 16<<20 || mode.Perm() != 0o600 {
 		return ErrUnsafeFile
@@ -119,8 +149,11 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err
 	}
 	stageFile := os.NewFile(uintptr(stageFD), "thothctl-safeio-stage")
 	if stageFile == nil {
-		_ = unix.Close(stageFD)
-		_ = unix.Unlinkat(dir, stage, 0)
+		closeErr := unix.Close(stageFD)
+		unlinkErr := darwinUnlinkat(dir, stage, 0)
+		if closeErr != nil || unlinkErr != nil {
+			return ErrIndeterminateFile
+		}
 		return ErrUnsafeFile
 	}
 	closed := false
@@ -129,24 +162,15 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err
 			return nil
 		}
 		closed = true
-		return stageFile.Close()
+		return darwinCloseStage(stageFile)
 	}
 	defer func() { _ = closeStage() }()
 	var staged unix.Stat_t
 	if err := unix.Fstat(stageFD, &staged); err != nil || staged.Nlink != 1 || staged.Mode&unix.S_IFMT != unix.S_IFREG {
 		_ = closeStage()
-		_ = unix.Unlinkat(dir, stage, 0)
-		return ErrUnsafeFile
+		return ErrIndeterminateFile
 	}
-	cleanup := func() {
-		// Trusted-parent mode makes this identity check + unlink pre-commit safe;
-		// never unlink a replacement observed at the stage name.
-		var current unix.Stat_t
-		if unix.Fstatat(dir, stage, &current, unix.AT_SYMLINK_NOFOLLOW) == nil && current.Ino == staged.Ino && current.Dev == staged.Dev {
-			_ = unix.Unlinkat(dir, stage, 0)
-		}
-	}
-	fail := func() error { _ = closeStage(); cleanup(); return ErrUnsafeFile }
+	fail := func() error { return cleanupDarwinStage(dir, stage, &staged, closeStage) }
 	if err := stageFile.Chmod(mode); err != nil {
 		return fail()
 	}

tools/thothctl/internal/safeio/files_darwin_test.go

@@ -0,0 +1,44 @@
+//go:build darwin
+
+package safeio
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"golang.org/x/sys/unix"
+)
+
+func TestWriteCanonicalExclusiveClassifiesUnlinkFailureAsIndeterminate(t *testing.T) {
+	root := t.TempDir()
+	stage := filepath.Join(root, "stage")
+	if err := os.WriteFile(stage, []byte("candidate"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	dir, err := unix.Open(root, unix.O_RDONLY|unix.O_DIRECTORY, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer unix.Close(dir)
+	fd, err := unix.Openat(dir, "stage", unix.O_RDWR, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	file := os.NewFile(uintptr(fd), "stage")
+	if file == nil {
+		t.Fatal("os.NewFile returned nil")
+	}
+	defer file.Close()
+	var staged unix.Stat_t
+	if err := unix.Fstat(fd, &staged); err != nil {
+		t.Fatal(err)
+	}
+	oldUnlink := darwinUnlinkat
+	t.Cleanup(func() { darwinUnlinkat = oldUnlink })
+	darwinUnlinkat = func(int, string, int) error { return errors.New("injected unlink failure") }
+	if err := cleanupDarwinStage(dir, "stage", &staged, file.Close); !errors.Is(err, ErrIndeterminateFile) {
+		t.Fatalf("unlink failure = %v, want ErrIndeterminateFile", err)
+	}
+}

tools/thothctl/internal/safeio/files_linux_test.go

@@ -0,0 +1,46 @@
+//go:build linux
+
+package safeio
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestReadCanonicalRegularRejectsReplacementDuringRead(t *testing.T) {
+	root := t.TempDir()
+	path := filepath.Join(root, "schema.sql")
+	replacement := filepath.Join(root, "replacement.sql")
+	parked := filepath.Join(root, "parked.sql")
+	contents := make([]byte, 64<<20)
+	if err := os.WriteFile(path, contents, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(replacement, contents, 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	entered := make(chan struct{})
+	proceed := make(chan struct{})
+	beforeBoundedRead = func() {
+		close(entered)
+		<-proceed
+	}
+	t.Cleanup(func() { beforeBoundedRead = nil })
+	result := make(chan error, 1)
+	go func() { _, err := ReadCanonicalRegular(path, int64(len(contents))); result <- err }()
+	<-entered
+	if err := os.Rename(path, parked); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Rename(replacement, path); err != nil {
+		t.Fatal(err)
+	}
+	close(proceed)
+	err := <-result
+	if !errors.Is(err, ErrUnsafeFile) {
+		t.Fatalf("replacement during read error = %v, want ErrUnsafeFile", err)
+	}
+}

tools/thothctl/internal/safeio/files_unix_test.go

@@ -1,14 +1,12 @@
-//go:build !windows
+//go:build darwin || linux
 
 package safeio
 
 import (
-	"bytes"
 	"errors"
 	"os"
 	"path/filepath"
 	"testing"
-	"time"
 
 	"golang.org/x/sys/unix"
 )
@@ -33,56 +31,6 @@ func TestReadCanonicalRegularRejectsNamedPipeWithoutBlocking(t *testing.T) {
 	}
 }
 
-func TestReadCanonicalRegularRejectsReplacementDuringRead(t *testing.T) {
-	root := t.TempDir()
-	path := filepath.Join(root, "schema.sql")
-	replacement := filepath.Join(root, "replacement.sql")
-	parked := filepath.Join(root, "parked.sql")
-	contents := bytes.Repeat([]byte("x"), 64<<20)
-	if err := os.WriteFile(path, contents, 0o600); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(replacement, contents, 0o600); err != nil {
-		t.Fatal(err)
-	}
-
-	var caught bool
-	for attempt := 0; attempt < 3 && !caught; attempt++ {
-		result := make(chan error, 1)
-		go func() {
-			_, err := ReadCanonicalRegular(path, int64(len(contents)))
-			result <- err
-		}()
-		time.Sleep(time.Millisecond)
-		var finalErr error
-		for i := 0; i < 20; i++ {
-			if err := os.Rename(path, parked); err == nil {
-				if err := os.Rename(replacement, path); err != nil {
-					t.Fatal(err)
-				}
-				time.Sleep(time.Millisecond)
-				if err := os.Rename(parked, replacement); err != nil {
-					t.Fatal(err)
-				}
-			}
-			select {
-			case finalErr = <-result:
-				i = 20
-			default:
-			}
-		}
-		if finalErr == nil {
-			finalErr = <-result
-		}
-		if errors.Is(finalErr, ErrUnsafeFile) {
-			caught = true
-		}
-	}
-	if !caught {
-		t.Fatal("replacement during read was not rejected")
-	}
-}
-
 func TestCanonicalDescriptorOwnershipDoesNotLeakAcrossNestedOperations(t *testing.T) {
 	root, err := filepath.EvalSymlinks(t.TempDir())
 	if err != nil {

tools/thothctl/internal/safeio/files_unsupported_unix.go

@@ -0,0 +1,13 @@
+//go:build !windows && !linux && !darwin
+
+package safeio
+
+import "io/fs"
+
+// Unsupported Unix targets fail closed rather than borrowing a platform-specific
+// publication primitive. Add a dedicated implementation only after auditing that
+// target's namespace and no-replace guarantees.
+func ReadCanonicalRegular(string, int64) ([]byte, error)        { return nil, ErrUnsafeFile }
+func writeCanonicalExclusive(string, []byte, fs.FileMode) error { return ErrUnsafeFile }
+func validateCanonicalOutputPath(string) error                  { return ErrUnsafeFile }
+func validatePlatformPathSyntax(string) error                   { return nil }

tools/thothctl/internal/safeio/files_windows.go

@@ -131,6 +131,22 @@ func closeWindowsHandles(handles []windows.Handle) {
 	}
 }
 
+var windowsDeleteHandle = deleteWindowsHandle
+var windowsCloseHandle = windows.CloseHandle
+var windowsCloseStage = func(file *os.File) error { return file.Close() }
+
+func cleanupWindowsStage(handle windows.Handle, closeStage func() error) error {
+	disposeErr := windowsDeleteHandle(handle)
+	closeErr := closeStage()
+	if disposeErr == nil && closeErr == nil {
+		// A successful disposition plus close proves the named stage is gone.
+		return ErrUnsafeFile
+	}
+	// Either a failed disposition or an uncertain close leaves the named
+	// candidate's reachability unresolved. Never classify this as retry-safe.
+	return ErrIndeterminateFile
+}
+
 func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) error {
 	if err := ValidateCanonicalPath(path); err != nil || len(contents) > 16<<20 || mode.Perm() != 0o600 {
 		return ErrUnsafeFile
@@ -160,8 +176,11 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err
 	}
 	stageFile := os.NewFile(uintptr(stageHandle), "thothctl-safeio-stage")
 	if stageFile == nil {
-		_ = deleteWindowsHandle(stageHandle)
-		_ = windows.CloseHandle(stageHandle)
+		disposeErr := windowsDeleteHandle(stageHandle)
+		closeErr := windowsCloseHandle(stageHandle)
+		if disposeErr != nil || closeErr != nil {
+			return ErrIndeterminateFile
+		}
 		return ErrUnsafeFile
 	}
 	closed := false
@@ -170,14 +189,14 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err
 			return nil
 		}
 		closed = true
-		return stageFile.Close()
+		return windowsCloseStage(stageFile)
 	}
 	defer func() { _ = closeStage() }()
 	var staged windows.ByHandleFileInformation
 	if err := windows.GetFileInformationByHandle(stageHandle, &staged); err != nil || staged.NumberOfLinks != 1 || staged.FileAttributes&windows.FILE_ATTRIBUTE_REPARSE_POINT != 0 || staged.FileAttributes&windows.FILE_ATTRIBUTE_DIRECTORY != 0 {
-		return failWindowsStage(stageHandle, closeStage)
+		return cleanupWindowsStage(stageHandle, closeStage)
 	}
-	fail := func() error { _ = deleteWindowsHandle(stageHandle); _ = closeStage(); return ErrUnsafeFile }
+	fail := func() error { return cleanupWindowsStage(stageHandle, closeStage) }
 	if n, err := stageFile.Write(contents); err != nil || n != len(contents) {
 		return fail()
 	}
@@ -199,13 +218,6 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err
 	return nil
 }
 
-// failWindowsStage disposes an exact handle when initial identity inspection fails.
-func failWindowsStage(handle windows.Handle, closeStage func() error) error {
-	_ = deleteWindowsHandle(handle)
-	_ = closeStage()
-	return ErrUnsafeFile
-}
-
 func deleteWindowsHandle(handle windows.Handle) error {
 	var disposition byte = 1
 	return windows.SetFileInformationByHandle(handle, windows.FileDispositionInfo, &disposition, uint32(unsafe.Sizeof(disposition)))

tools/thothctl/internal/safeio/files_windows_test.go

@@ -125,6 +125,34 @@ func TestWindowsStageHandleDeniesReadRenameDeleteAndHardlink(t *testing.T) {
 	}
 }
 
+func TestWriteCanonicalExclusiveClassifiesDispositionFailureAsIndeterminate(t *testing.T) {
+	root := t.TempDir()
+	path := filepath.Join(root, "candidate.yaml")
+	if err := os.WriteFile(path, []byte("existing"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	oldDelete, oldClose := windowsDeleteHandle, windowsCloseStage
+	t.Cleanup(func() { windowsDeleteHandle, windowsCloseStage = oldDelete, oldClose })
+	windowsDeleteHandle = func(windows.Handle) error { return errors.New("injected disposition failure") }
+	if err := writeCanonicalExclusive(path, []byte("candidate"), 0o600); !errors.Is(err, ErrIndeterminateFile) {
+		t.Fatalf("disposition failure = %v, want ErrIndeterminateFile", err)
+	}
+}
+
+func TestWriteCanonicalExclusiveClassifiesCloseFailureAsIndeterminate(t *testing.T) {
+	root := t.TempDir()
+	path := filepath.Join(root, "candidate.yaml")
+	if err := os.WriteFile(path, []byte("existing"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	oldDelete, oldClose := windowsDeleteHandle, windowsCloseStage
+	t.Cleanup(func() { windowsDeleteHandle, windowsCloseStage = oldDelete, oldClose })
+	windowsCloseStage = func(*os.File) error { return errors.New("injected close failure") }
+	if err := writeCanonicalExclusive(path, []byte("candidate"), 0o600); !errors.Is(err, ErrIndeterminateFile) {
+		t.Fatalf("close failure = %v, want ErrIndeterminateFile", err)
+	}
+}
+
 func TestWriteCanonicalExclusiveRequiresRestrictiveMode(t *testing.T) {
 	if err := writeCanonicalExclusive(`C:\\tmp\\thothctl-output.yaml`, []byte("x"), 0o640); err == nil {
 		t.Fatal("accepted non-restrictive output mode")

tools/thothctl/internal/workspaceops/operations.go

@@ -654,6 +654,9 @@ func publishCandidateBytes(x *hostExport, result Result, path string, b []byte)
 		return nil
 	}
 	if err := safeio.WriteCanonicalExclusive(path, b, 0o600); err != nil {
+		if errors.Is(err, safeio.ErrIndeterminateFile) {
+			return errors.New("candidate output state is indeterminate; reconcile the destination and private stages before retrying")
+		}
 		return errors.New("unsafe output file")
 	}
 	return nil