address review comments

Signed-off-by: Sertac Ozercan <[email protected]>

Author: sozercan

pkg/scanners/trivy/trivy.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net/http"
 	"os"
+	"os/exec"
 	"time"
 
 	"github.com/eraser-dev/eraser/api/unversioned"
@@ -147,19 +148,28 @@ func runProfileServer() {
 	log.Error(err, "pprof server failed")
 }
 
-func findTrivyExecutable() (string, error) {
-	// First, check if trivy exists at the hardcoded path
-	if _, err := os.Stat(trivyCommandName); err == nil {
-		return trivyCommandName, nil
+// ensureTrivyExecutable ensures that a trivy executable is available at the target path.
+// If the executable is not found at the target path, it searches for trivy in PATH
+// and creates a symlink at the target path pointing to the found executable.
+func ensureTrivyExecutable(targetPath string) error {
+	// Check if trivy already exists at the target path
+	if _, err := os.Stat(targetPath); err == nil {
+		return nil
 	}
 
-	// If not found at hardcoded path, try to find it in PATH
-	path, err := currentExecutingLookPath("trivy")
+	// If not found at target path, try to find it in PATH
+	pathLocation, err := exec.LookPath("trivy")
 	if err != nil {
-		return "", fmt.Errorf("trivy executable not found at %s and not found in PATH: %w", trivyCommandName, err)
+		return fmt.Errorf("trivy executable not found at %s and not found in PATH: %w", targetPath, err)
 	}
 
-	return path, nil
+	// Create symlink at target path pointing to the found executable
+	if err := os.Symlink(pathLocation, targetPath); err != nil {
+		return fmt.Errorf("failed to create symlink from %s to %s: %w", targetPath, pathLocation, err)
+	}
+
+	log.Info("created trivy symlink", "from", targetPath, "to", pathLocation)
+	return nil
 }
 
 func initScanner(userConfig *Config) (Scanner, error) {
@@ -180,19 +190,17 @@ func initScanner(userConfig *Config) (Scanner, error) {
 		Address: utils.CRIPath,
 	}
 
-	// Find trivy executable path during initialization
-	trivyPath, err := findTrivyExecutable()
-	if err != nil {
+	// Ensure trivy executable is available at the hardcoded path
+	if err := ensureTrivyExecutable(trivyCommandName); err != nil {
 		return nil, err
 	}
 
 	totalTimeout := time.Duration(userConfig.Timeout.Total)
 	timer := time.NewTimer(totalTimeout)
 
 	var s Scanner = &ImageScanner{
-		config:    *userConfig,
-		timer:     timer,
-		trivyPath: trivyPath,
+		config: *userConfig,
+		timer:  timer,
 	}
 	return s, nil
 }

pkg/scanners/trivy/types.go

@@ -14,10 +14,6 @@ import (
 	"github.com/eraser-dev/eraser/pkg/utils"
 )
 
-// currentExecutingLookPath is a variable that points to exec.LookPath by default,
-// but can be overridden for testing purposes.
-var currentExecutingLookPath = exec.LookPath
-
 const (
 	StatusFailed ScanStatus = iota
 	StatusNonCompliant
@@ -172,9 +168,8 @@ func (c *Config) getRuntimeVar() (string, error) {
 }
 
 type ImageScanner struct {
-	config    Config
-	timer     *time.Timer
-	trivyPath string
+	config Config
+	timer  *time.Timer
 }
 
 func (s *ImageScanner) Scan(img unversioned.Image) (ScanStatus, error) {
@@ -191,13 +186,13 @@ func (s *ImageScanner) Scan(img unversioned.Image) (ScanStatus, error) {
 		stderr := new(bytes.Buffer)
 
 		cliArgs := s.config.cliArgs(refs[i])
-		cmd := exec.Command(s.trivyPath, cliArgs...) // nolint:gosec // G204: Subprocess launched with variable
+		cmd := exec.Command(trivyCommandName, cliArgs...)
 		cmd.Stdout = stdout
 		cmd.Stderr = stderr
 		cmd.Env = append(cmd.Env, os.Environ()...)
 		cmd.Env = setRuntimeSocketEnvVars(cmd, s.config.Runtime)
 
-		log.V(1).Info("scanning image ref", "ref", refs[i], "cli_invocation", fmt.Sprintf("%s %s", s.trivyPath, strings.Join(cliArgs, " ")), "env", cmd.Env)
+		log.V(1).Info("scanning image ref", "ref", refs[i], "cli_invocation", fmt.Sprintf("%s %s", trivyCommandName, strings.Join(cliArgs, " ")), "env", cmd.Env)
 		if err := cmd.Run(); err != nil {
 			log.Error(err, "error scanning image", "imageID", img.ImageID, "reference", refs[i], "stderr", stderr.String())
 			continue

pkg/scanners/trivy/types_test.go

@@ -1,13 +1,14 @@
 package main
 
 import (
-	"errors"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/eraser-dev/eraser/api/unversioned"
-
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -172,103 +173,189 @@ func TestCLIArgs(t *testing.T) {
 	}
 }
 
-// TestFindTrivyExecutable tests the findTrivyExecutable function in isolation.
-func TestFindTrivyExecutable(t *testing.T) {
-	// Store original function to restore after tests
-	originalLookPath := currentExecutingLookPath
-	defer func() { currentExecutingLookPath = originalLookPath }()
+// TestEnsureTrivyExecutable tests the ensureTrivyExecutable function in isolation.
+func TestEnsureTrivyExecutable(t *testing.T) {
+	// Save original PATH to restore after tests
+	originalPath := os.Getenv("PATH")
+	defer func() { os.Setenv("PATH", originalPath) }()
 
 	testCases := []struct {
-		name               string
-		lookPathSetup      func()
-		expectedPath       string
-		expectedError      bool
-		expectedErrorMatch string
+		name             string
+		setupFunc        func(t *testing.T) (targetPath string, cleanup func())
+		expectedError    bool
+		expectedErrorMsg string
+		validateFunc     func(t *testing.T, targetPath string)
 	}{
 		{
-			name: "Trivy found in PATH only",
-			lookPathSetup: func() {
-				currentExecutingLookPath = func(file string) (string, error) {
-					if file == trivyExecutableName {
-						return trivyPathBin, nil
-					}
-					return "", errors.New("not found")
-				}
+			name: "Trivy already exists at target path",
+			setupFunc: func(t *testing.T) (string, func()) {
+				tempDir := t.TempDir()
+				targetPath := filepath.Join(tempDir, "trivy")
+
+				// Create a trivy executable at the target path
+				file, err := os.Create(targetPath)
+				require.NoError(t, err)
+				file.Close()
+				err = os.Chmod(targetPath, 0o755)
+				require.NoError(t, err)
+
+				return targetPath, func() {}
 			},
-			expectedPath:  trivyPathBin,
 			expectedError: false,
+			validateFunc: func(t *testing.T, targetPath string) {
+				// Should not create a symlink, original file should still exist
+				info, err := os.Lstat(targetPath)
+				require.NoError(t, err)
+				assert.Equal(t, os.FileMode(0o755), info.Mode().Perm(), "Original file should be preserved")
+
+				// Verify it's not a symlink
+				assert.Equal(t, 0, int(info.Mode()&os.ModeSymlink), "Should not be a symlink")
+			},
+		},
+		{
+			name: "Trivy found in PATH, symlink created successfully",
+			setupFunc: func(t *testing.T) (string, func()) {
+				tempDir := t.TempDir()
+				pathDir := filepath.Join(tempDir, "bin")
+				err := os.Mkdir(pathDir, 0o755)
+				require.NoError(t, err)
+
+				// Create a trivy executable in the PATH directory
+				trivyInPath := filepath.Join(pathDir, "trivy")
+				file, err := os.Create(trivyInPath)
+				require.NoError(t, err)
+				file.Close()
+				err = os.Chmod(trivyInPath, 0o755)
+				require.NoError(t, err)
+
+				// Set PATH to include our temp bin directory
+				os.Setenv("PATH", pathDir)
+
+				// Target path where symlink should be created
+				targetPath := filepath.Join(tempDir, "target_trivy")
+
+				return targetPath, func() {}
+			},
+			expectedError: false,
+			validateFunc: func(t *testing.T, targetPath string) {
+				// Should create a symlink at target path
+				info, err := os.Lstat(targetPath)
+				require.NoError(t, err)
+
+				// Verify it's a symlink
+				assert.NotEqual(t, 0, int(info.Mode()&os.ModeSymlink), "Should be a symlink")
+
+				// Verify symlink points to the correct location
+				linkTarget, err := os.Readlink(targetPath)
+				require.NoError(t, err)
+				assert.Contains(t, linkTarget, "trivy", "Symlink should point to trivy executable")
+			},
 		},
 		{
 			name: "Trivy not found anywhere",
-			lookPathSetup: func() {
-				currentExecutingLookPath = func(_ string) (string, error) {
-					return "", errors.New("executable file not found in $PATH")
+			setupFunc: func(t *testing.T) (string, func()) {
+				tempDir := t.TempDir()
+				emptyPathDir := filepath.Join(tempDir, "empty")
+				err := os.Mkdir(emptyPathDir, 0o755)
+				require.NoError(t, err)
+
+				// Set PATH to empty directory without trivy
+				os.Setenv("PATH", emptyPathDir)
+
+				targetPath := filepath.Join(tempDir, "target_trivy")
+				return targetPath, func() {}
+			},
+			expectedError:    true,
+			expectedErrorMsg: "trivy executable not found",
+			validateFunc: func(t *testing.T, targetPath string) {
+				// Should not create any file or symlink
+				_, err := os.Lstat(targetPath)
+				assert.True(t, os.IsNotExist(err), "Target path should not exist when trivy is not found")
+			},
+		},
+		{
+			name: "Symlink creation fails due to permission",
+			setupFunc: func(t *testing.T) (string, func()) {
+				if os.Getuid() == 0 {
+					t.Skip("Skipping permission test when running as root")
+				}
+
+				tempDir := t.TempDir()
+				pathDir := filepath.Join(tempDir, "bin")
+				err := os.Mkdir(pathDir, 0o755)
+				require.NoError(t, err)
+
+				// Create a trivy executable in PATH
+				trivyInPath := filepath.Join(pathDir, "trivy")
+				file, err := os.Create(trivyInPath)
+				require.NoError(t, err)
+				file.Close()
+				err = os.Chmod(trivyInPath, 0o755)
+				require.NoError(t, err)
+
+				os.Setenv("PATH", pathDir)
+
+				// Try to create symlink in root directory (should fail for non-root users)
+				targetPath := "/trivy_test_symlink"
+
+				return targetPath, func() {
+					// Clean up any created symlink
+					os.Remove(targetPath)
 				}
 			},
-			expectedPath:       "",
-			expectedError:      true,
-			expectedErrorMatch: "trivy executable not found at /trivy and not found in PATH",
+			expectedError:    true,
+			expectedErrorMsg: "failed to create symlink",
+			validateFunc: func(t *testing.T, targetPath string) {
+				// Should not create symlink due to permission error
+				_, err := os.Lstat(targetPath)
+				assert.True(t, os.IsNotExist(err), "Target path should not exist when symlink creation fails")
+			},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			tc.lookPathSetup()
+			targetPath, cleanup := tc.setupFunc(t)
+			defer cleanup()
 
-			path, err := findTrivyExecutable()
+			// Test the ensureTrivyExecutable function
+			err := ensureTrivyExecutable(targetPath)
 
 			if tc.expectedError {
 				assert.Error(t, err)
-				assert.Contains(t, err.Error(), tc.expectedErrorMatch)
-				assert.Empty(t, path)
+				assert.Contains(t, err.Error(), tc.expectedErrorMsg)
 			} else {
 				assert.NoError(t, err)
-				assert.Equal(t, tc.expectedPath, path)
+			}
+
+			if tc.validateFunc != nil {
+				tc.validateFunc(t, targetPath)
 			}
 		})
 	}
 }
 
-// TestImageScanner_Scan_TrivyPathLookup tests the logic for using the trivy executable path.
-func TestImageScanner_Scan_TrivyPathLookup(t *testing.T) {
-	// Base configuration for the scanner
-	baseConfig := DefaultConfig()
-	// Dummy image for testing
-	img := unversioned.Image{ImageID: "test-image-id", Names: []string{"test-image:latest"}}
+// TestInitScanner_TrivySymlinkCreation tests the symlink creation logic in initScanner.
+func TestInitScanner_TrivySymlinkCreation(t *testing.T) {
+	// This test verifies that initScanner properly calls ensureTrivyExecutable
+	// and handles errors appropriately. Since ensureTrivyExecutable is tested
+	// comprehensively above, this focuses on the integration aspect.
 
-	testCases := []struct {
-		name           string
-		trivyPath      string
-		expectedStatus ScanStatus
-	}{
-		{
-			name:           "Trivy path set to hardcoded path /trivy",
-			trivyPath:      trivyCommandName,
-			expectedStatus: StatusFailed, // Will fail during actual execution but not due to path issues
-		},
-		{
-			name:           "Trivy path set to system PATH location",
-			trivyPath:      trivyPathBin,
-			expectedStatus: StatusFailed, // Will fail during actual execution but not due to path issues
-		},
-	}
+	// Save original PATH to restore after test
+	originalPath := os.Getenv("PATH")
+	defer func() { os.Setenv("PATH", originalPath) }()
 
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			scanner := &ImageScanner{
-				config:    *baseConfig,
-				trivyPath: tc.trivyPath,
-			}
+	t.Run("initScanner fails when trivy not found", func(t *testing.T) {
+		// Set PATH to empty directory
+		tempDir := t.TempDir()
+		os.Setenv("PATH", tempDir)
 
-			status, err := scanner.Scan(img)
+		config := DefaultConfig()
+		scanner, err := initScanner(config)
 
-			// The scan will likely fail due to inability to run actual scan in test,
-			// but it should not be a "trivy not found" error since the path is already set
-			assert.Equal(t, tc.expectedStatus, status, "ScanStatus should be StatusFailed")
-			if err != nil {
-				assert.NotContains(t, err.Error(), "trivy executable not found",
-					"Error should not be about trivy not being found since path is pre-set")
-			}
-		})
-	}
+		// Should fail because trivy is not found
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "trivy executable not found")
+		assert.Nil(t, scanner)
+	})
 }