logging: add DirMode options and propagate FileMode to rotations (#7335)

Co-authored-by: Francis Lavoie <lavofr@gmail.com>

Author: DeRuina

caddytest/integration/caddyfile_adapt/log_roll_days.caddyfiletest

@@ -1,26 +1,43 @@
 :80
 
-log {
+log one {
 	output file /var/log/access.log {
+		mode 0644
+		dir_mode 0755
 		roll_size 1gb
 		roll_uncompressed
 		roll_local_time
 		roll_keep 5
 		roll_keep_for 90d
 	}
 }
+log two {
+	output file /var/log/access-2.log {
+		mode 0777
+		dir_mode from_file
+		roll_size 1gib
+		roll_interval 12h
+		roll_at 00:00 06:00 12:00,18:00
+		roll_minutes 10 40 45,46
+		roll_keep 10
+		roll_keep_for 90d
+	}
+}
 ----------
 {
 	"logging": {
 		"logs": {
 			"default": {
 				"exclude": [
-					"http.log.access.log0"
+					"http.log.access.one",
+					"http.log.access.two"
 				]
 			},
-			"log0": {
+			"one": {
 				"writer": {
+					"dir_mode": "0755",
 					"filename": "/var/log/access.log",
+					"mode": "0644",
 					"output": "file",
 					"roll_gzip": false,
 					"roll_keep": 5,
@@ -29,7 +46,34 @@ log {
 					"roll_size_mb": 954
 				},
 				"include": [
-					"http.log.access.log0"
+					"http.log.access.one"
+				]
+			},
+			"two": {
+				"writer": {
+					"dir_mode": "from_file",
+					"filename": "/var/log/access-2.log",
+					"mode": "0777",
+					"output": "file",
+					"roll_at": [
+						"00:00",
+						"06:00",
+						"12:00",
+						"18:00"
+					],
+					"roll_interval": 43200000000000,
+					"roll_keep": 10,
+					"roll_keep_days": 90,
+					"roll_minutes": [
+						10,
+						40,
+						45,
+						46
+					],
+					"roll_size_mb": 1024
+				},
+				"include": [
+					"http.log.access.two"
 				]
 			}
 		}
@@ -42,7 +86,7 @@ log {
 						":80"
 					],
 					"logs": {
-						"default_logger_name": "log0"
+						"default_logger_name": "two"
 					}
 				}
 			}

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 <filename> {
 //	    mode          <mode>
+//	    dir_mode      <mode|inherit|from_file>
 //	    roll_disabled
 //	    roll_size     <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
@@ -352,31 +456,48 @@ func (fw *FileWriter) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 			fw.RollInterval = duration
 
 		case "roll_minutes":
-			var minutesArrayStr string
-			if !d.AllArgs(&minutesArrayStr) {
+			// Accept either a single comma-separated argument or
+			// multiple space-separated arguments. Collect all
+			// remaining args on the line and split on commas.
+			args := d.RemainingArgs()
+			if len(args) == 0 {
 				return d.ArgErr()
 			}
-			minutesStr := strings.Split(minutesArrayStr, ",")
-			minutes := make([]int, len(minutesStr))
-			for i := range minutesStr {
-				ms := strings.Trim(minutesStr[i], " ")
-				m, err := strconv.Atoi(ms)
-				if err != nil {
-					return d.Errf("parsing roll_minutes number: %v", err)
+			var minutes []int
+			for _, arg := range args {
+				parts := strings.SplitSeq(arg, ",")
+				for p := range parts {
+					ms := strings.TrimSpace(p)
+					if ms == "" {
+						return d.Errf("parsing roll_minutes: empty value")
+					}
+					m, err := strconv.Atoi(ms)
+					if err != nil {
+						return d.Errf("parsing roll_minutes number: %v", err)
+					}
+					minutes = append(minutes, m)
 				}
-				minutes[i] = m
 			}
 			fw.RollAtMinutes = minutes
 
 		case "roll_at":
-			var timeArrayStr string
-			if !d.AllArgs(&timeArrayStr) {
+			// Accept either a single comma-separated argument or
+			// multiple space-separated arguments. Collect all
+			// remaining args on the line and split on commas.
+			args := d.RemainingArgs()
+			if len(args) == 0 {
 				return d.ArgErr()
 			}
-			timeStr := strings.Split(timeArrayStr, ",")
-			times := make([]string, len(timeStr))
-			for i := range timeStr {
-				times[i] = strings.Trim(timeStr[i], " ")
+			var times []string
+			for _, arg := range args {
+				parts := strings.SplitSeq(arg, ",")
+				for p := range parts {
+					ts := strings.TrimSpace(p)
+					if ts == "" {
+						return d.Errf("parsing roll_at: empty value")
+					}
+					times = append(times, ts)
+				}
 			}
 			fw.RollAt = times