fix: harden task1 workspace output and publication

tools/thothctl/internal/safeio/files.go

@@ -18,6 +18,9 @@ func ValidateCanonicalPath(path string) error {
 	if !filepath.IsAbs(path) || filepath.Clean(path) != path || strings.Contains(path, string(filepath.Separator)+".."+string(filepath.Separator)) {
 		return ErrUnsafeFile
 	}
+	if err := validatePlatformPathSyntax(path); err != nil {
+		return err
+	}
 	return nil
 }
 

tools/thothctl/internal/safeio/files_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/aritmolab/thothii/tools/thothctl/internal/testsupport"
@@ -86,6 +87,29 @@ func TestWriteCanonicalExclusiveRejectsExistingAndCreatesPrivateFile(t *testing.
 	}
 }
 
+func TestWriteCanonicalExclusiveCleansPrivateStageWhenPublicationRacesExistingLeaf(t *testing.T) {
+	root, err := filepath.EvalSymlinks(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	path := filepath.Join(root, "candidate.yaml")
+	if err := os.WriteFile(path, []byte("attacker"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if err := WriteCanonicalExclusive(path, []byte("candidate"), 0o600); !errors.Is(err, ErrUnsafeFile) {
+		t.Fatalf("write error=%v", err)
+	}
+	entries, err := os.ReadDir(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, entry := range entries {
+		if strings.HasPrefix(entry.Name(), ".thothctl-candidate-") {
+			t.Fatalf("private stage leaked: %s", entry.Name())
+		}
+	}
+}
+
 func TestReadCanonicalRegularRejectsHardlinkAndDirectory(t *testing.T) {
 	root, err := filepath.EvalSymlinks(t.TempDir())
 	if err != nil {

tools/thothctl/internal/safeio/files_unix.go

@@ -3,6 +3,8 @@
 package safeio
 
 import (
+	"crypto/rand"
+	"encoding/hex"
 	"io/fs"
 	"os"
 	"strings"
@@ -90,51 +92,117 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err
 	if err != nil {
 		return ErrUnsafeFile
 	}
-	defer func() { unix.Close(dir) }()
+	defer unix.Close(dir)
 	for _, component := range components[:len(components)-1] {
-		next, err := unix.Openat(dir, component, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0)
-		if err != nil {
+		next, openErr := unix.Openat(dir, component, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0)
+		if openErr != nil {
 			return ErrUnsafeFile
 		}
 		unix.Close(dir)
 		dir = next
 	}
-	var parentBefore unix.Stat_t
-	if err := unix.Fstat(dir, &parentBefore); err != nil {
+	if !recheckUnixParentPath(components[:len(components)-1], dir) {
 		return ErrUnsafeFile
 	}
-	fd, err := unix.Openat(dir, components[len(components)-1], unix.O_WRONLY|unix.O_CREAT|unix.O_EXCL|unix.O_CLOEXEC|unix.O_NOFOLLOW, uint32(mode.Perm()))
+
+	// Build the candidate under a private, same-parent name. Only after it is fully
+	// written, synced, and identity-checked do we link it into the requested leaf.
+	stage, err := privateStageName()
 	if err != nil {
 		return ErrUnsafeFile
 	}
-	f := os.NewFile(uintptr(fd), "thothctl-safeio-output")
-	if f == nil {
-		unix.Close(fd)
+	stageFD, err := unix.Openat(dir, stage, unix.O_WRONLY|unix.O_CREAT|unix.O_EXCL|unix.O_CLOEXEC|unix.O_NOFOLLOW, uint32(mode.Perm()))
+	if err != nil {
 		return ErrUnsafeFile
 	}
-	defer f.Close()
-	if err := f.Chmod(mode); err != nil {
+	stageFile := os.NewFile(uintptr(stageFD), "thothctl-safeio-stage")
+	if stageFile == nil {
+		unix.Close(stageFD)
+		_ = unix.Unlinkat(dir, stage, 0)
 		return ErrUnsafeFile
 	}
-	if _, err := f.Write(contents); err != nil {
+	stageCreated := true
+	var staged unix.Stat_t
+	if err := unix.Fstat(stageFD, &staged); err != nil || staged.Nlink != 1 || staged.Mode&unix.S_IFMT != unix.S_IFREG {
+		_ = stageFile.Close()
+		_ = unix.Unlinkat(dir, stage, 0)
 		return ErrUnsafeFile
 	}
-	if err := f.Sync(); err != nil {
+	published := false
+	var publishedIdentity unix.Stat_t
+	cleanup := func() {
+		if published {
+			var current unix.Stat_t
+			if unix.Fstatat(dir, components[len(components)-1], &current, unix.AT_SYMLINK_NOFOLLOW) == nil &&
+				current.Ino == publishedIdentity.Ino && current.Dev == publishedIdentity.Dev {
+				_ = unix.Unlinkat(dir, components[len(components)-1], 0)
+			}
+		}
+		if stageCreated {
+			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 {
+		_ = stageFile.Close()
+		cleanup()
 		return ErrUnsafeFile
 	}
-	var opened, named, parentAfter unix.Stat_t
-	if err := unix.Fstat(fd, &opened); err != nil || opened.Nlink != 1 {
+	if err := stageFile.Chmod(mode); err != nil {
+		return fail()
+	}
+	n, err := stageFile.Write(contents)
+	if err != nil || n != len(contents) {
+		return fail()
+	}
+	if err := stageFile.Sync(); err != nil {
+		return fail()
+	}
+	var afterWrite unix.Stat_t
+	if err := unix.Fstat(stageFD, &afterWrite); err != nil || afterWrite.Nlink != 1 || afterWrite.Mode&unix.S_IFMT != unix.S_IFREG || afterWrite.Size != int64(len(contents)) {
+		return fail()
+	}
+	if err := stageFile.Close(); err != nil {
+		cleanup()
 		return ErrUnsafeFile
 	}
-	if err := unix.Fstatat(dir, components[len(components)-1], &named, unix.AT_SYMLINK_NOFOLLOW); err != nil || named.Nlink != 1 || named.Ino != opened.Ino || named.Dev != opened.Dev {
-		return ErrUnsafeFile
+	stageCreated = true
+	if !recheckUnixParentPath(components[:len(components)-1], dir) {
+		return fail()
 	}
-	if err := unix.Fstat(dir, &parentAfter); err != nil || parentAfter.Ino != parentBefore.Ino || parentAfter.Dev != parentBefore.Dev || !recheckUnixParentPath(components[:len(components)-1], dir) {
-		return ErrUnsafeFile
+	if err := unix.Linkat(dir, stage, dir, components[len(components)-1], 0); err != nil {
+		return fail()
+	}
+	published = true
+	publishedIdentity = staged
+	if err := unix.Fstatat(dir, components[len(components)-1], &publishedIdentity, unix.AT_SYMLINK_NOFOLLOW); err != nil ||
+		publishedIdentity.Ino != staged.Ino || publishedIdentity.Dev != staged.Dev || publishedIdentity.Nlink != 2 {
+		return fail()
+	}
+	if err := unix.Unlinkat(dir, stage, 0); err != nil {
+		return fail()
+	}
+	stageCreated = false
+	if err := unix.Fstatat(dir, components[len(components)-1], &publishedIdentity, unix.AT_SYMLINK_NOFOLLOW); err != nil || publishedIdentity.Nlink != 1 {
+		return fail()
+	}
+	if err := unix.Fsync(dir); err != nil || !recheckUnixParentPath(components[:len(components)-1], dir) {
+		return fail()
 	}
 	return nil
 }
 
+func privateStageName() (string, error) {
+	var random [16]byte
+	if _, err := rand.Read(random[:]); err != nil {
+		return "", err
+	}
+	return ".thothctl-candidate-" + hex.EncodeToString(random[:]), nil
+}
+
 func validateCanonicalOutputPath(path string) error {
 	if err := ValidateCanonicalPath(path); err != nil {
 		return err
@@ -207,3 +275,5 @@ func recheckUnixParentPath(components []string, retained int) bool {
 	var got, want unix.Stat_t
 	return unix.Fstat(dir, &got) == nil && unix.Fstat(retained, &want) == nil && got.Ino == want.Ino && got.Dev == want.Dev
 }
+
+func validatePlatformPathSyntax(path string) error { return nil }

tools/thothctl/internal/safeio/files_windows.go

@@ -3,6 +3,8 @@
 package safeio
 
 import (
+	"crypto/rand"
+	"encoding/hex"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -137,50 +139,113 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err
 		return ErrUnsafeFile
 	}
 	defer closeWindowsHandles(retainedParents)
-	// Parent handles stay open with delete sharing denied until publication and
-	// identity recheck complete; this is the Windows equivalent of retained dirfds.
 	securityDescriptor, securityAttributes, err := ownerOnlySecurityAttributes()
 	if err != nil {
 		return ErrUnsafeFile
 	}
-	h, err := windows.CreateFile(windows.StringToUTF16Ptr(filepath.Join(parent, filepath.Base(path))), windows.GENERIC_WRITE, windowsOutputHandleShareMode, securityAttributes, windows.CREATE_NEW, windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_OPEN_REPARSE_POINT, 0)
-	if err != nil {
-		return ErrUnsafeFile
-	}
-	f := os.NewFile(uintptr(h), "thothctl-safeio-output")
-	if f == nil {
-		windows.CloseHandle(h)
-		return ErrUnsafeFile
-	}
-	defer f.Close()
-	// Go's Windows Chmod only toggles the read-only attribute; it cannot enforce
-	// owner-only permissions. The restrictive DACL is installed at creation time
-	// through SECURITY_ATTRIBUTES above.
 	_ = securityDescriptor
-	if mode.Perm() != 0o600 {
-		return ErrUnsafeFile
-	}
-	if _, err := f.Write(contents); err != nil {
-		return ErrUnsafeFile
-	}
-	if err := f.Sync(); err != nil {
-		return ErrUnsafeFile
-	}
-	var opened, named windows.ByHandleFileInformation
-	if windows.GetFileInformationByHandle(h, &opened) != nil || opened.NumberOfLinks != 1 {
-		return ErrUnsafeFile
-	}
-	check, err := openWindowsComponent(path, false)
+	stageName, err := privateWindowsStageName()
 	if err != nil {
 		return ErrUnsafeFile
 	}
-	defer windows.CloseHandle(check)
-	if windows.GetFileInformationByHandle(check, &named) != nil || named.NumberOfLinks != 1 || !sameWindowsFile(opened, named) {
+	stagePath := filepath.Join(parent, stageName)
+	stageHandle, err := windows.CreateFile(windows.StringToUTF16Ptr(stagePath), windows.GENERIC_WRITE, windowsOutputHandleShareMode, securityAttributes, windows.CREATE_NEW, windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_OPEN_REPARSE_POINT, 0)
+	if err != nil {
 		return ErrUnsafeFile
 	}
+	stageFile := os.NewFile(uintptr(stageHandle), "thothctl-safeio-stage")
+	if stageFile == nil {
+		windows.CloseHandle(stageHandle)
+		_ = windows.DeleteFile(windows.StringToUTF16Ptr(stagePath))
+		return ErrUnsafeFile
+	}
+	stageCreated := true
+	published := false
+	var staged, publishedIdentity windows.ByHandleFileInformation
+	if err := windows.GetFileInformationByHandle(stageHandle, &staged); err != nil || staged.NumberOfLinks != 1 {
+		_ = stageFile.Close()
+		_ = windows.DeleteFile(windows.StringToUTF16Ptr(stagePath))
+		return ErrUnsafeFile
+	}
+	cleanup := func() {
+		if published {
+			removeWindowsIfIdentity(filepath.Join(parent, filepath.Base(path)), publishedIdentity)
+		}
+		if stageCreated {
+			removeWindowsIfIdentity(stagePath, staged)
+		}
+	}
+	fail := func() error {
+		_ = stageFile.Close()
+		cleanup()
+		return ErrUnsafeFile
+	}
+	if n, err := stageFile.Write(contents); err != nil || n != len(contents) {
+		return fail()
+	}
+	if err := stageFile.Sync(); err != nil {
+		return fail()
+	}
+	var afterWrite windows.ByHandleFileInformation
+	if err := windows.GetFileInformationByHandle(stageHandle, &afterWrite); err != nil || afterWrite.NumberOfLinks != 1 || afterWrite.FileSizeHigh != uint32(uint64(len(contents))>>32) || afterWrite.FileSizeLow != uint32(len(contents)) {
+		return fail()
+	}
+	if err := stageFile.Close(); err != nil {
+		cleanup()
+		return ErrUnsafeFile
+	}
+	// CreateHardLink is an atomic, same-volume, no-replace publication. The final
+	// pathname can never refer to a partially written candidate.
+	finalPath := filepath.Join(parent, filepath.Base(path))
+	if err := windows.CreateHardLink(windows.StringToUTF16Ptr(finalPath), windows.StringToUTF16Ptr(stagePath), 0); err != nil {
+		return fail()
+	}
+	published = true
+	publishedIdentity = staged
+	check, identityErr := windowsFileIdentity(finalPath)
+	if identityErr != nil || check.NumberOfLinks != 2 || !sameWindowsFile(staged, check) {
+		return fail()
+	}
+	publishedIdentity = check
+	if err := windows.DeleteFile(windows.StringToUTF16Ptr(stagePath)); err != nil {
+		return fail()
+	}
+	stageCreated = false
+	finalIdentity, identityErr := windowsFileIdentity(finalPath)
+	if identityErr != nil || finalIdentity.NumberOfLinks != 1 || !sameWindowsFile(staged, finalIdentity) {
+		return fail()
+	}
 	return nil
 }
 
+func privateWindowsStageName() (string, error) {
+	var random [16]byte
+	if _, err := rand.Read(random[:]); err != nil {
+		return "", err
+	}
+	return ".thothctl-candidate-" + hex.EncodeToString(random[:]), nil
+}
+
+func windowsFileIdentity(path string) (windows.ByHandleFileInformation, error) {
+	h, err := windows.CreateFile(windows.StringToUTF16Ptr(path), windows.GENERIC_READ, windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE, nil, windows.OPEN_EXISTING, windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_OPEN_REPARSE_POINT, 0)
+	if err != nil {
+		return windows.ByHandleFileInformation{}, err
+	}
+	defer windows.CloseHandle(h)
+	var information windows.ByHandleFileInformation
+	if err := windows.GetFileInformationByHandle(h, &information); err != nil || information.NumberOfLinks == 0 || information.FileAttributes&windows.FILE_ATTRIBUTE_REPARSE_POINT != 0 || information.FileAttributes&windows.FILE_ATTRIBUTE_DIRECTORY != 0 {
+		return windows.ByHandleFileInformation{}, ErrUnsafeFile
+	}
+	return information, nil
+}
+
+func removeWindowsIfIdentity(path string, expected windows.ByHandleFileInformation) {
+	got, err := windowsFileIdentity(path)
+	if err == nil && sameWindowsFile(got, expected) {
+		_ = windows.DeleteFile(windows.StringToUTF16Ptr(path))
+	}
+}
+
 func openWindowsParents(path string) (string, []windows.Handle, error) {
 	volume := filepath.VolumeName(path)
 	root := volume + string(filepath.Separator)
@@ -217,6 +282,23 @@ func validateCanonicalOutputPath(path string) error {
 	return nil
 }
 
+func validatePlatformPathSyntax(path string) error {
+	// Win32 device namespaces and alternate data streams do not represent an
+	// independent regular file with owner-only output permissions.
+	lower := strings.ToLower(path)
+	if strings.HasPrefix(lower, `\\?\`) || strings.HasPrefix(lower, `\\.\`) ||
+		strings.HasPrefix(lower, `\device\`) || strings.HasPrefix(lower, `\??\`) || strings.HasPrefix(path, `\\`) {
+		return ErrUnsafeFile
+	}
+	volume := filepath.VolumeName(path)
+	for _, component := range strings.Split(strings.TrimPrefix(path, volume+string(filepath.Separator)), string(filepath.Separator)) {
+		if strings.Contains(component, ":") {
+			return ErrUnsafeFile
+		}
+	}
+	return nil
+}
+
 func sameWindowsFile(a, b windows.ByHandleFileInformation) bool {
 	return a.VolumeSerialNumber == b.VolumeSerialNumber && a.FileIndexHigh == b.FileIndexHigh && a.FileIndexLow == b.FileIndexLow
 }

tools/thothctl/internal/safeio/files_windows_test.go

@@ -22,6 +22,14 @@ var _ [expectedWindowsRetainedHandleShareMode - windowsRetainedHandleShareMode]s
 var _ [windowsOutputHandleShareMode - expectedWindowsOutputHandleShareMode]struct{}
 var _ [expectedWindowsOutputHandleShareMode - windowsOutputHandleShareMode]struct{}
 
+func TestValidateCanonicalPathRejectsWindowsNamespacesAndAlternateStreams(t *testing.T) {
+	for _, path := range []string{`C:\dir\existing.txt:candidate`, `\\?\C:\dir\candidate`, `\\.\pipe\candidate`, `\Device\HarddiskVolume1\candidate`, `\??\C:\candidate`} {
+		if err := ValidateCanonicalPath(path); !errors.Is(err, ErrUnsafeFile) {
+			t.Errorf("ValidateCanonicalPath(%q) = %v, want ErrUnsafeFile", path, err)
+		}
+	}
+}
+
 func TestOpenWindowsComponentBlocksMutationWhileHandleIsRetained(t *testing.T) {
 	t.Run("parent rename", func(t *testing.T) {
 		parent := filepath.Join(t.TempDir(), "parent")

tools/thothctl/internal/workspaceops/operations.go

@@ -11,6 +11,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"path/filepath"
 	"reflect"
 	"regexp"
 	"strings"
@@ -369,7 +370,7 @@ func makeInput(c Command) (inputEnvelope, string, error) {
 			if total > maxSQLTotal {
 				return env, "", errors.New("SQL input exceeds limit")
 			}
-			base := path[strings.LastIndexAny(path, "/\\")+1:]
+			base := filepath.Base(path)
 			env.SQL = append(env.SQL, sqlInput{base, base64.StdEncoding.EncodeToString(b), DigestBytes(b)})
 		}
 	case CheckSchemaRequest:
@@ -379,7 +380,7 @@ func makeInput(c Command) (inputEnvelope, string, error) {
 			if e != nil {
 				return env, "", errors.New("unsafe annotation input")
 			}
-			base := x.Annotations[strings.LastIndexAny(x.Annotations, "/\\")+1:]
+			base := filepath.Base(x.Annotations)
 			env.Annotations = &annotationInput{base, base64.StdEncoding.EncodeToString(b), DigestBytes(b)}
 		}
 	case IndexSchemaRequest:
@@ -589,7 +590,12 @@ func publishCandidate(x *hostExport, result Result, path string) error {
 		return errors.New("invalid candidate export")
 	}
 	var doc any
-	if yaml.Unmarshal(b, &doc) != nil {
+	decoder := yaml.NewDecoder(bytes.NewReader(b))
+	if decoder.Decode(&doc) != nil {
+		return errors.New("invalid candidate export")
+	}
+	var trailing any
+	if err := decoder.Decode(&trailing); err != io.EOF {
 		return errors.New("invalid candidate export")
 	}
 	if result.RunID == "" || !runIDPattern.MatchString(result.RunID) {

tools/thothctl/internal/workspaceops/operations_test.go

@@ -3,6 +3,7 @@ package workspaceops
 import (
 	"bytes"
 	"encoding/base64"
+	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -16,6 +17,41 @@ import (
 	"github.com/aritmolab/thothii/tools/thothctl/internal/safeio"
 )
 
+func TestMakeInputUsesPlatformBasename(t *testing.T) {
+	if filepath.Separator == '\\' {
+		t.Skip("Unix basename behavior")
+	}
+	root, err := filepath.EvalSymlinks(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	path := filepath.Join(root, "schema\\v2.sql")
+	if err := os.WriteFile(path, []byte("select 1"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	_, payload, err := makeInput(SuggestFksRequest{WorkspaceID: "psd", FromSQL: []string{path}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	var envelope inputEnvelope
+	if err := json.Unmarshal([]byte(payload), &envelope); err != nil {
+		t.Fatal(err)
+	}
+	if len(envelope.SQL) != 1 || envelope.SQL[0].Basename != "schema\\v2.sql" {
+		t.Fatalf("basename=%q", envelope.SQL[0].Basename)
+	}
+}
+
+func TestPublishCandidateRejectsTrailingYAMLDocument(t *testing.T) {
+	candidate := []byte("candidates: []\n---\n[\n")
+	digest := DigestBytes(candidate)
+	x := hostExport{MediaType: "application/yaml", SHA256: digest, ContentBase64: base64.StdEncoding.EncodeToString(candidate)}
+	result := Result{RunID: strings.Repeat("b", 32)}
+	if err := publishCandidate(&x, result, ""); err == nil {
+		t.Fatal("accepted malformed trailing YAML document")
+	}
+}
+
 func TestParseWorkspaceCommands(t *testing.T) {
 	cases := []struct {
 		name string