Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBG-4236: Create SG log files on startup with 0644 permission bits set #7128

Merged
merged 4 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion base/logger_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,12 @@ func (lfc *FileLoggerConfig) init(ctx context.Context, level LogLevel, name stri

var rotationDoneChan chan struct{}
if lfc.Output == nil {
rotationDoneChan = lfc.initLumberjack(ctx, name, filepath.Join(filepath.FromSlash(logFilePath), logFilePrefix+name+".log"))
logFileName := logFilePrefix + name + ".log"
logFileOutput := filepath.Join(filepath.FromSlash(logFilePath), logFileName)
if err := validateLogFileOutput(logFileOutput); err != nil {
return nil, err
}
rotationDoneChan = lfc.initLumberjack(ctx, name, logFileOutput)
}

if lfc.CollationBufferSize == nil {
Expand Down
52 changes: 13 additions & 39 deletions base/logging_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ const (
fileLoggerCollateFlushTimeout = 10 * time.Millisecond

rotatedLogDeletionInterval = time.Hour // not configurable

logFilePermission = 0644 // rw-r--r--
)

// ErrUnsetLogFilePath is returned when no log_file_path, or --defaultLogFilePath fallback can be used.
Expand Down Expand Up @@ -78,20 +80,11 @@ func InitLogging(ctx context.Context, logFilePath string,
return nil
}

err = validateLogFilePath(logFilePath)
if err != nil {
return err
}

ConsolefCtx(ctx, LevelInfo, KeyNone, "Logging: Files to %v", logFilePath)

auditLogFilePath := logFilePath
if audit != nil && audit.AuditLogFilePath != nil && BoolDefault(audit.Enabled, false) {
auditLogFilePath = *audit.AuditLogFilePath
err = validateLogFilePath(auditLogFilePath)
if err != nil {
return fmt.Errorf("error validating audit log file path: %w", err)
}
ConsolefCtx(ctx, LevelInfo, KeyNone, "Logging: Audit to %v", auditLogFilePath)
}

Expand Down Expand Up @@ -303,33 +296,6 @@ func BuildLoggingConfigFromLoggers(originalConfig LoggingConfig) *LoggingConfig
return &config
}

// validateLogFilePath ensures the given path is created and is a directory.
func validateLogFilePath(logFilePath string) error {
// Make full directory structure if it doesn't already exist
err := os.MkdirAll(logFilePath, 0700)
if err != nil {
return errors.Wrap(err, ErrInvalidLogFilePath.Error())
}

// Ensure LogFilePath is a directory. Lumberjack will check file permissions when it opens/creates the logfile.
if f, err := os.Stat(logFilePath); err != nil {
return errors.Wrap(err, ErrInvalidLogFilePath.Error())
} else if !f.IsDir() {
return errors.Wrap(ErrInvalidLogFilePath, "not a directory")
}

// Make temporary empty file to check if the log file path is writable
writeCheckFilePath := filepath.Join(logFilePath, ".SG_write_check")
err = os.WriteFile(writeCheckFilePath, nil, 0666)
if err != nil {
return errors.Wrap(err, ErrUnwritableLogFilePath.Error())
}
// best effort cleanup, but if we don't manage to remove it, WriteFile will overwrite on the next startup and try to remove again
_ = os.Remove(writeCheckFilePath)

return nil
}

// validateLogFileOutput ensures the given file has permission to be written to.
func validateLogFileOutput(logFileOutput string) error {
if logFileOutput == "" {
Expand All @@ -338,13 +304,21 @@ func validateLogFileOutput(logFileOutput string) error {

// Validate containing directory
logFileOutputDirectory := filepath.Dir(logFileOutput)
err := validateLogFilePath(logFileOutputDirectory)
// Make full directory structure if it doesn't already exist
err := os.MkdirAll(logFileOutputDirectory, 0700)
if err != nil {
return err
return errors.Wrap(err, ErrInvalidLogFilePath.Error())
}

// Ensure LogFilePath is a directory. We'll check file permissions when it opens/creates the logfile.
if f, err := os.Stat(logFileOutputDirectory); err != nil {
return errors.Wrap(err, ErrInvalidLogFilePath.Error())
} else if !f.IsDir() {
return errors.Wrap(ErrInvalidLogFilePath, "not a directory")
}

// Validate given file is writeable
bbrks marked this conversation as resolved.
Show resolved Hide resolved
file, err := os.OpenFile(logFileOutput, os.O_WRONLY|os.O_CREATE, 0666)
file, err := os.OpenFile(logFileOutput, os.O_WRONLY|os.O_CREATE, logFilePermission)
if err != nil {
if os.IsPermission(err) {
return errors.Wrap(err, "invalid file output")
Expand Down
3 changes: 2 additions & 1 deletion base/logging_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ func TestLogFilePathWritable(t *testing.T) {
err := os.Mkdir(logFilePath, test.logFilePathPerms)
require.NoError(t, err)

err = validateLogFilePath(logFilePath)
// The write-check is done inside validateLogFileOutput now.
err = validateLogFileOutput(filepath.Join(logFilePath, test.name+".log"))
if test.error {
assert.Error(t, err)
return
Expand Down
Loading