From b3fd6ae4dbaa2932d6629683d04e6eaa6b1a58f7 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Wed, 13 Nov 2024 16:23:22 -0800 Subject: [PATCH 1/3] Recognize circular links --- internal/smartlink/smartlink.go | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/internal/smartlink/smartlink.go b/internal/smartlink/smartlink.go index feb5134fa5..2441fa489a 100644 --- a/internal/smartlink/smartlink.go +++ b/internal/smartlink/smartlink.go @@ -7,6 +7,7 @@ import ( "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/fileutils" "github.com/ActiveState/cli/internal/logging" + "github.com/ActiveState/cli/internal/multilog" ) // LinkContents will link the contents of src to desc @@ -55,6 +56,15 @@ func Link(src, dest string) error { return errs.Wrap(err, "could not read directory %s", src) } for _, entry := range entries { + circular, err := isCircularLink(filepath.Join(src, entry.Name())) + if err != nil { + return errs.Wrap(err, "could not check for circular link") + } + if circular { + multilog.Error("Build contains a circular symlink: %s -> %s", filepath.Join(src, entry.Name()), dest) + continue + } + if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name())); err != nil { return errs.Wrap(err, "sub link failed") } @@ -80,6 +90,31 @@ func Link(src, dest string) error { return nil } +func isCircularLink(src string) (bool, error) { + if !fileutils.IsSymlink(src) { + return false, nil + } + + target, err := os.Readlink(src) + if err != nil { + return false, errs.Wrap(err, "could not read symlink %s", src) + } + + if !filepath.IsAbs(target) { + resolved, err := fileutils.ResolveUniquePath(src) + if err != nil { + return false, errs.Wrap(err, "Could not resolve src path") + } + target = resolved + } + + if fileutils.IsDir(src) && filepath.Dir(src) == target { + return true, nil + } + + return false, nil +} + // UnlinkContents will unlink the contents of src to dest if the links exist // WARNING: on windows smartlinks are hard links, and relating hard links back to their source is non-trivial, so instead // we just delete the target path. If the user modified the target in any way their changes will be lost. From 69f2cff65c2b4473cbad7ce3171c9827f3acf092 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 14 Nov 2024 09:27:34 -0800 Subject: [PATCH 2/3] Return early --- internal/smartlink/smartlink.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/smartlink/smartlink.go b/internal/smartlink/smartlink.go index 2441fa489a..2f7a33b6f5 100644 --- a/internal/smartlink/smartlink.go +++ b/internal/smartlink/smartlink.go @@ -91,7 +91,7 @@ func Link(src, dest string) error { } func isCircularLink(src string) (bool, error) { - if !fileutils.IsSymlink(src) { + if !fileutils.IsSymlink(src) || !fileutils.IsDir(src) { return false, nil } @@ -108,7 +108,8 @@ func isCircularLink(src string) (bool, error) { target = resolved } - if fileutils.IsDir(src) && filepath.Dir(src) == target { + // If the target points at the parent directory, we have a circular link + if filepath.Dir(src) == target { return true, nil } From aa2a4ee7dd4e2d4f7f1722868b6fa06ed7f723a0 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 14 Nov 2024 13:36:50 -0800 Subject: [PATCH 3/3] Move check and add test --- internal/smartlink/smartlink.go | 20 ++++----- internal/smartlink/smartlink_test.go | 67 ++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 internal/smartlink/smartlink_test.go diff --git a/internal/smartlink/smartlink.go b/internal/smartlink/smartlink.go index 2f7a33b6f5..8f85f77310 100644 --- a/internal/smartlink/smartlink.go +++ b/internal/smartlink/smartlink.go @@ -7,7 +7,6 @@ import ( "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/fileutils" "github.com/ActiveState/cli/internal/logging" - "github.com/ActiveState/cli/internal/multilog" ) // LinkContents will link the contents of src to desc @@ -41,7 +40,15 @@ func LinkContents(src, dest string) error { // Link creates a link from src to target. MS decided to support Symlinks but only if you opt into developer mode (go figure), // which we cannot reasonably force on our users. So on Windows we will instead create dirs and hardlinks. func Link(src, dest string) error { - var err error + isCircular, err := isCircularLink(src) + if err != nil { + return errs.Wrap(err, "could not check for circular link") + } + if isCircular { + logging.Warning("Skipping linking '%s' to '%s' as it is a circular link", src, dest) + return nil + } + src, dest, err = resolvePaths(src, dest) if err != nil { return errs.Wrap(err, "Could not resolve src and dest paths") @@ -56,15 +63,6 @@ func Link(src, dest string) error { return errs.Wrap(err, "could not read directory %s", src) } for _, entry := range entries { - circular, err := isCircularLink(filepath.Join(src, entry.Name())) - if err != nil { - return errs.Wrap(err, "could not check for circular link") - } - if circular { - multilog.Error("Build contains a circular symlink: %s -> %s", filepath.Join(src, entry.Name()), dest) - continue - } - if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name())); err != nil { return errs.Wrap(err, "sub link failed") } diff --git a/internal/smartlink/smartlink_test.go b/internal/smartlink/smartlink_test.go new file mode 100644 index 0000000000..bc90a81ab2 --- /dev/null +++ b/internal/smartlink/smartlink_test.go @@ -0,0 +1,67 @@ +package smartlink + +import ( + "os" + "path/filepath" + "testing" + + "github.com/ActiveState/cli/internal/fileutils" + "github.com/stretchr/testify/require" +) + +func TestLinkContentsWithCircularLink(t *testing.T) { + srcDir, err := os.MkdirTemp("", "src") + require.NoError(t, err) + defer os.RemoveAll(srcDir) + + destDir, err := os.MkdirTemp("", "dest") + require.NoError(t, err) + defer os.RemoveAll(destDir) + + // Create test file structure: + // src/ + // ├── regular.txt + // └── subdir/ + // ├── circle -> subdir (circular link) + // └── subfile.txt + + testFile := filepath.Join(srcDir, "regular.txt") + err = os.WriteFile(testFile, []byte("test content"), 0644) + require.NoError(t, err) + + subDir := filepath.Join(srcDir, "subdir") + err = os.Mkdir(subDir, 0755) + require.NoError(t, err) + + subFile := filepath.Join(subDir, "subfile.txt") + err = os.WriteFile(subFile, []byte("sub content"), 0644) + require.NoError(t, err) + + circularLink := filepath.Join(subDir, "circle") + err = os.Symlink(subDir, circularLink) + require.NoError(t, err) + + err = LinkContents(srcDir, destDir) + require.NoError(t, err) + + // Verify file structure: + // dest/ + // ├── regular.txt + // └── subdir/ + // └── subfile.txt + + destFile := filepath.Join(destDir, "regular.txt") + require.FileExists(t, destFile) + content, err := os.ReadFile(destFile) + require.NoError(t, err) + require.Equal(t, "test content", string(content)) + + destSubFile := filepath.Join(destDir, "subdir", "subfile.txt") + require.FileExists(t, destSubFile) + subContent, err := os.ReadFile(destSubFile) + require.NoError(t, err) + require.Equal(t, "sub content", string(subContent)) + + destCircular := filepath.Join(destDir, "subdir", "circle") + require.False(t, fileutils.TargetExists(destCircular), "Circular link should not have been created") +}