dw-0 / kiauh

Klipper Installation And Update Helper
GNU General Public License v3.0
3.3k stars 474 forks source link

mainsail file permissions 403 errors -- set umask to 022 #458

Closed fff7d1bc closed 1 week ago

fff7d1bc commented 5 months ago

Linux Distribution

Raspbian

What happened

I've run update from shell that had umask set to 077 which made ~/mainsail with permissions of 700 that then prevents it from working as the nginx cannot enter the directory if it lacks +x permissions.

What did you expect to happen

I would expect the permissions to be enforced. Best would be to just add umask 022 at the very top of kiauh.sh and it will have no effect on most default configurations but will fix any potential permissions issues for those that have umask of different value. I copied my .zshrc and this is how umask got changed.

How to reproduce

Additional information

-

fff7d1bc commented 1 week ago

@dw-0 you set it as not reproducible and not planned afterward. Why is that? You can just run umask 077 in active shell, then in that very shell use kiauh to update klipper and mainsail will return 403 for you.

dw-0 commented 1 week ago

use kiauh to update klipper and mainsail will return 403 for you

Im sorry but why would a klipper update affect mainsail?

fff7d1bc commented 1 week ago

Sorry I made a mistake in my comment. using the feature to update everything, which includes mainsail, will make the mainsail directory with wrong permissions, as those are not controlled within the script and the script expect to work with umask 022 as that would result in 755 permissions in directory, with umask 077 those are 700 and this makes nginx being unable to access the content of this.

This is because the function download_mainsail removes the directory and creates new one without any mention of what permissions to set so it picks up those from umask.

dw-0 commented 1 week ago

Okay now i think i got what you mean. Im not aware that the permissions of the mainsail directory actually matter, at least in regards to execute permissions. I know that for some time execute permissions are required on the users home directory so that nginx can access mainsail for example.

Okay but i think it shouldn't be a problem to simply explicity set the permissions after each update/download. That way, the issue should be fixed and i see no issue in doing so.

Sorry for the misunderstanding.

fff7d1bc commented 1 week ago

No no, I could make better job at explaining the issue. The problem originates from the fact that nginx runs as unprivileged user and it is then unable to serve static content out of that directory owned by for example pi.

What I would recommend is to just add line with umask 022 at the top of script as this is what you expect to have anyway, this way you do not have to worry about permissions anywhere later, this is just a explicit declaration of permissions you expect as opposite to expect it to come from environment, this will affect only kiauh and whatever it runs but does not affect anything else.

dw-0 commented 1 week ago

Okay i see, thanks for the input. I think now i fully understand the issue. I think umask 022 is usually the default, right? So setting this like you recommended at the very top of kiauh.sh should solve it. So it should even fix the issue for both KAIUH v5 and v6.

fff7d1bc commented 1 week ago

Yes, exactly 022 is the default of Linux kernel, some distributions change it, most notable is Ubuntu 22.04 LTS that uses 027 but then Ubuntu 24.04 LTS again uses 022. Thanks for taking a look into it!

dw-0 commented 1 week ago

Okay thanks! I provided a fix for this issue. Will be merged into master soon.

fff7d1bc commented 1 week ago

Fantastic, thank you very much!