diff --git a/modules/logging/filewriter.go b/modules/logging/filewriter.go index c3df562cb..8c0c804e5 100644 --- a/modules/logging/filewriter.go +++ b/modules/logging/filewriter.go @@ -90,6 +90,15 @@ type FileWriter struct { // 0600 by default. Mode fileMode `json:"mode,omitempty"` + // DirMode controls permissions for any directories created to reach Filename. + // Default: 0700 (current behavior). + // + // Special values: + // - "inherit" → copy the nearest existing parent directory's perms (with r→x normalization) + // - "from_file" → derive from the file Mode (with r→x), e.g. 0644 → 0755, 0600 → 0700 + // Numeric octal strings (e.g. "0755") are also accepted. Subject to process umask. + DirMode string `json:"dir_mode,omitempty"` + // Roll toggles log rolling or rotation, which is // enabled by default. Roll *bool `json:"roll,omitempty"` @@ -177,11 +186,33 @@ func (fw FileWriter) OpenWriter() (io.WriteCloser, error) { // roll log files as a sensible default to avoid disk space exhaustion roll := fw.Roll == nil || *fw.Roll - // create the file if it does not exist; create with the configured mode, or default - // to restrictive if not set. (timberjack will reuse the file mode across log rotation) - if err := os.MkdirAll(filepath.Dir(fw.Filename), 0o700); err != nil { - return nil, err + // Ensure directory exists before opening the file. + dirPath := filepath.Dir(fw.Filename) + switch strings.ToLower(strings.TrimSpace(fw.DirMode)) { + case "", "0": + // Preserve current behavior: locked-down directories by default. + if err := os.MkdirAll(dirPath, 0o700); err != nil { + return nil, err + } + case "inherit": + if err := mkdirAllInherit(dirPath); err != nil { + return nil, err + } + case "from_file": + if err := mkdirAllFromFile(dirPath, os.FileMode(fw.Mode)); err != nil { + return nil, err + } + default: + dm, err := parseFileMode(fw.DirMode) + if err != nil { + return nil, fmt.Errorf("dir_mode: %w", err) + } + if err := os.MkdirAll(dirPath, dm); err != nil { + return nil, err + } } + + // create/open the file file, err := os.OpenFile(fw.Filename, os.O_WRONLY|os.O_APPEND|os.O_CREATE, modeIfCreating) if err != nil { return nil, err @@ -234,13 +265,70 @@ func (fw FileWriter) OpenWriter() (io.WriteCloser, error) { RotateAtMinutes: fw.RollAtMinutes, RotateAt: fw.RollAt, BackupTimeFormat: fw.BackupTimeFormat, + FileMode: os.FileMode(fw.Mode), }, nil } +// normalizeDirPerm ensures that read bits also have execute bits set. +func normalizeDirPerm(p os.FileMode) os.FileMode { + if p&0o400 != 0 { + p |= 0o100 + } + if p&0o040 != 0 { + p |= 0o010 + } + if p&0o004 != 0 { + p |= 0o001 + } + return p +} + +// mkdirAllInherit creates missing dirs using the nearest existing parent's +// permissions, normalized with r→x. +func mkdirAllInherit(dir string) error { + if fi, err := os.Stat(dir); err == nil && fi.IsDir() { + return nil + } + cur := dir + var parent string + for { + next := filepath.Dir(cur) + if next == cur { + parent = next + break + } + if fi, err := os.Stat(next); err == nil { + if !fi.IsDir() { + return fmt.Errorf("path component %s exists and is not a directory", next) + } + parent = next + break + } + cur = next + } + perm := os.FileMode(0o700) + if fi, err := os.Stat(parent); err == nil && fi.IsDir() { + perm = fi.Mode().Perm() + } + perm = normalizeDirPerm(perm) + return os.MkdirAll(dir, perm) +} + +// mkdirAllFromFile creates missing dirs using the file's mode (with r→x) so +// 0644 → 0755, 0600 → 0700, etc. +func mkdirAllFromFile(dir string, fileMode os.FileMode) error { + if fi, err := os.Stat(dir); err == nil && fi.IsDir() { + return nil + } + perm := normalizeDirPerm(fileMode.Perm()) | 0o200 // ensure owner write on dir so files can be created + return os.MkdirAll(dir, perm) +} + // UnmarshalCaddyfile sets up the module from Caddyfile tokens. Syntax: // // file { // mode +// dir_mode // roll_disabled // roll_size // roll_uncompressed @@ -284,6 +372,22 @@ func (fw *FileWriter) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } fw.Mode = fileMode(mode) + case "dir_mode": + var val string + if !d.AllArgs(&val) { + return d.ArgErr() + } + val = strings.TrimSpace(val) + switch strings.ToLower(val) { + case "inherit", "from_file": + fw.DirMode = val + default: + if _, err := parseFileMode(val); err != nil { + return d.Errf("parsing dir_mode: %v", err) + } + fw.DirMode = val + } + case "roll_disabled": var f bool fw.Roll = &f diff --git a/modules/logging/filewriter_test.go b/modules/logging/filewriter_test.go index 2a246156c..915784b53 100644 --- a/modules/logging/filewriter_test.go +++ b/modules/logging/filewriter_test.go @@ -385,3 +385,225 @@ func TestFileModeModification(t *testing.T) { t.Errorf("file mode is %v, want %v", st.Mode(), want) } } + +func TestDirMode_Inherit(t *testing.T) { + m := syscall.Umask(0) + defer syscall.Umask(m) + + parent := t.TempDir() + if err := os.Chmod(parent, 0o755); err != nil { + t.Fatal(err) + } + + targetDir := filepath.Join(parent, "a", "b") + fw := &FileWriter{ + Filename: filepath.Join(targetDir, "test.log"), + DirMode: "inherit", + Mode: 0o640, + Roll: func() *bool { f := false; return &f }(), + } + w, err := fw.OpenWriter() + if err != nil { + t.Fatal(err) + } + _ = w.Close() + + st, err := os.Stat(targetDir) + if err != nil { + t.Fatal(err) + } + if got := st.Mode().Perm(); got != 0o755 { + t.Fatalf("dir perm = %o, want 0755", got) + } +} + +func TestDirMode_FromFile(t *testing.T) { + m := syscall.Umask(0) + defer syscall.Umask(m) + + base := t.TempDir() + + dir1 := filepath.Join(base, "logs1") + fw1 := &FileWriter{ + Filename: filepath.Join(dir1, "app.log"), + DirMode: "from_file", + Mode: 0o644, // => dir 0755 + Roll: func() *bool { f := false; return &f }(), + } + w1, err := fw1.OpenWriter() + if err != nil { + t.Fatal(err) + } + _ = w1.Close() + + st1, err := os.Stat(dir1) + if err != nil { + t.Fatal(err) + } + if got := st1.Mode().Perm(); got != 0o755 { + t.Fatalf("dir perm = %o, want 0755", got) + } + + dir2 := filepath.Join(base, "logs2") + fw2 := &FileWriter{ + Filename: filepath.Join(dir2, "app.log"), + DirMode: "from_file", + Mode: 0o600, // => dir 0700 + Roll: func() *bool { f := false; return &f }(), + } + w2, err := fw2.OpenWriter() + if err != nil { + t.Fatal(err) + } + _ = w2.Close() + + st2, err := os.Stat(dir2) + if err != nil { + t.Fatal(err) + } + if got := st2.Mode().Perm(); got != 0o700 { + t.Fatalf("dir perm = %o, want 0700", got) + } +} + +func TestDirMode_ExplicitOctal(t *testing.T) { + m := syscall.Umask(0) + defer syscall.Umask(m) + + base := t.TempDir() + dest := filepath.Join(base, "logs3") + fw := &FileWriter{ + Filename: filepath.Join(dest, "app.log"), + DirMode: "0750", + Mode: 0o640, + Roll: func() *bool { f := false; return &f }(), + } + w, err := fw.OpenWriter() + if err != nil { + t.Fatal(err) + } + _ = w.Close() + + st, err := os.Stat(dest) + if err != nil { + t.Fatal(err) + } + if got := st.Mode().Perm(); got != 0o750 { + t.Fatalf("dir perm = %o, want 0750", got) + } +} + +func TestDirMode_Default0700(t *testing.T) { + m := syscall.Umask(0) + defer syscall.Umask(m) + + base := t.TempDir() + dest := filepath.Join(base, "logs4") + fw := &FileWriter{ + Filename: filepath.Join(dest, "app.log"), + Mode: 0o640, + Roll: func() *bool { f := false; return &f }(), + } + w, err := fw.OpenWriter() + if err != nil { + t.Fatal(err) + } + _ = w.Close() + + st, err := os.Stat(dest) + if err != nil { + t.Fatal(err) + } + if got := st.Mode().Perm(); got != 0o700 { + t.Fatalf("dir perm = %o, want 0700", got) + } +} + +func TestDirMode_UmaskInteraction(t *testing.T) { + _ = syscall.Umask(0o022) // typical umask; restore after + defer syscall.Umask(0) + + base := t.TempDir() + dest := filepath.Join(base, "logs5") + fw := &FileWriter{ + Filename: filepath.Join(dest, "app.log"), + DirMode: "0755", + Mode: 0o644, + Roll: func() *bool { f := false; return &f }(), + } + w, err := fw.OpenWriter() + if err != nil { + t.Fatal(err) + } + _ = w.Close() + + st, err := os.Stat(dest) + if err != nil { + t.Fatal(err) + } + // 0755 &^ 0022 still 0755 for dirs; this just sanity-checks we didn't get stricter unexpectedly + if got := st.Mode().Perm(); got != 0o755 { + t.Fatalf("dir perm = %o, want 0755 (considering umask)", got) + } +} + +func TestCaddyfile_DirMode_Inherit(t *testing.T) { + d := caddyfile.NewTestDispenser(` +file /var/log/app.log { + dir_mode inherit + mode 0640 +}`) + var fw FileWriter + if err := fw.UnmarshalCaddyfile(d); err != nil { + t.Fatal(err) + } + if fw.DirMode != "inherit" { + t.Fatalf("got %q", fw.DirMode) + } + if fw.Mode != 0o640 { + t.Fatalf("mode = %o", fw.Mode) + } +} + +func TestCaddyfile_DirMode_FromFile(t *testing.T) { + d := caddyfile.NewTestDispenser(` +file /var/log/app.log { + dir_mode from_file + mode 0600 +}`) + var fw FileWriter + if err := fw.UnmarshalCaddyfile(d); err != nil { + t.Fatal(err) + } + if fw.DirMode != "from_file" { + t.Fatalf("got %q", fw.DirMode) + } + if fw.Mode != 0o600 { + t.Fatalf("mode = %o", fw.Mode) + } +} + +func TestCaddyfile_DirMode_Octal(t *testing.T) { + d := caddyfile.NewTestDispenser(` +file /var/log/app.log { + dir_mode 0755 +}`) + var fw FileWriter + if err := fw.UnmarshalCaddyfile(d); err != nil { + t.Fatal(err) + } + if fw.DirMode != "0755" { + t.Fatalf("got %q", fw.DirMode) + } +} + +func TestCaddyfile_DirMode_Invalid(t *testing.T) { + d := caddyfile.NewTestDispenser(` +file /var/log/app.log { + dir_mode nope +}`) + var fw FileWriter + if err := fw.UnmarshalCaddyfile(d); err == nil { + t.Fatal("expected error for invalid dir_mode") + } +} diff --git a/modules/logging/filewriter_test_windows.go b/modules/logging/filewriter_test_windows.go index d32a8d2c0..420dd0e91 100644 --- a/modules/logging/filewriter_test_windows.go +++ b/modules/logging/filewriter_test_windows.go @@ -53,3 +53,41 @@ func TestFileCreationMode(t *testing.T) { t.Fatalf("file mode is %v, want rw for user", st.Mode().Perm()) } } + +func TestDirMode_Windows_CreateSucceeds(t *testing.T) { + dir, err := os.MkdirTemp("", "caddytest") + if err != nil { + t.Fatalf("failed to create tempdir: %v", err) + } + defer os.RemoveAll(dir) + + tests := []struct { + name string + dirMode string + }{ + {"inherit", "inherit"}, + {"from_file", "from_file"}, + {"octal", "0755"}, + {"default", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + subdir := path.Join(dir, "logs-"+tt.name) + fw := &FileWriter{ + Filename: path.Join(subdir, "test.log"), + DirMode: tt.dirMode, + Mode: 0o600, + } + w, err := fw.OpenWriter() + if err != nil { + t.Fatalf("failed to open writer: %v", err) + } + defer w.Close() + + if _, err := os.Stat(fw.Filename); err != nil { + t.Fatalf("expected file to exist: %v", err) + } + }) + } +}