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

ignore exec.ErrDot when starting caddy in background #6512

Merged
merged 2 commits into from
Aug 13, 2024
Merged

Conversation

WeidiDeng
Copy link
Member

Fix 6508.

@mholt
Copy link
Member

mholt commented Aug 12, 2024

But isn't this a security risk? The reason this error is returned is because this may be unexpected. (Sigh, I don't understand why Windows does this. Of course putting binaries in an explicit PATH is a security benefit.)

@WeidiDeng
Copy link
Member Author

os.Arg[0] is the program name, which may or may not be a relative path. In case it's a relative path, it still points to the path where the executable is. The security risk is only when the executable it points to may not be what we wanted. That has to do with executable resolution in PWD.

The only way this path resolve to an executable that' not wanted is very complex and depends too much on timing. Caddy has to start and the original executable get unlinked and a new executable named caddy has to be created before the original caddy gets to the place where it spawns a new caddy process. This is very unlikely to happen in the real world.

I think caddy start is a convenient way to run a caddy. Denying user usage simply because it's not put in the PATH directory is against simple to use philosophy.

@mholt
Copy link
Member

mholt commented Aug 12, 2024

Ah, that's a good point, it's always the 0th CLI arg, which I guess makes it more trustworthy.

@mholt mholt merged commit 9ddb78f into master Aug 13, 2024
23 checks passed
@mholt mholt deleted the relative-path-fix branch August 13, 2024 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[windows] cannot run executable found relative to current directory
3 participants