Fix ignored umask env var by removing overwrite

[JorisMolnar]

---

We welcome all PR’s though this doesn’t guarantee it will be accepted.

Description:

I bumped into an issue that the new UMASK environment variable did not seem to work for the NZBGet service. The UMask was always 022.

4 years ago (so long before the UMASK env var) commit 64dc5f4cf9edb516dd3fb95b6f143c338291e9d6 added a line in the nzbget service run file to set the UMask to 022. I could not find a reason for this change. #26 doesn't give more information.

This static UMask got applied after the env var UMask got applied.

This PR removes the static UMask. This results in:

• UMASK env var is filled: The configured UMask is applied.
• UMASK env var is empty: The default UMask of the default abc user in the container is used. This is 022.

Benefits of this PR and context:

This PR fixes a bug for users that have configured a UMask using the environment variable. Users that have not configured a special UMask get the default 022, just like before. That should make the change safe for everyone.

How Has This Been Tested?

I started a container using the following docker-compose.yml service snippet:

  nzbget:
    container_name: nzbget
    build: ./nzbget-code/docker-nzbget
    restart: unless-stopped
    networks:
      - frontend
    volumes:
      - /mnt/Data1/media/NZBGet:/downloads
      - ./nzbget/config:/config
      - ./shared:/shared
    environment:
      - UMASK=002
      - PUID=${PUID_NZBGET}
      - PGID
      - TZ

Next I started a download and enter a shell in the container. There I ran the commands described in the Discord conversation below to check the UMask of the currently running nzbget and unrar processes.

root@f19036f8a0a2:/# ps aux
USER         PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root           1  0.0  0.0    196     4 ?        Ss   23:05   0:00 s6-svscan -t0 /var/run/s6/services
root          35  0.0  0.0    196     4 ?        S    23:05   0:00 s6-supervise s6-fdholderd
root         254  0.0  0.0    196     4 ?        S    23:05   0:00 s6-supervise nzbget
abc          258 39.9  1.0  49152 41716 ?        Ssl  23:05   3:58 /app/nzbget/nzbget -s -c /config/nzbget.conf -o OutputMode=log
root         282  0.0  0.0   2424  1148 pts/0    Ss   23:05   0:00 /bin/bash
abc        16577 23.2  0.0   4396  2100 ?        Ds   23:11   0:57 /app/nzbget/unrar x -y -p- -o+ *.rar /downloads/Downloads/Some.Random.Name/_unpack/
root       17132  0.0  0.0   1616   524 pts/0    R+   23:15   0:00 ps aux
root@f19036f8a0a2:/# grep '^Umask:' "/proc/16577/status"
Umask:  0002
root@f19036f8a0a2:/# grep '^Umask:' "/proc/258/status"
Umask:  0002

Source / References:

The following is the short Discord discussion I had about this subject.

JMolnar Hi, I'm trying to configure the umask for my nzbget container as described here: https://github.com/linuxserver/docker-nzbget#umask-for-running-applications But it seems like this setting doesn't work.

I test this by connecting to the container using sudo docker exec -it nzbget /bin/bash and then running grep '^Umask:' "/proc/12345/status" where 12345 is the PID of the /app/nzbget/nzbget process. (I get the PID by running ps aux) The response I get in the nzbget container is Umask: 0022. I also tried running it in the rutorrent container, where I get Umask: 0002 as expected.

My docker-compose service snippet is:

  nzbget:
    container_name: nzbget
    image: linuxserver/nzbget
    restart: unless-stopped
    networks:
      - frontend
    volumes:
      - /mnt/Data1/media/NZBGet:/downloads
      - ./nzbget/config:/config
      - ./shared:/shared
    environment:
      - UMASK=002
      - PUID=${PUID_NZBGET}
      - PGID
      - TZ

j0nnymoe @JorisMolnar you can set the UMASK via the nzbget settings I believe aswell

JMolnar I noticed the following file always sets the umask to 022, but I don't know if there is some other system that should change it again: https://github.com/linuxserver/docker-nzbget/blob/master/root/etc/services.d/nzbget/run Thanks @j0nnymoe, I'll take a look at that. I found the setting in nzbget and can confirm that works :smile: Thanks The original setting was 1000, which nzbget says disables changing the umask, so it seems there still is an issue with the nzbget image. But at least there is a workaround :slight_smile:

JMolnar Regarding that umask issue I mentioned 2 hours ago, it seems like we aren't completely there yet. NZBGet just extracted something and it had the wrong permissions. I'd have to test further, but it's probably because the unrar process doesn't inherit the umask configured in nzbget settings. So unrar got 022 again :sweat_smile: I'll take a look if I can spot the difference between the containers where the umask env variable works and doesn't work.

JMolnar I think I fixed the issue. If my test succeeds I'll submit a pull request.

[aptalca]

This should probably be changed to

UMASK=${UMASK:-022}
umask ${UMASK}

so it defaults to 022 if unset (probably was the initial intention)

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/nzbget/v21.0-pkg-4d7b0218-pr-111/index.html https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/nzbget/v21.0-pkg-4d7b0218-pr-111/shellcheck-result.xml

[JorisMolnar]

so it defaults to 022 if unset (probably was the initial intention)

As far as I could see in my testing, the default (if you don't call umask) is already 022 implicitly. But if you prefer I can obviously make it explicit.

In that case, I am wondering if this change with a default umask doesn't belong in the base images, but that's probably too big a risk for breaking changes 😅

[aptalca]

No, the default behavior in the baseimage would be not setting umask at all.

But since it was set to 022 specifically here, I assumed the default would be something other, and that nzbget should use 022. In other words, I took it at face value that it would be needed but it may not be the case.

Looking at the history, I see that the umask was incorrectly set to 000 at initial release 4 years ago, and was changed to 022 shortly after. So it's likely not needed at all.

[aptalca]

@JorisMolnar thanks for the PR. Can you please submit the same to the testing branch as well?

Thanks

[JorisMolnar]

Can you please submit the same to the testing branch as well?

I'll do that. Sorry I didn't know.

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions[bot]]

This pull request is locked due to inactivity