logging: Customize log file permissions (#6314)

Adding a "mode" option to overwrite the default logfile permissions.
Default remains "0600" which is the one currently used by lumberjack.

modules/logging/filewriter.go

@@ -15,6 +15,7 @@
 package logging
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"math"
@@ -33,6 +34,43 @@ func init() {
 	caddy.RegisterModule(FileWriter{})
 }
 
+// fileMode is a string made of 1 to 4 octal digits representing
+// a numeric mode as specified with the `chmod` unix command.
+// `"0777"` and `"777"` are thus equivalent values.
+type fileMode os.FileMode
+
+// UnmarshalJSON satisfies json.Unmarshaler.
+func (m *fileMode) UnmarshalJSON(b []byte) error {
+	if len(b) == 0 {
+		return io.EOF
+	}
+
+	var s string
+	if err := json.Unmarshal(b, &s); err != nil {
+		return err
+	}
+
+	mode, err := parseFileMode(s)
+	if err != nil {
+		return err
+	}
+
+	*m = fileMode(mode)
+	return err
+}
+
+// parseFileMode parses a file mode string,
+// adding support for `chmod` unix command like
+// 1 to 4 digital octal values.
+func parseFileMode(s string) (os.FileMode, error) {
+	modeStr := fmt.Sprintf("%04s", s)
+	mode, err := strconv.ParseUint(modeStr, 8, 32)
+	if err != nil {
+		return 0, err
+	}
+	return os.FileMode(mode), nil
+}
+
 // FileWriter can write logs to files. By default, log files
 // are rotated ("rolled") when they get large, and old log
 // files get deleted, to ensure that the process does not
@@ -41,6 +79,10 @@ type FileWriter struct {
 	// Filename is the name of the file to write.
 	Filename string `json:"filename,omitempty"`
 
+	// The file permissions mode.
+	// 0600 by default.
+	Mode fileMode `json:"mode,omitempty"`
+
 	// Roll toggles log rolling or rotation, which is
 	// enabled by default.
 	Roll *bool `json:"roll,omitempty"`
@@ -100,6 +142,10 @@ func (fw FileWriter) WriterKey() string {
 
 // OpenWriter opens a new file writer.
 func (fw FileWriter) OpenWriter() (io.WriteCloser, error) {
+	if fw.Mode == 0 {
+		fw.Mode = 0o600
+	}
+
 	// roll log files by default
 	if fw.Roll == nil || *fw.Roll {
 		if fw.RollSizeMB == 0 {
@@ -116,6 +162,9 @@ func (fw FileWriter) OpenWriter() (io.WriteCloser, error) {
 			fw.RollKeepDays = 90
 		}
 
+		f_tmp, _ := os.OpenFile(fw.Filename, os.O_WRONLY|os.O_APPEND|os.O_CREATE, os.FileMode(fw.Mode))
+		f_tmp.Close()
+
 		return &lumberjack.Logger{
 			Filename:   fw.Filename,
 			MaxSize:    fw.RollSizeMB,
@@ -127,12 +176,13 @@ func (fw FileWriter) OpenWriter() (io.WriteCloser, error) {
 	}
 
 	// otherwise just open a regular file
-	return os.OpenFile(fw.Filename, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o666)
+	return os.OpenFile(fw.Filename, os.O_WRONLY|os.O_APPEND|os.O_CREATE, os.FileMode(fw.Mode))
 }
 
 // UnmarshalCaddyfile sets up the module from Caddyfile tokens. Syntax:
 //
 //	file <filename> {
+//	    mode          <mode>
 //	    roll_disabled
 //	    roll_size     <size>
 //	    roll_uncompressed
@@ -150,7 +200,7 @@ func (fw FileWriter) OpenWriter() (io.WriteCloser, error) {
 // The roll_keep_for duration has day resolution.
 // Fractional values are rounded up to the next whole number of days.
 //
-// If any of the roll_size, roll_keep, or roll_keep_for subdirectives are
+// If any of the mode, roll_size, roll_keep, or roll_keep_for subdirectives are
 // omitted or set to a zero value, then Caddy's default value for that
 // subdirective is used.
 func (fw *FileWriter) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
@@ -165,6 +215,17 @@ func (fw *FileWriter) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 
 	for d.NextBlock(0) {
 		switch d.Val() {
+		case "mode":
+			var modeStr string
+			if !d.AllArgs(&modeStr) {
+				return d.ArgErr()
+			}
+			mode, err := parseFileMode(modeStr)
+			if err != nil {
+				return d.Errf("parsing mode: %v", err)
+			}
+			fw.Mode = fileMode(mode)
+
 		case "roll_disabled":
 			var f bool
 			fw.Roll = &f

modules/logging/filewriter_test.go

@@ -0,0 +1,308 @@
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build !windows
+
+package logging
+
+import (
+	"encoding/json"
+	"os"
+	"path"
+	"syscall"
+	"testing"
+
+	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
+)
+
+func TestFileCreationMode(t *testing.T) {
+	on := true
+	off := false
+
+	tests := []struct {
+		name     string
+		fw       FileWriter
+		wantMode os.FileMode
+	}{
+		{
+			name: "default mode no roll",
+			fw: FileWriter{
+				Roll: &off,
+			},
+			wantMode: 0o600,
+		},
+		{
+			name: "default mode roll",
+			fw: FileWriter{
+				Roll: &on,
+			},
+			wantMode: 0o600,
+		},
+		{
+			name: "custom mode no roll",
+			fw: FileWriter{
+				Roll: &off,
+				Mode: 0o666,
+			},
+			wantMode: 0o666,
+		},
+		{
+			name: "custom mode roll",
+			fw: FileWriter{
+				Roll: &on,
+				Mode: 0o666,
+			},
+			wantMode: 0o666,
+		},
+	}
+
+	m := syscall.Umask(0o000)
+	defer syscall.Umask(m)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			dir, err := os.MkdirTemp("", "caddytest")
+			if err != nil {
+				t.Fatalf("failed to create tempdir: %v", err)
+			}
+			defer os.RemoveAll(dir)
+			fpath := path.Join(dir, "test.log")
+			tt.fw.Filename = fpath
+
+			logger, err := tt.fw.OpenWriter()
+			if err != nil {
+				t.Fatalf("failed to create file: %v", err)
+			}
+			defer logger.Close()
+
+			st, err := os.Stat(fpath)
+			if err != nil {
+				t.Fatalf("failed to check file permissions: %v", err)
+			}
+
+			if st.Mode() != tt.wantMode {
+				t.Errorf("file mode is %v, want %v", st.Mode(), tt.wantMode)
+			}
+		})
+	}
+}
+
+func TestFileRotationPreserveMode(t *testing.T) {
+	m := syscall.Umask(0o000)
+	defer syscall.Umask(m)
+
+	dir, err := os.MkdirTemp("", "caddytest")
+	if err != nil {
+		t.Fatalf("failed to create tempdir: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	fpath := path.Join(dir, "test.log")
+
+	roll := true
+	mode := fileMode(0o640)
+	fw := FileWriter{
+		Filename:   fpath,
+		Mode:       mode,
+		Roll:       &roll,
+		RollSizeMB: 1,
+	}
+
+	logger, err := fw.OpenWriter()
+	if err != nil {
+		t.Fatalf("failed to create file: %v", err)
+	}
+	defer logger.Close()
+
+	b := make([]byte, 1024*1024-1000)
+	logger.Write(b)
+	logger.Write(b[0:2000])
+
+	files, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatalf("failed to read temporary log dir: %v", err)
+	}
+
+	// We might get 2 or 3 files depending
+	// on the race between compressed log file generation,
+	// removal of the non compressed file and reading the directory.
+	// Ordering of the files are [ test-*.log test-*.log.gz test.log ]
+	if len(files) < 2 || len(files) > 3 {
+		t.Log("got files: ", files)
+		t.Fatalf("got %v files want 2", len(files))
+	}
+
+	wantPattern := "test-*-*-*-*-*.*.log"
+	test_date_log := files[0]
+	if m, _ := path.Match(wantPattern, test_date_log.Name()); m != true {
+		t.Fatalf("got %v filename want %v", test_date_log.Name(), wantPattern)
+	}
+
+	st, err := os.Stat(path.Join(dir, test_date_log.Name()))
+	if err != nil {
+		t.Fatalf("failed to check file permissions: %v", err)
+	}
+
+	if st.Mode() != os.FileMode(mode) {
+		t.Errorf("file mode is %v, want %v", st.Mode(), mode)
+	}
+
+	test_dot_log := files[len(files)-1]
+	if test_dot_log.Name() != "test.log" {
+		t.Fatalf("got %v filename want test.log", test_dot_log.Name())
+	}
+
+	st, err = os.Stat(path.Join(dir, test_dot_log.Name()))
+	if err != nil {
+		t.Fatalf("failed to check file permissions: %v", err)
+	}
+
+	if st.Mode() != os.FileMode(mode) {
+		t.Errorf("file mode is %v, want %v", st.Mode(), mode)
+	}
+}
+
+func TestFileModeConfig(t *testing.T) {
+	tests := []struct {
+		name    string
+		d       *caddyfile.Dispenser
+		fw      FileWriter
+		wantErr bool
+	}{
+		{
+			name: "set mode",
+			d: caddyfile.NewTestDispenser(`
+file test.log {
+	mode 0666
+}
+`),
+			fw: FileWriter{
+				Mode: 0o666,
+			},
+			wantErr: false,
+		},
+		{
+			name: "set mode 3 digits",
+			d: caddyfile.NewTestDispenser(`
+file test.log {
+	mode 666
+}
+`),
+			fw: FileWriter{
+				Mode: 0o666,
+			},
+			wantErr: false,
+		},
+		{
+			name: "set mode 2 digits",
+			d: caddyfile.NewTestDispenser(`
+file test.log {
+	mode 66
+}
+`),
+			fw: FileWriter{
+				Mode: 0o066,
+			},
+			wantErr: false,
+		},
+		{
+			name: "set mode 1 digits",
+			d: caddyfile.NewTestDispenser(`
+file test.log {
+	mode 6
+}
+`),
+			fw: FileWriter{
+				Mode: 0o006,
+			},
+			wantErr: false,
+		},
+		{
+			name: "invalid mode",
+			d: caddyfile.NewTestDispenser(`
+file test.log {
+	mode foobar
+}
+`),
+			fw:      FileWriter{},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fw := &FileWriter{}
+			if err := fw.UnmarshalCaddyfile(tt.d); (err != nil) != tt.wantErr {
+				t.Fatalf("UnmarshalCaddyfile() error = %v, want %v", err, tt.wantErr)
+			}
+			if fw.Mode != tt.fw.Mode {
+				t.Errorf("got mode %v, want %v", fw.Mode, tt.fw.Mode)
+			}
+		})
+	}
+}
+
+func TestFileModeJSON(t *testing.T) {
+	tests := []struct {
+		name    string
+		config  string
+		fw      FileWriter
+		wantErr bool
+	}{
+		{
+			name: "set mode",
+			config: `
+{
+	"mode": "0666"
+}
+`,
+			fw: FileWriter{
+				Mode: 0o666,
+			},
+			wantErr: false,
+		},
+		{
+			name: "set mode invalid value",
+			config: `
+{
+	"mode": "0x666"
+}
+`,
+			fw:      FileWriter{},
+			wantErr: true,
+		},
+		{
+			name: "set mode invalid string",
+			config: `
+{
+	"mode": 777
+}
+`,
+			fw:      FileWriter{},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fw := &FileWriter{}
+			if err := json.Unmarshal([]byte(tt.config), fw); (err != nil) != tt.wantErr {
+				t.Fatalf("UnmarshalJSON() error = %v, want %v", err, tt.wantErr)
+			}
+			if fw.Mode != tt.fw.Mode {
+				t.Errorf("got mode %v, want %v", fw.Mode, tt.fw.Mode)
+			}
+		})
+	}
+}

modules/logging/filewriter_test_windows.go

@@ -0,0 +1,55 @@
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build windows
+
+package logging
+
+import (
+	"os"
+	"path"
+	"testing"
+)
+
+// Windows relies on ACLs instead of unix permissions model.
+// Go allows to open files with a particular mode put it is limited to read or write.
+// See https://cs.opensource.google/go/go/+/refs/tags/go1.22.3:src/syscall/syscall_windows.go;l=708.
+// This is pretty restrictive and has few interest for log files and thus we just test that log files are
+// opened with R/W permissions by default on Windows too.
+func TestFileCreationMode(t *testing.T) {
+	dir, err := os.MkdirTemp("", "caddytest")
+	if err != nil {
+		t.Fatalf("failed to create tempdir: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	fw := &FileWriter{
+		Filename: path.Join(dir, "test.log"),
+	}
+
+	logger, err := fw.OpenWriter()
+	if err != nil {
+		t.Fatalf("failed to create file: %v", err)
+	}
+	defer logger.Close()
+
+	st, err := os.Stat(fw.Filename)
+	if err != nil {
+		t.Fatalf("failed to check file permissions: %v", err)
+	}
+
+	if st.Mode().Perm()&0o600 != 0o600 {
+		t.Fatalf("file mode is %v, want rw for user", st.Mode().Perm())
+	}
+}