Skip to content

[AssetMapper] Dotfiles exposed due to cache prefix #52697

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.

Sign up for GitHub

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

Jump to bottom
Closed
smnandre opened this issue Nov 23, 2023 · 4 comments
Closed

[AssetMapper] Dotfiles exposed due to cache prefix #52697

smnandre opened this issue Nov 23, 2023 · 4 comments
Labels
AssetMapper Bug Status: Needs Review

Comments

@smnandre
Copy link
Member

smnandre commented Nov 23, 2023 •
edited
Loading

Symfony version(s) affected

6.4,7.0,7.1

Description

Any hidden / dotfile in the assets directory is deployed and publically accessible.

Assets files are versionned via a prefix inserted before the file extension.

The servers standard protection against dotfile access cannot work, as the mapped files are not "dotfiles" anymore.

your-project/
├─ assets/
│  ├─ .DS_Store 
│  ├─ .cert
│  └─ ... 

your-project/
├─ public/
│  ├─ assets/
│  │   ├─ -XXXXX.DS_Store
│  │  └─ -YYYYY.cert

How to reproduce

symfony new asset-mapper-dot-files --webapp --version="next" \
	&& cd asset-mapper-dot-files \
	&& composer req symfony/asset-mapper \
	&& touch assets/.FOO \
	&& APP_ENV=prod php bin/console asset-map:compile 
	
ls -la public/assets

Possible Solution

Exclude patterns

This behaviour can be prevented with a specific config directive

# config/asset_mapper.yaml
framework:
    asset_mapper:
        # The paths to make available to the asset mapper.
        paths:
            - assets/

        excluded_patterns:
            - '*/.*'

As of today this is neither documented or "default behaviour".

That could be a first move, but this is something risky, as the pattern is not self-explanatory

Fix asset collection

Another way would be to modify the assets collection (in the Repository) to ignore dotfiles

..

What do you think ?
@smnandre smnandre added the Bug label Nov 23, 2023
@94noni
Copy link
Contributor

94noni commented Nov 23, 2023

Or perhaps go with a list of default file extension (.js/.css/.png etc) allowed, this being possible to override ?

but indeed having all compiles to public may be risky in case of wrong/accidental commit on assets folders

@maelanleborgne
Copy link
Contributor

It feels like a list of allowed files would be really long and hard to maintain, and it may be confusing for developers to see some files missing when compiling assets after an upgrade.
Maybe the easier path would be changing the default configuration of the asset mapper to use the excluded_pattern: '*/.*', as well as adding it to the config file delivered by the recipe along with a comment on what this directive does and adding this to the docs ?
It could also be worth displaying a warning while compiling assets to suggest the user to check the content of the public/assets dir for any unwanted files and create exclusion rules accordingly.

wdyt ?

@smnandre
Copy link
Member Author

What about an option "exclude_dotfiles" (default true) ?

@weaverryan weaverryan mentioned this issue Nov 24, 2023
Merged
@weaverryan
Copy link
Member

What about an option "exclude_dotfiles" (default true) ?

Yes, I like this option. I'd rather not bother at all - and make it clear that ALL files in your mapped assets/ directory are made publicly accessible (that's actually the whole point of AssetMapper), but it might be better to be on the safe side. But hiding dot files by default (but allowing it to be turned of) seems reasonable to me, and safe by default. Dot files, I think, are a special case.

Btw, for comparison, Propshaft from Ruby has the same behavior as AssetMapper and, as far as I know, doesn't exclude anything by default: https://github.com/rails/propshaft#usage

So, if we are going to do this, it's at #52712

@fabpot fabpot closed this as completed Nov 25, 2023
fabpot added a commit that referenced this issue Nov 25, 2023
@fabpot
feature #52712 [AssetMapper] Exclude dot files (weaverryan)
1386c95 
This PR was merged into the 6.4 branch.

Discussion
----------

[AssetMapper] Exclude dot files

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes - could possibly be considered a security fix
| New feature?  | yes
| Deprecations? | no
| Issues        | Fix #52697
| License       | MIT

See #52697. The biggest question is: should we do this? Is it enough to say "Hey! When you map an assets directory, EVERYTHING is published publicly?". Or should we be on the safe side and exclude dot files by default.

Cheers!

Commits
-------

85c0ef6 [AssetMapper] Adding an option (true by default) to not publish dot files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
AssetMapper Bug Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @weaverryan @94noni @smnandre @maelanleborgne @carsonbot