From c96da4f77c64eafc3d565d7da8a53cf791f99f7b Mon Sep 17 00:00:00 2001 From: Giacomo Lanciano Date: Tue, 28 Oct 2025 17:51:38 +0100 Subject: [PATCH 1/4] chore: update .gitignore --- .gitignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index d5108eaa..f87ba09b 100644 --- a/.gitignore +++ b/.gitignore @@ -10,4 +10,6 @@ # Output of the go coverage tool, specifically when used with LiteIDE *.out -.idea \ No newline at end of file +.idea + +.vscode From 5631101117b1098a9bc9b685b9a6c6062efc2811 Mon Sep 17 00:00:00 2001 From: Giacomo Lanciano Date: Tue, 28 Oct 2025 18:02:54 +0100 Subject: [PATCH 2/4] feat: check for problematic dirs also in '/root/...' --- pkg/assessor/cache/cache.go | 25 ++++++++++++++-- pkg/assessor/cache/cache_test.go | 49 ++++++++++++++++++++++++++++++++ pkg/scanner/scan.go | 9 ++++++ 3 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 pkg/assessor/cache/cache_test.go diff --git a/pkg/assessor/cache/cache.go b/pkg/assessor/cache/cache.go index d6102c1a..51b22de0 100644 --- a/pkg/assessor/cache/cache.go +++ b/pkg/assessor/cache/cache.go @@ -32,7 +32,7 @@ func (a CacheAssessor) Assess(fileMap deckodertypes.FileMap) ([]*types.Assessmen dirBase := filepath.Base(dirName) // match Directory - if utils.StringInSlice(dirBase+"/", reqDirs) || utils.StringInSlice(dirName+"/", reqDirs) { + if utils.StringInSlice(dirBase+"/", reqDirs) || utils.StringInSlice(dirName+"/", reqDirs) || isUnderRootReqDir(dirName, reqDirs) { if _, ok := detectedDir[dirName]; ok { continue } @@ -77,8 +77,29 @@ func inIgnoreDir(filename string) bool { return false } +// isUnderRootReqDir checks if directory is under /root/ or root/ with a required directory +func isUnderRootReqDir(dirName string, reqDirs []string) bool { + for _, dir := range reqDirs { + cleanDir := strings.TrimSuffix(dir, "/") + if strings.HasPrefix(dirName, "root/"+cleanDir) || strings.HasPrefix(dirName, "/root/"+cleanDir) { + return true + } + } + return false +} + func (a CacheAssessor) RequiredFiles() []string { - return append(reqFiles, reqDirs...) + result := append(reqFiles, reqDirs...) + + // Add /root/ and root/ variants for directories only + // (tar files may or may not have leading slash) + // Files are already matched by their base name, so no need to add file variants + for _, dir := range reqDirs { + result = append(result, "/root/"+dir) + result = append(result, "root/"+dir) + } + + return result } func (a CacheAssessor) RequiredExtensions() []string { diff --git a/pkg/assessor/cache/cache_test.go b/pkg/assessor/cache/cache_test.go new file mode 100644 index 00000000..38ec9859 --- /dev/null +++ b/pkg/assessor/cache/cache_test.go @@ -0,0 +1,49 @@ +package cache + +import ( + "testing" +) + +func TestIsUnderRootReqDir(t *testing.T) { + tests := []struct { + name string + dirName string + expected bool + }{ + { + name: "Directory is root/.aws", + dirName: "root/.aws", + expected: true, + }, + { + name: "Directory is /root/.git", + dirName: "/root/.git", + expected: true, + }, + { + name: "Directory deep under root/.cache", + dirName: "root/.cache/pip/http-v2/1/2/3", + expected: true, + }, + { + name: "Directory not under root", + dirName: "/home/user/.aws", + expected: false, + }, + { + name: "Directory in root but not in reqDir", + dirName: "/root/somedir", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isUnderRootReqDir(tt.dirName, reqDirs) + if result != tt.expected { + t.Errorf("isUnderRootReqDir(%s) = %v, expected %v", tt.dirName, result, tt.expected) + } + }) + } +} + diff --git a/pkg/scanner/scan.go b/pkg/scanner/scan.go index 62d32de9..3fd0ca00 100644 --- a/pkg/scanner/scan.go +++ b/pkg/scanner/scan.go @@ -5,6 +5,7 @@ import ( "context" "os" "path/filepath" + "strings" "github.com/goodwithtech/deckoder/analyzer" "github.com/goodwithtech/deckoder/extractor" @@ -115,6 +116,14 @@ func createPathPermissionFilterFunc(filenames, extensions []string, permissions return true, nil } + // Check if file is under any required directory (not just immediate parent) + for reqDir := range requiredDirNames { + // Check if the file path starts with the required directory + if strings.HasPrefix(filePath, reqDir+"/") { + return true, nil + } + } + fi := h.FileInfo() fileMode := fi.Mode() for _, p := range permissions { From 8c5148c4b0d18d8d91b55f9f5e7b52183e63308e Mon Sep 17 00:00:00 2001 From: Giacomo Lanciano Date: Wed, 29 Oct 2025 12:43:33 +0100 Subject: [PATCH 3/4] feat: make detection logic more general --- pkg/assessor/cache/cache.go | 39 ++++----- pkg/assessor/cache/cache_test.go | 142 ++++++++++++++++++++++++++----- pkg/scanner/scan.go | 19 ++--- 3 files changed, 143 insertions(+), 57 deletions(-) diff --git a/pkg/assessor/cache/cache.go b/pkg/assessor/cache/cache.go index 51b22de0..d5bf0fd8 100644 --- a/pkg/assessor/cache/cache.go +++ b/pkg/assessor/cache/cache.go @@ -31,8 +31,20 @@ func (a CacheAssessor) Assess(fileMap deckodertypes.FileMap) ([]*types.Assessmen dirName := filepath.Dir(filename) dirBase := filepath.Base(dirName) - // match Directory - if utils.StringInSlice(dirBase+"/", reqDirs) || utils.StringInSlice(dirName+"/", reqDirs) || isUnderRootReqDir(dirName, reqDirs) { + // match Directory - check if any component of the directory path matches a required dir + matched := utils.StringInSlice(dirBase+"/", reqDirs) || utils.StringInSlice(dirName+"/", reqDirs) + if !matched { + // Check if any directory component in the path matches a required directory + parts := strings.Split(dirName, "/") + for _, part := range parts { + if utils.StringInSlice(part+"/", reqDirs) { + matched = true + break + } + } + } + + if matched { if _, ok := detectedDir[dirName]; ok { continue } @@ -77,29 +89,8 @@ func inIgnoreDir(filename string) bool { return false } -// isUnderRootReqDir checks if directory is under /root/ or root/ with a required directory -func isUnderRootReqDir(dirName string, reqDirs []string) bool { - for _, dir := range reqDirs { - cleanDir := strings.TrimSuffix(dir, "/") - if strings.HasPrefix(dirName, "root/"+cleanDir) || strings.HasPrefix(dirName, "/root/"+cleanDir) { - return true - } - } - return false -} - func (a CacheAssessor) RequiredFiles() []string { - result := append(reqFiles, reqDirs...) - - // Add /root/ and root/ variants for directories only - // (tar files may or may not have leading slash) - // Files are already matched by their base name, so no need to add file variants - for _, dir := range reqDirs { - result = append(result, "/root/"+dir) - result = append(result, "root/"+dir) - } - - return result + return append(reqFiles, reqDirs...) } func (a CacheAssessor) RequiredExtensions() []string { diff --git a/pkg/assessor/cache/cache_test.go b/pkg/assessor/cache/cache_test.go index 38ec9859..658a9757 100644 --- a/pkg/assessor/cache/cache_test.go +++ b/pkg/assessor/cache/cache_test.go @@ -2,48 +2,152 @@ package cache import ( "testing" + + "github.com/SpazioDati/dockle/pkg/log" + "github.com/SpazioDati/dockle/pkg/types" + deckodertypes "github.com/goodwithtech/deckoder/types" ) -func TestIsUnderRootReqDir(t *testing.T) { +func init() { + // Initialize logger for tests + log.InitLogger(false, false) +} + +func TestAssess(t *testing.T) { tests := []struct { - name string - dirName string - expected bool + name string + fileMap deckodertypes.FileMap + expectedCount int + expectedFiles []string }{ { - name: "Directory is root/.aws", - dirName: "root/.aws", - expected: true, + name: "detects suspicious directories anywhere in filesystem", + fileMap: deckodertypes.FileMap{ + "root/.cache/pip/http-v2/1/2/3/file": {}, + "root/.aws/credentials": {}, + "/root/.git/config": {}, + "home/user/.npm/registry": {}, + ".cache/direct": {}, + }, + expectedCount: 5, + expectedFiles: []string{"root/.cache/pip/http-v2/1/2/3", "root/.aws", "/root/.git", "home/user/.npm", ".cache"}, + }, + { + name: "detects required files by basename", + fileMap: deckodertypes.FileMap{ + "root/Dockerfile": {}, + "app/docker-compose.yml": {}, + "home/.vimrc": {}, + "project/.DS_Store": {}, + }, + expectedCount: 4, + expectedFiles: []string{"root/Dockerfile", "app/docker-compose.yml", "home/.vimrc", "project/.DS_Store"}, + }, + { + name: "deduplicates directories with multiple files", + fileMap: deckodertypes.FileMap{ + "root/.cache/file1": {}, + "root/.cache/file2": {}, + "root/.cache/file3": {}, + }, + expectedCount: 1, + expectedFiles: []string{"root/.cache"}, }, { - name: "Directory is /root/.git", - dirName: "/root/.git", + name: "ignores uncontrollable directories", + fileMap: deckodertypes.FileMap{ + "app/node_modules/.cache/file": {}, + "lib/vendor/.git/config": {}, + }, + expectedCount: 0, + expectedFiles: []string{}, + }, + { + name: "handles edge cases", + fileMap: deckodertypes.FileMap{ + "usr/bin/app": {}, + "etc/config.conf": {}, + }, + expectedCount: 0, + expectedFiles: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Reset detectedDir map before each test + detectedDir = map[string]struct{}{} + + assessor := CacheAssessor{} + assessments, err := assessor.Assess(tt.fileMap) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(assessments) != tt.expectedCount { + t.Errorf("expected %d assessments, got %d", tt.expectedCount, len(assessments)) + for i, a := range assessments { + t.Logf(" [%d] %s", i, a.Filename) + } + } + + // Verify expected files are present + assessmentMap := make(map[string]*types.Assessment) + for _, a := range assessments { + assessmentMap[a.Filename] = a + } + + for _, expectedFile := range tt.expectedFiles { + if _, found := assessmentMap[expectedFile]; !found { + t.Errorf("expected assessment for '%s' not found", expectedFile) + } + } + + // Verify all have correct code + for _, a := range assessments { + if a.Code != types.InfoDeletableFiles { + t.Errorf("wrong code for %s: got %v, want %v", a.Filename, a.Code, types.InfoDeletableFiles) + } + } + }) + } +} + +func TestInIgnoreDir(t *testing.T) { + tests := []struct { + name string + filename string + expected bool + }{ + { + name: "File in node_modules", + filename: "app/node_modules/.cache/file", expected: true, }, { - name: "Directory deep under root/.cache", - dirName: "root/.cache/pip/http-v2/1/2/3", + name: "File in vendor", + filename: "lib/vendor/.git/config", expected: true, }, { - name: "Directory not under root", - dirName: "/home/user/.aws", + name: "File not in ignore dir", + filename: "app/.cache/file", expected: false, }, { - name: "Directory in root but not in reqDir", - dirName: "/root/somedir", - expected: false, + name: "File with node_modules in name but not as directory", + filename: "app/my-node_modules-file.txt", + expected: false, // Only matches "node_modules/" with trailing slash }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := isUnderRootReqDir(tt.dirName, reqDirs) + result := inIgnoreDir(tt.filename) if result != tt.expected { - t.Errorf("isUnderRootReqDir(%s) = %v, expected %v", tt.dirName, result, tt.expected) + t.Errorf("inIgnoreDir(%s) = %v, expected %v", tt.filename, result, tt.expected) } }) } } - diff --git a/pkg/scanner/scan.go b/pkg/scanner/scan.go index 3fd0ca00..f5f22853 100644 --- a/pkg/scanner/scan.go +++ b/pkg/scanner/scan.go @@ -106,20 +106,11 @@ func createPathPermissionFilterFunc(filenames, extensions []string, permissions return true, nil } - // Check with file directory name - fileDir := filepath.Dir(filePath) - if _, ok := requiredDirNames[fileDir]; ok { - return true, nil - } - fileDirBase := filepath.Base(fileDir) - if _, ok := requiredDirNames[fileDirBase]; ok { - return true, nil - } - - // Check if file is under any required directory (not just immediate parent) - for reqDir := range requiredDirNames { - // Check if the file path starts with the required directory - if strings.HasPrefix(filePath, reqDir+"/") { + // Check if any directory component in the path matches a required directory + // This catches .cache/, .git/, etc. anywhere in the filesystem tree + parts := strings.Split(filePath, "/") + for _, part := range parts { + if _, ok := requiredDirNames[part]; ok { return true, nil } } From d226212f1aa6badaf9bf3e2928711f531c9b4d92 Mon Sep 17 00:00:00 2001 From: Giacomo Lanciano Date: Wed, 29 Oct 2025 12:55:30 +0100 Subject: [PATCH 4/4] feat: make output less noisy --- pkg/assessor/cache/cache.go | 15 ++++++++++----- pkg/assessor/cache/cache_test.go | 10 +++++----- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/pkg/assessor/cache/cache.go b/pkg/assessor/cache/cache.go index d5bf0fd8..01049245 100644 --- a/pkg/assessor/cache/cache.go +++ b/pkg/assessor/cache/cache.go @@ -33,22 +33,27 @@ func (a CacheAssessor) Assess(fileMap deckodertypes.FileMap) ([]*types.Assessmen // match Directory - check if any component of the directory path matches a required dir matched := utils.StringInSlice(dirBase+"/", reqDirs) || utils.StringInSlice(dirName+"/", reqDirs) + reportDir := dirName + if !matched { // Check if any directory component in the path matches a required directory + // and find the top-level occurrence to report parts := strings.Split(dirName, "/") - for _, part := range parts { + for i, part := range parts { if utils.StringInSlice(part+"/", reqDirs) { matched = true + // Report only up to and including the first required directory component + reportDir = strings.Join(parts[:i+1], "/") break } } } if matched { - if _, ok := detectedDir[dirName]; ok { + if _, ok := detectedDir[reportDir]; ok { continue } - detectedDir[dirName] = struct{}{} + detectedDir[reportDir] = struct{}{} // Skip uncontrollable dependency directory e.g) npm : node_modules, php: composer if inIgnoreDir(filename) { @@ -59,8 +64,8 @@ func (a CacheAssessor) Assess(fileMap deckodertypes.FileMap) ([]*types.Assessmen assesses, &types.Assessment{ Code: types.InfoDeletableFiles, - Filename: dirName, - Desc: fmt.Sprintf("Suspicious directory : %s ", dirName), + Filename: reportDir, + Desc: fmt.Sprintf("Suspicious directory : %s ", reportDir), }) } diff --git a/pkg/assessor/cache/cache_test.go b/pkg/assessor/cache/cache_test.go index 658a9757..68888f2d 100644 --- a/pkg/assessor/cache/cache_test.go +++ b/pkg/assessor/cache/cache_test.go @@ -30,7 +30,7 @@ func TestAssess(t *testing.T) { ".cache/direct": {}, }, expectedCount: 5, - expectedFiles: []string{"root/.cache/pip/http-v2/1/2/3", "root/.aws", "/root/.git", "home/user/.npm", ".cache"}, + expectedFiles: []string{"root/.cache", "root/.aws", "/root/.git", "home/user/.npm", ".cache"}, }, { name: "detects required files by basename", @@ -44,11 +44,11 @@ func TestAssess(t *testing.T) { expectedFiles: []string{"root/Dockerfile", "app/docker-compose.yml", "home/.vimrc", "project/.DS_Store"}, }, { - name: "deduplicates directories with multiple files", + name: "reports only top-level suspicious directory once", fileMap: deckodertypes.FileMap{ - "root/.cache/file1": {}, - "root/.cache/file2": {}, - "root/.cache/file3": {}, + "root/.cache/file1": {}, + "root/.cache/pip/file2": {}, + "root/.cache/pip/http-v2/deep/file": {}, }, expectedCount: 1, expectedFiles: []string{"root/.cache"},