From 09c3b7cbea24ddbf905ec1ea75448ec21663a485 Mon Sep 17 00:00:00 2001 From: VictorHuu Date: Thu, 1 May 2025 02:47:32 +0800 Subject: [PATCH] fix:Resolve ancestral symlinks correctly (#3783) * Resolve upstream symlinks correctly Signed-off-by: Yuntao Hu * in case of the root directory Signed-off-by: Yuntao Hu * for static analysis check pass Signed-off-by: Yuntao Hu * add unit test cases for the symlink scenarios Signed-off-by: Yuntao Hu --------- Signed-off-by: Yuntao Hu --- .../fileresolver/directory_indexer.go | 22 ++++++++ .../fileresolver/directory_indexer_test.go | 51 +++++++++++++++++++ .../fileresolver/filetree_resolver_test.go | 2 +- .../module_1/module_1_1/place | 1 + .../system_paths/target/symlinks-to-dev | 1 + .../target/symlinks-to-hierarchical-dev | 1 + .../fileresolver/unindexed_directory_test.go | 2 +- 7 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 syft/internal/fileresolver/test-fixtures/system_paths/target/hierarchical-dev/module_1/module_1_1/place create mode 120000 syft/internal/fileresolver/test-fixtures/system_paths/target/symlinks-to-dev create mode 120000 syft/internal/fileresolver/test-fixtures/system_paths/target/symlinks-to-hierarchical-dev diff --git a/syft/internal/fileresolver/directory_indexer.go b/syft/internal/fileresolver/directory_indexer.go index 2d9101999..0e3509655 100644 --- a/syft/internal/fileresolver/directory_indexer.go +++ b/syft/internal/fileresolver/directory_indexer.go @@ -379,6 +379,28 @@ func (r directoryIndexer) addSymlinkToIndex(p string, info os.FileInfo) (string, // if the base is set, then we first need to resolve the link, // before finding it's location in the base dir, err := filepath.Rel(r.base, filepath.Dir(p)) + // if the relative path to the base contains "..",i.e. p is the parent or ancestor of the base + // For example: + // dir: "/root/asymlink" -> "/root/realdir" (linkTarget:"realdir") + // base: "/root/asymlink" + // so the relative path of /root to the "/root/asymlink" is ".." + // we cannot directly concatenate ".." to "/root/symlink",however, + // the parent directory of linkTarget should be "/root" + for strings.HasPrefix(dir, "..") { + if strings.HasPrefix(dir, "../") { + dir = strings.TrimPrefix(dir, "../") + } else { + dir = strings.TrimPrefix(dir, "..") + } + lastSlash := strings.LastIndex(r.base, "/") + if lastSlash != -1 { + r.base = r.base[:lastSlash] + } + // In case of the root directory + if r.base == "" { + r.base = "/" + } + } if err != nil { return "", fmt.Errorf("unable to resolve relative path for path=%q: %w", p, err) } diff --git a/syft/internal/fileresolver/directory_indexer_test.go b/syft/internal/fileresolver/directory_indexer_test.go index 3b736b667..1fe77106a 100644 --- a/syft/internal/fileresolver/directory_indexer_test.go +++ b/syft/internal/fileresolver/directory_indexer_test.go @@ -1,10 +1,12 @@ package fileresolver import ( + "fmt" "io/fs" "os" "path" "path/filepath" + "runtime" "sort" "strings" "testing" @@ -223,6 +225,55 @@ func TestDirectoryIndexer_index(t *testing.T) { } } +func TestDirectoryIndexer_index_for_AncestorSymlinks(t *testing.T) { + // note: this test is testing the effects from NewFromDirectory, indexTree, and addPathToIndex + _, filename, _, ok := runtime.Caller(0) + require.True(t, ok) + dir := filepath.Dir(filename) + + tests := []struct { + name string + path string + relative_base string + }{ + { + name: "the parent directory has symlink target", + path: "test-fixtures/system_paths/target/symlinks-to-dev", + relative_base: "test-fixtures/system_paths/target/symlinks-to-dev", + }, + { + name: "the ancestor directory has symlink target", + path: "test-fixtures/system_paths/target/symlinks-to-hierarchical-dev", + relative_base: "test-fixtures/system_paths/target/symlinks-to-hierarchical-dev/module_1/module_1_1", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + indexer := newDirectoryIndexer("test-fixtures/system_paths/target", + fmt.Sprintf("%v/%v", dir, test.relative_base)) + tree, index, err := indexer.build() + require.NoError(t, err) + info, err := os.Stat(test.path) + assert.NoError(t, err) + + // note: the index uses absolute paths, so assertions MUST keep this in mind + cwd, err := os.Getwd() + require.NoError(t, err) + + p := file.Path(path.Join(cwd, test.path)) + assert.Equal(t, true, tree.HasPath(p)) + exists, ref, err := tree.File(p) + assert.Equal(t, true, exists) + if assert.NoError(t, err) { + return + } + + entry, err := index.Get(*ref.Reference) + require.NoError(t, err) + assert.Equal(t, info.Mode(), entry.Mode) + }) + } +} func TestDirectoryIndexer_index_survive_badSymlink(t *testing.T) { // test-fixtures/bad-symlinks // ├── root diff --git a/syft/internal/fileresolver/filetree_resolver_test.go b/syft/internal/fileresolver/filetree_resolver_test.go index 1a3d66172..7687a41cc 100644 --- a/syft/internal/fileresolver/filetree_resolver_test.go +++ b/syft/internal/fileresolver/filetree_resolver_test.go @@ -811,7 +811,7 @@ func TestDirectoryResolverDoesNotIgnoreRelativeSystemPaths(t *testing.T) { // 4: within target/ // 1: target/link --> relative path to "place" // NOTE: this is filtered out since it not unique relative to outside_root/link_target/place // 1: outside_root/link_target/place - assert.Len(t, locations, 5) + assert.Len(t, locations, 6) // ensure that symlink indexing outside of root worked testLocation := "test-fixtures/system_paths/outside_root/link_target/place" diff --git a/syft/internal/fileresolver/test-fixtures/system_paths/target/hierarchical-dev/module_1/module_1_1/place b/syft/internal/fileresolver/test-fixtures/system_paths/target/hierarchical-dev/module_1/module_1_1/place new file mode 100644 index 000000000..44d6628cd --- /dev/null +++ b/syft/internal/fileresolver/test-fixtures/system_paths/target/hierarchical-dev/module_1/module_1_1/place @@ -0,0 +1 @@ +bad \ No newline at end of file diff --git a/syft/internal/fileresolver/test-fixtures/system_paths/target/symlinks-to-dev b/syft/internal/fileresolver/test-fixtures/system_paths/target/symlinks-to-dev new file mode 120000 index 000000000..90012116c --- /dev/null +++ b/syft/internal/fileresolver/test-fixtures/system_paths/target/symlinks-to-dev @@ -0,0 +1 @@ +dev \ No newline at end of file diff --git a/syft/internal/fileresolver/test-fixtures/system_paths/target/symlinks-to-hierarchical-dev b/syft/internal/fileresolver/test-fixtures/system_paths/target/symlinks-to-hierarchical-dev new file mode 120000 index 000000000..9e59e02b5 --- /dev/null +++ b/syft/internal/fileresolver/test-fixtures/system_paths/target/symlinks-to-hierarchical-dev @@ -0,0 +1 @@ +hierarchical-dev \ No newline at end of file diff --git a/syft/internal/fileresolver/unindexed_directory_test.go b/syft/internal/fileresolver/unindexed_directory_test.go index d21615fdd..8945e53b2 100644 --- a/syft/internal/fileresolver/unindexed_directory_test.go +++ b/syft/internal/fileresolver/unindexed_directory_test.go @@ -833,7 +833,7 @@ func Test_UnindexedDirectoryResolverDoesNotIgnoreRelativeSystemPaths(t *testing. // 4: within target/ // 1: target/link --> relative path to "place" // NOTE: this is filtered out since it not unique relative to outside_root/link_target/place // 1: outside_root/link_target/place - assert.Len(t, locations, 5) + assert.Len(t, locations, 6) // ensure that symlink indexing outside of root worked testLocation := "../outside_root/link_target/place"