Skip to content

Extracted file permissions don't respect UMask when completion follows a failed DirectUnpack + par2 repair #884

Description

@xbmc4lyfe

Platform: Linux/Docker

Environment:

Device: x86_64 server, systemd service
OS version: Ubuntu 24.04, 64bit
CPU architecture: 64bit
Includes libs or tool that apply: unrar 7.00, 7-Zip, par2-turbo
Running in Docker: No
Relevant config: UMask=0002, DirectUnpack=yes, ParCheck=auto, ParRepair=yes

Current Behavior:

When DirectUnpack fully extracts an archive on its own — the normal, successful case, no failed articles or repair involved — the resulting media file can be left with restrictive Unix permissions (owner+group only) instead of the permissions every other completed item gets from the configured UMask.

This is silent in the log: the item reports SUCCESS at full health, the file is present at the correct size, so nothing indicates a problem. It only surfaces when something else reads the file — an NFS/SMB client mounting the completed-downloads share under a different UID, a media server or add-on running as a different user, etc. — which gets Permission denied on open, despite stat() on the same file succeeding (the containing directory is still listable). Confirmed independently by a second consumer of a completed file, accessed over two different network protocols, both reporting the identical "listable but not readable" signature.

Permissions on affected items vs. other recently-completed items in the same DestDir (all owned by the same daemon user:group):

640  <affected item 1>/<file>.mkv    <- broken
640  <affected item 2>/<file>.mkv    <- broken
664  <other completed item 1>/<file>.mkv
664  <other completed item 2>/<file>.mkv
664  <other completed item 3>/<file>.mkv

664 is exactly 0666 & ~0002 — the configured UMask applied correctly, matching everything else in the library. 640 doesn't correspond to that UMask value at all. Manually chmod 664-ing an affected file to match its siblings fixes the read immediately; no other change is needed.

Expected Behavior:

Every file the post-processing pipeline finalizes should end up with the same permissions the configured UMask produces, regardless of which internal path finished extracting it.

Steps To Reproduce:

  1. DirectUnpack=yes.
  2. Download an archived release where DirectUnpack extracts successfully on its own (log shows Direct unpack for <item> successful).
  3. Post-processing logs Using directly unpacked files (it reuses DirectUnpack's output rather than re-extracting).
  4. The item completes as SUCCESS, but the extracted file's permissions don't match the configured UMask.

No failed articles or par2 repair are required — this reproduces on a plain, fully successful download.

Logs:

Log excerpt (collection/file names replaced with placeholders; everything else verbatim):

INFO  Unrar: All OK
INFO  Direct unpack for <collection> successful
INFO  Checking renamed files for <collection>
INFO  No renamed files found for <collection>
INFO  Checking renamed archive files for <collection>
INFO  No renamed archive files found for <collection>
INFO  Unpacking <collection>
INFO  Using directly unpacked files
INFO  Deleting archive files
INFO  Unpack for <collection> successful
...
INFO  Cleaning up <collection>
INFO  Cleanup for <collection> successful
INFO  Moving completed files for <collection>
INFO  Move for <collection> successful

Nothing in the log flags the permission problem.

Extra information:

Root cause, traced in daemon/postprocess/UnpackController.cpp's Run():

if (m_postInfo->GetNzbInfo()->GetDirectUnpackStatus() == NzbInfo::nsSuccess)
{
    ...
}
else
{
    PrintMessage(Message::mkInfo, "Using directly unpacked files");
}

When DirectUnpack already extracted everything successfully, UnpackController takes this shortcut and never calls UnpackArchives()/ExecuteUnrar() again. FileSystem::RestoreFileOrDirPermissions() — the codebase's only fix-up for permissions unrar/7z restore from an archive's stored Unix-permission metadata (comment: "Fixing file or directory permissions overridden by the unpacker") — is only reached from inside that unpack-and-move sequence, so this shortcut skips it entirely.

The later, separate final move — daemon/postprocess/Cleanup.cpp, MoveController::MoveFiles(), which is what actually lands the file in DestDir and runs for every completed item regardless of which unpack path was taken — calls plain FileSystem::MoveFile() with no permission handling at all. So nothing downstream of the DirectUnpack-success shortcut ever normalizes the permissions the unpacker itself set from the archive.

Suggested fix: since MoveController::MoveFiles() is the one place every completed item's files pass through on their way to DestDir, that looks like the right place to centralize the permission normalization, rather than relying on it being called only from inside UnpackController's own move step (which, as shown here, is easy to bypass via the direct-unpack shortcut).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions