[Backport 7.74.x] privileged logs: Handle unsearchable directories

pkg/logs/internal/util/opener/open_linux.go

@@ -16,3 +16,8 @@ import (
 func OpenLogFile(path string) (*os.File, error) {
 	return privilegedlogsclient.Open(path)
 }
+
+// StatLogFile stats a log file with the privileged logs client
+func StatLogFile(path string) (os.FileInfo, error) {
+	return privilegedlogsclient.Stat(path)
+}

pkg/logs/internal/util/opener/open_other.go

@@ -18,3 +18,8 @@ import (
 func OpenLogFile(path string) (*os.File, error) {
 	return filesystem.OpenShared(path)
 }
+
+// StatLogFile stats a log file
+func StatLogFile(path string) (os.FileInfo, error) {
+	return os.Stat(path)
+}

pkg/logs/launchers/file/launcher_privileged_logs_test.go

@@ -9,6 +9,8 @@
 package file
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -19,15 +21,33 @@ import (
 )
 
 type PrivilegedLogsTestSetupStrategy struct {
-	tempDirs [2]string
+	searchableTempDirs   [2]string
+	unsearchableTempDirs [2]string
 }
 
 func (s *PrivilegedLogsTestSetupStrategy) Setup(t *testing.T) TestSetupResult {
 	handler := privilegedlogstest.Setup(t, func() {
-		s.tempDirs = [2]string{}
+		s.searchableTempDirs = [2]string{}
+		s.unsearchableTempDirs = [2]string{}
 		for i := 0; i < 2; i++ {
 			testDir := t.TempDir()
-			s.tempDirs[i] = testDir
+
+			s.searchableTempDirs[i] = testDir
+
+			// Use a subdirectory without execute permissions so that the
+			// unprivileged user can't even stat(2) the file.
+			subdir := filepath.Join(testDir, "subdir")
+			err := os.Mkdir(subdir, 0)
+			require.NoError(t, err)
+
+			s.unsearchableTempDirs[i] = subdir
+
+			// Restore permissions after the test so that the cleanup of the
+			// temporary directories doesn't fail.
+			t.Cleanup(func() {
+				err := os.Chmod(subdir, 0755)
+				require.NoError(t, err)
+			})
 		}
 	})
 
@@ -41,7 +61,24 @@ func (s *PrivilegedLogsTestSetupStrategy) Setup(t *testing.T) TestSetupResult {
 	systemProbeConfig.SetWithoutSource("privileged_logs.enabled", true)
 	systemProbeConfig.SetWithoutSource("system_probe_config.sysprobe_socket", handler.SocketPath)
 
-	return TestSetupResult{TestDirs: s.tempDirs[:]}
+	return TestSetupResult{TestDirs: s.unsearchableTempDirs[:], TestOps: TestOps{
+		create: func(name string) (*os.File, error) {
+			var file *os.File
+			err := privilegedlogstest.WithParentPermFixup(t, name, func() error {
+				var err error
+				file, err = os.Create(name)
+				return err
+			})
+			return file, err
+		}, rename: func(oldPath, newPath string) error {
+			return privilegedlogstest.WithParentPermFixup(t, newPath, func() error {
+				return os.Rename(oldPath, newPath)
+			})
+		}, remove: func(name string) error {
+			return privilegedlogstest.WithParentPermFixup(t, name, func() error {
+				return os.Remove(name)
+			})
+		}}}
 }
 
 type PrivilegedLogsLauncherTestSuite struct {
@@ -125,10 +162,13 @@ type privilegedLogsTestSetup struct {
 // setupPrivilegedLogsTest sets up the privileged logs test environment
 func setupPrivilegedLogsTest(t *testing.T) *privilegedLogsTestSetup {
 	strategy := &PrivilegedLogsTestSetupStrategy{}
-	result := strategy.Setup(t)
+	strategy.Setup(t)
 
 	return &privilegedLogsTestSetup{
-		tempDirs: [2]string{result.TestDirs[0], result.TestDirs[1]},
+		// Use the searchable temp dirs since several of the non-suite tests use
+		// wildcard patterns which do not work with non-searchable directories
+		// since the logs agent is unable to scan the directory for files.
+		tempDirs: [2]string{strategy.searchableTempDirs[0], strategy.searchableTempDirs[1]},
 	}
 }
 

pkg/logs/launchers/file/launcher_test.go

@@ -41,7 +41,12 @@ import (
 type RegularTestSetupStrategy struct{}
 
 func (s *RegularTestSetupStrategy) Setup(t *testing.T) TestSetupResult {
-	return TestSetupResult{TestDirs: []string{t.TempDir(), t.TempDir()}}
+	return TestSetupResult{TestDirs: []string{t.TempDir(), t.TempDir()},
+		TestOps: TestOps{
+			create: os.Create,
+			rename: os.Rename,
+			remove: os.Remove,
+		}}
 }
 
 type LauncherTestSuite struct {
@@ -110,8 +115,15 @@ type TestSetupStrategy interface {
 	Setup(t *testing.T) TestSetupResult
 }
 
+type TestOps struct {
+	create func(name string) (*os.File, error)
+	rename func(oldPath, newPath string) error
+	remove func(name string) error
+}
+
 type TestSetupResult struct {
 	TestDirs []string
+	TestOps  TestOps
 }
 
 type BaseLauncherTestSuite struct {
@@ -132,6 +144,8 @@ type BaseLauncherTestSuite struct {
 
 	setupStrategy TestSetupStrategy
 	setupResult   TestSetupResult
+
+	ops TestOps
 }
 
 const DefaultFileLimit = 100
@@ -190,13 +204,15 @@ func (suite *BaseLauncherTestSuite) SetupTest() {
 	var err error
 	suite.testDir = suite.setupResult.TestDirs[0]
 
+	suite.ops = suite.setupResult.TestOps
+
 	suite.testPath = fmt.Sprintf("%s/launcher.log", suite.testDir)
 	suite.testRotatedPath = fmt.Sprintf("%s.1", suite.testPath)
 
-	f, err := os.Create(suite.testPath)
+	f, err := suite.ops.create(suite.testPath)
 	suite.Nil(err)
 	suite.testFile = f
-	f, err = os.Create(suite.testRotatedPath)
+	f, err = suite.ops.create(suite.testRotatedPath)
 	suite.Nil(err)
 	suite.testRotatedFile = f
 
@@ -263,8 +279,9 @@ func (suite *BaseLauncherTestSuite) TestLauncherScanWithLogRotation() {
 	suite.Equal("hello world", string(msg.GetContent()))
 
 	tailer, _ = s.tailers.Get(getScanKey(suite.testPath, suite.source))
-	os.Rename(suite.testPath, suite.testRotatedPath)
-	f, err := os.Create(suite.testPath)
+	err = suite.ops.rename(suite.testPath, suite.testRotatedPath)
+	suite.Nil(err)
+	f, err := suite.ops.create(suite.testPath)
 	suite.Nil(err)
 	s.resolveActiveTailers(suite.s.fileProvider.FilesToTail(context.Background(), suite.s.validatePodContainerID, suite.s.activeSources, suite.s.registry))
 	newTailer, _ = s.tailers.Get(getScanKey(suite.testPath, suite.source))
@@ -333,8 +350,9 @@ func (suite *BaseLauncherTestSuite) TestLauncherScanWithLogRotationAndChecksum_R
 	s.registry.(*auditorMock.Registry).SetFingerprint(fingerprint)
 
 	// Rotate file
-	os.Rename(suite.testPath, suite.testRotatedPath)
-	f, err := os.Create(suite.testPath)
+	err = suite.ops.rename(suite.testPath, suite.testRotatedPath)
+	suite.Nil(err)
+	f, err := suite.ops.create(suite.testPath)
 	suite.Nil(err)
 
 	// Write different content
@@ -457,13 +475,13 @@ func (suite *BaseLauncherTestSuite) TestLauncherScanWithFileRemovedAndCreated()
 	var err error
 
 	// remove file
-	err = os.Remove(suite.testPath)
+	err = suite.ops.remove(suite.testPath)
 	suite.Nil(err)
 	s.resolveActiveTailers(suite.s.fileProvider.FilesToTail(context.Background(), suite.s.validatePodContainerID, suite.s.activeSources, suite.s.registry))
 	suite.Equal(tailerLen-1, s.tailers.Count())
 
 	// create file
-	_, err = os.Create(suite.testPath)
+	_, err = suite.ops.create(suite.testPath)
 	suite.Nil(err)
 	s.resolveActiveTailers(suite.s.fileProvider.FilesToTail(context.Background(), suite.s.validatePodContainerID, suite.s.activeSources, suite.s.registry))
 	suite.Equal(tailerLen, s.tailers.Count())
@@ -1260,11 +1278,11 @@ func (suite *BaseLauncherTestSuite) TestLauncherDoesNotCreateTailerForRotatedUnd
 
 	// Simulate file rotation: move current file to .1 and create a new empty file
 	rotatedPath := suite.testPath + ".1"
-	err = os.Rename(suite.testPath, rotatedPath)
+	err = suite.ops.rename(suite.testPath, rotatedPath)
 	suite.Nil(err)
 
 	// Create a new file that is undersized (empty, which results in fingerprint = 0)
-	newFile, err := os.Create(suite.testPath)
+	newFile, err := suite.ops.create(suite.testPath)
 	suite.Nil(err)
 	newFile.Close()
 
@@ -1328,10 +1346,10 @@ func (suite *BaseLauncherTestSuite) TestRotatedTailersNotStoppedDuringScan() {
 
 	// Rotate the file
 	rotatedPath := suite.testPath + ".1"
-	err = os.Rename(suite.testPath, rotatedPath)
+	err = suite.ops.rename(suite.testPath, rotatedPath)
 	suite.Nil(err)
 
-	newFile, err := os.Create(suite.testPath)
+	newFile, err := suite.ops.create(suite.testPath)
 	suite.Nil(err)
 	_, err = newFile.WriteString("new content\n")
 	suite.Nil(err)
@@ -1398,11 +1416,11 @@ func (suite *BaseLauncherTestSuite) TestRestartTailerAfterFileRotationRemovesTai
 
 	// Simulate rotation
 	rotatedPath := suite.testPath + ".1"
-	err = os.Rename(suite.testPath, rotatedPath)
+	err = suite.ops.rename(suite.testPath, rotatedPath)
 	suite.Nil(err)
 
 	// Create new file
-	newFile, err := os.Create(suite.testPath)
+	newFile, err := suite.ops.create(suite.testPath)
 	suite.Nil(err)
 	_, err = newFile.WriteString("line2\n")
 	suite.Nil(err)
@@ -1477,7 +1495,7 @@ func (suite *LauncherTestSuite) TestTailerReceivesConfigWhenDisabled() {
 	defer status.Clear()
 
 	// Create file with content
-	f, err := os.Create(suite.testPath)
+	f, err := suite.ops.create(suite.testPath)
 	suite.Nil(err)
 	_, err = f.WriteString("test data\n")
 	suite.Nil(err)

pkg/logs/launchers/file/provider/file_provider.go

@@ -17,6 +17,7 @@ import (
 	"time"
 
 	auditor "github.com/DataDog/datadog-agent/comp/logs/auditor/def"
+	"github.com/DataDog/datadog-agent/pkg/logs/internal/util/opener"
 	"github.com/DataDog/datadog-agent/pkg/logs/sources"
 	"github.com/DataDog/datadog-agent/pkg/logs/status"
 	tailer "github.com/DataDog/datadog-agent/pkg/logs/tailers/file"
@@ -267,7 +268,7 @@ func (p *FileProvider) FilesToTail(ctx context.Context, validatePodContainerID b
 // with ordering defined by 'wildcardOrder'
 func (p *FileProvider) CollectFiles(source *sources.LogSource) ([]*tailer.File, error) {
 	path := source.Config.Path
-	_, err := os.Stat(path)
+	_, err := opener.StatLogFile(path)
 	switch {
 	case err == nil:
 		return []*tailer.File{

pkg/privileged-logs/client/open.go

@@ -111,6 +111,24 @@ func OpenPrivileged(socketPath string, filePath string) (*os.File, error) {
 	return nil, fmt.Errorf("no file descriptor received")
 }
 
+func maybeOpenPrivileged(path string, originalError error) (*os.File, error) {
+	enabled := pkgconfigsetup.SystemProbe().GetBool("privileged_logs.enabled")
+	if !enabled {
+		return nil, originalError
+	}
+
+	log.Debugf("Permission denied, opening file with system-probe: %v", path)
+
+	socketPath := pkgconfigsetup.SystemProbe().GetString("system_probe_config.sysprobe_socket")
+	file, spErr := OpenPrivileged(socketPath, path)
+	log.Tracef("Opened file with system-probe: %v, err: %v", path, spErr)
+	if spErr != nil {
+		return nil, fmt.Errorf("failed to open file with system-probe: %w, original error: %w", spErr, originalError)
+	}
+
+	return file, nil
+}
+
 // Open attempts to open a file, and if it fails due to permissions, it opens
 // the file using system-probe if the privileged logs module is available.
 func Open(path string) (*os.File, error) {
@@ -119,19 +137,33 @@ func Open(path string) (*os.File, error) {
 		return file, err
 	}
 
-	enabled := pkgconfigsetup.SystemProbe().GetBool("privileged_logs.enabled")
-	if !enabled {
-		return file, err
+	file, err = maybeOpenPrivileged(path, err)
+	if err != nil {
+		return nil, err
 	}
 
-	log.Debugf("Permission denied, opening file with system-probe: %v", path)
+	return file, nil
+}
 
-	socketPath := pkgconfigsetup.SystemProbe().GetString("system_probe_config.sysprobe_socket")
-	fd, spErr := OpenPrivileged(socketPath, path)
-	log.Tracef("Opened file with system-probe: %v, err: %v", path, spErr)
-	if spErr != nil {
-		return file, fmt.Errorf("failed to open file with system-probe: %w, original error: %w", spErr, err)
+// Stat attempts to stat a file, and if it fails due to permissions, it opens
+// the file using system-probe if the privileged logs module is available and
+// stats the opened file.
+func Stat(path string) (os.FileInfo, error) {
+	info, err := os.Stat(path)
+	if err == nil || !errors.Is(err, os.ErrPermission) {
+		return info, err
+	}
+
+	file, err := maybeOpenPrivileged(path, err)
+	if err != nil {
+		return nil, err
+	}
+	defer file.Close()
+
+	info, err = file.Stat()
+	if err != nil {
+		return nil, err
 	}
 
-	return fd, nil
+	return info, nil
 }

pkg/privileged-logs/client/open_other.go

@@ -16,3 +16,8 @@ import (
 func Open(path string) (*os.File, error) {
 	return os.Open(path)
 }
+
+// Stat provides a fallback for non-Linux platforms where the privileged logs module is not available.
+func Stat(path string) (os.FileInfo, error) {
+	return os.Stat(path)
+}

pkg/privileged-logs/test/handler.go

@@ -130,6 +130,28 @@ func Setup(t *testing.T, callback func()) *Handler {
 	return handler
 }
 
+// WithParentPermFixup ensures that the parent directory of the given path has
+// execute permissions before operations that manipulate the log files from the
+// tests.  This is necessary to ensure that the tests can manipulate the log
+// files even when the parent directory does not have search permissions.
+func WithParentPermFixup(t *testing.T, path string, op func() error) error {
+	parent := filepath.Dir(path)
+	err := os.Chmod(parent, 0755)
+	require.NoError(t, err)
+
+	opErr := op()
+
+	err = os.Chmod(parent, 0)
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		err := os.Chmod(parent, 0755)
+		require.NoError(t, err)
+	})
+
+	return opErr
+}
+
 func setupTestServer(t *testing.T) *Handler {
 	cfg := &sysconfigtypes.Config{}
 	deps := module.FactoryDependencies{}