diff --git a/internal/observability/rename_noreplace.go b/internal/observability/rename_noreplace.go new file mode 100644 index 000000000..ef715cddc --- /dev/null +++ b/internal/observability/rename_noreplace.go @@ -0,0 +1,14 @@ +package observability + +import ( + "os" + + "github.com/Twigpine/zero/internal/pathjail" +) + +// renameNoReplace publishes a crash report without replacing an existing one. +// The implementation lives in pathjail because the file tools need the same +// atomic no-replace guarantee. +func renameNoReplace(root *os.Root, oldname, newname string) error { + return pathjail.RenameNoReplace(root, oldname, newname) +} diff --git a/internal/observability/rename_noreplace_darwin.go b/internal/observability/rename_noreplace_darwin.go deleted file mode 100644 index 7be01852b..000000000 --- a/internal/observability/rename_noreplace_darwin.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build darwin - -package observability - -import ( - "fmt" - "os" - - "golang.org/x/sys/unix" -) - -func renameNoReplace(root *os.Root, oldname, newname string) error { - directory, err := root.Open(".") - if err != nil { - return fmt.Errorf("open crash directory for publication: %w", err) - } - defer directory.Close() - return unix.RenameatxNp(int(directory.Fd()), oldname, int(directory.Fd()), newname, unix.RENAME_EXCL) -} diff --git a/internal/observability/rename_noreplace_linux.go b/internal/observability/rename_noreplace_linux.go deleted file mode 100644 index bab6dc4b8..000000000 --- a/internal/observability/rename_noreplace_linux.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build linux - -package observability - -import ( - "fmt" - "os" - - "golang.org/x/sys/unix" -) - -func renameNoReplace(root *os.Root, oldname, newname string) error { - directory, err := root.Open(".") - if err != nil { - return fmt.Errorf("open crash directory for publication: %w", err) - } - defer directory.Close() - return unix.Renameat2(int(directory.Fd()), oldname, int(directory.Fd()), newname, unix.RENAME_NOREPLACE) -} diff --git a/internal/pathjail/pathjail.go b/internal/pathjail/pathjail.go index 407dc71c3..af5ec598a 100644 --- a/internal/pathjail/pathjail.go +++ b/internal/pathjail/pathjail.go @@ -142,12 +142,22 @@ func trimTrailingSeparators(path string) string { var randomBytes = rand.Read // CreateTemp makes an O_EXCL temp file named prefix.suffix inside dir, -// relative to handle, and returns it with its relative path. +// relative to handle, and returns it with its relative path. The file is +// created 0o600 so bytes staged in it are not readable before publication. // // O_EXCL rather than a fixed name: a link planted at a predictable temp path // between the check and the create is the same race this package exists to // close, and O_EXCL makes the kernel refuse it rather than us. func CreateTemp(handle *os.Root, dir, prefix, suffix string) (*os.File, string, error) { + return CreateTempMode(handle, dir, prefix, suffix, 0o600) +} + +// CreateTempMode is CreateTemp with an explicit create mode. The mode is +// passed to the kernel's exclusive create, so the process umask still applies; +// callers that must publish the permissions a normal create would have been +// granted should read the returned file's mode rather than assigning perm +// later, because Chmod does not consult the umask. +func CreateTempMode(handle *os.Root, dir, prefix, suffix string, perm os.FileMode) (*os.File, string, error) { // prefix and suffix are NAME fragments, not path fragments. A separator or a // "."/".." in either would relocate the file out of dir — a prefix of // "../escaped", or a suffix of ".tmp/../evil" that also erases the random @@ -164,7 +174,7 @@ func CreateTemp(handle *os.Root, dir, prefix, suffix string) (*os.File, string, return nil, "", fmt.Errorf("generate a temporary name: %w", err) } relative := filepath.Join(dir, prefix+"."+hex.EncodeToString(noise)+suffix) - file, err := handle.OpenFile(relative, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o600) + file, err := handle.OpenFile(relative, os.O_RDWR|os.O_CREATE|os.O_EXCL, perm) if err == nil { return file, relative, nil } diff --git a/internal/pathjail/rename_noreplace.go b/internal/pathjail/rename_noreplace.go new file mode 100644 index 000000000..adc4eacc0 --- /dev/null +++ b/internal/pathjail/rename_noreplace.go @@ -0,0 +1,28 @@ +package pathjail + +import ( + "fmt" + "os" + "path/filepath" +) + +// RenameNoReplace renames oldname to newname within the directory bound by +// root, and refuses to replace an existing newname with the same atomic +// guarantee the kernel gives a create's exclusive open. A caller that loses +// the race receives fs.ErrExist and nothing at newname changes. +// +// Both names must be single path components: they are resolved relative to +// root's directory handle, so a separator (or an ancestor link) must not be +// able to redirect the syscall outside the checked root. Callers with a path +// in a subdirectory open that directory with Root.OpenRoot first. +// +// Platforms whose kernel cannot publish without replacing fall back to +// ErrUnsupported, which lets the caller use a no-replace hard link instead. +func RenameNoReplace(root *os.Root, oldname, newname string) error { + for _, name := range []string{oldname, newname} { + if name == "" || name == "." || name == ".." || filepath.Base(name) != name { + return fmt.Errorf("%w: rename name %q must be a single path component", ErrEscapes, name) + } + } + return renameNoReplace(root, oldname, newname) +} diff --git a/internal/pathjail/rename_noreplace_darwin.go b/internal/pathjail/rename_noreplace_darwin.go new file mode 100644 index 000000000..7866d939b --- /dev/null +++ b/internal/pathjail/rename_noreplace_darwin.go @@ -0,0 +1,22 @@ +//go:build darwin + +package pathjail + +import ( + "fmt" + "os" + + "golang.org/x/sys/unix" +) + +func renameNoReplace(root *os.Root, oldname, newname string) error { + directory, err := root.Open(".") + if err != nil { + return fmt.Errorf("open directory for rename: %w", err) + } + defer directory.Close() + if err := unix.RenameatxNp(int(directory.Fd()), oldname, int(directory.Fd()), newname, unix.RENAME_EXCL); err != nil { + return fmt.Errorf("rename %s to %s: %w", oldname, newname, err) + } + return nil +} diff --git a/internal/pathjail/rename_noreplace_linux.go b/internal/pathjail/rename_noreplace_linux.go new file mode 100644 index 000000000..4adb2d599 --- /dev/null +++ b/internal/pathjail/rename_noreplace_linux.go @@ -0,0 +1,22 @@ +//go:build linux + +package pathjail + +import ( + "fmt" + "os" + + "golang.org/x/sys/unix" +) + +func renameNoReplace(root *os.Root, oldname, newname string) error { + directory, err := root.Open(".") + if err != nil { + return fmt.Errorf("open directory for rename: %w", err) + } + defer directory.Close() + if err := unix.Renameat2(int(directory.Fd()), oldname, int(directory.Fd()), newname, unix.RENAME_NOREPLACE); err != nil { + return fmt.Errorf("rename %s to %s: %w", oldname, newname, err) + } + return nil +} diff --git a/internal/observability/rename_noreplace_other.go b/internal/pathjail/rename_noreplace_other.go similarity index 87% rename from internal/observability/rename_noreplace_other.go rename to internal/pathjail/rename_noreplace_other.go index ee43bc1b2..4839ad303 100644 --- a/internal/observability/rename_noreplace_other.go +++ b/internal/pathjail/rename_noreplace_other.go @@ -1,6 +1,6 @@ //go:build !darwin && !linux && !windows -package observability +package pathjail import ( "errors" diff --git a/internal/pathjail/rename_noreplace_test.go b/internal/pathjail/rename_noreplace_test.go new file mode 100644 index 000000000..223d14d2d --- /dev/null +++ b/internal/pathjail/rename_noreplace_test.go @@ -0,0 +1,79 @@ +package pathjail + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +func openRenameRoot(t *testing.T) (*os.Root, string) { + t.Helper() + base := t.TempDir() + root, err := os.OpenRoot(base) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = root.Close() }) + return root, base +} + +// A CREATE'S NAME MUST NOT BE LOST TO A RACE. The no-replace rename reports the +// collision as fs.ErrExist and leaves both files exactly as they were, so the +// caller can refuse the create instead of replacing the other writer. +func TestRenameNoReplaceRefusesAnExistingDestination(t *testing.T) { + root, base := openRenameRoot(t) + if err := os.WriteFile(filepath.Join(base, "source"), []byte("source\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(base, "target"), []byte("target\n"), 0o644); err != nil { + t.Fatal(err) + } + + if err := RenameNoReplace(root, "source", "target"); !errors.Is(err, os.ErrExist) { + t.Fatalf("RenameNoReplace over an existing name = %v, want os.ErrExist", err) + } + for name, want := range map[string]string{"source": "source\n", "target": "target\n"} { + got, err := os.ReadFile(filepath.Join(base, name)) + if err != nil || string(got) != want { + t.Fatalf("%s = %q, err=%v; a refused rename must not move either file", name, got, err) + } + } +} + +func TestRenameNoReplacePublishesAnAbsentDestination(t *testing.T) { + root, base := openRenameRoot(t) + if err := os.WriteFile(filepath.Join(base, "source"), []byte("whole\n"), 0o644); err != nil { + t.Fatal(err) + } + + if err := RenameNoReplace(root, "source", "target"); err != nil { + t.Fatalf("RenameNoReplace = %v", err) + } + if got, err := os.ReadFile(filepath.Join(base, "target")); err != nil || string(got) != "whole\n" { + t.Fatalf("target = %q, err=%v", got, err) + } + if _, err := os.Stat(filepath.Join(base, "source")); !os.IsNotExist(err) { + t.Fatalf("source survived a successful rename: %v", err) + } +} + +// The syscall is given a directory handle and bare names. Allowing a +// separator here would let an ancestor component resolve outside the directory +// the caller checked, so it is refused before the syscall. +func TestRenameNoReplaceRejectsPathComponents(t *testing.T) { + root, _ := openRenameRoot(t) + for _, tc := range []struct{ oldname, newname string }{ + {"", "target"}, + {"source", ""}, + {".", "target"}, + {"source", ".."}, + {"dir/source", "target"}, + {"source", "dir/target"}, + {filepath.Join("dir", "source"), "target"}, + } { + if err := RenameNoReplace(root, tc.oldname, tc.newname); !errors.Is(err, ErrEscapes) { + t.Errorf("RenameNoReplace(%q, %q) = %v, want ErrEscapes", tc.oldname, tc.newname, err) + } + } +} diff --git a/internal/observability/rename_noreplace_windows.go b/internal/pathjail/rename_noreplace_windows.go similarity index 99% rename from internal/observability/rename_noreplace_windows.go rename to internal/pathjail/rename_noreplace_windows.go index 7c19df596..01baf71c7 100644 --- a/internal/observability/rename_noreplace_windows.go +++ b/internal/pathjail/rename_noreplace_windows.go @@ -1,6 +1,6 @@ //go:build windows -package observability +package pathjail import ( "errors" diff --git a/internal/tools/file_commit.go b/internal/tools/file_commit.go index 287ef68ed..c019b095f 100644 --- a/internal/tools/file_commit.go +++ b/internal/tools/file_commit.go @@ -5,6 +5,11 @@ import ( "fmt" "io" "os" + "path/filepath" + "strings" + "syscall" + + "github.com/Twigpine/zero/internal/pathjail" ) var errFileChangedDuringWrite = errors.New("file changed on disk before the write committed") @@ -14,11 +19,39 @@ var errFileChangedDuringWrite = errors.New("file changed on disk before the writ // object that will actually be mutated. var fileWriteBeforeCommit func(path string) +// fileWriteBeforePublish is a deterministic test hook that runs after the +// replacement bytes have been staged and before the destination's identity is +// re-checked for the publish. Tests use it to install a competing writer at the +// last point before publication. +var fileWriteBeforePublish func(path string) + // fileWriteStat is a deterministic test seam for proving that the opened-file // identity is captured before the final preimage comparison. Production uses // the file descriptor directly. var fileWriteStat = func(file *os.File) (os.FileInfo, error) { return file.Stat() } +// fileWriteTempWrite is a deterministic test seam for a failure while the +// replacement bytes are written to the side file, after the observed file's +// identity and preimage have been accepted. Production writes through +// io.WriteString. The failure it stands in for (a full disk, a failing +// device) is exactly the moment a truncate-then-write has already destroyed +// the observed file. +var fileWriteTempWrite = func(file *os.File, content string) error { + _, err := io.WriteString(file, content) + return err +} + +// fileWriteRename publishes the staged side file over the destination. +// Tests replace it to exercise the in-place fallback deterministically. +var fileWriteRename = func(root *os.Root, oldName, newName string) error { + return root.Rename(oldName, newName) +} + +// fileWriteNoReplaceRename publishes a create without replacing an existing +// name. Tests replace it to exercise the hard-link fallback that filesystems +// without a no-replace rename use. +var fileWriteNoReplaceRename = publishRootedNoReplace + // commitRootedFileContents binds an overwrite to the file identity and bytes // that the caller observed, opening every component through an already-open // *os.Root descriptor rather than re-resolving the absolute path. Because the @@ -26,11 +59,28 @@ var fileWriteStat = func(file *os.File) (os.FileInfo, error) { return file.Stat( // swapped for a symlink that escapes the workspace between validation and this // call is refused by the kernel instead of redirecting the write. // -// A create uses exclusive creation THROUGH THE ROOT. An overwrite opens the -// observed object without truncation, verifies identity/content through that -// handle, then truncates and writes the same handle. relativePath is the target -// expressed relative to root; absolutePath is retained only for the test hook -// and error reporting. +// Both publishes are atomic against a concurrent reader: the replacement bytes +// are written whole to a side file in the target's directory and the target is +// swapped wholesale. A create publishes through a kernel no-replace rename, so +// losing the create race refuses instead of replacing the raced file. An +// overwrite publishes through a rename after re-checking that the destination +// still names the observed object; that check is the last operation before the +// rename, but no supported platform offers a compare-and-swap rename, so a +// writer that replaces the name between the check and the syscall can still be +// replaced. A failure at any point before the publish removes the side file +// and leaves the observed bytes standing — the caller's file outlives a write +// that could not complete, and no partial state is ever visible at the path +// itself. +// +// A final-component symlink keeps main's behaviour: the link stays a link and +// the resolved file is what is replaced. A hard link is detached by the rename +// instead of rewritten through the shared inode. Where the platform refuses a +// rename (notably Windows with an open reader), the write falls back to the +// previous in-place publish through a freshly validated handle; that is the +// only case in which the preimage is not protected by staging. +// +// relativePath is the target expressed relative to root; absolutePath is +// retained only for the test hooks and error reporting. // // expectedInfo nil means the caller observed a missing path. expectedContent // may be nil for an existing but unreadable file; that path may still be @@ -41,16 +91,7 @@ func commitRootedFileContents(root *os.Root, absolutePath, relativePath string, } if expectedInfo == nil { - file, err := root.OpenFile(relativePath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) - if err != nil { - return err - } - openedInfo, err := fileWriteStat(file) - if err != nil { - _ = file.Close() - return err - } - return writeAndVerifyRootedFileIdentity(root, relativePath, file, openedInfo, content, false) + return commitRootedFileCreate(root, relativePath, content) } flags := os.O_WRONLY @@ -86,24 +127,246 @@ func commitRootedFileContents(root *os.Root, absolutePath, relativePath string, return errFileChangedDuringWrite } } - return writeAndVerifyRootedFileIdentity(root, relativePath, file, openedInfo, content, true) -} -func writeAndVerifyRootedFileIdentity(root *os.Root, relativePath string, file *os.File, openedInfo os.FileInfo, content string, truncate bool) error { - if truncate { - if err := file.Truncate(0); err != nil { + // A device, FIFO or socket is not a file this path should replace with a + // regular one, and the previous in-place write is the only publish that + // keeps its semantics. Regular files (including symlink targets) go through + // staging and rename. + if !openedInfo.Mode().IsRegular() { + return writeRootedFileInPlace(root, relativePath, file, openedInfo, expectedContent, content) + } + mode := openedInfo.Mode().Perm() + publishPath, err := resolveRootedFinalPath(root, relativePath) + if err != nil { + _ = file.Close() + return err + } + if publishPath != relativePath { + targetInfo, statErr := root.Stat(publishPath) + if statErr != nil || !os.SameFile(openedInfo, targetInfo) { _ = file.Close() - return err + return fmt.Errorf("%w: link target changed", errFileChangedDuringWrite) } - if _, err := file.Seek(0, io.SeekStart); err != nil { - _ = file.Close() + } + if err := file.Close(); err != nil { + return err + } + return commitRootedFileOverwrite(root, absolutePath, relativePath, publishPath, openedInfo, mode, expectedContent, content) +} + +// commitRootedFileCreate publishes a brand-new file whose absence the caller +// observed. The bytes are written whole to a private side file beside the +// target and handed to the name with a kernel no-replace rename: the create is +// either absent or the whole file, a concurrent create is refused rather than +// replaced, and a write that cannot complete leaves no file at all. +func commitRootedFileCreate(root *os.Root, relativePath, content string) error { + tempName, err := writeRootedSideFile(root, relativePath, 0o644, true, content) + if err != nil { + return err + } + if err := publishRootedCreate(root, tempName, relativePath); err != nil { + return errors.Join(err, removeWriteSideFile(root, tempName)) + } + return nil +} + +// publishRootedCreate hands the staged side file to a name that must not +// already exist. RenameNoReplace is the primary primitive; a hard link is the +// no-replace fallback for filesystems whose rename cannot promise it. Neither +// fallback copies into a visible target, so a reader sees only the absent name +// or the complete file. +func publishRootedCreate(root *os.Root, tempName, relativePath string) error { + renameErr := fileWriteNoReplaceRename(root, tempName, relativePath) + if renameErr == nil { + return nil + } + if errors.Is(renameErr, os.ErrExist) { + return fmt.Errorf("creating %s: %w", relativePath, renameErr) + } + if !renameNoReplaceUnsupported(renameErr) { + return fmt.Errorf("creating %s: %w", relativePath, renameErr) + } + if linkErr := root.Link(tempName, relativePath); linkErr != nil { + return fmt.Errorf("creating %s: %w", relativePath, errors.Join(renameErr, linkErr)) + } + if cleanupErr := removeWriteSideFile(root, tempName); cleanupErr != nil { + return fmt.Errorf("creating %s: published, but removing the staging name failed: %w", relativePath, cleanupErr) + } + return nil +} + +// publishRootedNoReplace performs the no-replace rename through a root handle +// bound to the target's own directory. Binding the directory first is what +// keeps the raw rename inside the workspace: a parent link swapped after the +// caller resolved the path is refused when the directory is opened, and both +// names are single components below that handle. +func publishRootedNoReplace(root *os.Root, tempName, relativePath string) error { + parent := filepath.Dir(relativePath) + if filepath.Dir(tempName) != parent { + return fmt.Errorf("staging file %s is not beside %s", tempName, relativePath) + } + directory, err := root.OpenRoot(parent) + if err != nil { + return err + } + defer directory.Close() + return pathjail.RenameNoReplace(directory, filepath.Base(tempName), filepath.Base(relativePath)) +} + +// renameNoReplaceUnsupported reports whether the platform or filesystem told +// us only that it cannot perform a no-replace rename, so the no-replace hard +// link is the right fallback. A collision is not an unsupported operation and +// is handled by the caller before this is consulted. +func renameNoReplaceUnsupported(err error) bool { + return errors.Is(err, errors.ErrUnsupported) || errors.Is(err, syscall.EINVAL) +} + +// commitRootedFileOverwrite publishes a replacement for an object whose +// identity and preimage just passed validation: the bytes are written to a +// side file beside the target, the observed identity is re-checked against the +// path as late as the rename allows, and a rename hands the path over in one +// step. A failure while the side file is being written — ENOSPC, a failing +// device — therefore costs a removed side file, not the observed file. +// +// If the platform refuses the rename (most commonly a Windows reader holding +// the destination open), the previous in-place publish is used after reopening +// and re-validating the same object. That fallback cannot be atomic, but it is +// the only way to write a file another process has open without delete +// sharing, and it is what this tool did before staging existed. +func commitRootedFileOverwrite(root *os.Root, absolutePath, relativePath, publishPath string, openedInfo os.FileInfo, mode os.FileMode, expectedContent *string, content string) error { + tempName, err := writeRootedSideFile(root, publishPath, mode, false, content) + if err != nil { + return err + } + if fileWriteBeforePublish != nil { + fileWriteBeforePublish(absolutePath) + } + if err := verifyRootedPublishTarget(root, publishPath, openedInfo); err != nil { + _ = removeWriteSideFile(root, tempName) + return err + } + if err := fileWriteRename(root, tempName, publishPath); err != nil { + renameErr := fmt.Errorf("writing %s: atomic replacement refused: %w", relativePath, err) + fallbackErr := rewriteRootedFileInPlace(root, relativePath, openedInfo, expectedContent, content) + cleanupErr := removeWriteSideFile(root, tempName) + if fallbackErr == nil { + if cleanupErr != nil { + return fmt.Errorf("%w: the file was written in place, but removing the staged replacement failed: %v", renameErr, cleanupErr) + } + return nil + } + return errors.Join(renameErr, fmt.Errorf("writing %s: in-place fallback failed: %w", relativePath, fallbackErr), cleanupErr) + } + return nil +} + +// verifyRootedPublishTarget refuses to replace anything but the exact object +// whose identity and preimage were validated. It is deliberately the last +// operation before the rename; no portable rename offers a compare-and-swap, +// so this check is the strongest condition available before an atomic replace. +func verifyRootedPublishTarget(root *os.Root, publishPath string, openedInfo os.FileInfo) error { + info, err := root.Lstat(publishPath) + if err != nil || info.Mode()&(os.ModeSymlink|os.ModeIrregular) != 0 || !os.SameFile(openedInfo, info) { + return fmt.Errorf("%w: path identity changed", errFileChangedDuringWrite) + } + return nil +} + +// resolveRootedFinalPath follows final-component symlinks inside the root and +// returns the path of the file the caller's link resolves to. Publishing over +// that path preserves the link, unlike a rename over the link itself. +func resolveRootedFinalPath(root *os.Root, relativePath string) (string, error) { + current := relativePath + for links := 0; links < 255; links++ { + info, err := root.Lstat(current) + if err != nil { + return "", err + } + if info.Mode()&os.ModeSymlink == 0 { + return current, nil + } + target, err := root.Readlink(current) + if err != nil { + return "", err + } + if filepath.IsAbs(target) || filepath.VolumeName(target) != "" { + return "", fmt.Errorf("%w: %s is an absolute link", errFileChangedDuringWrite, current) + } + next := filepath.Clean(filepath.Join(filepath.Dir(current), target)) + if next == ".." || strings.HasPrefix(next, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("%w: %s points outside the workspace", errFileChangedDuringWrite, current) + } + current = next + } + return "", fmt.Errorf("%w: more than 255 links at %s", errFileChangedDuringWrite, relativePath) +} + +// writeRootedSideFile writes content to a fresh side file beside the target, +// relative to the root handle, and leaves it closed and ready to publish. The +// caller owns removing the side file when the publish that follows does not +// commit. +// +// umaskDerived selects the permission contract. A create asks the kernel for +// 0o644 and remembers what the process umask actually granted; the bytes are +// made owner-only while they are staged, then the granted bits are restored. +// An overwrite stages 0o600 and finally carries the observed file's permission +// bits over, so the published file looks like the one it replaced. +func writeRootedSideFile(root *os.Root, relativePath string, mode os.FileMode, umaskDerived bool, content string) (string, error) { + createMode := mode.Perm() + if !umaskDerived { + createMode = 0o600 + } + temp, tempName, err := pathjail.CreateTempMode(root, filepath.Dir(relativePath), "zero-write", ".tmp", createMode) + if err != nil { + return "", fmt.Errorf("writing %s: %w", relativePath, err) + } + granted := mode.Perm() + if umaskDerived { + info, statErr := temp.Stat() + if statErr != nil { + _ = temp.Close() + _ = removeWriteSideFile(root, tempName) + return "", fmt.Errorf("writing %s: %w", relativePath, statErr) + } + granted = info.Mode().Perm() + if chmodErr := temp.Chmod(0o600); chmodErr != nil { + _ = temp.Close() + _ = removeWriteSideFile(root, tempName) + return "", fmt.Errorf("writing %s: %w", relativePath, chmodErr) + } + } + writeErr := func() error { + if err := fileWriteTempWrite(temp, content); err != nil { return err } + return temp.Chmod(granted.Perm()) + }() + closeErr := temp.Close() + if writeErr != nil || closeErr != nil { + _ = removeWriteSideFile(root, tempName) + return "", fmt.Errorf("writing %s: %w", relativePath, errors.Join(writeErr, closeErr)) } - if _, err := io.WriteString(file, content); err != nil { + return tempName, nil +} + +// writeRootedFileInPlace publishes through an already-validated handle. It is +// the non-regular and rename-refused fallback, so it keeps the previous +// truncate-and-write behaviour and restores the observed bytes if a write +// fails and the preimage is available. +func writeRootedFileInPlace(root *os.Root, relativePath string, file *os.File, openedInfo os.FileInfo, expectedContent *string, content string) error { + if err := file.Truncate(0); err != nil { _ = file.Close() return err } + if _, err := file.Seek(0, io.SeekStart); err != nil { + _ = file.Close() + return err + } + if _, err := io.WriteString(file, content); err != nil { + restoreErr := restoreRootedFile(file, expectedContent) + closeErr := file.Close() + return errors.Join(err, restoreErr, closeErr) + } if err := file.Close(); err != nil { return err } @@ -113,3 +376,84 @@ func writeAndVerifyRootedFileIdentity(root *os.Root, relativePath string, file * } return nil } + +// rewriteRootedFileInPlace reopens the observed path, re-validates identity +// and preimage through that handle, and only then falls back to an in-place +// write. A replacement installed during the refused publish is therefore not +// touched. +func rewriteRootedFileInPlace(root *os.Root, relativePath string, expectedInfo os.FileInfo, expectedContent *string, content string) error { + flags := os.O_WRONLY + if expectedContent != nil { + flags = os.O_RDWR + } + file, err := root.OpenFile(relativePath, flags, 0) + if err != nil { + return err + } + openedInfo, err := file.Stat() + if err != nil { + _ = file.Close() + return err + } + if !os.SameFile(expectedInfo, openedInfo) { + _ = file.Close() + return errFileChangedDuringWrite + } + pathInfo, err := root.Stat(relativePath) + if err != nil || !os.SameFile(openedInfo, pathInfo) { + _ = file.Close() + return errFileChangedDuringWrite + } + if expectedContent != nil { + current, readErr := io.ReadAll(file) + if readErr != nil { + _ = file.Close() + return readErr + } + if string(current) != *expectedContent { + _ = file.Close() + return errFileChangedDuringWrite + } + } + return writeRootedFileInPlace(root, relativePath, file, openedInfo, expectedContent, content) +} + +// restoreRootedFile puts the observed bytes back after a failed in-place +// write. The write already truncated the file, so this is best effort: the +// caller receives both the original failure and any restore failure. +func restoreRootedFile(file *os.File, preimage *string) error { + if preimage == nil { + return nil + } + if err := file.Truncate(0); err != nil { + return fmt.Errorf("restoring the preimage: %w", err) + } + if _, err := file.Seek(0, io.SeekStart); err != nil { + return fmt.Errorf("restoring the preimage: %w", err) + } + if _, err := io.WriteString(file, *preimage); err != nil { + return fmt.Errorf("restoring the preimage: %w", err) + } + return nil +} + +// removeWriteSideFile removes a staged replacement that never reached (or did +// not survive) its publish, through the root handle. +func removeWriteSideFile(root *os.Root, name string) error { + err := root.Remove(name) + if err == nil || errors.Is(err, os.ErrNotExist) { + return nil + } + // Windows refuses to delete a read-only file. The staged file only carries + // that bit after its published mode was applied; clear it through the root + // and retry. The name belongs to a random side file this commit created. + if errors.Is(err, os.ErrPermission) { + if chmodErr := root.Chmod(name, 0o600); chmodErr == nil { + err = root.Remove(name) + if err == nil || errors.Is(err, os.ErrNotExist) { + return nil + } + } + } + return err +} diff --git a/internal/tools/file_commit_publish_test.go b/internal/tools/file_commit_publish_test.go new file mode 100644 index 000000000..35363cbb6 --- /dev/null +++ b/internal/tools/file_commit_publish_test.go @@ -0,0 +1,283 @@ +package tools + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +func installFileWriteRename(t *testing.T, rename func(*os.Root, string, string) error) { + t.Helper() + prior := fileWriteRename + fileWriteRename = rename + t.Cleanup(func() { fileWriteRename = prior }) +} + +func installFileWriteBeforePublish(t *testing.T, hook func(string)) { + t.Helper() + prior := fileWriteBeforePublish + fileWriteBeforePublish = hook + t.Cleanup(func() { fileWriteBeforePublish = prior }) +} + +func installFileWriteNoReplaceRename(t *testing.T, rename func(*os.Root, string, string) error) { + t.Helper() + prior := fileWriteNoReplaceRename + fileWriteNoReplaceRename = rename + t.Cleanup(func() { fileWriteNoReplaceRename = prior }) +} + +// A FINAL-COMPONENT SYMLINK STAYS A LINK. Main opened the target through the +// link and wrote through it, so replacing the link with a regular file leaves +// the link's target stuck on its old bytes. Publishing over the resolved path +// preserves both halves: the link is still a link, and the file it names has +// the new content. +// +// The public tools reject a symlink during path validation today, so this is +// asserted at the commit layer, which is where the publish semantics live and +// where a scoped write that resolves through an extra root reaches them. +func TestOverwriteKeepsAnInWorkspaceSymlink(t *testing.T) { + for _, tc := range []struct { + name string + chain bool + }{ + {name: "direct link"}, + {name: "link chain", chain: true}, + } { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "target.txt") + if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil { + t.Fatal(err) + } + link := filepath.Join(root, "link.txt") + if err := os.Symlink("target.txt", link); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + publishPath, links := "link.txt", []string{link} + if tc.chain { + chained := filepath.Join(root, "chained.txt") + if err := os.Symlink("link.txt", chained); err != nil { + t.Fatalf("create second symlink: %v", err) + } + publishPath, links = "chained.txt", append(links, chained) + } + + workspace, err := os.OpenRoot(root) + if err != nil { + t.Fatal(err) + } + defer workspace.Close() + expectedInfo, err := workspace.Stat(publishPath) + if err != nil { + t.Fatal(err) + } + preimage := "observed\n" + if err := commitRootedFileContents(workspace, filepath.Join(root, publishPath), publishPath, expectedInfo, &preimage, "zero\n"); err != nil { + t.Fatalf("symlink overwrite: %v", err) + } + for _, path := range links { + info, err := os.Lstat(path) + if err != nil { + t.Fatalf("inspect %s: %v", path, err) + } + if info.Mode()&os.ModeSymlink == 0 { + t.Fatalf("%s is no longer a symlink: mode=%v", path, info.Mode()) + } + } + if got, err := os.ReadFile(target); err != nil || string(got) != "zero\n" { + t.Fatalf("link target = %q, err=%v; the link must keep pointing at the rewritten file", got, err) + } + entries, err := os.ReadDir(root) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + if strings.HasSuffix(entry.Name(), ".tmp") { + t.Fatalf("staging file %s survived a successful publish", entry.Name()) + } + } + }) + } +} + +// A READER CAN HOLD THE FILE OPEN. Windows refuses a rename over a name while +// another handle has it open without delete sharing, so the atomic publish is +// not available there; the commit must fall back to writing through the +// validated handle rather than failing an ordinary overwrite. The same test +// also runs on the rename-capable platforms, where it proves the common path. +func TestOverwriteSucceedsWithAnOpenReader(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "existing.txt") + if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil { + t.Fatal(err) + } + reader, err := os.Open(target) + if err != nil { + t.Fatal(err) + } + defer reader.Close() + + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "existing.txt", "content": "zero\n", "overwrite": true, + }) + if result.Status != StatusOK { + t.Fatalf("overwrite with an open reader = %s: %s", result.Status, result.Output) + } + if got, err := os.ReadFile(target); err != nil || string(got) != "zero\n" { + t.Fatalf("target = %q, err=%v", got, err) + } +} + +// WHEN THE PLATFORM REFUSES THE REPLACEMENT, the previous in-place publish is +// still available. The fallback reopens and re-validates the object first, so +// this is not a licence to write into whatever currently carries the name. +func TestOverwriteFallsBackToInPlaceWhenReplacementIsRefused(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "existing.txt") + if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil { + t.Fatal(err) + } + installFileWriteRename(t, func(*os.Root, string, string) error { + return errors.New("replacement refused") + }) + + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "existing.txt", "content": "zero\n", "overwrite": true, + }) + if result.Status != StatusOK { + t.Fatalf("fallback overwrite = %s: %s", result.Status, result.Output) + } + if got, err := os.ReadFile(target); err != nil || string(got) != "zero\n" { + t.Fatalf("fallback target = %q, err=%v", got, err) + } + entries, err := os.ReadDir(root) + if err != nil { + t.Fatal(err) + } + if len(entries) != 1 || entries[0].Name() != "existing.txt" { + t.Fatalf("fallback left %d entries; the staged replacement must be removed", len(entries)) + } +} + +// THE FALLBACK IS NOT A REPLACEMENT LICENCE. A competing writer that installs +// a new file during the refused publish must be detected by identity before +// the fallback truncates anything, even when the bytes are identical. +func TestOverwriteFallbackRefusesAReplacedTarget(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "existing.txt") + if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil { + t.Fatal(err) + } + installFileWriteRename(t, func(*os.Root, string, string) error { + preserved := target + ".preserved" + if err := os.Rename(target, preserved); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil { + t.Fatal(err) + } + return errors.New("replacement refused") + }) + + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "existing.txt", "content": "zero\n", "overwrite": true, + }) + if result.Status != StatusError || !strings.Contains(result.Output, errFileChangedDuringWrite.Error()) { + t.Fatalf("replaced fallback target = %s: %s", result.Status, result.Output) + } + if got, err := os.ReadFile(target); err != nil || string(got) != "observed\n" { + t.Fatalf("replacement = %q, err=%v; the fallback must not truncate a file it did not validate", got, err) + } +} + +// A FILESYSTEM MAY NOT OFFER A NO-REPLACE RENAME. The hard-link fallback is +// still atomic against readers — a reader sees the absent name or the complete +// inode — and refuses a name another writer created. +func TestCreateFallsBackToAHardLinkWhenNoReplaceRenameIsUnsupported(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "created.txt") + installFileWriteNoReplaceRename(t, func(*os.Root, string, string) error { + return errors.ErrUnsupported + }) + + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "created.txt", "content": "zero\n", + }) + if result.Status != StatusOK { + t.Fatalf("hard-link create = %s: %s", result.Status, result.Output) + } + if got, err := os.ReadFile(target); err != nil || string(got) != "zero\n" { + t.Fatalf("created file = %q, err=%v", got, err) + } + entries, err := os.ReadDir(root) + if err != nil { + t.Fatal(err) + } + if len(entries) != 1 || entries[0].Name() != "created.txt" { + t.Fatalf("root holds %d entries; the staging name must not survive the publish", len(entries)) + } +} + +func TestCreateHardLinkFallbackStillRefusesAnExistingName(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "existing.txt") + if err := os.WriteFile(target, []byte("other writer\n"), 0o644); err != nil { + t.Fatal(err) + } + installFileWriteNoReplaceRename(t, func(*os.Root, string, string) error { + return errors.ErrUnsupported + }) + + workspace, err := os.OpenRoot(root) + if err != nil { + t.Fatal(err) + } + defer workspace.Close() + if err := commitRootedFileContents(workspace, target, "existing.txt", nil, nil, "zero\n"); !errors.Is(err, os.ErrExist) { + t.Fatalf("hard-link create over an existing name = %v, want os.ErrExist", err) + } + if got, err := os.ReadFile(target); err != nil || string(got) != "other writer\n" { + t.Fatalf("existing file = %q, err=%v; the fallback must not replace it", got, err) + } + entries, err := os.ReadDir(root) + if err != nil { + t.Fatal(err) + } + if len(entries) != 1 || entries[0].Name() != "existing.txt" { + t.Fatalf("root holds %d entries; a refused create must remove its staging file", len(entries)) + } +} + +// A REPLACEMENT INSTALLED AFTER THE BYTES WERE STAGED must not be silently +// overwritten by the publish. The identity check immediately before the rename +// is what keeps the conflict guarantee for the window that remains. +func TestOverwriteRefusesAReplacementInstalledBeforePublish(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "existing.txt") + if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil { + t.Fatal(err) + } + installFileWriteBeforePublish(t, func(path string) { + preserved := path + ".preserved" + if err := os.Rename(path, preserved); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("competing\n"), 0o644); err != nil { + t.Fatal(err) + } + }) + + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "existing.txt", "content": "zero\n", "overwrite": true, + }) + if result.Status != StatusError || !strings.Contains(result.Output, errFileChangedDuringWrite.Error()) { + t.Fatalf("pre-publish replacement = %s: %s", result.Status, result.Output) + } + if got, err := os.ReadFile(target); err != nil || string(got) != "competing\n" { + t.Fatalf("competing file = %q, err=%v; the publish must refuse a replacement it did not observe", got, err) + } +} diff --git a/internal/tools/file_commit_test.go b/internal/tools/file_commit_test.go index c6baa21b7..b21f19b9d 100644 --- a/internal/tools/file_commit_test.go +++ b/internal/tools/file_commit_test.go @@ -350,3 +350,68 @@ func TestWriteFileRefusesSameNameSubstitutionWithIdenticalBytes(t *testing.T) { t.Fatalf("substituted object = %q, err=%v; the write must not land in an object the preimage did not observe", got, err) } } + +// AN OVERWRITTEN PATH'S NEIGHBOURS THROUGH ANOTHER NAME KEEP THEIR CONTENT. +// +// The replace-overwrite truncates and rewrites the observed file in place, and +// truncation follows the inode: a sibling reached through a hard link loses +// its content when this path's content is replaced, because both names were +// pointing at the same bytes. Publishing through a rename detaches this name +// from the shared inode instead, so the link sibling keeps what it shares. +func TestOverwriteDoesNotTruncateAHardlinkSibling(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "target.txt") + sibling := filepath.Join(root, "sibling.txt") + if err := os.WriteFile(target, []byte("shared\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Link(target, sibling); err != nil { + t.Skipf("hard links unavailable: %v", err) + } + + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "target.txt", "content": "replaced\n", "overwrite": true, + }) + if result.Status != StatusOK { + t.Fatalf("overwrite status = %s: %s", result.Status, result.Output) + } + shared, err := os.ReadFile(sibling) + if err != nil || string(shared) != "shared\n" { + t.Fatalf("hard-link sibling = %q, err=%v; replacing this name must not rewrite the bytes the sibling shares", shared, err) + } +} + +// THE PUBLISHED FILE LOOKS LIKE THE FILE IT REPLACED. The permission bits are +// carried over from the file that was observed, not from the side file the +// bytes were staged in. +func TestOverwriteKeepsTheObservedPermissionBits(t *testing.T) { + // 0o640 differs from the 0o600 the side file is staged with, so a publish + // that forgot to carry the observed bits cannot pass by coincidence. + want := os.FileMode(0o640) + if runtime.GOOS == "windows" { + // A Windows file carries only the read-only bit, so a 0o640 file is + // reported as 0o666; the write bit, not the exact value, is the fact + // being pinned. + want = os.FileMode(0o666) + } + + root := t.TempDir() + target := filepath.Join(root, "existing.txt") + if err := os.WriteFile(target, []byte("observed\n"), want); err != nil { + t.Fatal(err) + } + + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "existing.txt", "content": "zero\n", "overwrite": true, + }) + if result.Status != StatusOK { + t.Fatalf("overwrite status = %s: %s", result.Status, result.Output) + } + info, err := os.Stat(target) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != want { + t.Fatalf("published permission bits = %o, want %o", got, want) + } +} diff --git a/internal/tools/file_commit_umask_test.go b/internal/tools/file_commit_umask_test.go new file mode 100644 index 000000000..a3b82705c --- /dev/null +++ b/internal/tools/file_commit_umask_test.go @@ -0,0 +1,56 @@ +//go:build unix + +package tools + +import ( + "context" + "os" + "path/filepath" + "syscall" + "testing" +) + +// THE PUBLISHED FILE CARRIES THE PERMISSIONS THE PROCESS UMASK PRODUCES, NOT +// THE RAW MODE AN ASSIGNMENT WOULD SET. +// +// The create path opens its staging file with the same exclusive 0o644 create +// this branch of the write path always honored, and the kernel applies the +// process umask to that open. The publish must carry the granted bits — taken +// from the staging file itself — because a chmod never consults the umask: a +// process that requests tight defaults on new files would find every write it +// publishes shared with the world. Under the restrictive umask a plain mode +// assignment would publish 0o644 where the kernel granted 0o600, so the test +// fails before the granted bits are carried. +func TestCreatePublishesTheUmaskedPermissionsOfTheStagedCreate(t *testing.T) { + for name, testCase := range map[string]struct { + umask int + want os.FileMode + }{ + "a restrictive umask keeps every stranger out": {umask: 0o077, want: 0o600}, + "a permissive umask keeps the readable bits": {umask: 0o022, want: 0o644}, + } { + t.Run(name, func(t *testing.T) { + previous := syscall.Umask(testCase.umask) + defer syscall.Umask(previous) + root := t.TempDir() + target := filepath.Join(root, "created.txt") + + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "created.txt", "content": "zero\n", + }) + if result.Status != StatusOK { + t.Fatalf("create status = %s: %s", result.Status, result.Output) + } + info, err := os.Stat(target) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != testCase.want { + t.Fatalf("published mode = %#o, want the %#o the kernel granted the staging create under this umask; a raw mode assignment would publish %#o and share the file with hands the umask excludes", got, testCase.want, os.FileMode(0o644)) + } + if got, readErr := os.ReadFile(target); readErr != nil || string(got) != "zero\n" { + t.Fatalf("published content = %q, err=%v", got, readErr) + } + }) + } +} diff --git a/internal/tools/file_commit_write_failure_test.go b/internal/tools/file_commit_write_failure_test.go new file mode 100644 index 000000000..640151314 --- /dev/null +++ b/internal/tools/file_commit_write_failure_test.go @@ -0,0 +1,128 @@ +package tools + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" +) + +func installFileWriteTempWrite(t *testing.T, write func(*os.File, string) error) { + t.Helper() + prior := fileWriteTempWrite + fileWriteTempWrite = write + t.Cleanup(func() { fileWriteTempWrite = prior }) +} + +// A WRITE THAT CANNOT COMPLETE MUST NOT TAKE THE FILE IT REPLACED WITH IT. +// +// Running out of disk space part-way through a write used to be fatal to the +// preimage: the overwrite truncated the observed file first, so the failed +// write left an empty husk where a working file stood. The replacement bytes +// are now written whole to a side file, so a failure there costs the side file +// and the observed bytes survive untouched. Each case reopens the target after +// the tool has run and checks the preimage is what still stands, and that no +// side file was left behind. +func TestOverwriteFailureKeepsThePreimageStanding(t *testing.T) { + for name, run := range map[string]func(string) Result{ + "write overwrite": func(root string) Result { + return NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "existing.txt", "content": "zero\n", "overwrite": true, + }) + }, + "edit": func(root string) Result { + return NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "existing.txt", "old_string": "observed", "new_string": "zero", + }) + }, + } { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "existing.txt") + if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil { + t.Fatal(err) + } + // The write that stands in for ENOSPC fails after the file was + // observed, accepted and staged — the exact point the old + // truncate-then-write had nothing left to lose. + writeErr := errors.New("injected unrecoverable write failure") + installFileWriteTempWrite(t, func(*os.File, string) error { return writeErr }) + + result := run(root) + if result.Status != StatusError { + t.Fatalf("failed-write overwrite = %s, want error: %s", result.Status, result.Output) + } + got, err := os.ReadFile(target) + if err != nil || string(got) != "observed\n" { + t.Fatalf("preimage = %q, err=%v; the observed file must outlive a write that failed", got, err) + } + entries, err := os.ReadDir(root) + if err != nil { + t.Fatal(err) + } + if len(entries) != 1 || entries[0].Name() != "existing.txt" { + t.Fatalf("root holds %d entries; a failed write must remove its side file", len(entries)) + } + }) + } +} + +// A NEW FILE WHOSE WRITE FAILS MUST NOT APPEAR AT ALL. An exclusive create used +// to leave whatever the write had managed to write behind, half-written and +// complete-looking to anything that did not read past the visible length. +func TestCreateFailureCreatesNothing(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "created.txt") + writeErr := errors.New("injected unrecoverable write failure") + installFileWriteTempWrite(t, func(*os.File, string) error { return writeErr }) + + result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{ + "path": "created.txt", "content": "zero\n", + }) + if result.Status != StatusError { + t.Fatalf("failed-write create = %s, want error: %s", result.Status, result.Output) + } + if _, statErr := os.Stat(target); !os.IsNotExist(statErr) { + t.Fatalf("target = %v; a create whose bytes never made it must leave no file", statErr) + } + entries, err := os.ReadDir(root) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Fatalf("root holds %d entries; a failed create must remove its side file", len(entries)) + } +} + +// AT THE LAYER THE COMMIT RUNS, the injected failure is reported as the +// failure it injected and nothing else, so a caller can distinguish a failed +// write from a refused write. +func TestCommitRootedOverwriteFailedWriteIsTheWriteError(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "existing.txt") + if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil { + t.Fatal(err) + } + workspace, err := os.OpenRoot(root) + if err != nil { + t.Fatal(err) + } + defer workspace.Close() + info, err := os.Stat(target) + if err != nil { + t.Fatal(err) + } + preimage := "observed\n" + writeErr := errors.New("injected unrecoverable write failure") + installFileWriteTempWrite(t, func(*os.File, string) error { return writeErr }) + + err = commitRootedFileContents(workspace, filepath.Join(root, "existing.txt"), "existing.txt", info, &preimage, "zero\n") + if !errors.Is(err, writeErr) { + t.Fatalf("commit error = %v, want the injected write failure", err) + } + got, readErr := os.ReadFile(target) + if readErr != nil || string(got) != "observed\n" { + t.Fatalf("preimage = %q, err=%v; the observed file must outlive a write that failed", got, readErr) + } +}