fix: validate and remove dangling in-bindle symlinks

fixes #57

Author: Zebradil

pkg/vendir/cmd/sync.go

@@ -128,12 +128,13 @@ func (o *SyncOptions) Run() error {
 		return fmt.Errorf("Unable to create cache: %s", err)
 	}
 	syncOpts := ctldir.SyncOpts{
-		RefFetcher:     ctldir.NewNamedRefFetcher(secrets, configMaps),
-		GithubAPIToken: os.Getenv("VENDIR_GITHUB_API_TOKEN"),
-		HelmBinary:     os.Getenv("VENDIR_HELM_BINARY"),
-		Cache:          cache,
-		Lazy:           o.Lazy,
-		Partial:        len(dirs) > 0,
+		RefFetcher:                  ctldir.NewNamedRefFetcher(secrets, configMaps),
+		GithubAPIToken:              os.Getenv("VENDIR_GITHUB_API_TOKEN"),
+		HelmBinary:                  os.Getenv("VENDIR_HELM_BINARY"),
+		Cache:                       cache,
+		Lazy:                        o.Lazy,
+		Partial:                     len(dirs) > 0,
+		AllowAllSymlinkDestinations: o.AllowAllSymlinkDestinations,
 	}
 	newLockConfig := ctlconf.NewLockConfig()
 
@@ -145,12 +146,6 @@ func (o *SyncOptions) Run() error {
 		if err != nil {
 			return fmt.Errorf("Syncing directory '%s': %s", dirConf.Path, err)
 		}
-		if !o.AllowAllSymlinkDestinations {
-			err = ctldir.ValidateSymlinks(dirConf.Path)
-			if err != nil {
-				return err
-			}
-		}
 
 		newLockConfig.Directories = append(newLockConfig.Directories, dirLockConf)
 	}

pkg/vendir/directory/directory.go

@@ -38,12 +38,13 @@ func NewDirectory(opts ctlconf.Directory, lockDirectory ctlconf.LockDirectory, u
 }
 
 type SyncOpts struct {
-	RefFetcher     ctlfetch.RefFetcher
-	GithubAPIToken string
-	HelmBinary     string
-	Cache          ctlcache.Cache
-	Lazy           bool
-	Partial        bool
+	RefFetcher                  ctlfetch.RefFetcher
+	GithubAPIToken              string
+	HelmBinary                  string
+	Cache                       ctlcache.Cache
+	Lazy                        bool
+	Partial                     bool
+	AllowAllSymlinkDestinations bool
 }
 
 func createConfigDigest(contents ctlconf.DirectoryContents) (string, error) {
@@ -230,11 +231,22 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) {
 			return lockConfig, fmt.Errorf("Unknown contents type for directory '%s'", contents.Path)
 		}
 
+		if !syncOpts.AllowAllSymlinkDestinations {
+			err = ValidateSymlinks(stagingDstPath)
+			if err != nil {
+				return lockConfig, fmt.Errorf("Validating symlinks in directory '%s': %s", contents.Path, err)
+			}
+		}
+
 		if !skipFileFilter {
 			err = FileFilter{contents}.Apply(stagingDstPath)
 			if err != nil {
 				return lockConfig, fmt.Errorf("Filtering paths in directory '%s': %s", contents.Path, err)
 			}
+			err = RemoveDanglingSymlinks(stagingDstPath)
+			if err != nil {
+				return lockConfig, fmt.Errorf("Removing dangling symlinks in directory '%s': %s", contents.Path, err)
+			}
 		}
 
 		if !skipNewRootPath && len(contents.NewRootPath) > 0 {

pkg/vendir/directory/symlink.go

@@ -11,6 +11,22 @@ import (
 	"strings"
 )
 
+// RemoveDanglingSymlinks removes symlinks within path whose targets do not exist.
+func RemoveDanglingSymlinks(path string) error {
+	return filepath.WalkDir(path, func(entryPath string, info fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.Type()&os.ModeSymlink == os.ModeSymlink {
+			_, statErr := os.Stat(entryPath)
+			if statErr != nil && os.IsNotExist(statErr) {
+				return os.Remove(entryPath)
+			}
+		}
+		return nil
+	})
+}
+
 // ValidateSymlinks enforces that symlinks inside the given path resolve to inside the path
 func ValidateSymlinks(path string) error {
 	absRoot, err := filepath.Abs(path)

pkg/vendir/directory/symlink_test.go

@@ -9,6 +9,52 @@ import (
 	"testing"
 )
 
+func TestRemoveDanglingSymlinks(t *testing.T) {
+	root, err := os.MkdirTemp("", "vendir-test")
+	if err != nil {
+		t.Fatalf("failed to create tmpdir: %v", err)
+	}
+	defer os.RemoveAll(root)
+
+	root, err = filepath.EvalSymlinks(root)
+	if err != nil {
+		t.Fatalf("failed to read link tmpdir: %v", err)
+	}
+
+	// existing file that a valid symlink points to
+	existingFile := filepath.Join(root, "existing.txt")
+	f, err := os.Create(existingFile)
+	if err != nil {
+		t.Fatalf("failed to create file: %v", err)
+	}
+	f.Close()
+
+	// valid symlink: points to existing file
+	validLink := filepath.Join(root, "valid_link")
+	if err = os.Symlink(existingFile, validLink); err != nil {
+		t.Fatalf("failed to create symlink: %v", err)
+	}
+
+	// dangling symlink: points to non-existent file
+	danglingLink := filepath.Join(root, "dangling_link")
+	if err = os.Symlink(filepath.Join(root, "nonexistent.txt"), danglingLink); err != nil {
+		t.Fatalf("failed to create symlink: %v", err)
+	}
+
+	if err = RemoveDanglingSymlinks(root); err != nil {
+		t.Fatalf("RemoveDanglingSymlinks() error = %v", err)
+	}
+
+	// valid symlink must still exist
+	if _, err = os.Lstat(validLink); err != nil {
+		t.Errorf("valid symlink was unexpectedly removed: %v", err)
+	}
+	// dangling symlink must be gone
+	if _, err = os.Lstat(danglingLink); err == nil {
+		t.Errorf("dangling symlink was not removed")
+	}
+}
+
 func TestValidateSymlinks(t *testing.T) {
 	root, err := os.MkdirTemp("", "vendir-test")
 	if err != nil {

test/e2e/invalid_symlink_test.go

@@ -13,6 +13,75 @@ import (
 	"sigs.k8s.io/yaml"
 )
 
+// TestSymlinkWithIncludePaths verifies that a sync with includePaths succeeds even when
+// the filtered content contains symlinks pointing to paths that are excluded by includePaths.
+// The dangling symlinks must be silently removed rather than causing an error.
+func TestSymlinkWithIncludePaths(t *testing.T) {
+	env := BuildEnv(t)
+	vendir := Vendir{t, env.BinaryPath, Logger{}}
+
+	tmpDir, err := os.MkdirTemp("", "vendir-test")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpDir)
+
+	tmpDir, err = filepath.EvalSymlinks(tmpDir)
+	require.NoError(t, err)
+
+	// Source layout:
+	//   src/
+	//     dir1/
+	//       kept.txt        <- included by includePaths
+	//       link_to_other   -> ../dir2/other.txt  (will dangle after filtering)
+	//     dir2/
+	//       other.txt       <- NOT included by includePaths
+	srcDir := filepath.Join(tmpDir, "src")
+	dir1 := filepath.Join(srcDir, "dir1")
+	dir2 := filepath.Join(srcDir, "dir2")
+	require.NoError(t, os.MkdirAll(dir1, os.ModePerm))
+	require.NoError(t, os.MkdirAll(dir2, os.ModePerm))
+
+	keptFile, err := os.Create(filepath.Join(dir1, "kept.txt"))
+	require.NoError(t, err)
+	keptFile.Close()
+
+	otherFile, err := os.Create(filepath.Join(dir2, "other.txt"))
+	require.NoError(t, err)
+	otherFile.Close()
+
+	// Symlink uses a relative path (../dir2/other.txt) so it is internal to the bundle
+	require.NoError(t, os.Symlink("../dir2/other.txt", filepath.Join(dir1, "link_to_other")))
+
+	cfg := config.Config{
+		APIVersion: "vendir.k14s.io/v1alpha1",
+		Kind:       "Config",
+		Directories: []config.Directory{{
+			Path: "result",
+			Contents: []config.DirectoryContents{{
+				Path: "out",
+				Directory: &config.DirectoryContentsDirectory{
+					Path: "src",
+				},
+				IncludePaths: []string{"dir1/**"},
+			}},
+		}},
+	}
+
+	cfgBytes, err := yaml.Marshal(cfg)
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "vendir.yml"), cfgBytes, 0666))
+
+	_, err = vendir.RunWithOpts([]string{"sync"}, RunOpts{Dir: tmpDir, AllowError: true})
+	require.NoError(t, err, "sync should succeed even though includePaths causes a dangling symlink")
+
+	// The dangling symlink must have been removed
+	_, err = os.Lstat(filepath.Join(tmpDir, "result", "out", "dir1", "link_to_other"))
+	require.True(t, os.IsNotExist(err), "dangling symlink should have been removed after filtering")
+
+	// The kept file must still be present
+	_, err = os.Stat(filepath.Join(tmpDir, "result", "out", "dir1", "kept.txt"))
+	require.NoError(t, err, "kept.txt should be present after filtering")
+}
+
 func TestInvalidSymlink(t *testing.T) {
 	env := BuildEnv(t)
 	vendir := Vendir{t, env.BinaryPath, Logger{}}