Skip to content

Commit 356615e

Browse files
committed
precheck log file permission
1 parent e48040d commit 356615e

File tree

2 files changed

+89
-7
lines changed

2 files changed

+89
-7
lines changed

log.go

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"errors"
1919
"fmt"
2020
"os"
21+
"path/filepath"
2122
"sync"
2223
"sync/atomic"
2324
"time"
@@ -174,12 +175,42 @@ func (s *lockWithTimeoutWrapper) Sync() error {
174175

175176
// initFileLog initializes file based logging options.
176177
func initFileLog(cfg *FileLogConfig) (*lumberjack.Logger, error) {
178+
// Verify file permissions early to prevent silent failures.
179+
// Since lumberjack creates log files lazily, permission issues wouldn't be detected
180+
// until the first write attempt. This is problematic when tools redirect their
181+
// error output to these log files.
182+
// e.g. dumpling use the log file as ErrorOutputPath, but the log file will not be created
183+
// if there has permission issues, and dumpling will not print any error message.
184+
185+
// Create the directory if it doesn't exist
186+
dir := filepath.Dir(cfg.Filename)
187+
if err := os.MkdirAll(dir, 0755); err != nil {
188+
return nil, fmt.Errorf("cannot create log directory: %v", err)
189+
}
190+
191+
// Check if the path is a directory which is invalid
177192
if st, err := os.Stat(cfg.Filename); err == nil {
178193
if st.IsDir() {
179194
return nil, errors.New("can't use directory as log file name")
180195
}
181-
} else if !os.IsNotExist(err) {
182-
return nil, err
196+
197+
// Check if the file is writable
198+
file, err := os.OpenFile(cfg.Filename, os.O_WRONLY|os.O_APPEND, 0666)
199+
if err != nil {
200+
return nil, fmt.Errorf("can't write to log file: %v", err)
201+
}
202+
file.Close()
203+
} else if os.IsNotExist(err) {
204+
// File doesn't exist, verify we can create it
205+
file, err := os.Create(cfg.Filename)
206+
if err != nil {
207+
return nil, fmt.Errorf("can't create log file: %v", err)
208+
}
209+
file.Close()
210+
// Remove the empty file since lumberjack will create it
211+
os.Remove(cfg.Filename)
212+
} else {
213+
return nil, fmt.Errorf("error checking log file: %v", err)
183214
}
184215

185216
if cfg.MaxSize == 0 {

zap_log_test.go

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"math"
2020
"net"
2121
"os"
22+
"path/filepath"
2223
"strings"
2324
"testing"
2425
"time"
@@ -288,23 +289,73 @@ func TestCompressError(t *testing.T) {
288289
}
289290

290291
func TestLogFileNoPermission(t *testing.T) {
291-
tempDir, _ := os.MkdirTemp("/tmp", "tests-log")
292-
defer os.RemoveAll(tempDir)
292+
tempDir := t.TempDir()
293+
294+
// Directory permission denied
295+
dirPath := filepath.Join(tempDir, "noperm-dir")
296+
require.NoError(t, os.Mkdir(dirPath, 0755))
293297
conf := &Config{
294298
Level: "info",
295299
File: FileLogConfig{
296-
Filename: tempDir + "/test.log",
300+
Filename: filepath.Join(dirPath, "test.log"),
297301
MaxSize: 1,
298302
},
299303
}
300304
_, _, err := InitLogger(conf)
301305
require.NoError(t, err)
306+
require.NoError(t, os.Chmod(dirPath, 0))
307+
_, _, err = InitLogger(conf)
308+
require.Contains(t, err.Error(), "permission denied")
309+
require.NoError(t, os.Chmod(dirPath, 0755))
310+
311+
// Directory exists but doesn't allow file creation
312+
readOnlyDirPath := filepath.Join(tempDir, "readonly-dir")
313+
require.NoError(t, os.Mkdir(readOnlyDirPath, 0755))
314+
require.NoError(t, os.Chmod(readOnlyDirPath, 0555)) // Read-only directory
315+
conf = &Config{
316+
Level: "info",
317+
File: FileLogConfig{
318+
Filename: filepath.Join(readOnlyDirPath, "test.log"),
319+
MaxSize: 1,
320+
},
321+
}
322+
_, _, err = InitLogger(conf)
323+
require.Contains(t, err.Error(), "permission denied")
324+
require.NoError(t, os.Chmod(readOnlyDirPath, 0755))
325+
326+
// Using a directory as log file
327+
dirLogPath := filepath.Join(tempDir, "dir-as-log")
328+
require.NoError(t, os.Mkdir(dirLogPath, 0755))
329+
conf = &Config{
330+
Level: "info",
331+
File: FileLogConfig{
332+
Filename: dirLogPath,
333+
MaxSize: 1,
334+
},
335+
}
336+
_, _, err = InitLogger(conf)
337+
require.Contains(t, err.Error(), "can't use directory as log file name")
302338

303-
err = os.Chmod(tempDir, 0)
339+
// File exists but is not writable
340+
filePath := filepath.Join(tempDir, "readonly.log")
341+
file, err := os.Create(filePath)
304342
require.NoError(t, err)
343+
file.Close()
344+
require.NoError(t, os.Chmod(filePath, 0444))
305345

346+
// Ensure parent directory is created successfully
347+
nestedPath := filepath.Join(tempDir, "nested/path/to")
348+
conf = &Config{
349+
Level: "info",
350+
File: FileLogConfig{
351+
Filename: filepath.Join(nestedPath, "test.log"),
352+
MaxSize: 1,
353+
},
354+
}
306355
_, _, err = InitLogger(conf)
307-
require.Contains(t, err.Error(), "permission denied")
356+
require.NoError(t, err)
357+
_, err = os.Stat(nestedPath)
358+
require.NoError(t, err)
308359
}
309360

310361
// testLogSpy is a testing.TB that captures logged messages.

0 commit comments

Comments
 (0)