fix: make everything written into the mirror group-readable

The mirror is written by one account and read by another -- an SMB share, or
whatever else serves it -- but nothing here produced a group-readable file.
Encodes go through `tempfile.mkstemp`, which creates 0600 regardless of the
umask and keeps that mode through the rename into place, so every encoded
track landed unreadable. Copies of existing MP3s inherit the mode of a source
file in a library this tool does not own, which may be no better.

Add the group-read bit explicitly: to the temporary file before it is renamed,
so a mirror file is never visible without it, and to a copy once it has landed.
Directories are handled by clearing the group bits from the process umask
rather than chmod'ing each one, since a file the group cannot reach is no more
useful than one it cannot read. Only the group bits are touched; the world bits
and ownership stay with the umask as before.

Mirror files written before this are repaired on the next pass. Their mtimes
are correct, so no other part of the pass would revisit them, and topping up
the mode costs a stat rather than a re-encode.
This commit is contained in:
Emma Thorpe
2026-08-24 11:36:08 +01:00
parent 9984bedd02
commit 100671da99
3 changed files with 121 additions and 0 deletions
+37
View File
@@ -77,6 +77,15 @@ MIRROR_SUFFIX = ".mp3"
# Filesystems disagree about mtime precision; SMB in particular rounds.
MTIME_TOLERANCE_SECONDS = 2
# The mirror exists to be read back by something else -- an SMB share, another
# account on the box -- so everything written into it has to be group-readable.
# Neither writer manages that unaided: tempfile.mkstemp forces 0600 whatever the
# umask, and shutil.copy2 carries the source file's mode across from a library
# that may be tighter still. Directories need the execute bit too, or the group
# cannot enter them to reach the readable files inside.
GROUP_READ = 0o040
GROUP_ENTER = 0o050
@dataclass
class Result:
@@ -124,6 +133,13 @@ def is_current(source, mirror):
return abs(source.stat().st_mtime - mirror.stat().st_mtime) <= MTIME_TOLERANCE_SECONDS
def make_group_readable(path):
"""Add the group-read bit to a mirror file, leaving the rest of the mode alone."""
mode = path.stat().st_mode
if not mode & GROUP_READ:
path.chmod(mode | GROUP_READ)
@functools.lru_cache(maxsize=4096)
def find_cover(directory):
"""Return an external cover image for a directory, if one is present.
@@ -234,6 +250,9 @@ def encode(source, mirror, quality_args, dry_run):
lines = completed.stderr.strip().splitlines()
return Result("failed", source, lines[-1] if lines else "ffmpeg failed")
os.utime(temporary, (stat.st_atime, stat.st_mtime))
# Before the rename, so the file is never visible in the mirror without
# the bit.
make_group_readable(temporary)
os.replace(temporary, mirror)
except Exception as error: # noqa: BLE001 - reported per file, run continues
return Result("failed", source, str(error))
@@ -253,6 +272,9 @@ def copy(source, mirror, dry_run):
mirror.parent.mkdir(parents=True, exist_ok=True)
try:
shutil.copy2(source, mirror)
# copy2 brings the source's mode with it, and the source library is not
# ours to have permissions opinions about.
make_group_readable(mirror)
except OSError as error:
return Result("failed", source, str(error))
@@ -263,6 +285,14 @@ def copy(source, mirror, dry_run):
def process(source, mirror, quality_args, dry_run):
"""Bring one source file's mirror entry up to date."""
if is_current(source, mirror):
# A mirror written before this bit was set has a correct mtime, so
# nothing else in the pass would ever revisit it. Top it up here
# instead: one stat per file, and no chmod at all once it is right.
if not dry_run:
try:
make_group_readable(mirror)
except OSError as error:
return Result("failed", mirror, str(error))
return Result("skipped", mirror)
if source.suffix.lower() in COPY_EXTENSIONS:
return copy(source, mirror, dry_run)
@@ -463,6 +493,13 @@ def main(argv=None):
logging.basicConfig(format="%(asctime)s %(levelname)s %(message)s", level=logging.INFO)
args = build_parser().parse_args(argv)
# Directories are created with 0o777 masked by the umask, so clear the group
# bits from it once here rather than chmod'ing every directory the walk
# creates. Files cannot be handled this way -- mkstemp and copy2 both set a
# mode outright -- so they get an explicit chmod instead.
inherited = os.umask(0o077)
os.umask(inherited & ~GROUP_ENTER)
if not args.source or not args.mirror:
logger.error("both --source and --mirror are required")
return 2