Prevent direct access for .blade.php files #1075
Conversation
Done. |
|
This should be ready to get merged assuming we don't want to add another default extension. I believe Blade and Twig as defaults should be sane. |
Return a 401 in all cases to prevent snooping
|
|
||
| {% block blade_twig_templates -%} | ||
| # Prevent Blade and Twig templates from being accessed directly. | ||
| location ~* \.(blade\.php|twig)(/.*)?$ { |
There was a problem hiding this comment.
Why the (/.*)? This is going to match a lot more than just files ending in .blade.php or .twig
There was a problem hiding this comment.
I think it was a valid point raised– unless I'm missing something in the nginx syntax above compared to Apache. Tests show it works as intended though.
There was a problem hiding this comment.
🤔 okay but I still don't understand that situation. Maybe @QWp6t can clear it up
There was a problem hiding this comment.
Meh, you're probably right in terms of it not being an issue in Trellis unless the end-user messed with nginx configuration. Do you just want to leave it as a safe-guard or revert it? I don't think it really "hurts" anything persay. It'd just make example.com/test.blade.php/asdf return the 401. It was mainly suggested due to certain webserver configurations seeing a URL like that and "smartly" returning the closest match which would then result in a status 200 response ignoring the exact location rule.
There was a problem hiding this comment.
The twig case is the one that worries me slightly more since there's no .php in it as well. So example.com/foo.twig/asdf would match? It likely doesn't matter, but I don't want to break any URLs which might be valid.
There was a problem hiding this comment.
It would indeed match– but as far as WordPress permalinks go it'd be a non-issue as . is replaced with - when editing a slug.
There was a problem hiding this comment.
@swalkinshaw I don't think (/.*) is needed here.
https://roots-example-project.com/app/themes/sage/resources/views/index.blade.php/more-stuff-here
Seems to work fine.
I was only suggesting it for Apache because after testing on a few Apache servers, they all displayed the blade file.
|
Curious which error code we should settle on? Possibilities are |
|
I would suggest returning whatever the Not Allowed code is. I think that's 403 iirc (I'm on my phone, too lazy to look it up). |
Revert to a 403 (`deny all`)
|
Apologies for the delay, I had sworn I had pushed an update already. The regex is now reverted as well as returning a 403 ( This should now be ready. |
In Trellis' current state,
.blade.phpfiles are viewable in plain-text by accessing them directly. Laravel avoids this problem by having views outside of thepublicdirectory, but unfortunately, that isn't a sane solution with WordPress.Outside of Trellis, this also calls for an addition to the Sage docs to provide examples on preventing access with apache and/or nginx.
Credit to pkarjala from this thread for finding this.
Real World Example