getgrav / grav

Modern, Crazy Fast, Ridiculously Easy and Amazingly Powerful Flat-File CMS powered by PHP, Markdown, Twig, and Symfony
https://getgrav.org
MIT License
14.58k stars 1.41k forks source link

Not sure if this should be deeply concerning or not #1269

Closed matthew-dean closed 7 years ago

matthew-dean commented 7 years ago

So I did bin/gpm selfupgrade and got this:

...
Cleared:  /Users/[redacted]/cache/*
ERROR: rmdir(/Applications): Operation not permitted

Touched: /Users/[redacted]/user/config/system.yaml
...

Um.... is there any reason why Grav is trying to remove the entire Applications directory from my Mac? 'Cause if it's doing what it says it's trying to do, it probably should not be.

matthew-dean commented 7 years ago

Soooo..... some of my Applications are now missing, such as Google Chrome. Concern level: Rising

It appears now that Grav was able to delete all Applications from my Mac that were not installed via the App Store. I would recommend perhaps flagging this as "Worst bug ever"? Concern level: Near Maximum

rhukster commented 7 years ago

This is very strange, certainly we've never seen anything like this before. I'm looking over the Cache::clearCache() method: https://github.com/getgrav/grav/blob/develop/system/src/Grav/Common/Cache.php#L333-L404, and the remove should simply loop over some streams:

    protected static $standard_remove = [
        'cache://twig/',
        'cache://doctrine/',
        'cache://compiled/',
        'cache://validated-',
        'cache://images',
        'asset://',
    ];

These are the standard ones, and they are built on top of the current Grav root path. I'm guessing you have some strange symlink back to applications or root within one of your cache clearable folders? I'm not sure how else this could happen, because Grav certainly has no clue about anything outside of Grav.

matthew-dean commented 7 years ago

Well, it's hard for me to blame Grav entirely. All I know is that the deletion of Applications happened at that moment. And no, there should be no symlink (certainly none I can see) that would point to Applications.

w00fz commented 7 years ago

How did you install your Grav? What's the location it's in? What server are you running on your mac? Apache, Nginx? What PHP version? Was this your first install or did you have it working before?

Also just so you know, all the core developers are using Mac and we have hundreds of instances running Grav with different implementations, symlinks, no-symlinks, git checkouts, unzipped skeletons and so on. We never seen Grav ever trying to eliminate a folder that was outside its own scope.

rhukster commented 7 years ago

Are you running a multi-site setup? Do you have a custom setup.php that might be overidding or changing the streams?

rhukster commented 7 years ago

Would you mind sending us the 'un-redacted' copy of the output. The path could be quite important. Also what webserver are you running this under.

matthew-dean commented 7 years ago

Would you mind sending us the 'un-redacted' copy of the output. The path could be quite important. Also what webserver are you running this under.

Webserver was under Mamp Pro.


MacBook-Pro:beedie Matthew$ bin/gpm selfupgrade

GPM Releases Configuration: Stable

Grav v1.1.14 is now available [release date: Wed Jan 18 22:13:42 2017].
You are currently using v1.1.12.
Would you like to read the changelog before proceeding? [y|N] N
Would you like to upgrade now? [y|N] y

Preparing to upgrade to v1.1.14..
  |- Downloading upgrade [2.48M]...   100%
  |- Installing upgrade...    ok                             
  '- Success!  

Clearing cache

Cleared:  /Users/Matthew/git/BAM/beedie/cache/*
ERROR: rmdir(/Applications): Operation not permitted

Touched: /Users/Matthew/git/BAM/beedie/user/config/system.yaml

MacBook-Pro:beedie Matthew$ 
matthew-dean commented 7 years ago

Do you know of any Mac-specific feature or bug that would remove all third-party apps? I don't disagree with you that this is straight-up bizarre as hell. It might be a complete red herring that it happened during a Grav update, where it not for the output about Applications.

rhukster commented 7 years ago

The more I look at this the more I think it must of been a symlink in your /cache folder that somehow linked back to your root, or even Applications folder. The logic just deletes everything under cache/ wherever that is, at that seems to be found correctly as it is trying to delete:

/Users/[your_user]/[grav_location]/cache/*

that is correct, and in fact with clear --all which is what selfupgrade() calls, that is all it's doing, just recursively deleting the cache folders under the cache/ folder.

Can you look in your grav/cache folder and do an ls -la ?

matthew-dean commented 7 years ago
MacBook-Pro:cache Matthew$ ls -la
total 0
drwxr-xr-x   2 Matthew  staff   68 24 Jan 14:12 .
drwxr-xr-x  25 Matthew  staff  850 24 Jan 14:38 ..
rhukster commented 7 years ago

There' certainly nothing mac specific. Grav is developed on Mac, we all use macs, i've been doing this for several years, clearing cache multiple times per day as I develop stuff, and have never seen anything like this.

w00fz commented 7 years ago

Are you still able to recover that instance you had? A find /Users/.../beedie -type l -ls might help determine if effectively there were any symlinks. That's the only thing I can personally think of that would make Grav follow outside the scope.

matthew-dean commented 7 years ago

@w00fz find /Users/Matthew/git/BAM/beedie -type l -ls produces nothing.

Thanks guys for a quick response. I realize now it's probably not your fault based on the code you shared. But.... what the hell, lol.

rhukster commented 7 years ago

Would you be willing to download a fresh Grav and try it again? I know it might be a little scary based on what happened before, but it would definitely rule out a core issue.

It really is an odd one.

matthew-dean commented 7 years ago

@rhukster Well, first, I wanted to see if I could replicate similar conditions.

I did a reset in git back to where I started. Then I held my breath and did bin/gpm selfupgrade.

This time, /Applications was not mentioned. You may be right that somehow a symlink ended up in there, but I can't for the life of me figure out how that happened.

Regardless, would it be reasonable to ask that Grav does a check to not follow symlinks from inside cache? It should delete a symlink if it exists (presuming that was the issue), but it shouldn't be deleting the location the symlink is pointing to. It seems like a small change for cache clearing, but would prevent huge repercussions for someone else, however slim the chance.

As in: if you want to decide it's user error, I'm fine with that. But I don't understand a cache clearing strategy that should have enabled this error, symlink or not. Cache should always be local, and not deleted remotely, no?

rhukster commented 7 years ago

I'm going to look at adding a check in the delete to ensure that symlinks are either skipped, or warnings thrown and skipped. Really there's no real reason to have symlinks in the cache folder, can only lead to issues.

mahagr commented 7 years ago

I don't think there is really a need to skip symlink -- just unlink it as it was a file.

mahagr commented 7 years ago

After taking a look at the code, Folder::delete() already takes care of symlinks by unlinking them. As it doesn't follow the links, having a symlink in cache folder should have deleted nothing. Hard links, on the other hand, would have been deleted, but it doesn't matter either as the files would still exist in their original location.

I don't see that the fix would have made any difference; there must have been something else going on in the installation. Maybe bad skeleton was used? Maybe there was a bad configuration?

mahagr commented 7 years ago

One thing came to my mind -- is there a realpath() call somewhere? As it does resolve symlink to a real path...

Again, the fix wouldn't have detected that either.

timcosgrove commented 6 years ago

This just happened to me. I ran bin/gpm self-upgrade and now all my third party apps in /Applications are gone.

timcosgrove commented 6 years ago

I had done nothing re: symlinking from within that Grav directory to anywhere else in the system. Let me know what else I might do to help diagnose.

timcosgrove commented 6 years ago

But I am able to confirm similar output:

Clearing cache

Cleared:  /Users/timcosgrove/workspace/redacted/app/cache/*
ERROR: rmdir(/Applications): Operation not permitted

Touched: /Users/timcosgrove/workspace/redacted/app/user/config/system.yaml

I am unable to get this to happen again. I rolled back the Grav changes (it was managed as a git repo) and re-ran as bin/gpm -vvv self-upgrade, but did not see similar behavior.

I attempted self-upgrade on a different Grav project I have locally, but again did not see the deletion this time. It only happened on the first pass.