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

Docs: Added hide for *.php files to file_server directives. #1463

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gnat
Copy link

@gnat gnat commented Mar 27, 2025

Thought I'd make a quick documentation PR as this has always spooked me with FrankenPHP. Prevents accidental exposure of raw .php files when using file_server. It'd be a security nightmare if we had an issue with unwanted PHP code downloads, particularly with projects that mix assets with code.

Protects us against scenarios where a .php ends up at file_server, ex: Caddyfile misconfiguration (could be as lame as a syntax issue or commenting out php or php_server) or FrankenPHP issue/crash.

@@ -123,7 +123,9 @@ route {
# FrankenPHP!
@phpFiles path *.php
php @phpFiles
file_server
file_server {
hide *.php
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it makes sense to add hide *.php to this specific configuration. It might make people think they need to add it everywhere, even though in this case it's (as you mentioned) redundant.

Maybe it would make more sense to add a separate section with a specific scenario in which this would be necessary for security?

@gnat
Copy link
Author

gnat commented Mar 27, 2025

If there's any issue at the php or php_server directives, it'll fall through and your code is going to be publicly served by file_server. FrankenPHP is lovely but we've had crash fixes already.

Agreed I don't like extra lines either, but if there's any regression/crash in FrankenPHP or someone messes up a Caddyfile edit, you get a very scary failure scenario.

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.

2 participants