fix: every file is written under a name nothing else holds

CHANGELOG.md

@@ -56,6 +56,32 @@ because it turns other people's test suites red.
   same new code and were checked against their recorded hashes and across every
   format at five sizes and two seeds.
 
+### Security
+
+- **A file is never written under a name something else already holds.** Every
+  file this tool writes goes to a temporary name first and is renamed into
+  place. Three of those temporary names were created in a way that follows a
+  link, so a link left at one of them by somebody else sent the bytes wherever
+  it pointed - outside the directory you gave - and the run still reported
+  success.
+
+  Reproduced against the previous build. A link at the manifest's temporary name
+  put the manifest onto a file outside the output directory and exited 0, after
+  which `verify` called that run a match and `cleanup` reported it removed. The
+  same shape made `recipe fmt -w` write your recipe onto somebody else's file
+  and leave your recipe itself as a link. On Windows none of this needs a
+  privilege, because a hard link is enough.
+
+  Every one of those names is now claimed rather than created, and a name
+  something else holds is a refusal that says which name and what to do.
+  Pointing `--out` at a directory reached through a link keeps working, which is
+  the setup this was measured against.
+
+  **What changes for an ordinary run: nothing.** The one case you can meet
+  without somebody working against you is a leftover `.tfg-writing` file from a
+  run that was killed part way through. That used to be written over in silence.
+  It is now a refusal naming the file, so remove it and run again.
+
 ### Changed
 
 - **Files are written over several threads, so a run of many files is several

internal/core/createnew.go

@@ -0,0 +1,90 @@
+package core
+
+import (
+	"errors"
+	"io/fs"
+	"os"
+)
+
+// CreateNew creates a file under a name nobody is holding, and refuses when
+// something already is.
+//
+// Every file this tool writes goes through here, and there is one reason for
+// that rather than a preference for tidiness. A create that is not exclusive
+// follows whatever the name points at, so a name somebody else put there first
+// decides where the bytes land. Measured on 2026-09-06 against the shipped
+// binary, in a scratch directory outside the repository:
+//
+//	a link at <manifest>.tfg-writing   the manifest landed on the file the
+//	                                   link pointed at, outside the output
+//	                                   directory, and the run exited 0
+//	a link at <recipe>.tfg-writing     "recipe fmt -w" wrote the recipe onto
+//	                                   somebody else's file and left the
+//	                                   recipe itself as a link, exit 0
+//
+// Neither name was checked anywhere, because the checks that exist are about
+// the file a run produces and the manifest it records - and these two are the
+// names those files are written under before they are renamed into place.
+//
+// THE CHECK CANNOT BE A LOOK BEFORE THE WRITE, and that is what makes this a
+// create rather than a question. os.Stat follows a link, so a link pointing at
+// nothing answers "there is nothing here" - and a hard link is not a link at
+// all as far as any question goes: os.Lstat reports it as an ordinary file,
+// because that is what it is. On Windows an ordinary user creates one without
+// any privilege, which is measured rather than read. So the only answer that
+// holds is the one the operating system settles while it creates the file.
+//
+// O_EXCL IS NOT RELIABLE EVERYWHERE, and that was measured too, on 2026-08-03
+// and again on 2026-08-25. On Windows, Go asks for the reparse point rather
+// than for what it points at when O_EXCL is set, and the create then reports
+// "the file exists" about a file that is not there whenever any part of the
+// path is a symbolic link or a junction. A directory reached through a link is
+// an ordinary setup - a redirected workspace, a mounted scratch disk - and this
+// tool supports it on purpose.
+//
+// So a refusal is believed only when something really is there, and the
+// question that settles it is os.Lstat rather than os.Stat: a link pointing at
+// nothing is a name being taken, whatever it points at. Where O_EXCL works this
+// is exactly O_EXCL. Where it lies, this is what the tool did before it, and
+// what is left is the window between the two calls - narrow, on that one
+// platform, and smaller than the whole of the door it replaces.
+func CreateNew(path string, perm os.FileMode) (*os.File, error) {
+	f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
+	if err == nil {
+		return f, nil
+	}
+
+	_, lookErr := os.Lstat(path)
+	if lookErr == nil {
+		// Something is genuinely there. This is the refusal that matters, and
+		// it is the one the escapes above went round.
+		return nil, &NameTakenError{Path: path, Err: err}
+	}
+	if !errors.Is(lookErr, fs.ErrNotExist) {
+		// A name we cannot ask about is not a name we may write over. Reported
+		// as the create failed rather than as the look did, because the create
+		// is what the caller asked for.
+		return nil, err
+	}
+
+	return os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
+}
+
+// NameTakenError is refusing to write under a name something else is holding.
+//
+// It carries the create's own error so that a caller can still ask
+// errors.Is(err, fs.ErrExist) and give the refusal its own words - which the
+// engine does, because "this run will not write over it" is a better sentence
+// about a generated file than anything a general purpose helper could write.
+type NameTakenError struct {
+	Path string
+	Err  error
+}
+
+func (e *NameTakenError) Error() string {
+	return "the name " + e.Path + " is already in use, so nothing was written. " +
+		"This tool writes under a temporary name and renames it into place, and it never writes over a name somebody else holds. " +
+		"Remove what is at that name, or work in a directory nothing else is writing to"
+}
+
+func (e *NameTakenError) Unwrap() error { return e.Err }

internal/core/replace.go

@@ -1,6 +1,9 @@
 package core
 
-import "os"
+import (
+	"errors"
+	"os"
+)
 
 // writingSuffix marks the half written copy while it is being filled.
 //
@@ -47,7 +50,14 @@ func ReplaceFile(path string, content []byte) error {
 
 	tmp := path + writingSuffix
 	if err := writeWhole(tmp, content, mode); err != nil {
-		_ = os.Remove(tmp)
+		// Only what this call created is taken away. A refusal from CreateNew
+		// means the name was already somebody's - a leftover from an
+		// interrupted run, or something planted there - and untouchable rule 7
+		// is that this tool does not remove what it did not write.
+		var taken *NameTakenError
+		if !errors.As(err, &taken) {
+			_ = os.Remove(tmp)
+		}
 		return err
 	}
 	if err := os.Rename(tmp, path); err != nil {
@@ -83,13 +93,20 @@ func modeToKeep(path string) (os.FileMode, error) {
 
 // writeWhole fills the copy and makes sure it carries the mode it was given.
 //
-// The mode is set explicitly rather than left to the create call, for two
-// reasons that both bite quietly: a create only applies its mode when the file
-// is new, so a leftover copy from an interrupted run would keep whatever it
-// had, and the process umask takes bits away from a create and not from a
-// chmod.
+// The mode is set explicitly rather than left to the create call, because the
+// process umask takes bits away from a create and not from a chmod.
+//
+// A second reason stood here until 2026-09-06 and it went with the create it
+// described: a create only applies its mode when the file is new, so a leftover
+// copy from an interrupted run used to keep whatever mode it had. CreateNew
+// refuses a name something is already holding, so there is no leftover to
+// inherit a mode from - there is a refusal naming the file instead. That
+// changed because this name is beside a file in somebody's repository, and a
+// create that is not exclusive wrote through a link planted at it. Measured on
+// 2026-09-06: "recipe fmt -w" put the recipe on a file outside the directory
+// and left the recipe itself as a link, exit 0.
 func writeWhole(path string, content []byte, mode os.FileMode) error {
-	f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode)
+	f, err := CreateNew(path, mode)
 	if err != nil {
 		return err
 	}

internal/engine/parallel.go

@@ -6,13 +6,17 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"runtime"
 	"sync"
 	"sync/atomic"
+
+	"github.com/donislawdev/TestingFilesGenerator/internal/core"
 )
 
 // This file is the only place in internal/engine that runs anything beside
@@ -289,30 +293,40 @@ func writeOne(ctx context.Context, f PlannedFile, outDir string, p *fileProgress
 	// files that want the same name and this name is built from that one.
 	tmp := tempPathFor(outDir, f.Name)
 
-	// os.Create, and O_EXCL was tried here and taken back out on 2026-08-25.
+	// Claimed rather than created, and core.CreateNew carries the measurement
+	// that settles how.
 	//
-	// The idea was sound: the check in preflight answers "this name is free"
-	// a few hundred lines before the write, and O_EXCL would have the
-	// filesystem answer it at the moment of writing instead. What it costs on
-	// Windows is not sound. Measured with a probe, a file created in a
+	// O_EXCL on its own was tried here and taken back out on 2026-08-25, for a
+	// reason that has not changed. Measured with a probe, a file created in a
 	// directory reached through a symbolic link:
 	//
 	//   os.Create                 works
 	//   O_CREATE|O_EXCL|O_WRONLY  fails with "The file exists"
 	//
 	// about a file that does not exist. Go asks for the reparse point rather
 	// than what it points at when O_EXCL is set, so every file of a run whose
-	// output directory is a link fails - and this tool supports exactly that
+	// output directory is a link failed - and this tool supports exactly that
 	// on purpose, because people keep fixtures on a mounted workspace or a
 	// scratch disk. Two guards said so within a minute of the change.
 	//
-	// The window O_EXCL would have closed is a real one and it is small:
-	// preflight refuses every name that is taken before the run starts, so
-	// what is left is somebody else creating our temporary name, with our
-	// process id in it, during the run. Trading a supported way of pointing
-	// the tool at a directory for that is the wrong way round.
-	fh, err := os.Create(tmp)
+	// What came back on 2026-09-06 is not that flag on its own. It is the
+	// pair: create exclusively, and believe the refusal only when os.Lstat
+	// says something is really there. The supported setup keeps working, and
+	// the window this file used to leave open closes with it.
+	//
+	// That window is small and it was the last one of its kind: preflight
+	// refuses every name that is taken before the run starts, so what was left
+	// is somebody creating our temporary name - with our process id in it -
+	// during the run, and a create that follows links putting the bytes
+	// wherever it pointed. Owner's call on 2026-09-06, after the same class
+	// was found unguarded in two other places. See core.CreateNew.
+	fh, err := core.CreateNew(tmp, 0o666)
 	if err != nil {
+		if errors.Is(err, fs.ErrExist) {
+			// In our own words. The fault is the one preflight names, arriving
+			// later than preflight can look.
+			return "", &CollisionError{Path: tmp}
+		}
 		return "", err
 	}