From 3c092fbe34125abf7705a3bbe2aee56188213d58 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 2 Sep 2026 00:56:53 +0000 Subject: [PATCH] Replace Notarius bundle promotion with rename --- docs/internal/fileops.md | 6 +- docs/operations.md | 8 +- docs/roadmap/implementation.md | 2 +- docs/troubleshooting.md | 23 - internal/fileops/cleanup_test.go | 10 + internal/fileops/confined.go | 326 ---------- internal/fileops/directory.go | 555 ------------------ .../fileops/directory_special_unix_test.go | 35 -- internal/fileops/directory_test.go | 524 ----------------- .../fileops/directory_unsupported_test.go | 37 -- internal/fileops/modes_posix_test.go | 25 - internal/fileops/rename_noreplace_darwin.go | 19 - internal/fileops/rename_noreplace_linux.go | 19 - internal/fileops/rename_noreplace_other.go | 21 - .../rename_noreplace_supported_test.go | 26 - internal/fileops/rename_noreplace_windows.go | 18 - internal/fileops/sync_directory_unix.go | 30 - .../fileops/sync_directory_unsupported.go | 17 - internal/fileops/sync_directory_windows.go | 52 -- internal/stage/extract.go | 54 +- internal/stage/extract_test.go | 10 +- 21 files changed, 72 insertions(+), 1745 deletions(-) delete mode 100644 internal/fileops/confined.go delete mode 100644 internal/fileops/directory.go delete mode 100644 internal/fileops/directory_special_unix_test.go delete mode 100644 internal/fileops/directory_test.go delete mode 100644 internal/fileops/directory_unsupported_test.go delete mode 100644 internal/fileops/rename_noreplace_darwin.go delete mode 100644 internal/fileops/rename_noreplace_linux.go delete mode 100644 internal/fileops/rename_noreplace_other.go delete mode 100644 internal/fileops/rename_noreplace_supported_test.go delete mode 100644 internal/fileops/rename_noreplace_windows.go delete mode 100644 internal/fileops/sync_directory_unix.go delete mode 100644 internal/fileops/sync_directory_unsupported.go delete mode 100644 internal/fileops/sync_directory_windows.go diff --git a/docs/internal/fileops.md b/docs/internal/fileops.md index dae5363..1cc67d1 100644 --- a/docs/internal/fileops.md +++ b/docs/internal/fileops.md @@ -14,10 +14,8 @@ writer receives a `fileops`-owned, already-open sibling temporary file rather than a mutable destination path. Callers still own remote object selection, validation, conflict handling, and final mode. -Directory promotion keeps the verified destination parent open while it creates -the temporary tree, copies regular source entries, and performs the platform -no-replace rename. Platforms without a verified handle-relative atomic -no-replace primitive reject promotion before writing a temporary tree. +Directory bundles are not promoted by `internal/fileops`; extraction owns its +validated Notarius bundle rename. ## Cleanup Contract diff --git a/docs/operations.md b/docs/operations.md index cffb218..2c86a6e 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -284,11 +284,9 @@ record does not fall back to the earlier outputs. The earlier run manifest and immutable bundle remain available for inspection, but downstream resolution requires a new current successful extraction record. -Atomic Notarius bundle promotion is supported on Linux and macOS. On Windows -and other operating systems, extraction fails before copying the bundle into a -temporary promotion tree because Narratio has no verified atomic no-replace -directory primitive there. This is an extraction limitation, not a broader -platform-support guarantee for every Narratio workflow. +Notarius extraction validates its run-local bundle and moves it once to the +unique durable bundle path. A pre-existing durable destination is a collision; +no duplicate promotion tree is created. ## External Command Lifecycle diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 260d719..a5be375 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -532,7 +532,7 @@ complete validation and manifest-last recovery semantics. ## Stage 7 — Replace Notarius Tree Copy With One Validated Rename -**Status: Pending** +**Status: Completed** ### Goal diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 5dd90b1..be6d63a 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -282,29 +282,6 @@ Narratio validates selector structure and prepared evidence, while Notarius owns slot declarations, media compatibility, and generated-handoff conflicts. Relevant reference: [Notarius integration](./integrations/notarius.md). -## Atomic Notarius promotion unsupported - -Symptom: - -- extraction fails with `atomic no-replace directory promotion is unsupported` - before a durable bundle or temporary promotion tree is created. - -Likely cause: - -- Narratio is running on an operating system other than Linux, macOS, or - Windows, where the required atomic no-replace directory primitive has not - been implemented and verified. - -Safe fix: - -- run extraction on Linux, macOS, or Windows. Do not replace the atomic commit - with a manual copy or move; the session manifest must never observe a partial - or overwritten bundle. - -This is an extraction-specific platform boundary, not a support statement for -unrelated Narratio workflows. See -[Operations: Extraction Workflow](./operations.md#extraction-workflow). - ## Notarius receipt or index incompatible Symptom: diff --git a/internal/fileops/cleanup_test.go b/internal/fileops/cleanup_test.go index 44740a0..84d6ded 100644 --- a/internal/fileops/cleanup_test.go +++ b/internal/fileops/cleanup_test.go @@ -8,6 +8,16 @@ import ( "testing" ) +func mustWriteFile(t *testing.T, path string, data []byte, mode os.FileMode) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll(%q) error = %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, data, mode); err != nil { + t.Fatalf("WriteFile(%q) error = %v", path, err) + } +} + func TestRemoveAllUnderRootRemovesNestedTreeIdempotently(t *testing.T) { root := t.TempDir() target := filepath.Join(root, "runs", "run-1") diff --git a/internal/fileops/confined.go b/internal/fileops/confined.go deleted file mode 100644 index 0d4aa2e..0000000 --- a/internal/fileops/confined.go +++ /dev/null @@ -1,326 +0,0 @@ -package fileops - -import ( - "crypto/rand" - "encoding/hex" - "errors" - "fmt" - "os" - "path/filepath" - "strings" -) - -// openConfinedParent opens dst's parent without following a symbolic-link -// component. It starts from the filesystem root, so the resulting handle stays -// valid even if a pathname ancestor is later renamed or replaced. -func openConfinedParent(dst string, create bool, mode os.FileMode) (*os.Root, string, error) { - if strings.TrimSpace(dst) == "" { - return nil, "", fmt.Errorf("destination path is required") - } - abs, err := filepath.Abs(dst) - if err != nil { - return nil, "", fmt.Errorf("resolve destination path: %w", err) - } - parts, err := absolutePathParts(abs) - if err != nil { - return nil, "", err - } - if len(parts) == 0 { - return nil, "", fmt.Errorf("destination file name is required") - } - - rootPath := filesystemRoot(abs) - root, err := os.OpenRoot(rootPath) - if err != nil { - return nil, "", fmt.Errorf("open filesystem root %q: %w", rootPath, err) - } - for _, part := range parts[:len(parts)-1] { - child, err := openConfinedChild(root, part, create, mode) - if err != nil { - _ = root.Close() - return nil, "", err - } - _ = root.Close() - root = child - } - return root, parts[len(parts)-1], nil -} - -func ensureConfinedDirectory(directory string, mode os.FileMode) error { - abs, err := filepath.Abs(directory) - if err != nil { - return fmt.Errorf("resolve workspace directory: %w", err) - } - parts, err := absolutePathParts(abs) - if err != nil { - return err - } - rootPath := filesystemRoot(abs) - root, err := os.OpenRoot(rootPath) - if err != nil { - return fmt.Errorf("open filesystem root %q: %w", rootPath, err) - } - defer func() { _ = root.Close() }() - - for _, part := range parts { - child, err := openConfinedChild(root, part, true, mode) - if err != nil { - return err - } - _ = root.Close() - root = child - } - return setOpenedDirectoryMode(root, mode) -} - -func absolutePathParts(path string) ([]string, error) { - root := filesystemRoot(path) - relative, err := filepath.Rel(root, path) - if err != nil { - return nil, fmt.Errorf("resolve path below filesystem root: %w", err) - } - if relative == "." { - return nil, nil - } - parts := strings.FieldsFunc(relative, func(r rune) bool { return r == filepath.Separator || r == '/' || r == '\\' }) - for _, part := range parts { - if part == "" || part == "." || part == ".." { - return nil, fmt.Errorf("unsafe filesystem path component %q", part) - } - } - return parts, nil -} - -func filesystemRoot(path string) string { - volume := filepath.VolumeName(path) - if volume == "" { - return string(filepath.Separator) - } - return volume + string(filepath.Separator) -} - -func openConfinedChild(parent *os.Root, name string, create bool, mode os.FileMode) (*os.Root, error) { - info, err := parent.Lstat(name) - created := false - if errors.Is(err, os.ErrNotExist) { - if !create { - return nil, fmt.Errorf("destination ancestor %q does not exist", name) - } - if err := parent.Mkdir(name, mode.Perm()); err != nil && !errors.Is(err, os.ErrExist) { - return nil, fmt.Errorf("create destination ancestor %q: %w", name, err) - } - created = true - info, err = parent.Lstat(name) - } - if err != nil { - return nil, fmt.Errorf("inspect destination ancestor %q: %w", name, err) - } - if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { - return nil, fmt.Errorf("destination ancestor %q is not a regular directory", name) - } - - child, err := parent.OpenRoot(name) - if err != nil { - return nil, fmt.Errorf("open destination ancestor %q: %w", name, err) - } - opened, err := child.Stat(".") - if err != nil { - _ = child.Close() - return nil, fmt.Errorf("inspect opened destination ancestor %q: %w", name, err) - } - current, err := parent.Lstat(name) - if err != nil || current.Mode()&os.ModeSymlink != 0 || !current.IsDir() || !os.SameFile(opened, current) { - _ = child.Close() - if err != nil { - return nil, fmt.Errorf("reinspect destination ancestor %q: %w", name, err) - } - return nil, fmt.Errorf("destination ancestor %q changed while being opened", name) - } - if created { - if err := setOpenedDirectoryMode(child, mode); err != nil { - _ = child.Close() - return nil, fmt.Errorf("set destination ancestor permissions %q: %w", name, err) - } - } - return child, nil -} - -func setOpenedDirectoryMode(root *os.Root, mode os.FileMode) error { - directory, err := root.Open(".") - if err != nil { - return err - } - defer func() { _ = directory.Close() }() - return directory.Chmod(mode) -} - -func createSiblingTemp(parent *os.Root, base string) (*os.File, string, error) { - return createSiblingTempWithFlags(parent, base, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o600) -} - -func createSiblingTempWithFlags(parent *os.Root, base string, flags int, mode os.FileMode) (*os.File, string, error) { - for attempt := 0; attempt < 100; attempt++ { - var randomBytes [16]byte - if _, err := rand.Read(randomBytes[:]); err != nil { - return nil, "", fmt.Errorf("generate temporary file name: %w", err) - } - name := "." + base + ".tmp-" + hex.EncodeToString(randomBytes[:]) - file, err := parent.OpenFile(name, flags|os.O_CREATE|os.O_EXCL, mode) - if errors.Is(err, os.ErrExist) { - continue - } - if err != nil { - return nil, "", err - } - return file, name, nil - } - return nil, "", fmt.Errorf("create unique sibling temporary file") -} - -func syncOpenedDirectory(parent *os.Root) error { - directory, err := parent.Open(".") - if err != nil { - return err - } - defer func() { _ = directory.Close() }() - return syncDirectoryFile(directory, parent.Name()) -} - -// OpenFileConfined opens a file after verifying its parent hierarchy without -// mutating through symbolic links. Truncating opens atomically replace the leaf -// with a newly created sibling, while other creating opens never carry O_CREATE -// into an existing-leaf open. Existing symbolic-link and non-regular leaves are -// rejected. -func OpenFileConfined(path string, flags int, mode os.FileMode) (*os.File, error) { - return openFileConfined(path, flags, mode, nil) -} - -func openFileConfined(path string, flags int, mode os.FileMode, beforeOpen func() error) (*os.File, error) { - parent, name, err := openConfinedParent(path, false, 0) - if err != nil { - return nil, err - } - defer func() { _ = parent.Close() }() - - if flags&(os.O_CREATE|os.O_EXCL) == os.O_CREATE|os.O_EXCL { - if err := runBeforeConfinedOpen(beforeOpen); err != nil { - return nil, err - } - file, err := parent.OpenFile(name, flags, mode) - if err != nil { - return nil, err - } - return validateOpenedConfinedFile(parent, name, file) - } - - exists, err := validateConfinedLeaf(parent, name) - if err != nil { - return nil, err - } - if flags&os.O_TRUNC != 0 { - if !exists && flags&os.O_CREATE == 0 { - return nil, fmt.Errorf("open destination file %q: %w", name, os.ErrNotExist) - } - return replaceAndOpenConfinedFile(parent, name, flags, mode, beforeOpen) - } - - if err := runBeforeConfinedOpen(beforeOpen); err != nil { - return nil, err - } - if !exists && flags&os.O_CREATE != 0 { - file, err := parent.OpenFile(name, flags|os.O_EXCL, mode) - if err == nil { - return validateOpenedConfinedFile(parent, name, file) - } - if !errors.Is(err, os.ErrExist) { - return nil, err - } - if _, validationErr := validateConfinedLeaf(parent, name); validationErr != nil { - return nil, validationErr - } - } - - // The leaf now exists, so removing O_CREATE prevents a raced dangling - // symlink from creating its target. Opening an existing symlink can expose a - // handle, but no caller can mutate through it before the identity check below. - file, err := parent.OpenFile(name, flags&^os.O_CREATE, mode) - if err != nil { - return nil, err - } - return validateOpenedConfinedFile(parent, name, file) -} - -func validateConfinedLeaf(parent *os.Root, name string) (bool, error) { - info, err := parent.Lstat(name) - if errors.Is(err, os.ErrNotExist) { - return false, nil - } - if err != nil { - return false, fmt.Errorf("inspect destination file %q: %w", name, err) - } - if info.Mode()&os.ModeSymlink != 0 { - return false, fmt.Errorf("destination file %q is a symbolic link", name) - } - if !info.Mode().IsRegular() { - return false, fmt.Errorf("destination file %q is not a regular file", name) - } - return true, nil -} - -func replaceAndOpenConfinedFile(parent *os.Root, name string, flags int, mode os.FileMode, beforeOpen func() error) (_ *os.File, resultErr error) { - temporary, temporaryName, err := createSiblingTempWithFlags(parent, name, flags&^os.O_TRUNC, mode) - if err != nil { - return nil, fmt.Errorf("create replacement for destination file %q: %w", name, err) - } - installed := false - defer func() { - if installed { - return - } - closeErr := temporary.Close() - removeErr := parent.Remove(temporaryName) - if errors.Is(removeErr, os.ErrNotExist) { - removeErr = nil - } - resultErr = errors.Join(resultErr, closeErr, removeErr) - }() - - if err := runBeforeConfinedOpen(beforeOpen); err != nil { - return nil, err - } - // Rename replaces the directory entry itself rather than following it. A - // leaf changed to a symlink after validation therefore cannot redirect the - // truncation to its target. - if err := parent.Rename(temporaryName, name); err != nil { - return nil, fmt.Errorf("install replacement for destination file %q: %w", name, err) - } - installed = true - return validateOpenedConfinedFile(parent, name, temporary) -} - -func runBeforeConfinedOpen(beforeOpen func() error) error { - if beforeOpen == nil { - return nil - } - if err := beforeOpen(); err != nil { - return fmt.Errorf("check before confined file open: %w", err) - } - return nil -} - -func validateOpenedConfinedFile(parent *os.Root, name string, file *os.File) (*os.File, error) { - opened, err := file.Stat() - if err != nil { - _ = file.Close() - return nil, fmt.Errorf("inspect opened destination file %q: %w", name, err) - } - current, err := parent.Lstat(name) - if err != nil || !opened.Mode().IsRegular() || current.Mode()&os.ModeSymlink != 0 || !current.Mode().IsRegular() || !os.SameFile(opened, current) { - _ = file.Close() - if err != nil { - return nil, fmt.Errorf("reinspect destination file %q: %w", name, err) - } - return nil, fmt.Errorf("destination file %q changed while being opened", name) - } - return file, nil -} diff --git a/internal/fileops/directory.go b/internal/fileops/directory.go deleted file mode 100644 index 3bb02e1..0000000 --- a/internal/fileops/directory.go +++ /dev/null @@ -1,555 +0,0 @@ -package fileops - -import ( - "crypto/rand" - "encoding/hex" - "errors" - "fmt" - "io" - "os" - "path/filepath" - "sort" - "strings" -) - -// ErrAtomicDirectoryPromotionUnsupported indicates that the current operating -// system lacks the atomic no-replace primitive required by PromoteDirectory. -var ErrAtomicDirectoryPromotionUnsupported = errors.New("atomic no-replace directory promotion is unsupported") - -// PromoteDirectory copies an existing regular-file tree into a new directory -// and installs the complete copy atomically. It never removes the source or -// replaces an existing destination. -func PromoteDirectory(src, dst string) error { - if err := checkAtomicDirectoryPromotionSupport(); err != nil { - return err - } - parent, destinationName, err := openConfinedParent(dst, false, 0) - if err != nil { - return fmt.Errorf("open destination parent: %w", err) - } - defer func() { _ = parent.Close() }() - return promoteDirectoryConfined(src, dst, parent, destinationName, sourceTraversalHooks{}) -} - -func promoteDirectoryConfined(src, dst string, parent *os.Root, destinationName string, hooks sourceTraversalHooks) (resultErr error) { - if strings.TrimSpace(src) == "" || strings.TrimSpace(dst) == "" { - return fmt.Errorf("source and destination directory paths are required") - } - sourceInfo, err := os.Lstat(src) - if err != nil { - return fmt.Errorf("inspect source directory: %w", err) - } - if sourceInfo.Mode()&os.ModeSymlink != 0 || !sourceInfo.IsDir() { - return fmt.Errorf("source path %q is not a directory", src) - } - if _, err := parent.Lstat(destinationName); err == nil { - return fmt.Errorf("destination path %q already exists", dst) - } else if !errors.Is(err, os.ErrNotExist) { - return fmt.Errorf("inspect destination path: %w", err) - } - insideSource, err := pathWithin(src, dst) - if err != nil { - return err - } - if insideSource { - return fmt.Errorf("destination path %q must not be inside source directory %q", dst, src) - } - - temporary, temporaryName, err := createSiblingTempDirectory(parent, destinationName) - if err != nil { - return err - } - removeTemporary := true - defer func() { - var closeErr error - if temporary != nil { - closeErr = temporary.Close() - } - if removeTemporary { - removeErr := parent.RemoveAll(temporaryName) - if removeErr != nil { - resultErr = errors.Join(resultErr, fmt.Errorf("remove temporary destination directory: %w", removeErr)) - } - } - if closeErr != nil { - resultErr = errors.Join(resultErr, fmt.Errorf("close temporary destination directory: %w", closeErr)) - } - }() - - sourceRoot, err := openVerifiedSourceRoot(src, sourceInfo, hooks) - if err != nil { - return err - } - defer func() { _ = sourceRoot.Close() }() - if err := copyRegularTreeToRoot(sourceRoot, src, temporary, hooks); err != nil { - return err - } - if err := setOpenedDirectoryMode(temporary, WorkspaceDirectoryMode); err != nil { - return fmt.Errorf("set temporary root permissions: %w", err) - } - if err := syncOpenedDirectory(temporary); err != nil { - return fmt.Errorf("sync temporary root: %w", err) - } - if err := temporary.Close(); err != nil { - return fmt.Errorf("close temporary root: %w", err) - } - temporary = nil - - directory, err := parent.Open(".") - if err != nil { - return fmt.Errorf("open destination parent for promotion: %w", err) - } - renameErr := renameDirectoryNoReplaceAt(directory, temporaryName, destinationName) - closeErr := directory.Close() - if renameErr != nil { - return fmt.Errorf("install promoted directory: %w", renameErr) - } - if closeErr != nil { - return fmt.Errorf("close destination parent after promotion: %w", closeErr) - } - removeTemporary = false - if err := syncOpenedDirectory(parent); err != nil { - return fmt.Errorf("sync destination parent: %w", err) - } - return nil -} - -func createSiblingTempDirectory(parent *os.Root, base string) (*os.Root, string, error) { - for attempt := 0; attempt < 100; attempt++ { - var randomBytes [16]byte - if _, err := rand.Read(randomBytes[:]); err != nil { - return nil, "", fmt.Errorf("generate temporary directory name: %w", err) - } - name := "." + base + ".tmp-" + hex.EncodeToString(randomBytes[:]) - if err := parent.Mkdir(name, WorkspaceDirectoryMode.Perm()); err != nil { - if errors.Is(err, os.ErrExist) { - continue - } - return nil, "", fmt.Errorf("create temporary destination directory: %w", err) - } - directory, err := openConfinedChild(parent, name, false, 0) - if err != nil { - _ = parent.Remove(name) - return nil, "", err - } - return directory, name, nil - } - return nil, "", fmt.Errorf("create unique temporary destination directory") -} - -func copyRegularTreeToRoot(src *os.Root, sourcePath string, dst *os.Root, hooks sourceTraversalHooks) error { - directory, err := src.Open(".") - if err != nil { - return fmt.Errorf("open source directory %q for traversal: %w", sourcePath, err) - } - entries, readErr := directory.ReadDir(-1) - closeErr := directory.Close() - if readErr != nil { - return fmt.Errorf("read source directory %q: %w", sourcePath, readErr) - } - if closeErr != nil { - return fmt.Errorf("close source directory %q: %w", sourcePath, closeErr) - } - sort.Slice(entries, func(i, j int) bool { return entries[i].Name() < entries[j].Name() }) - for _, entry := range entries { - entryPath := filepath.Join(sourcePath, entry.Name()) - info, err := src.Lstat(entry.Name()) - if err != nil { - return fmt.Errorf("inspect source entry %q: %w", entryPath, err) - } - if hooks.afterEntryInspect != nil { - hooks.afterEntryInspect(entryPath) - } - switch { - case info.Mode().IsRegular(): - if err := copyRegularFileToRoot(src, dst, entry.Name(), entryPath, info); err != nil { - return err - } - case info.IsDir(): - if err := copyRegularDirectoryToRoot(src, dst, entry.Name(), entryPath, info, hooks); err != nil { - return err - } - default: - return fmt.Errorf("source entry %q has unsupported file type %s", entryPath, info.Mode().Type()) - } - } - return nil -} - -func copyRegularDirectoryToRoot(sourceParent, destinationParent *os.Root, name, sourcePath string, inspected os.FileInfo, hooks sourceTraversalHooks) error { - source, err := openVerifiedChildDirectory(sourceParent, name, sourcePath, inspected) - if err != nil { - return err - } - defer func() { _ = source.Close() }() - if err := destinationParent.Mkdir(name, WorkspaceDirectoryMode.Perm()); err != nil { - return fmt.Errorf("create destination directory %q: %w", sourcePath, err) - } - destination, err := openConfinedChild(destinationParent, name, false, 0) - if err != nil { - return err - } - err = copyRegularTreeToRoot(source, sourcePath, destination, hooks) - if err == nil { - err = setOpenedDirectoryMode(destination, WorkspaceDirectoryMode) - } - if err == nil { - err = syncOpenedDirectory(destination) - } - closeErr := destination.Close() - if err != nil { - return err - } - if closeErr != nil { - return fmt.Errorf("close destination directory %q: %w", sourcePath, closeErr) - } - return nil -} - -func copyRegularFileToRoot(sourceRoot, destinationRoot *os.Root, name, sourcePath string, inspected os.FileInfo) error { - in, err := openVerifiedSourceFile(sourceRoot, name, sourcePath, inspected) - if err != nil { - return err - } - defer func() { _ = in.Close() }() - out, err := destinationRoot.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, WorkspaceFileMode) - if err != nil { - return fmt.Errorf("create destination file %q: %w", sourcePath, err) - } - if _, err := io.Copy(out, in); err != nil { - _ = out.Close() - return fmt.Errorf("copy source file %q: %w", sourcePath, err) - } - if err := out.Chmod(WorkspaceFileMode); err != nil { - _ = out.Close() - return fmt.Errorf("set destination file permissions %q: %w", sourcePath, err) - } - if err := out.Sync(); err != nil { - _ = out.Close() - return fmt.Errorf("sync destination file %q: %w", sourcePath, err) - } - if err := out.Close(); err != nil { - return fmt.Errorf("close destination file %q: %w", sourcePath, err) - } - return nil -} - -func openVerifiedChildDirectory(parent *os.Root, name, sourcePath string, inspected os.FileInfo) (*os.Root, error) { - child, err := parent.OpenRoot(name) - if err != nil { - return nil, fmt.Errorf("open source directory %q: %w", sourcePath, err) - } - opened, err := child.Stat(".") - if err != nil { - _ = child.Close() - return nil, fmt.Errorf("inspect opened source directory %q: %w", sourcePath, err) - } - current, err := parent.Lstat(name) - if err != nil || current.Mode()&os.ModeSymlink != 0 || !current.IsDir() || !os.SameFile(opened, current) { - _ = child.Close() - if err != nil { - return nil, fmt.Errorf("reinspect source directory %q: %w", sourcePath, err) - } - return nil, fmt.Errorf("source directory %q changed while being copied", sourcePath) - } - return child, nil -} - -func openVerifiedSourceFile(root *os.Root, name, sourcePath string, inspected os.FileInfo) (*os.File, error) { - in, err := root.Open(name) - if err != nil { - return nil, fmt.Errorf("open source file %q: %w", sourcePath, err) - } - opened, err := in.Stat() - if err != nil { - _ = in.Close() - return nil, fmt.Errorf("inspect opened source file %q: %w", sourcePath, err) - } - current, err := root.Lstat(name) - if err != nil || current.Mode()&os.ModeSymlink != 0 || !current.Mode().IsRegular() || !os.SameFile(opened, current) { - _ = in.Close() - if err != nil { - return nil, fmt.Errorf("reinspect source file %q: %w", sourcePath, err) - } - return nil, fmt.Errorf("source file %q changed while being copied", sourcePath) - } - return in, nil -} - -func promoteDirectory(src, dst string, install func(string, string) error) error { - return promoteDirectoryWithHooks(src, dst, install, sourceTraversalHooks{}) -} - -type sourceTraversalHooks struct { - afterRootInspect func() - afterEntryInspect func(string) -} - -func promoteDirectoryWithHooks( - src, dst string, - install func(string, string) error, - hooks sourceTraversalHooks, -) error { - if strings.TrimSpace(src) == "" || strings.TrimSpace(dst) == "" { - return fmt.Errorf("source and destination directory paths are required") - } - - sourceInfo, err := os.Lstat(src) - if err != nil { - return fmt.Errorf("inspect source directory: %w", err) - } - if !sourceInfo.IsDir() { - return fmt.Errorf("source path %q is not a directory", src) - } - - if _, err := os.Lstat(dst); err == nil { - return fmt.Errorf("destination path %q already exists", dst) - } else if !errors.Is(err, os.ErrNotExist) { - return fmt.Errorf("inspect destination path: %w", err) - } - - insideSource, err := pathWithin(src, dst) - if err != nil { - return err - } - if insideSource { - return fmt.Errorf("destination path %q must not be inside source directory %q", dst, src) - } - - destinationParent := filepath.Dir(dst) - parentInfo, err := os.Lstat(destinationParent) - if err != nil { - return fmt.Errorf("inspect destination parent: %w", err) - } - if !parentInfo.IsDir() { - return fmt.Errorf("destination parent %q is not a directory", destinationParent) - } - - temporary, err := os.MkdirTemp(destinationParent, "."+filepath.Base(dst)+".tmp-*") - if err != nil { - return fmt.Errorf("create temporary destination directory: %w", err) - } - removeTemporary := true - defer func() { - if removeTemporary { - _ = RemoveAllUnderRoot(destinationParent, temporary) - } - }() - - sourceRoot, err := openVerifiedSourceRoot(src, sourceInfo, hooks) - if err != nil { - return err - } - defer func() { _ = sourceRoot.Close() }() - - if err := copyRegularTree(sourceRoot, src, temporary, hooks); err != nil { - return err - } - if err := os.Chmod(temporary, WorkspaceDirectoryMode); err != nil { - return fmt.Errorf("set temporary root permissions: %w", err) - } - if err := syncDirectory(temporary); err != nil { - return fmt.Errorf("sync temporary root: %w", err) - } - if err := install(temporary, dst); err != nil { - return fmt.Errorf("install promoted directory: %w", err) - } - - removeTemporary = false - if err := syncDirectory(destinationParent); err != nil { - return fmt.Errorf("sync destination parent: %w", err) - } - return nil -} - -func openVerifiedSourceRoot(path string, inspected os.FileInfo, hooks sourceTraversalHooks) (*os.Root, error) { - if hooks.afterRootInspect != nil { - hooks.afterRootInspect() - } - - root, err := os.OpenRoot(path) - if err != nil { - return nil, fmt.Errorf("open source directory %q: %w", path, err) - } - verified := false - defer func() { - if !verified { - _ = root.Close() - } - }() - - opened, err := root.Stat(".") - if err != nil { - return nil, fmt.Errorf("inspect opened source directory %q: %w", path, err) - } - if !opened.IsDir() || !os.SameFile(inspected, opened) { - return nil, fmt.Errorf("source directory %q changed while being opened", path) - } - current, err := os.Lstat(path) - if err != nil { - return nil, fmt.Errorf("reinspect source directory %q: %w", path, err) - } - if current.Mode()&os.ModeSymlink != 0 || !current.IsDir() || !os.SameFile(opened, current) { - return nil, fmt.Errorf("source directory %q changed while being opened", path) - } - - verified = true - return root, nil -} - -func copyRegularTree(src *os.Root, sourcePath, dst string, hooks sourceTraversalHooks) error { - directory, err := src.Open(".") - if err != nil { - return fmt.Errorf("open source directory %q for traversal: %w", sourcePath, err) - } - defer func() { _ = directory.Close() }() - - entries, err := directory.ReadDir(-1) - if err != nil { - return fmt.Errorf("read source directory %q: %w", sourcePath, err) - } - sort.Slice(entries, func(i, j int) bool { - return entries[i].Name() < entries[j].Name() - }) - - for _, entry := range entries { - entryPath := filepath.Join(sourcePath, entry.Name()) - destinationPath := filepath.Join(dst, entry.Name()) - info, err := src.Lstat(entry.Name()) - if err != nil { - return fmt.Errorf("inspect source entry %q: %w", entryPath, err) - } - - switch { - case info.Mode().IsRegular(): - if hooks.afterEntryInspect != nil { - hooks.afterEntryInspect(entryPath) - } - if err := copyRegularFile(src, entry.Name(), entryPath, destinationPath, info); err != nil { - return err - } - case info.IsDir(): - if hooks.afterEntryInspect != nil { - hooks.afterEntryInspect(entryPath) - } - if err := copyRegularDirectory(src, entry.Name(), entryPath, destinationPath, info, hooks); err != nil { - return err - } - default: - return fmt.Errorf("source entry %q has unsupported file type %s", entryPath, info.Mode().Type()) - } - } - - return nil -} - -func copyRegularDirectory( - parent *os.Root, - name, sourcePath, dst string, - inspected os.FileInfo, - hooks sourceTraversalHooks, -) error { - child, err := parent.OpenRoot(name) - if err != nil { - return fmt.Errorf("open source directory %q: %w", sourcePath, err) - } - defer func() { _ = child.Close() }() - - opened, err := child.Stat(".") - if err != nil { - return fmt.Errorf("inspect opened source directory %q: %w", sourcePath, err) - } - if !opened.IsDir() || !os.SameFile(inspected, opened) { - return fmt.Errorf("source directory %q changed while being copied", sourcePath) - } - current, err := parent.Lstat(name) - if err != nil { - return fmt.Errorf("reinspect source directory %q: %w", sourcePath, err) - } - if current.Mode()&os.ModeSymlink != 0 || !current.IsDir() || !os.SameFile(opened, current) { - return fmt.Errorf("source directory %q changed while being copied", sourcePath) - } - - if err := os.Mkdir(dst, WorkspaceDirectoryMode); err != nil { - return fmt.Errorf("create destination directory %q: %w", dst, err) - } - if err := copyRegularTree(child, sourcePath, dst, hooks); err != nil { - return err - } - if err := os.Chmod(dst, WorkspaceDirectoryMode); err != nil { - return fmt.Errorf("set destination directory permissions %q: %w", dst, err) - } - if err := syncDirectory(dst); err != nil { - return fmt.Errorf("sync destination directory %q: %w", dst, err) - } - return nil -} - -func copyRegularFile( - root *os.Root, - name, sourcePath, dst string, - inspected os.FileInfo, -) error { - in, err := root.Open(name) - if err != nil { - return fmt.Errorf("open source file %q: %w", sourcePath, err) - } - defer func() { _ = in.Close() }() - - opened, err := in.Stat() - if err != nil { - return fmt.Errorf("inspect opened source file %q: %w", sourcePath, err) - } - if !opened.Mode().IsRegular() || !os.SameFile(inspected, opened) { - return fmt.Errorf("source file %q changed while being copied", sourcePath) - } - current, err := root.Lstat(name) - if err != nil { - return fmt.Errorf("reinspect source file %q: %w", sourcePath, err) - } - if current.Mode()&os.ModeSymlink != 0 || !current.Mode().IsRegular() || !os.SameFile(opened, current) { - return fmt.Errorf("source file %q changed while being copied", sourcePath) - } - - out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, WorkspaceFileMode) - if err != nil { - return fmt.Errorf("create destination file %q: %w", dst, err) - } - closed := false - defer func() { - if !closed { - _ = out.Close() - } - }() - - if _, err := io.Copy(out, in); err != nil { - return fmt.Errorf("copy source file %q: %w", sourcePath, err) - } - if err := out.Chmod(WorkspaceFileMode); err != nil { - return fmt.Errorf("set destination file permissions %q: %w", dst, err) - } - if err := out.Sync(); err != nil { - return fmt.Errorf("sync destination file %q: %w", dst, err) - } - if err := out.Close(); err != nil { - return fmt.Errorf("close destination file %q: %w", dst, err) - } - closed = true - return nil -} - -func pathWithin(parent, candidate string) (bool, error) { - absoluteParent, err := filepath.Abs(parent) - if err != nil { - return false, fmt.Errorf("resolve source directory: %w", err) - } - absoluteCandidate, err := filepath.Abs(candidate) - if err != nil { - return false, fmt.Errorf("resolve destination directory: %w", err) - } - relative, err := filepath.Rel(absoluteParent, absoluteCandidate) - if err != nil { - return false, fmt.Errorf("compare source and destination directories: %w", err) - } - return relative != "." && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)), nil -} diff --git a/internal/fileops/directory_special_unix_test.go b/internal/fileops/directory_special_unix_test.go deleted file mode 100644 index 02c514f..0000000 --- a/internal/fileops/directory_special_unix_test.go +++ /dev/null @@ -1,35 +0,0 @@ -//go:build linux || darwin - -package fileops - -import ( - "os" - "path/filepath" - "syscall" - "testing" -) - -func TestPromoteDirectoryRejectsNamedPipe(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - dst := filepath.Join(root, "destination") - if err := os.Mkdir(src, 0o755); err != nil { - t.Fatalf("Mkdir(source) error = %v", err) - } - pipe := filepath.Join(src, "events.pipe") - if err := syscall.Mkfifo(pipe, 0o600); err != nil { - t.Skipf("Mkfifo() unavailable: %v", err) - } - - if err := PromoteDirectory(src, dst); err == nil { - t.Fatal("PromoteDirectory() error = nil, want named pipe rejection") - } - info, err := os.Lstat(pipe) - if err != nil || info.Mode()&os.ModeNamedPipe == 0 { - t.Fatalf("source named pipe was not preserved: info=%v err=%v", info, err) - } - if _, err := os.Lstat(dst); !os.IsNotExist(err) { - t.Fatalf("Lstat(destination) error = %v, want not exist", err) - } - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} diff --git a/internal/fileops/directory_test.go b/internal/fileops/directory_test.go deleted file mode 100644 index 5c88882..0000000 --- a/internal/fileops/directory_test.go +++ /dev/null @@ -1,524 +0,0 @@ -//go:build linux || darwin || windows - -package fileops - -import ( - "bytes" - "os" - "path/filepath" - "reflect" - "runtime" - "strings" - "testing" -) - -func TestPromoteDirectoryCopiesNestedRegularTree(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - dst := filepath.Join(root, "promoted") - mustWriteFile(t, filepath.Join(src, "z-last.txt"), []byte("last"), 0o777) - mustWriteFile(t, filepath.Join(src, "nested", "binary.dat"), []byte{0, 1, 2, 0xff}, 0o600) - mustWriteFile(t, filepath.Join(src, "a-first.txt"), []byte("first"), 0o400) - if err := os.Mkdir(filepath.Join(src, "empty"), 0o700); err != nil { - t.Fatalf("Mkdir(empty) error = %v", err) - } - - if err := PromoteDirectory(src, dst); err != nil { - t.Fatalf("PromoteDirectory() error = %v", err) - } - - wantLayout := []string{".", "a-first.txt", "empty", "nested", "nested/binary.dat", "z-last.txt"} - if got := treeLayout(t, dst); !reflect.DeepEqual(got, wantLayout) { - t.Fatalf("promoted layout = %#v, want %#v", got, wantLayout) - } - assertFileBytes(t, filepath.Join(dst, "a-first.txt"), []byte("first")) - assertFileBytes(t, filepath.Join(dst, "nested", "binary.dat"), []byte{0, 1, 2, 0xff}) - assertFileBytes(t, filepath.Join(dst, "z-last.txt"), []byte("last")) - - if runtime.GOOS != "windows" { - for _, path := range []string{dst, filepath.Join(dst, "nested"), filepath.Join(dst, "empty")} { - info, err := os.Stat(path) - if err != nil { - t.Fatalf("Stat(%q) error = %v", path, err) - } - if got := info.Mode().Perm(); got != WorkspaceDirectoryMode.Perm() { - t.Fatalf("directory mode for %q = %o, want %o", path, got, WorkspaceDirectoryMode.Perm()) - } - } - for _, path := range []string{filepath.Join(dst, "a-first.txt"), filepath.Join(dst, "nested", "binary.dat"), filepath.Join(dst, "z-last.txt")} { - info, err := os.Stat(path) - if err != nil { - t.Fatalf("Stat(%q) error = %v", path, err) - } - if got := info.Mode().Perm(); got != WorkspaceFileMode.Perm() { - t.Fatalf("file mode for %q = %o, want %o", path, got, WorkspaceFileMode.Perm()) - } - } - } - - assertFileBytes(t, filepath.Join(src, "nested", "binary.dat"), []byte{0, 1, 2, 0xff}) - assertNoMatchingTempDirectories(t, root, ".promoted.tmp-") -} - -func TestPromoteDirectoryRejectsInvalidPaths(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - if err := os.Mkdir(src, 0o755); err != nil { - t.Fatalf("Mkdir(source) error = %v", err) - } - - tests := []struct { - name string - src string - dst string - }{ - {name: "empty source", src: "", dst: filepath.Join(root, "out-a")}, - {name: "empty destination", src: src, dst: " "}, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - if err := PromoteDirectory(test.src, test.dst); err == nil { - t.Fatal("PromoteDirectory() error = nil, want path validation failure") - } - }) - } -} - -func TestPromoteDirectoryRejectsExistingDestination(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - dst := filepath.Join(root, "destination") - mustWriteFile(t, filepath.Join(src, "value.txt"), []byte("source"), 0o644) - mustWriteFile(t, filepath.Join(dst, "value.txt"), []byte("existing"), 0o644) - - err := PromoteDirectory(src, dst) - if err == nil || !strings.Contains(err.Error(), "already exists") { - t.Fatalf("PromoteDirectory() error = %v, want existing destination error", err) - } - assertFileBytes(t, filepath.Join(dst, "value.txt"), []byte("existing")) - assertFileBytes(t, filepath.Join(src, "value.txt"), []byte("source")) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryRejectsNonDirectorySource(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source.txt") - dst := filepath.Join(root, "destination") - mustWriteFile(t, src, []byte("source"), 0o644) - - if err := PromoteDirectory(src, dst); err == nil { - t.Fatal("PromoteDirectory() error = nil, want non-directory source error") - } - assertFileBytes(t, src, []byte("source")) - if _, err := os.Lstat(dst); !os.IsNotExist(err) { - t.Fatalf("Lstat(destination) error = %v, want not exist", err) - } -} - -func TestPromoteDirectoryRejectsSymlinksWithoutFollowingThem(t *testing.T) { - root := t.TempDir() - externalFile := filepath.Join(root, "external.txt") - externalDirectory := filepath.Join(root, "external-directory") - mustWriteFile(t, externalFile, []byte("outside"), 0o644) - mustWriteFile(t, filepath.Join(externalDirectory, "secret.txt"), []byte("secret"), 0o644) - - tests := []struct { - name string - target string - link string - }{ - {name: "file", target: externalFile, link: "file-link"}, - {name: "directory", target: externalDirectory, link: "directory-link"}, - {name: "escaping", target: filepath.Join("..", "external.txt"), link: "escaping-link"}, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - src := filepath.Join(root, "source-"+test.name) - dst := filepath.Join(root, "destination-"+test.name) - if err := os.Mkdir(src, 0o755); err != nil { - t.Fatalf("Mkdir(source) error = %v", err) - } - if err := os.Symlink(test.target, filepath.Join(src, test.link)); err != nil { - t.Skipf("Symlink() unavailable: %v", err) - } - - if err := PromoteDirectory(src, dst); err == nil { - t.Fatal("PromoteDirectory() error = nil, want symlink rejection") - } - linkInfo, err := os.Lstat(filepath.Join(src, test.link)) - if err != nil || linkInfo.Mode()&os.ModeSymlink == 0 { - t.Fatalf("source symlink was not preserved: info=%v err=%v", linkInfo, err) - } - if _, err := os.Lstat(dst); !os.IsNotExist(err) { - t.Fatalf("Lstat(destination) error = %v, want not exist", err) - } - assertNoMatchingTempDirectories(t, root, ".destination-"+test.name+".tmp-") - }) - } - assertFileBytes(t, externalFile, []byte("outside")) - assertFileBytes(t, filepath.Join(externalDirectory, "secret.txt"), []byte("secret")) -} - -func TestPromoteDirectoryRejectsSymlinkSourceRoot(t *testing.T) { - root := t.TempDir() - target := filepath.Join(root, "target") - src := filepath.Join(root, "source") - dst := filepath.Join(root, "destination") - mustWriteFile(t, filepath.Join(target, "value.txt"), []byte("outside"), 0o644) - if err := os.Symlink(target, src); err != nil { - t.Skipf("Symlink() unavailable: %v", err) - } - - if err := PromoteDirectory(src, dst); err == nil { - t.Fatal("PromoteDirectory() error = nil, want source-root symlink rejection") - } - assertFileBytes(t, filepath.Join(target, "value.txt"), []byte("outside")) - assertPathMissing(t, dst) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryRejectsSourceRootReplacementBeforeOpen(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - preserved := filepath.Join(root, "source-preserved") - replacement := filepath.Join(root, "replacement") - dst := filepath.Join(root, "destination") - mustWriteFile(t, filepath.Join(src, "value.txt"), []byte("original"), 0o644) - mustWriteFile(t, filepath.Join(replacement, "value.txt"), []byte("replacement"), 0o644) - - err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ - afterRootInspect: func() { - if err := os.Rename(src, preserved); err != nil { - t.Fatalf("Rename(original source) error = %v", err) - } - if err := os.Rename(replacement, src); err != nil { - t.Fatalf("Rename(replacement source) error = %v", err) - } - }, - }) - if err == nil { - t.Fatal("promoteDirectoryWithHooks() error = nil, want source identity failure") - } - assertFileBytes(t, filepath.Join(preserved, "value.txt"), []byte("original")) - assertFileBytes(t, filepath.Join(src, "value.txt"), []byte("replacement")) - assertPathMissing(t, dst) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryRejectsIdentityPreservingSourceRootSymlinkReplacement(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - preserved := filepath.Join(root, "source-preserved") - dst := filepath.Join(root, "destination") - mustWriteFile(t, filepath.Join(src, "value.txt"), []byte("original"), 0o644) - - err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ - afterRootInspect: func() { - if err := os.Rename(src, preserved); err != nil { - t.Fatalf("Rename(inspected source) error = %v", err) - } - if err := os.Symlink(filepath.Base(preserved), src); err != nil { - t.Skipf("Symlink() unavailable: %v", err) - } - }, - }) - if err == nil { - t.Fatal("promoteDirectoryWithHooks() error = nil, want source-root symlink replacement failure") - } - assertFileBytes(t, filepath.Join(preserved, "value.txt"), []byte("original")) - assertPathIsSymlink(t, src) - assertPathMissing(t, dst) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryRejectsInspectedDirectorySymlinkReplacement(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - child := filepath.Join(src, "child") - preserved := filepath.Join(src, "child-preserved") - outside := filepath.Join(root, "outside") - dst := filepath.Join(root, "destination") - mustWriteFile(t, filepath.Join(child, "value.txt"), []byte("original"), 0o644) - mustWriteFile(t, filepath.Join(outside, "sentinel.txt"), []byte("outside"), 0o644) - - replaced := false - err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ - afterEntryInspect: func(path string) { - if replaced || path != child { - return - } - replaced = true - if err := os.Rename(child, preserved); err != nil { - t.Fatalf("Rename(inspected child) error = %v", err) - } - if err := os.Symlink(filepath.Join("..", "outside"), child); err != nil { - t.Skipf("Symlink() unavailable: %v", err) - } - if err := os.Mkdir(dst, 0o755); err != nil { - t.Fatalf("Mkdir(concurrent destination) error = %v", err) - } - mustWriteFile(t, filepath.Join(dst, "value.txt"), []byte("concurrent"), 0o644) - }, - }) - if err == nil { - t.Fatal("promoteDirectoryWithHooks() error = nil, want symlink replacement failure") - } - assertFileBytes(t, filepath.Join(preserved, "value.txt"), []byte("original")) - assertFileBytes(t, filepath.Join(outside, "sentinel.txt"), []byte("outside")) - assertFileBytes(t, filepath.Join(dst, "value.txt"), []byte("concurrent")) - assertPathMissing(t, filepath.Join(dst, "sentinel.txt")) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryRejectsInspectedFileIdentityMismatch(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - file := filepath.Join(src, "value.txt") - preserved := filepath.Join(src, "value-preserved.txt") - dst := filepath.Join(root, "destination") - mustWriteFile(t, file, []byte("original"), 0o644) - - replaced := false - err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ - afterEntryInspect: func(path string) { - if replaced || path != file { - return - } - replaced = true - if err := os.Rename(file, preserved); err != nil { - t.Fatalf("Rename(inspected file) error = %v", err) - } - mustWriteFile(t, file, []byte("replacement"), 0o644) - }, - }) - if err == nil { - t.Fatal("promoteDirectoryWithHooks() error = nil, want file identity failure") - } - assertFileBytes(t, preserved, []byte("original")) - assertFileBytes(t, file, []byte("replacement")) - assertPathMissing(t, dst) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryRejectsIdentityPreservingFileSymlinkReplacement(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - file := filepath.Join(src, "value.txt") - preserved := filepath.Join(src, "value-preserved.txt") - dst := filepath.Join(root, "destination") - mustWriteFile(t, file, []byte("original"), 0o644) - - replaced := false - err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ - afterEntryInspect: func(path string) { - if replaced || path != file { - return - } - replaced = true - if err := os.Rename(file, preserved); err != nil { - t.Fatalf("Rename(inspected file) error = %v", err) - } - if err := os.Symlink(filepath.Base(preserved), file); err != nil { - t.Skipf("Symlink() unavailable: %v", err) - } - }, - }) - if err == nil { - t.Fatal("promoteDirectoryWithHooks() error = nil, want file symlink replacement failure") - } - assertFileBytes(t, preserved, []byte("original")) - assertPathIsSymlink(t, file) - assertPathMissing(t, dst) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryRejectsInspectedDirectoryIdentityMismatch(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - child := filepath.Join(src, "child") - preserved := filepath.Join(src, "child-preserved") - dst := filepath.Join(root, "destination") - mustWriteFile(t, filepath.Join(child, "value.txt"), []byte("original"), 0o644) - - replaced := false - err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ - afterEntryInspect: func(path string) { - if replaced || path != child { - return - } - replaced = true - if err := os.Rename(child, preserved); err != nil { - t.Fatalf("Rename(inspected directory) error = %v", err) - } - mustWriteFile(t, filepath.Join(child, "value.txt"), []byte("replacement"), 0o644) - }, - }) - if err == nil { - t.Fatal("promoteDirectoryWithHooks() error = nil, want directory identity failure") - } - assertFileBytes(t, filepath.Join(preserved, "value.txt"), []byte("original")) - assertFileBytes(t, filepath.Join(child, "value.txt"), []byte("replacement")) - assertPathMissing(t, dst) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryRejectsIdentityPreservingDirectorySymlinkReplacement(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - child := filepath.Join(src, "child") - preserved := filepath.Join(src, "child-preserved") - dst := filepath.Join(root, "destination") - mustWriteFile(t, filepath.Join(child, "value.txt"), []byte("original"), 0o644) - - replaced := false - err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ - afterEntryInspect: func(path string) { - if replaced || path != child { - return - } - replaced = true - if err := os.Rename(child, preserved); err != nil { - t.Fatalf("Rename(inspected directory) error = %v", err) - } - if err := os.Symlink(filepath.Base(preserved), child); err != nil { - t.Skipf("Symlink() unavailable: %v", err) - } - }, - }) - if err == nil { - t.Fatal("promoteDirectoryWithHooks() error = nil, want directory symlink replacement failure") - } - assertFileBytes(t, filepath.Join(preserved, "value.txt"), []byte("original")) - assertPathIsSymlink(t, child) - assertPathMissing(t, dst) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryDoesNotReplaceDestinationCreatedBeforeInstall(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - dst := filepath.Join(root, "destination") - mustWriteFile(t, filepath.Join(src, "value.txt"), []byte("source"), 0o644) - - err := promoteDirectory(src, dst, func(temporary, destination string) error { - if err := os.Mkdir(destination, 0o755); err != nil { - t.Fatalf("Mkdir(concurrent destination) error = %v", err) - } - mustWriteFile(t, filepath.Join(destination, "value.txt"), []byte("concurrent"), 0o644) - return renameDirectoryNoReplace(temporary, destination) - }) - if err == nil { - t.Fatal("promoteDirectory() error = nil, want no-replace install failure") - } - assertFileBytes(t, filepath.Join(dst, "value.txt"), []byte("concurrent")) - assertFileBytes(t, filepath.Join(src, "value.txt"), []byte("source")) - assertNoMatchingTempDirectories(t, root, ".destination.tmp-") -} - -func TestPromoteDirectoryRejectsDestinationInsideSource(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - dst := filepath.Join(src, "nested", "destination") - if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { - t.Fatalf("MkdirAll(destination parent) error = %v", err) - } - - if err := PromoteDirectory(src, dst); err == nil { - t.Fatal("PromoteDirectory() error = nil, want nested destination rejection") - } - if _, err := os.Lstat(dst); !os.IsNotExist(err) { - t.Fatalf("Lstat(destination) error = %v, want not exist", err) - } -} - -func TestPromoteDirectoryRejectsSymlinkedDestinationAncestor(t *testing.T) { - root := t.TempDir() - source := filepath.Join(root, "source") - if err := os.Mkdir(source, 0o755); err != nil { - t.Fatalf("Mkdir(source) error = %v", err) - } - if err := os.WriteFile(filepath.Join(source, "value.txt"), []byte("source"), 0o644); err != nil { - t.Fatalf("WriteFile(source) error = %v", err) - } - outside := t.TempDir() - if err := os.Symlink(outside, filepath.Join(root, "redirect")); err != nil { - t.Skipf("Symlink unavailable: %v", err) - } - - err := PromoteDirectory(source, filepath.Join(root, "redirect", "bundle")) - if err == nil { - t.Fatal("PromoteDirectory() error = nil, want symlink ancestor rejection") - } - if _, err := os.Stat(filepath.Join(outside, "bundle")); !os.IsNotExist(err) { - t.Fatalf("outside bundle exists: stat err = %v", err) - } -} - -func mustWriteFile(t *testing.T, path string, data []byte, mode os.FileMode) { - t.Helper() - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - t.Fatalf("MkdirAll(%q) error = %v", filepath.Dir(path), err) - } - if err := os.WriteFile(path, data, mode); err != nil { - t.Fatalf("WriteFile(%q) error = %v", path, err) - } -} - -func assertFileBytes(t *testing.T, path string, want []byte) { - t.Helper() - got, err := os.ReadFile(path) - if err != nil { - t.Fatalf("ReadFile(%q) error = %v", path, err) - } - if !bytes.Equal(got, want) { - t.Fatalf("ReadFile(%q) = %v, want %v", path, got, want) - } -} - -func assertPathMissing(t *testing.T, path string) { - t.Helper() - if _, err := os.Lstat(path); !os.IsNotExist(err) { - t.Fatalf("Lstat(%q) error = %v, want not exist", path, err) - } -} - -func assertPathIsSymlink(t *testing.T, path string) { - t.Helper() - info, err := os.Lstat(path) - if err != nil || info.Mode()&os.ModeSymlink == 0 { - t.Fatalf("Lstat(%q) info = %v, error = %v, want symlink", path, info, err) - } -} - -func treeLayout(t *testing.T, root string) []string { - t.Helper() - var layout []string - err := filepath.WalkDir(root, func(path string, _ os.DirEntry, err error) error { - if err != nil { - return err - } - relative, err := filepath.Rel(root, path) - if err != nil { - return err - } - layout = append(layout, filepath.ToSlash(relative)) - return nil - }) - if err != nil { - t.Fatalf("WalkDir(%q) error = %v", root, err) - } - return layout -} - -func assertNoMatchingTempDirectories(t *testing.T, parent, prefix string) { - t.Helper() - entries, err := os.ReadDir(parent) - if err != nil { - t.Fatalf("ReadDir(%q) error = %v", parent, err) - } - for _, entry := range entries { - if strings.HasPrefix(entry.Name(), prefix) { - t.Fatalf("unexpected temporary directory residue: %s", filepath.Join(parent, entry.Name())) - } - } -} diff --git a/internal/fileops/directory_unsupported_test.go b/internal/fileops/directory_unsupported_test.go deleted file mode 100644 index 4ff079a..0000000 --- a/internal/fileops/directory_unsupported_test.go +++ /dev/null @@ -1,37 +0,0 @@ -//go:build !linux && !darwin && !windows - -package fileops - -import ( - "errors" - "os" - "path/filepath" - "strings" - "testing" -) - -func TestPromoteDirectoryFailsBeforeCreatingTemporaryTree(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - dst := filepath.Join(root, "destination") - if err := os.Mkdir(src, 0o755); err != nil { - t.Fatalf("Mkdir(source) error = %v", err) - } - if err := os.WriteFile(filepath.Join(src, "value.txt"), []byte("source"), 0o644); err != nil { - t.Fatalf("WriteFile(source) error = %v", err) - } - - err := PromoteDirectory(src, dst) - if !errors.Is(err, ErrAtomicDirectoryPromotionUnsupported) { - t.Fatalf("PromoteDirectory() error = %v, want unsupported capability", err) - } - entries, readErr := os.ReadDir(root) - if readErr != nil { - t.Fatalf("ReadDir(root) error = %v", readErr) - } - for _, entry := range entries { - if entry.Name() == filepath.Base(dst) || strings.HasPrefix(entry.Name(), ".destination.tmp-") { - t.Fatalf("unsupported promotion created %q", entry.Name()) - } - } -} diff --git a/internal/fileops/modes_posix_test.go b/internal/fileops/modes_posix_test.go index 02b8087..78f17c1 100644 --- a/internal/fileops/modes_posix_test.go +++ b/internal/fileops/modes_posix_test.go @@ -36,31 +36,6 @@ func TestWorkspaceModesOverrideRestrictiveUmask(t *testing.T) { assertWorkspaceFileMode(t, file) } -func TestPromoteDirectoryUsesWorkspaceModesUnderRestrictiveUmask(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - if err := os.MkdirAll(filepath.Join(src, "nested"), 0o755); err != nil { - t.Fatalf("MkdirAll(source) error = %v", err) - } - if err := os.WriteFile(filepath.Join(src, "nested", "result.json"), []byte("result"), 0o600); err != nil { - t.Fatalf("WriteFile(source) error = %v", err) - } - - destinationParent := filepath.Join(root, "workspace", "artifacts", "notarius") - if err := EnsureWorkspaceDirectory(destinationParent); err != nil { - t.Fatalf("EnsureWorkspaceDirectory(destination parent) error = %v", err) - } - restoreUmask := syscall.Umask(0o077) - t.Cleanup(func() { syscall.Umask(restoreUmask) }) - destination := filepath.Join(destinationParent, "run-1") - if err := PromoteDirectory(src, destination); err != nil { - t.Fatalf("PromoteDirectory() error = %v", err) - } - assertWorkspaceDirectoryMode(t, destination) - assertWorkspaceDirectoryMode(t, filepath.Join(destination, "nested")) - assertWorkspaceFileMode(t, filepath.Join(destination, "nested", "result.json")) -} - func assertWorkspaceDirectoryMode(t *testing.T, path string) { t.Helper() info, err := os.Stat(path) diff --git a/internal/fileops/rename_noreplace_darwin.go b/internal/fileops/rename_noreplace_darwin.go deleted file mode 100644 index 36bd3d0..0000000 --- a/internal/fileops/rename_noreplace_darwin.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build darwin - -package fileops - -import ( - "os" - - "golang.org/x/sys/unix" -) - -func checkAtomicDirectoryPromotionSupport() error { return nil } - -func renameDirectoryNoReplace(src, dst string) error { - return unix.RenamexNp(src, dst, unix.RENAME_EXCL) -} - -func renameDirectoryNoReplaceAt(parent *os.File, src, dst string) error { - return unix.RenameatxNp(int(parent.Fd()), src, int(parent.Fd()), dst, unix.RENAME_EXCL) -} diff --git a/internal/fileops/rename_noreplace_linux.go b/internal/fileops/rename_noreplace_linux.go deleted file mode 100644 index db65b90..0000000 --- a/internal/fileops/rename_noreplace_linux.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build linux - -package fileops - -import ( - "os" - - "golang.org/x/sys/unix" -) - -func checkAtomicDirectoryPromotionSupport() error { return nil } - -func renameDirectoryNoReplace(src, dst string) error { - return unix.Renameat2(unix.AT_FDCWD, src, unix.AT_FDCWD, dst, unix.RENAME_NOREPLACE) -} - -func renameDirectoryNoReplaceAt(parent *os.File, src, dst string) error { - return unix.Renameat2(int(parent.Fd()), src, int(parent.Fd()), dst, unix.RENAME_NOREPLACE) -} diff --git a/internal/fileops/rename_noreplace_other.go b/internal/fileops/rename_noreplace_other.go deleted file mode 100644 index cb93a9b..0000000 --- a/internal/fileops/rename_noreplace_other.go +++ /dev/null @@ -1,21 +0,0 @@ -//go:build !linux && !darwin && !windows - -package fileops - -import ( - "fmt" - "os" - "runtime" -) - -func checkAtomicDirectoryPromotionSupport() error { - return fmt.Errorf("%w on %s", ErrAtomicDirectoryPromotionUnsupported, runtime.GOOS) -} - -func renameDirectoryNoReplace(_, _ string) error { - return checkAtomicDirectoryPromotionSupport() -} - -func renameDirectoryNoReplaceAt(_ *os.File, _, _ string) error { - return checkAtomicDirectoryPromotionSupport() -} diff --git a/internal/fileops/rename_noreplace_supported_test.go b/internal/fileops/rename_noreplace_supported_test.go deleted file mode 100644 index 2685464..0000000 --- a/internal/fileops/rename_noreplace_supported_test.go +++ /dev/null @@ -1,26 +0,0 @@ -//go:build linux || darwin || windows - -package fileops - -import ( - "os" - "path/filepath" - "testing" -) - -func TestRenameDirectoryNoReplacePreservesExistingDestination(t *testing.T) { - root := t.TempDir() - src := filepath.Join(root, "source") - dst := filepath.Join(root, "destination") - mustWriteFile(t, filepath.Join(src, "value.txt"), []byte("source"), 0o644) - mustWriteFile(t, filepath.Join(dst, "value.txt"), []byte("existing"), 0o644) - - if err := renameDirectoryNoReplace(src, dst); err == nil { - t.Fatal("renameDirectoryNoReplace() error = nil, want existing destination failure") - } - assertFileBytes(t, filepath.Join(src, "value.txt"), []byte("source")) - assertFileBytes(t, filepath.Join(dst, "value.txt"), []byte("existing")) - if info, err := os.Stat(src); err != nil || !info.IsDir() { - t.Fatalf("source directory was not preserved: info=%v err=%v", info, err) - } -} diff --git a/internal/fileops/rename_noreplace_windows.go b/internal/fileops/rename_noreplace_windows.go deleted file mode 100644 index cb345f3..0000000 --- a/internal/fileops/rename_noreplace_windows.go +++ /dev/null @@ -1,18 +0,0 @@ -//go:build windows - -package fileops - -import ( - "fmt" - "os" -) - -func checkAtomicDirectoryPromotionSupport() error { - return fmt.Errorf("%w on windows", ErrAtomicDirectoryPromotionUnsupported) -} - -func renameDirectoryNoReplace(src, dst string) error { return checkAtomicDirectoryPromotionSupport() } - -func renameDirectoryNoReplaceAt(parent *os.File, src, dst string) error { - return checkAtomicDirectoryPromotionSupport() -} diff --git a/internal/fileops/sync_directory_unix.go b/internal/fileops/sync_directory_unix.go deleted file mode 100644 index 2001d17..0000000 --- a/internal/fileops/sync_directory_unix.go +++ /dev/null @@ -1,30 +0,0 @@ -//go:build linux || darwin - -package fileops - -import ( - "errors" - "fmt" - "os" - "syscall" -) - -func syncDirectory(path string) error { - directory, err := os.Open(path) - if err != nil { - return err - } - defer func() { _ = directory.Close() }() - - return syncDirectoryFile(directory, path) -} - -func syncDirectoryFile(directory *os.File, path string) error { - err := directory.Sync() - // Some Unix filesystems do not implement directory syncing. Report this - // explicitly so callers do not confuse visible replacement with a durable one. - if errors.Is(err, syscall.EINVAL) || errors.Is(err, syscall.ENOTSUP) { - return fmt.Errorf("%w for %q: %w", ErrDirectorySyncUnsupported, path, err) - } - return err -} diff --git a/internal/fileops/sync_directory_unsupported.go b/internal/fileops/sync_directory_unsupported.go deleted file mode 100644 index 8750fdb..0000000 --- a/internal/fileops/sync_directory_unsupported.go +++ /dev/null @@ -1,17 +0,0 @@ -//go:build !linux && !darwin && !windows - -package fileops - -import ( - "fmt" - "os" - "runtime" -) - -func syncDirectory(path string) error { - return fmt.Errorf("%w for %q on %s", ErrDirectorySyncUnsupported, path, runtime.GOOS) -} - -func syncDirectoryFile(_ *os.File, path string) error { - return syncDirectory(path) -} diff --git a/internal/fileops/sync_directory_windows.go b/internal/fileops/sync_directory_windows.go deleted file mode 100644 index f91b06d..0000000 --- a/internal/fileops/sync_directory_windows.go +++ /dev/null @@ -1,52 +0,0 @@ -//go:build windows - -package fileops - -import ( - "errors" - "fmt" - "os" - - "golang.org/x/sys/windows" -) - -func syncDirectory(path string) error { - pathPointer, err := windows.UTF16PtrFromString(path) - if err != nil { - return err - } - directory, err := windows.CreateFile( - pathPointer, - windows.GENERIC_WRITE, - windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, - nil, - windows.OPEN_EXISTING, - windows.FILE_FLAG_BACKUP_SEMANTICS, - 0, - ) - if err != nil { - return err - } - defer func() { _ = windows.CloseHandle(directory) }() - - err = windows.FlushFileBuffers(directory) - // Windows filesystems may reject flushing a directory handle even when it - // was opened correctly. Report documented unavailable forms explicitly so - // callers do not confuse visible replacement with a durable one. - if errors.Is(err, windows.ERROR_INVALID_FUNCTION) || - errors.Is(err, windows.ERROR_INVALID_HANDLE) || - errors.Is(err, windows.ERROR_NOT_SUPPORTED) { - return fmt.Errorf("%w for %q: %w", ErrDirectorySyncUnsupported, path, err) - } - return err -} - -func syncDirectoryFile(directory *os.File, path string) error { - err := directory.Sync() - if errors.Is(err, windows.ERROR_INVALID_FUNCTION) || - errors.Is(err, windows.ERROR_INVALID_HANDLE) || - errors.Is(err, windows.ERROR_NOT_SUPPORTED) { - return fmt.Errorf("%w for %q: %w", ErrDirectorySyncUnsupported, path, err) - } - return err -} diff --git a/internal/stage/extract.go b/internal/stage/extract.go index fff26ee..e7e04bc 100644 --- a/internal/stage/extract.go +++ b/internal/stage/extract.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "os" "os/exec" @@ -33,7 +34,7 @@ type extractStage struct{} func (extractStage) Name() string { return "extract" } -func (extractStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*StageResult, error) { +func (extractStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (result *StageResult, resultErr error) { if env == nil || env.Config == nil || env.Config.Pipeline == nil || env.Config.Session == nil { return nil, fmt.Errorf("extract: resolved stage environment config is required") } @@ -154,6 +155,15 @@ func (extractStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S if err != nil { return nil, fmt.Errorf("extract: run notarius: %w", err) } + bundleInstalled := false + defer func() { + if bundleInstalled { + return + } + if cleanupErr := os.RemoveAll(outputRoot); cleanupErr != nil { + resultErr = errors.Join(resultErr, fmt.Errorf("extract: remove failed Notarius output root: %w", cleanupErr)) + } + }() if strings.TrimSpace(adapterResult.BundleRoot) == "" || strings.TrimSpace(adapterResult.Index.Path) == "" { return nil, fmt.Errorf("extract: notarius result is missing bundle or index path") } @@ -189,8 +199,24 @@ func (extractStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S return nil, fmt.Errorf("extract: validate staging index: %w", err) } - if err := fileops.PromoteDirectory(adapterResult.BundleRoot, durableBundle); err != nil { - return nil, fmt.Errorf("extract: promote notarius bundle: %w", err) + bundleRelative, err := pathsafe.SlashRelativeFromRoot(outputRoot, adapterResult.BundleRoot) + if err != nil || bundleRelative == "." || bundleRelative == "" { + return nil, fmt.Errorf("extract: Notarius bundle must be a proper descendant of the run output root") + } + if err := normalizeNotariusBundle(adapterResult.BundleRoot); err != nil { + return nil, fmt.Errorf("extract: normalize Notarius bundle: %w", err) + } + if _, err := os.Lstat(durableBundle); err == nil { + return nil, fmt.Errorf("extract: durable Notarius bundle destination already exists") + } else if !os.IsNotExist(err) { + return nil, fmt.Errorf("extract: inspect durable Notarius bundle destination: %w", err) + } + if err := os.Rename(adapterResult.BundleRoot, durableBundle); err != nil { + return nil, fmt.Errorf("extract: install Notarius bundle: %w", err) + } + bundleInstalled = true + if err := os.Remove(outputRoot); err != nil && !os.IsNotExist(err) { + return nil, fmt.Errorf("extract: remove empty Notarius output root: %w", err) } promotedIndexPath, err := pathsafe.JoinSlashRelativeUnderRoot(durableBundle, indexRelative) if err != nil { @@ -293,6 +319,28 @@ func (extractStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S }, nil } +func normalizeNotariusBundle(root string) error { + return filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + info, err := entry.Info() + if err != nil { + return err + } + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("bundle path %q is a symlink", path) + } + if info.IsDir() { + return os.Chmod(path, fileops.WorkspaceDirectoryMode) + } + if !info.Mode().IsRegular() { + return fmt.Errorf("bundle path %q is not regular", path) + } + return os.Chmod(path, fileops.WorkspaceFileMode) + }) +} + type selectedNotariusLane struct { Key string Descriptor notarius.LaneDescriptor diff --git a/internal/stage/extract_test.go b/internal/stage/extract_test.go index b7c1742..63e4602 100644 --- a/internal/stage/extract_test.go +++ b/internal/stage/extract_test.go @@ -310,8 +310,8 @@ func TestExtractStageProducesImmutableManifestReadyOutputs(t *testing.T) { t.Fatalf("promoted file %q missing: %v", relative, err) } } - if _, err := os.Stat(filepath.Join(stagingBundle, "unknown", "private-debug.json")); err != nil { - t.Fatalf("source bundle was not preserved: %v", err) + if _, err := os.Stat(stagingBundle); !os.IsNotExist(err) { + t.Fatalf("staging bundle still exists after rename: %v", err) } for _, output := range result.Outputs { if strings.Contains(output.AbsolutePath, "unconfigured") || strings.Contains(output.AbsolutePath, "private-debug") { @@ -507,11 +507,11 @@ func TestExtractStageAdapterAndPromotionFailuresReturnNoOutputs(t *testing.T) { t.Fatalf("MkdirAll(destination) error = %v", err) } result, err := (extractStage{}).Run(context.Background(), env, m) - if err == nil || !strings.Contains(err.Error(), "promote") || result != nil { + if err == nil || !strings.Contains(err.Error(), "destination already exists") || result != nil { t.Fatalf("Run() result = %#v error = %v", result, err) } - if _, err := os.Stat(fake.Result.BundleRoot); err != nil { - t.Fatalf("failed promotion removed staging bundle: %v", err) + if _, err := os.Stat(fake.Result.BundleRoot); !os.IsNotExist(err) { + t.Fatalf("failed installation staging bundle still exists: %v", err) } }) }