Skip to content

Conversation

@Nerahikada
Copy link
Contributor

@Nerahikada Nerahikada commented Mar 31, 2025

Description

This PR will fix permission issues (#2535, #3281, #3399) by making permissions as lax as possible when handling files and directories.
However, I know nothing about the possible (and dangerous) side effects of this change.

Further comments

Tested on Ubuntu 24.04.2 LTS and Go 1.23.0.
file0, dir0 is the file created before the change, file1, dir1 is the file created by the command line, and file2, dir2 is the file created by the modified filebrowser.

ubuntu@8d98ab8f6f40:~/filebrowser/working$ uname -a
Linux 8d98ab8f6f40 6.11.0-21-generic #21-Ubuntu SMP PREEMPT_DYNAMIC Wed Feb 19 16:50:40 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
ubuntu@8d98ab8f6f40:~/filebrowser/working$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=24.04
DISTRIB_CODENAME=noble
DISTRIB_DESCRIPTION="Ubuntu 24.04.2 LTS"
ubuntu@8d98ab8f6f40:~/filebrowser/working$ go version
go version go1.23.0 linux/amd64
ubuntu@8d98ab8f6f40:~/filebrowser/working$ ls -lha
total 64K
drwxrwxr-x 5 ubuntu ubuntu 4.0K Mar 31 19:29 .
drwxrwxr-x 1 ubuntu ubuntu 4.0K Mar 31 19:43 ..
drwxr-xr-x 2 ubuntu ubuntu 4.0K Mar 31 19:25 dir0
drwxrwxr-x 2 ubuntu ubuntu 4.0K Mar 31 19:27 dir1
drwxrwxr-x 2 ubuntu ubuntu 4.0K Mar 31 19:28 dir2
-rw-r--r-- 1 ubuntu ubuntu    0 Mar 31 19:25 file0
-rw-rw-r-- 1 ubuntu ubuntu    0 Mar 31 19:27 file1
-rw-rw-r-- 1 ubuntu ubuntu    0 Mar 31 19:29 file2
-rw------- 1 ubuntu ubuntu  64K Mar 31 19:28 filebrowser.db

@Nerahikada Nerahikada requested a review from o1egl as a code owner March 31, 2025 10:49
@Nerahikada
Copy link
Contributor Author

Why do people mention me without creating a PR with only two lines of changes?

@domme1908
Copy link

@o1egl could this be merged?

@str0ngb4d
Copy link

I could use this fix as well. Without it, it's difficult to use filebrowser to share files across OS users.

@o1egl
Copy link
Member

o1egl commented Jun 5, 2025

Elevating permissions might be dangerous in some installations. I'd make it configurable, keeping current permissions as defaults

@hacdias
Copy link
Member

hacdias commented Jun 25, 2025

This feature - it is not a fix, since it is working as intended - will not be published in the current form.

Just like @o1egl mentioned, if you want this, it needs to be optional. Please add a flag to change the default file and directory permissions instead.

@hacdias hacdias marked this pull request as draft June 25, 2025 15:55
@Nerahikada
Copy link
Contributor Author

I'll close this because the maintainer is back

@Nerahikada Nerahikada closed this Jun 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants