search

pixelfed / pixelfed

Photo Sharing. For Everyone.
https://pixelfed.org
GNU Affero General Public License v3.0
5.61k stars 670 forks source link

Picture display broken after upgrade to v0.11.5 : folder rights issue #4275

Open lapineige opened 1 year ago

lapineige commented 1 year ago

Hello,

On Yunohost installations, since version 0.11.5 new uploaded pictures are broken. They are not shown, neither the preview is. Even in compose view. Alternative text is working. I don't really know how to debug this. No horizon failed job, no supervisor issue, I can't find laravel log, no web console error… No other error to report. You can see an example here : https://photo.lapineige.fr/i/web/post/548580903387685397

Is that a know Pixelfed bug ? How can I diagnose this ?

Thank you

Pixelfed : 0.11.5 Postgres: 13.9 PHP: 8.1 - recently upgraded, before Pixelfed 0.11.5 we were using php 8.0 Laravel : 9.52.4

wgahnagl commented 1 year ago

I'm getting this

Private Folder > 770 Public Folder > 775 (not 750) .... Private File > 660 Public File > 664

Several people reported that they used this with no success. (example here YunoHost-Apps/pixelfed_ynh#210 (comment))

One or maybe two reported they did an upgrade and had no change in the filesystems.php, just as it was the case for me the first time.

I persist to think there is a bug, but I can't find if it's on Yunohost side or rather in Pixelfed itself.

I'm getting this on a regular pixelfed install 🤔 Trying to debug it as well I've changed the filesystems.php but to no avail

lapineige commented 1 year ago

You mean you have the correct rights in that filesystems.php file after a new install, or that it is not updated ?

mitexleo commented 1 year ago

For the record, I tried to upgrade to this commit : #4287 With no change regarding this issue. Also filesystems.php wasn't affected, no change in that file.

I wonder why an upgrade don't overwrite those files content. Any idea ?

It won't update previously uploaded images permission. Only new ones will be affected

lapineige commented 1 year ago

I mean for filesystems.php and other (php) config files.

When upgrading to a new source code they should be overwritten. I wonder if it as a link with the fact we don't use a release archive in this case.

lapineige commented 1 year ago

The bug remains after upgrading to v0.11.6 : https://github.com/YunoHost-Apps/pixelfed_ynh/pull/210

As a result this assumption is wrong :

I wonder if it as a link with the fact we don't use a release archive in this case.

lapineige commented 1 year ago

https://forum.yunohost.org/t/pixelfed-pictures-not-loading/24244/31 Someone noticed that with the old UI, the issue is transformed into a "Something went wrong".

lapineige commented 1 year ago

Following the linked discussion, I was able to reproduce the fix:

Steps to reproduce:

  1. change filesystems.php private folder right to 0750. Note: after the upgrade the permission were back to default, so it was 700

  2. php8.2 config:cache

  3. php8.2 cache:clear

  4. chown -R :www-data public/storage/m/_v2/1/someRandomNumber/anotherRandomOne_FolderThanContainsTheNewPictures

The thing is, it only fixes it partly. In Pixelfed this works https://photo.lapineige.fr/i/web/post/548580903387685397 From Mastodon, the picture is broken. The link to the image is valid.

So it turns out that now newly created pictures are owned by pixelfed:pixelfed, not pixelfed:www-data. I don't know if it's a Pixelfed or Yunohost issue ?

mitexleo commented 1 year ago

Following the linked discussion, I was able to reproduce the fix:

Steps to reproduce:

  • change filesystems.php private folder right to 0750. Note: after the upgrade the permission were back to default, so it was 700
  • php8.2 config:cache
  • php8.2 cache:clear
  • chown -R :www-data public/storage/m/_v2/1/someRandomNumber/anotherRandomOne_FolderThanContainsTheNewPictures

The thing is, it only fixes it partly. In Pixelfed this works https://photo.lapineige.fr/i/web/post/548580903387685397 From Mastodon, the picture is broken. The link to the image is valid.

So it turns out that now newly created pictures are owned by pixelfed:pixelfed, not pixelfed:www-data. I don't know if it's a Pixelfed or Yunohost issue ?

You're not doing it correctly. KINDLY DO AS I SAID EARLIER.

In order to everything work correctly you have to add the user "pixelfed" to "www-data" group.

Then go to your Pixelfed root dir and run : sudo chown -R www-data:www-data .

lapineige commented 1 year ago

add the user "pixelfed" to "www-data" group.

To my understanding that should be the case by default. I'll check. And by default all files and folder belongs to pixelfed:www-data (as seen here https://github.com/YunoHost-Apps/pixelfed_ynh/blob/master/scripts/install#L102).

Then go to your Pixelfed root dir and run : sudo chown -R www-data:www-data .

I don't understand why all files should suddenly belong to another user, while it was working perfectly fine until version 0.11.5. In addition, that's not really how it is supposed to work in Yunohost so I'm not sure we should do that. And security-wise I'm not sure it's a good thing to give write permission to another user (if we can avoid it).

What is the root problem that this solution would fix ? That's what I need to understand to find another solution that would fit better to this use case.

Also to me this doesn't explain why newly create files are with pixelfed:pixelfed, even if we chown the folder to pixelfed:www-data.

lapineige commented 1 year ago

What I don't understand is :

lapineige commented 1 year ago 
1. change `filesystems.php` private folder right to 0750. _Note: after the upgrade the permission were back to default, so it was 700_

2. `php8.2 config:cache`

3. `php8.2 cache:clear`

4. `chown -R :www-data public/storage/m/_v2/1/someRandomNumber/anotherRandomOne_FolderThanContainsTheNewPictures`

From 1), and the fact it fails without 4), I'm guessing that it's www-data (NGINX ?) that has to be able to read the file. And by default group permission setting for private directory is 0 (nothing) and group is pixelfed and not :www-data (while it should according to https://github.com/YunoHost-Apps/pixelfed_ynh/blob/master/scripts/install#L102).

I don't know how to fix the wrong group setting issue. While for the setting issue, I guess we could patch filesystems.php to change private folder setting from default 0700 to 0750 (@dansup, considering you made the patch, what's you're advice here ?).

lapineige commented 1 year ago

I noticed one more thing:

Both have proper rights and owners.

lapineige commented 1 year ago

We're making progress !

I implemented here the changes described above and here https://github.com/YunoHost-Apps/pixelfed_ynh/issues/211#issuecomment-1535026590

✅ On Pixelfed side, apart from the issue I mentioned right above, problem solved ! 🎉 ✅ On external clients such as PixelDroid, problem solved as reported by one user. ❌ On Mastodon side at least, problem not solved. ✅ On other software (namely GoToSocial) that generates its own preview, problem solved.

lapineige commented 1 year ago

So, to summarise what I understood of this issue:

That said, a big thank you to @neonota, we would never have found a solution without your help to diagnose the issue 😃 💚

lapineige commented 1 year ago

A couple new findings:

As you can see, it's 644 (as expected) and due owner for the picture… but not for the thumbnail ! Is the thumbnail generated by another process ? (not php ? 🤔)

Everything work except from Mastodon.

ashemsay commented 1 year ago

I'm kind of lost, here is what I have before and after posting:

-rw-r--r-- 1 pixelfed pixelfed 13529 May  5 15:31 SomePicture.png
-rw-r--r-- 1 pixelfed pixelfed 45340 May  5 15:31 SomePicture_thumb.png

Yet it works perfectly, even from mastodon (I copy/pasted the post link into my mastodon search field and can see the picture)

lapineige commented 1 year ago

That's on Yunohost right ? Did you upgrade to the lasted testing version (0.11.6~ynh2) ?

That is super strange…

ashemsay commented 1 year ago

Yes on Yunohost, and I did the upgrade.

That was on my test instance.

I compared with my prod instance which is still in 0.11.5~ynh1 and I have the same permissions and ownership on the final files, I noticed a difference on the directories though.

Here's what I have on my upgraded (0.11.6~ynh2) test instance:

namei -om /var/www/pixelfed/storage/app/public/m/_v2/randomnumbers/random-string/randomstring/SomePicture_thumb.png
f: /var/www/pixelfed/storage/app/public/m/_v2/randomnumbers/random-string/randomstring/SomePicture_thumb.png
 drwxr-xr-x root     root     /
 drwxr-xr-x root     root     var
 drwxr-xr-x root     root     www
 drwxr-x--- pixelfed www-data pixelfed
 drwxrwx--- pixelfed www-data storage
 drwxrwx--- pixelfed www-data app
 drwxrwx--- pixelfed www-data public
 drwxrwx--- pixelfed www-data m
 drwxr-x--- pixelfed www-data _v2
 drwxr-x--- pixelfed www-data randomnumbers
 drwxr-x--- pixelfed www-data random-string
 drwxr-x--- pixelfed www-data randomstring
 -rw-r--r-- pixelfed pixelfed SomePicture_thumb.png

And here's what I have on my prod one (0.11.5~ynh1):

namei -om /var/www/pixelfed/storage/app/public/m/_v2/randomnumbers/random-string/randomstring/SomePicture_thumb.png
f: /var/www/pixelfed/storage/app/public/m/_v2/randomnumbers/random-string/randomstring/SomePicture_thumb.png
 drwxr-xr-x root     root     /
 drwxr-xr-x root     root     var
 drwxr-xr-x root     root     www
 drwxr-x--- pixelfed www-data pixelfed
 drwxrwx--- pixelfed www-data storage
 drwxrwx--- pixelfed www-data app
 drwxrwx--- pixelfed www-data public
 drwxrwx--- pixelfed www-data m
 drwxrwx--- pixelfed www-data _v2
 drwxrwx--- pixelfed www-data randomnumbers
 drwx------ pixelfed pixelfed random-string
 drwx------ pixelfed pixelfed randomstring
 -rw-r--r-- pixelfed pixelfed SomePicture_thumb.png
lapineige commented 1 year ago

I compared with my prod instance which is still in 0.11.5~ynh1 and I have the same permissions and ownership on the final files, I noticed a difference on the directories though.

That would confirm that the issue is really on directory rights (that I patched) and not on file rights, even ground owner :thinking:

drwxr-x--- pixelfed www-data _v2 drwxr-x--- pixelfed www-data randomnumbers

drwxrwx--- pixelfed www-data _v2 drwxrwx--- pixelfed www-data randomnumbers

Hum wait, maybe I should have left permission for write access for group on the folder ? @dansup @neonota any idea ? This is more complex to handle than this, I need to to find a good way to test the existence of such folders (without the check, upgrade might fail, and CI tests will fail), and set proper rights… edit: I think I found a way.

dansup commented 1 year ago

@lapineige You can adjust file/directory permissions here.

I can make these env variables if that would make your life easier!

lapineige commented 1 year ago

@lapineige You can adjust file/directory permissions here.

This is patched on the fly during upgrade now. I changed it so it's fine, we have the good (group) owner.

What's missing for now is an explanation of this issue https://github.com/pixelfed/pixelfed/issues/4275#issuecomment-1536063409 (why does it fails on Mastodon / why the thumbnail as invalid owner contrary to the final file)

I can make these env variables if that would make your life easier!

Thanks for the proposal. For now on, this is configured on Yunohost side. However it's done by searching for the original line and replacing it, so it would be robust to a syntax change or if those permissions are configured elsewhere. So it's not needed, but it might be more future proof to make such env variables, especially if you plan to include these settings elsewhere.

lapineige commented 1 year ago

Do you know which file is loaded from Mastodon (and so on), the thumbnail or the final one ? And which process (php, nginx…) manage it ? This might help me to fix the rights.

lapineige commented 1 year ago

One more finding: an user discovered that the rights issue affects avatars too. So new files uploaded in public/storage/avatars are affected too. Change permissions from 700 to 750 fixes it.

I'll come back with more details about their configuration, but here is mine (which is prior to the upgrade… uh not sure for default files considering the dates):

> ls -l /var/www/pixelfed/public/storage/avatars .rw-rw---- 1.5k pixelfed www-data 3 May 8:30 default.jpg .rw-rw---- 14k pixelfed www-data 3 May 8:30 default.png

And for my avatar, in sub-folder:

.rw-r----- pixelfed www-data

So right user+group, and final file is readable by user and group (php and nginx ?).

webmink commented 1 year ago

OK, I have done all the chmod suggested and it has made previous uploads under 0.11.5 appear but new uploads and uploads under 0.11.6 still failed. I then used 755 instead of 750 for user storage and avatars and it seems to be working now for previous posts!

However, the image is still broken during posting and new uploads fail to be visible image post

I can then chmod m/_v2/* to 755 and images appear in the post OK

Any idea how to fix this so I don't have to manually chmod please? My filesystem.php is as @dansup suggests, and everything belongs to pixelfed/www-data

lapineige commented 1 year ago

OK, I have done all the chmod suggested and it has made previous uploads under 0.11.5 appear but new uploads and uploads under 0.11.6 still failed.

This is exactly the expected behaviour. Great.

I then used 755 instead of 750 for user storage and avatars and it seems to be working now for previous posts!

Didn't you say previous post where fixed by the chmod before that ? Or that you did one single chmod but with 755 instead of 750 ? If that's so, it should make no difference in the result on Pixelfed side.

I can then chmod m/_v2/* to 755 and images appear in the post OK

As expected.

Any idea how to fix this so I don't have to manually chmod please? My filesystem.php is as @dansup suggests, and everything belongs to pixelfed/www-data

This is exactly what we are trying to do. For now on, if you are using Yunohost (I guess ?), if you upgrade to this branch https://github.com/YunoHost-Apps/pixelfed_ynh/pull/215 it does the chmod for you. It doesn't fix fully new image upload (Mastodon can't see it for instance), but it changes the group owner of those file so on Pixelfed side at least it is fixed. In fact the chmod is not enough : you need to change the group owner of the file to www-data (chown :www-data -R FoldersToChange). With the testing branch, those file are now created directly with the right group owner (www-data instead of Pixelfed).

webmink commented 1 year ago

Didn't you say previous post where fixed by the chmod before that ? Or that you did one single chmod but with 755 instead of 750 ?

Sorry, I meant that I first did chmod on all the public storage to 750 and it fixed the 11.5 uploads but not the 11.6 uploads, then did chmod to 755 and it also changed the 11.6 uploads

This is exactly what we are trying to do.

Ah, I had intuited that your install was now working OK, sorry.

I think I'll stop messing at this point, this is very much "Sorcerers Apprentice" now. New uploads are still broken so I'll pause using Pixelfed until I hear good news!

lapineige commented 1 year ago

Didn't you say previous post where fixed by the chmod before that ? Or that you did one single chmod but with 755 instead of 750 ? Sorry, I meant that I first did chmod on all the public storage to 750 and it fixed the 11.5 uploads but not the 11.6 uploads, then did chmod to 755 and it also changed the 11.6 uploads

Wait… can you confirm you're using 0.11.6~ynh1 and not ~ynh2 (the testing branch) ?

For your information all you chmod operation that affected public/storage/m/_v2/ and sub-folders will be erased by an upgrade (hence harmonised with the standard configuration).

 Ah, I had intuited that your install was now working OK, sorry.

It is (except from Mastodon at least) for the testing branch, 0.11.6~ynh2.

webmink commented 1 year ago

Wait… can you confirm you're using 0.11.6~ynh1 and not ~ynh2 (the testing branch) ?

Yes, I have 0.11.6~ynh1 installed at the moment.

harmonised with the standard configuration

Good to know - I have had Yunohost apps I had to stop using (notably Hedgedoc) because they avoided manual changes during upgrades and that left the environment broken...

lapineige commented 1 year ago

Yes, I have 0.11.6~ynh1 installed at the moment.

Ok then everything is fine.

mitexleo commented 1 year ago

Could you please check which user is running Laravel Horizon?

webmink commented 1 year ago

On my system ps says it's run by "pixelfed".

However, the supervisord log includes this line:

Apr 27 23:31:10 supervisord[2701095]: 2023-04-27 23:31:10,298 CRIT Supervisor is running as root. Privileges were not dropped because no user is specified in the config file. If you intend to run as root, you can set user=root in the config file to avoid this message.

mitexleo commented 1 year ago

On my system ps says it's run by "pixelfed".

However, the supervisord log includes this line:

Apr 27 23:31:10 supervisord[2701095]: 2023-04-27 23:31:10,298 CRIT Supervisor is running as root. Privileges were not dropped because no user is specified in the config file. If you intend to run as root, you can set user=root in the config file to avoid this message.

Well ! This is the culprit. You need to run Horizon as the pixelfed user !!!!!

lapineige commented 1 year ago

Oh ! I didn't even know how this was managed, I don't even really understand what horizon is doing (the server is working even when its service is off 🤔).

Do you know which config file we are supposed to adjust ?

mitexleo commented 1 year ago

Oh ! I didn't even know how this was managed, I don't even really understand what horizon is doing (the server is working even when its service is off 🤔).

Do you know which config file we are supposed to adjust ?

Yunohost uses Supervisor. So you need to edit a Supervisor config. (I don't know where it's located, need some time)

mitexleo commented 1 year ago

Run find /etc -name 'pixelfed.conf'.

mitexleo commented 1 year ago

Run find /etc -name 'pixelfed.conf'.

It should be locatedin /etc/supervisor/conf.d/

webmink commented 1 year ago

Here's what I have in /etc/supervisor/conf.d/pixelfed-horizon.conf:

[program:pixelfed-horizon]
process_name=%(program_name)s
command=php8.2 /var/www/pixelfed/artisan horizon
autostart=true
autorestart=true
user=pixelfed
redirect_stderr=true
stdout_logfile=/var/log/pixelfed/pixelfed-horizon.log
stopwaitsecs=3600

mitexleo commented 1 year ago

Here's what I have in /etc/supervisor/conf.d/pixelfed-horizon.conf:

[program:pixelfed-horizon] process_name=%(program_name)s command=php8.2 /var/www/pixelfed/artisan horizon autostart=true autorestart=true user=pixelfed redirect_stderr=true stdout_logfile=/var/log/pixelfed/pixelfed-horizon.log stopwaitsecs=3600

Looks good 👍

lapineige commented 1 year ago

And so it still try to run as root or something like that, even with pixelfed user in the config ? :thinking:

webmink commented 1 year ago

Exactly so. Just restarted to be sure:

May 12 09:36:11 supervisord[30299]: 2023-05-12 09:36:11,472 CRIT Supervisor is running as root. Privileges were not dropped because no user is specified in the config file. If you intend to run as root, you can set user=root in the config file to avoid this message. May 12 09:36:11 supervisord[30299]: 2023-05-12 09:36:11,472 INFO Included extra file "/etc/supervisor/conf.d/pixelfed-horizon.conf" during parsing

ashemsay commented 1 year ago

I think the message not dropping privileges in the logs is due to the fact that there is indeed no user specified in the global supervisor configuration (/etc/supervisor/supervisod.conf), we can see that the pixelfed-horizon.conf file is included after that message in the logs.

When checking, I can see all my horizon processes running as pixelfed:

ps -ef | grep horizon
pixelfed 1084281 1084280  0 12:10 ?        00:00:00 php8.2 /var/www/pixelfed/artisan horizon
pixelfed 1084290 1084281  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:supervisor servicescoupoufr-XbTY:supervisor-1 redis --workers-name=default --balance=auto --max-processes=20 --min-processes=1 --nice=0 --balance-cooldown=3 --balance-max-shift=1 --parent-id=1084281 --auto-scaling-strategy=time --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=high,default,follow,shared,inbox,feed,low,story,delete,mmo --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084298 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=high --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084299 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=default --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084300 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=follow --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084301 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=shared --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084302 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=inbox --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084303 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=feed --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084304 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=low --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084305 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=story --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084306 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=delete --sleep=3 --timeout=300 --tries=3 --rest=0
pixelfed 1084307 1084290  0 12:10 ?        00:00:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=servicescoupoufr-XbTY:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=64 --queue=mmo --sleep=3 --timeout=300 --tries=3 --rest=0
root     1085028 4186105  0 12:12 pts/8    00:00:00 grep horizon
lapineige commented 1 year ago

So everything is normal ?

ashemsay commented 1 year ago

So it seems to me, at least on my instance. I am running version 0.11.6~ynh2 though, that's the version we're testing, right?

mitexleo commented 1 year ago

We're missing something! It's happening because of miscofiguration, I'm sure about it!

lapineige commented 1 year ago

Do you have an idea of the config file we should look at ?

mitexleo commented 1 year ago

What does sudo systemctl status supervisor returns ?

lapineige commented 1 year ago

Running, nothing to report, it's using php artisan horizon.

lapineige commented 1 year ago

Should I try right after uploading a picture ?

mitexleo commented 1 year ago

Should I try right after uploading a picture ?

Why not?

lapineige commented 1 year ago

Nothing to report :/