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

Regression: Caddy 2.9.0 log writer requires existing directory path #6766

Open
mattgarber opened this issue Jan 4, 2025 · 8 comments
Open
Labels
bug 🐞 Something isn't working
Milestone

Comments

@mattgarber
Copy link

Overview

Using Caddy 2.8.4, an output log file could be specified such as output file /var/log/caddy/access.log, and Caddy would create the caddy subdirectory path in /var/log if it was missing before creating/opening the access.log file – assuming proper ownership and permissions, e.g., Caddy running as root.

Under Caddy 2.9.0, if the /var/log/caddy directory path doesn't already exist when Caddy starts, it will exit with a log writer error rather than creating the necessary containing directory for the log file:

Error: loading initial config: loading new config: setting up custom log 'log0': opening log writer using &logging.FileWriter{Filename:"/var/log/caddy/access.log", Mode:0x0, Roll:(*bool)(nil), RollSizeMB:239, RollCompress:(*bool)(nil), RollLocalTime:false, RollKeep:4, RollKeepDays:30}: open /var/log/caddy/access.log: no such file or directory

After looking through recent activity, I had a guess it might be related to the FileWriter changes (#6314) and chmod improvements within the past few days (#6761), although reproduced the same regression with v2.9.1-0.20250103220605-010cd982731b built with xcaddy against that latest commit (010cd98).

To Reproduce

Use the following two Dockerfiles (for v2.8.4 and v2.9.0) with identical Caddyfile to test the regression in behavior. (The v2.9.0 Dockerfile will require the /var/log/caddy directory to be created before Caddy starts, commented at first to show the error being thrown.)

Dockerfile (v2.8.4)

FROM caddy:2.8-alpine
 
COPY Caddyfile /etc/caddy/Caddyfile

EXPOSE 80
EXPOSE 443

Dockerfile (v2.9.0)

FROM caddy:2.9-alpine
 
COPY Caddyfile /etc/caddy/Caddyfile
#RUN mkdir /var/log/caddy

EXPOSE 80
EXPOSE 443

Dockerfile (commit 010cd98)

FROM caddy:2.9-builder AS builder
 
RUN xcaddy build 010cd982731b91bf60a3afd313bedcd01cd7ab0e


FROM caddy:2.9-alpine

COPY --from=builder /usr/bin/caddy /usr/bin/caddy
COPY Caddyfile /etc/caddy/Caddyfile
#RUN mkdir /var/log/caddy

EXPOSE 80
EXPOSE 443

Caddyfile

example.com {
    root /srv/www/example.com
    encode zstd gzip
    file_server

    log {
        output file /var/log/caddy/access.log {
            roll_size 250m
            roll_keep 4
            roll_keep_for 30d
        }
    }
}
@mattgarber
Copy link
Author

Alternative steps to reproduce

My initial steps used Docker as that was quickest for me to test and capture on a server, but the following should also work easily on a Linux or macOS desktop (I verified on macOS):

  1. Create a temporary directory as your normal user, e.g., $HOME/caddy-test.
  2. Inside that directory, create a Caddyfile with the contents below.
  3. Download the v2.8.4 and v2.9.0 Caddy binaries for your platform, named as caddy-2.8.4 and caddy-2.9.0 into this test directory.
  4. Execute ./caddy-2.8.4 run. You should see the normal Caddy startup messages.
  5. Visit https://localhost:8080 in your browser; you should get a 404 as no directory index or existing files have been requested.
  6. Hit Ctrl-C to stop the running Caddy process. There should be a some-non-existent-path subdirectory created with access.log inside it.
  7. Remove the some-non-existent-path subdirectory. If you forget to remove the log directory in between, it will fail to reproduce and start up fine.
  8. Execute ./caddy-2.9.0 run. It will immediately throw the log writer error and fail to start:
2025/01/05 20:53:07.946	INFO	using adjacent Caddyfile
2025/01/05 20:53:07.947	INFO	adapted config to JSON	{"adapter": "caddyfile"}
2025/01/05 20:53:07.947	WARN	Caddyfile input is not formatted; run 'caddy fmt --overwrite' to fix inconsistencies	
{"adapter": "caddyfile", "file": "Caddyfile", "line": 2}
Error: loading initial config: loading new config: setting up custom log 'log0': opening log writer using 
&logging.FileWriter{Filename:"./some-non-existent-path/access.log", Mode:0x0, Roll:(*bool)(nil), RollSizeMB:239, 
RollCompress:(*bool)(nil), RollLocalTime:false, RollKeep:4, RollKeepDays:30}: 
open ./some-non-existent-path/access.log: no such file or directory

Caddyfile

localhost:8080 {
    root .
    encode zstd gzip
    file_server

    log {
        output file ./some-non-existent-path/access.log {
            roll_size 250m
            roll_keep 4
            roll_keep_for 30d
        }
    }
}

@Thedarkmatter10
Copy link

Hey @mattgarber I want to work on this issue . Can you assign this issue to me ?

@francislavoie
Copy link
Member

We don't generally assign issues. If you want to work on it, just open a PR.

@mholt
Copy link
Member

mholt commented Jan 6, 2025

Should be an easy fix. What will the permissions on the directory(ies) be? I was thinking 0700.

@mattgarber
Copy link
Author

mattgarber commented Jan 6, 2025

@mholt That's a good question… I believe that the previous behavior (through v2.8.4) was based on the umask of the user running the caddy process at creation time. So for example, root on many server distributions (and containers) has a umask of 022, leading to the containing directories being created with permissions of 0755 (drwxr-xr-x) in the running instance I'm looking at now, although the log file itself was created more strictly as 0600 (-rw-------).

If you think that permissions should start to be a bit more restrictive on log locations by default, permissions of 0700 on the containing directory/ies (and 0600 on the file) would make sense, too.

@mholt
Copy link
Member

mholt commented Jan 6, 2025

People were apparently surprised with the tighter permissions on the log files in 2.9 (which, I don't think I realized we had changed them -- maybe a dependency changed the defaults?, anyway...), but I don't think, if the directory doesn't even exist yet, it would inconvenience anyone. If they cared, they'd have made the directory already.

So maybe 0700 would be a conservative default, and we can go from there. Caddy makes it, Caddy owns it. ✊

@mattgarber
Copy link
Author

I think that sounds very reasonable:

  • If the containing directory already exists, Caddy doesn't alter it (chmod), so different permissions can be set if necessary by creating the directory in advance.
  • If the containing directory(ies) doesn't already exist, Caddy creates it with conservative defaults of 0700.
  • This basically matches the secure default of the log files created now with 0600.

@mholt
Copy link
Member

mholt commented Jan 7, 2025

Ok, sounds good. MkdirAll with 0700 permission. If a PR shows up in the next day or so I'll review it; otherwise I'll just push a commit myself.

@mholt mholt added this to the v2.9.1 milestone Jan 7, 2025
@mholt mholt added the bug 🐞 Something isn't working label Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants