fix(thothctl): fail closed on partial secret loads

tools/thothctl/cmd/thothctl/main.go

@@ -307,6 +307,12 @@ func containsDeclaredSecretBytes(contents []byte, secrets []string) bool {
 // deterministic safe line exists it emits empty stderr rather than risk a
 // declared-secret collision.
 func writeWorkspaceError(stderr io.Writer, err error, secrets []string, code int) int {
+	// A nil set means the complete declared-secret set was not loaded. No
+	// nonempty diagnostic is safe because a successfully read secret may equal
+	// the fixed error chrome. Callers must fail closed without output.
+	if secrets == nil {
+		return code
+	}
 	message := "workspace operation failed"
 	if err != nil {
 		message = output.Sanitize(err.Error(), secrets)

tools/thothctl/cmd/thothctl/main_test.go

@@ -218,6 +218,23 @@ func TestRunWorkspaceRejectsDeclaredSecretInHumanChrome(t *testing.T) {
 	}
 }
 
+func TestRunWorkspacePartialSecretLoadEmitsNoOutput(t *testing.T) {
+	fixture := newCLIFixture(t, "")
+	firstSecret := filepath.Join(fixture.root, "first-secret")
+	missingSecret := filepath.Join(fixture.root, "missing-secret")
+	if err := os.WriteFile(firstSecret, []byte("workspace operation failed"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	fixture.setEnvContents(t, "FIRST_SECRET_FILE="+firstSecret+"\nSECOND_SECRET_FILE="+missingSecret+"\n")
+
+	var stdout, stderr bytes.Buffer
+	code := run(context.Background(), []string{"--installation", fixture.installationPath, "workspace", "inspect", "--workspace", "psd"}, &stdout, &stderr)
+	if code != 2 || stdout.Len() != 0 || stderr.Len() != 0 {
+		t.Fatalf("exit=%d stdout=%q stderr=%q, want exit 2 and no output", code, stdout.String(), stderr.String())
+	}
+	assertDockerNotInvoked(t, fixture)
+}
+
 func TestRunWorkspaceErrorPrefixNeverLeaksDeclaredSecret(t *testing.T) {
 	fixture := newCLIFixture(t, "UNLABELLED_SECRET_FILE=%s\n")
 	secretPath := filepath.Join(fixture.root, "secret")

tools/thothctl/internal/safeio/files.go

@@ -18,10 +18,6 @@ var ErrUnsafeFile = errors.New("unsafe file")
 // 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)) {
@@ -33,7 +29,7 @@ func ValidateCanonicalPath(path string) error {
 	return nil
 }
 
-func readBoundedRegularFile(file *os.File, maximum int64) ([]byte, error) {
+func readBoundedRegularFile(file *os.File, maximum int64, before func()) ([]byte, error) {
 	if maximum < 0 || maximum == int64(^uint64(0)>>1) {
 		return nil, ErrUnsafeFile
 	}
@@ -41,8 +37,8 @@ func readBoundedRegularFile(file *os.File, maximum int64) ([]byte, error) {
 	if err != nil || !info.Mode().IsRegular() {
 		return nil, ErrUnsafeFile
 	}
-	if beforeBoundedRead != nil {
+	if before != nil {
-		beforeBoundedRead()
+		before()
 	}
 	contents, err := io.ReadAll(io.LimitReader(file, maximum+1))
 	if err != nil || int64(len(contents)) > maximum {

tools/thothctl/internal/safeio/files_darwin.go

@@ -16,6 +16,10 @@ import (
 // descriptor. O_NOFOLLOW rejects symlinks at every component, and the open directory descriptors
 // prevent later parent replacement from redirecting the final open.
 func ReadCanonicalRegular(path string, maximum int64) ([]byte, error) {
+	return readCanonicalRegularWithHook(path, maximum, nil)
+}
+
+func readCanonicalRegularWithHook(path string, maximum int64, beforeRead func()) ([]byte, error) {
 	if err := ValidateCanonicalPath(path); err != nil {
 		return nil, err
 	}
@@ -54,7 +58,7 @@ func ReadCanonicalRegular(path string, maximum int64) ([]byte, error) {
 	if err := unix.Fstat(int(file.Fd()), &before); err != nil || before.Nlink > 1 || before.Mode&unix.S_IFMT != unix.S_IFREG {
 		return nil, ErrUnsafeFile
 	}
-	contents, err := readBoundedRegularFile(file, maximum)
+	contents, err := readBoundedRegularFile(file, maximum, beforeRead)
 	if err != nil {
 		return nil, ErrUnsafeFile
 	}

tools/thothctl/internal/safeio/files_linux.go

@@ -16,6 +16,10 @@ import (
 // descriptor. O_NOFOLLOW rejects symlinks at every component, and the open directory descriptors
 // prevent later parent replacement from redirecting the final open.
 func ReadCanonicalRegular(path string, maximum int64) ([]byte, error) {
+	return readCanonicalRegularWithHook(path, maximum, nil)
+}
+
+func readCanonicalRegularWithHook(path string, maximum int64, beforeRead func()) ([]byte, error) {
 	if err := ValidateCanonicalPath(path); err != nil {
 		return nil, err
 	}
@@ -54,7 +58,7 @@ func ReadCanonicalRegular(path string, maximum int64) ([]byte, error) {
 	if err := unix.Fstat(int(file.Fd()), &before); err != nil || before.Nlink > 1 || before.Mode&unix.S_IFMT != unix.S_IFREG {
 		return nil, ErrUnsafeFile
 	}
-	contents, err := readBoundedRegularFile(file, maximum)
+	contents, err := readBoundedRegularFile(file, maximum, beforeRead)
 	if err != nil {
 		return nil, ErrUnsafeFile
 	}

tools/thothctl/internal/safeio/files_replacement_unix_test.go

@@ -1,4 +1,4 @@
-//go:build linux
+//go:build darwin || linux
 
 package safeio
 
@@ -10,7 +10,7 @@ import (
 )
 
 func TestReadCanonicalRegularRejectsReplacementDuringRead(t *testing.T) {
-	root := t.TempDir()
+	root := canonicalSafeioTempDir(t)
 	path := filepath.Join(root, "schema.sql")
 	replacement := filepath.Join(root, "replacement.sql")
 	parked := filepath.Join(root, "parked.sql")
@@ -24,13 +24,14 @@ func TestReadCanonicalRegularRejectsReplacementDuringRead(t *testing.T) {
 
 	entered := make(chan struct{})
 	proceed := make(chan struct{})
-	beforeBoundedRead = func() {
+	result := make(chan error, 1)
+	go func() {
+		_, err := readCanonicalRegularWithHook(path, int64(len(contents)), func() {
 			close(entered)
 			<-proceed
-	}
+		})
-	t.Cleanup(func() { beforeBoundedRead = nil })
+		result <- err
-	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)
@@ -44,3 +45,17 @@ func TestReadCanonicalRegularRejectsReplacementDuringRead(t *testing.T) {
 		t.Fatalf("replacement during read error = %v, want ErrUnsafeFile", err)
 	}
 }
+
+func canonicalSafeioTempDir(t *testing.T) string {
+	t.Helper()
+	root, err := filepath.EvalSymlinks(os.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	directory, err := os.MkdirTemp(root, "thothctl-safeio-test-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.RemoveAll(directory) })
+	return directory
+}

tools/thothctl/internal/safeio/files_windows.go

@@ -71,7 +71,7 @@ func ReadCanonicalRegular(path string, maximum int64) ([]byte, error) {
 	if windows.GetFileInformationByHandle(handle, &before) != nil || before.NumberOfLinks > 1 {
 		return nil, ErrUnsafeFile
 	}
-	contents, err := readBoundedRegularFile(file, maximum)
+	contents, err := readBoundedRegularFile(file, maximum, nil)
 	if err != nil {
 		return nil, ErrUnsafeFile
 	}