transmission / transmission

Official Transmission BitTorrent client repository
https://transmissionbt.com
Other
12.31k stars 1.22k forks source link

make files in .config/transmission-daemon/ group readable #7006

Open ilf opened 4 months ago

ilf commented 4 months ago

What is the issue?

In /var/lib/transmission-daemon/.config/transmission-daemon/, all files (dht.dat, stats.json, resume/*.resume and torrents/*.torrent) are created -rw------- (umask 077).

This is "just an unintended side effect", because they are created with mkstemp() without adjusting their permission.

I would like to change the permission to at least -rw-r----- (umask 027), so I can back them up as a different user in the transmission group. (Or even better: -rw-r--r-- (umask 022), like everything else.)

settings.json has a setting umask, but apparently that's only for files in download-dir.

Which application of Transmission?

transmission-daemon

Which version of Transmission?

3.00

killemov commented 4 months ago

Correct, this is not possible. But you can install a (*)cron job to change the flags or user:group every couple of minutes.

ilf commented 4 months ago

@killemov: Thanks for confirming. I believe these facts (it's currently not possible, it can be implemented, people want this) fill the definition of a feature request.

Thank you for the suggested workaround. But that's rather hacky and might cause unintented side-effects.

killemov commented 4 months ago

But that's rather hacky and might cause unintented side-effects.

Not really, I can't even fathom the amount of services around the world and beyond that depend on cron. Also, if you don't remove rights you should be okay.

ilf commented 4 months ago

Can anyone explain to me why transmission-daemon makes config-files unreadable for its own group? Is there a good reason? Or could we change the default value from group unreadable to group readable?

killemov commented 4 months ago

unreadable for its own group

This is actually a very astute observation. Why indeed?

tearfur commented 3 months ago

Can anyone explain to me why transmission-daemon makes config-files unreadable for its own group? Is there a good reason?

After reading the code, my guess is that it's just an unintended side effect.

When saving each of these files, Transmission first uses mkstemp() to save the contents to a temp file, then after the temp file is written successfully, the temp file is moved to the final destination. mkstemp() is hard coded to create the temp file with permission 0600.

The reason for first saving the contents to a temp file instead of creating the file directly is explained in the source code in this function:

https://github.com/transmission/transmission/blob/3e9f5f614a875aae987235d3836d0ca555a80d80/libtransmission/utils.cc#L179

ilf commented 3 months ago

@tearfur: Great find, thanks!

If it is indeed "just an unintended side effect", can we adjust it and fix the permissions?

There's already a post-save step to "move it from '.tmp' to the correct filename":

https://github.com/transmission/transmission/blob/3e9f5f614a875aae987235d3836d0ca555a80d80/libtransmission/utils.cc#L210

ilf commented 1 month ago

I just discovered this has been open for three months.

The description provided by @tearfur sounds like this would be relatively easy to fix, they also marked it as "good first issue".

Unfortunately, I do not know C++. Is anyone up for this?

I would very much appreciate a patch :) Thanks, all!

jggimi commented 1 month ago

The patch is relatively small, and it was easy once I figured out I should have been using std::string_view, new with C++17.

Tested on OpenBSD, working fine with new .json files.