From 1ad6a8df47d6a9b632bcb05e516a1a4f40e2665f Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Mon, 18 Dec 2023 09:17:50 -0500 Subject: [PATCH] ensure file moves do not occur across filesystems Signed-off-by: Alex Goodman --- store.go | 1 + tool/install.go | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/store.go b/store.go index 2dce47b..41fd48e 100644 --- a/store.go +++ b/store.go @@ -152,6 +152,7 @@ func (s *Store) AddTool(toolName string, resolvedVersion, pathOutsideRoot string // move the file into the store at root/basename targetName := toolName targetPath := filepath.Join(s.root, toolName) + if err := os.Rename(pathOutsideRoot, targetPath); err != nil { return err } diff --git a/tool/install.go b/tool/install.go index 1a4e7ea..d55b986 100644 --- a/tool/install.go +++ b/tool/install.go @@ -28,7 +28,12 @@ func Install(tool binny.Tool, intent binny.VersionIntent, store *binny.Store, ve } }() - tmpdir, err := os.MkdirTemp("", fmt.Sprintf("binny-install-%s-", tool.Name())) + // note: we choose a staging directory that is within the store to ensure the final move is atomic + // and not across filesystems (which would not succeed). We do this instead of a copy in case one of the binaries + // being managed is in use, in which case a copy would fail with "text file busy" error, whereas a move would + // allow for the in-use binary to continue to be used (since an unlink is performed on the path to the binary + // or the path to the parent of the binary). + tmpdir, err := os.MkdirTemp(store.Root(), fmt.Sprintf("binny-install-%s-", tool.Name())) if err != nil { return fmt.Errorf("failed to create temp directory: %w", err) }