Revert "Disable permission flag checks on imported logs (#82)" (#85)

This reverts commit fd1bbe3263.
This commit is contained in:
Liza Tsibur
2025-12-09 13:22:09 -07:00
committed by GitHub
parent fd1bbe3263
commit 63b6e3d6e7
2 changed files with 27 additions and 40 deletions
+7 -10
View File
@@ -161,19 +161,15 @@ func RunImportCmd(startTime time.Time, cfg *config.Config, afs afero.Fs, logDir
// get list of hourly log maps of all days of log files in directory
logMap, walkErrors, err := WalkFiles(afs, logDir, db.Rolling)
// log any errors that occurred during the walk, before returning
// this is especially useful when all files in the directory are invalid
// instead of only logging 'no valid files found'
for _, walkErr := range walkErrors {
logger.Debug().Str("path", walkErr.Path).Err(walkErr.Error).Msg("file was left out of import due to error or incompatibility")
}
// return if the walk failed completely
if err != nil {
return importResults, err
}
// log any errors that occurred during the walk
for _, walkErr := range walkErrors {
logger.Debug().Str("path", walkErr.Path).Err(walkErr.Error).Msg("file was left out of import due to error or incompatibility")
}
var elapsedTime int64
// loop through each day
@@ -429,7 +425,8 @@ func WalkFiles(afs afero.Fs, root string, rolling bool) ([]HourlyZeekLogs, []Wal
}
// check if the file is readable
if _, err := afs.Open(path); err != nil {
_, err := afs.Open(path)
if err != nil || !(info.Mode().Perm()&0444 == 0444) {
walkErrors = append(walkErrors, WalkError{Path: path, Error: ErrInsufficientReadPermissions})
return nil //nolint:nilerr // log the issue and continue walking
}
+20 -30
View File
@@ -622,6 +622,7 @@ func createExpectedResults(logs []cmd.HourlyZeekLogs) []cmd.HourlyZeekLogs {
}
func TestWalkFiles(t *testing.T) {
afs := afero.NewMemMapFs()
tests := []struct {
name string
@@ -1158,33 +1159,25 @@ func TestWalkFiles(t *testing.T) {
},
expectedError: cmd.ErrNoValidFilesFound,
},
// Previously, read permissions were checked with !(info.Mode().Perm()&0444 == 0444), but
// this requires all read permissions (user, group, others)/0644 to be set which is not ideal.
// A better check would be to see if any read permission is set, i.e., (info.Mode().Perm()&0444 != 0).
// However, since some ACL systems/SELinux might interfere with this, it's better to let the Open() call
// return an error if permission is denied.
// Unfortunately, afero.MemMapFs does not support file permissions when using Open, so this test is skipped.
// https://github.com/spf13/afero/issues/150
// {
// name: "No Read Permissions on Files",
// directory: "/logs",
// directoryPermissions: iofs.FileMode(0o775),
// filePermissions: iofs.FileMode(0o000),
// files: []string{
// "conn.log", "dns.log", "http.log", "ssl.log", "open_conn.log", "open_http.log", "open_ssl.log",
// },
// expectedWalkErrors: []cmd.WalkError{
// {Path: "/logs/conn.log", Error: cmd.ErrInsufficientReadPermissions},
// {Path: "/logs/dns.log", Error: cmd.ErrInsufficientReadPermissions},
// {Path: "/logs/http.log", Error: cmd.ErrInsufficientReadPermissions},
// {Path: "/logs/ssl.log", Error: cmd.ErrInsufficientReadPermissions},
// {Path: "/logs/open_conn.log", Error: cmd.ErrInsufficientReadPermissions},
// {Path: "/logs/open_http.log", Error: cmd.ErrInsufficientReadPermissions},
// {Path: "/logs/open_ssl.log", Error: cmd.ErrInsufficientReadPermissions},
// },
// expectedError: cmd.ErrNoValidFilesFound,
// },
{
name: "No Read Permissions on Files",
directory: "/logs",
directoryPermissions: iofs.FileMode(0o775),
filePermissions: iofs.FileMode(0o000),
files: []string{
"conn.log", "dns.log", "http.log", "ssl.log", "open_conn.log", "open_http.log", "open_ssl.log",
},
expectedWalkErrors: []cmd.WalkError{
{Path: "/logs/conn.log", Error: cmd.ErrInsufficientReadPermissions},
{Path: "/logs/dns.log", Error: cmd.ErrInsufficientReadPermissions},
{Path: "/logs/http.log", Error: cmd.ErrInsufficientReadPermissions},
{Path: "/logs/ssl.log", Error: cmd.ErrInsufficientReadPermissions},
{Path: "/logs/open_conn.log", Error: cmd.ErrInsufficientReadPermissions},
{Path: "/logs/open_http.log", Error: cmd.ErrInsufficientReadPermissions},
{Path: "/logs/open_ssl.log", Error: cmd.ErrInsufficientReadPermissions},
},
expectedError: cmd.ErrNoValidFilesFound,
},
{
name: "No Files, Only SubDirectories",
directory: "/logs",
@@ -1224,8 +1217,6 @@ func TestWalkFiles(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// create a new in-memory filesystem for each test
afs := afero.NewMemMapFs()
// Create the directory
if test.directory != "" {
@@ -1307,7 +1298,6 @@ func TestWalkFiles(t *testing.T) {
if test.expectedError == nil {
require.NoError(t, err, "running WalkFiles should not produce an error")
} else {
require.Error(t, err, "running WalkFiles should produce an error")
require.ErrorIs(t, err, test.expectedError, "error should match expected value")
}