gitea
gitea copied to clipboard
go-gitea
Save files in local storage as group readable
Go creates temporary files as 600 (https://github.com/golang/go/blob/334a591a3f4d868368913328b3e81ddf5b0f46fa/src/os/tempfile.go#L44), but sometimes we want the group to be able to read them (for example, for another user to back up the storage.)
I'm not sure if this qualifies as a breaking change. In one hand, I think no one is relying on Gitea saving these files (in my case LFS files) as group unreadable, but in the other it could be a possibility.
I do not think hard-coded 0o640 is a general value for everyone.
In most cases, Gitea respects the umask, I think it should also use umask to decide the file modes here.
Okay, so should 0o777 be set instead?
IIRC Gitea should read the umask and calculate the new file mode then apply it (and it only works for POSIX, not for Windows, so the code should be splitted for different OS types).
So, although I prefer to use umask to get a general solution, it's not a blocker. If the code could use umask (I could help and approve it), or if other maintainers also agree to use 0o640 (wait for more feedbacks), either is good to me.
If I understand correctly, for the umask to be applied, 777 (or 666 as it's not executable, I guess) would need to be used. https://www.yesthatblog.com/post/0072-go-and-umask/
Yup, but that's only for os.OpenFile/os.Mkdir/os.MkdirAll, not for the Chown.
@catdevnull I made some changes for this PR, now the ApplyUmask can help to respect the user's umask setting.
LGTM, I was going to make similar changes when I had the time. However, Go (at least 7 years ago...) took a different approach by creating a temporary file with 0o666 to get the default permissions and then copying the permissions to the file..
It's still done today: https://github.com/golang/go/blob/629c0b3a6e3e2ff11c9c989a8dcd1a538cdd76f4/src/cmd/go/internal/work/exec.go#L1759