fix: group-readable mirror output, and atomic copies #4

2026-08-24T11:34:45+01:00

lyrathorpe commented

2026-08-24 11:34:45 +01:00

Problem

Two ways the mirror ends up wrong, both invisible until someone tries to read
or play a track.

Nothing written into the mirror is group-readable, so the account that
serves it cannot read it. Encoded tracks go through tempfile.mkstemp, which
creates 0600 whatever the umask, and os.replace carries that mode into the
mirror unchanged. Copied MP3s go through shutil.copy2, which reproduces the
mode of a source file in a library this tool does not own.

Copies are not atomic. Encodes are written to a temporary file and renamed
into place; copies were written straight to their destination. A copy cut short
by a full disk, a killed container or an I/O error leaves a truncated MP3 in
the mirror -- and copy2 brings the source's mtime across with the bytes, so
staleness detection reads that fragment as up to date and never replaces it.

Change

The group-read bit is added to the temporary file before the rename, so a
mirror file is never visible without it, and to a copy before it lands.
Directories are covered by clearing the group bits from the process umask
once at startup rather than chmod'ing each one -- a file the group cannot
reach is no better than one it cannot read. Files cannot be done that way
because mkstemp and copy2 both set a mode outright.
Only the group bits are touched. World bits and ownership stay with the
umask, as before.
An existing mirror is repaired on the next pass: those files have correct
mtimes, so nothing else would revisit them, and the fix is a stat and a
chmod, not a re-encode.
copy() now writes to a temporary file in the destination directory and
renames it into place, so the destination has either the whole file or
nothing.

Testing

Five new tests, each failing on main and passing here: encoded file, copied
file from a 0600 source, directory traversability under a 0077 umask,
repair-without-re-encode, and an interrupted copy (a patched copy2 that
writes 64 bytes and raises) leaving neither a mirror file nor a .part
straggler. A sixth asserts --dry-run still changes nothing. 27 passed
locally and in docker build --target test, which is what CI runs.

## Problem Two ways the mirror ends up wrong, both invisible until someone tries to read or play a track. **Nothing written into the mirror is group-readable**, so the account that serves it cannot read it. Encoded tracks go through `tempfile.mkstemp`, which creates `0600` whatever the umask, and `os.replace` carries that mode into the mirror unchanged. Copied MP3s go through `shutil.copy2`, which reproduces the mode of a source file in a library this tool does not own. **Copies are not atomic.** Encodes are written to a temporary file and renamed into place; copies were written straight to their destination. A copy cut short by a full disk, a killed container or an I/O error leaves a truncated MP3 in the mirror -- and `copy2` brings the source's mtime across with the bytes, so staleness detection reads that fragment as up to date and never replaces it. ## Change - The group-read bit is added to the temporary file *before* the rename, so a mirror file is never visible without it, and to a copy before it lands. - Directories are covered by clearing the group bits from the process umask once at startup rather than chmod'ing each one -- a file the group cannot reach is no better than one it cannot read. Files cannot be done that way because `mkstemp` and `copy2` both set a mode outright. - Only the group bits are touched. World bits and ownership stay with the umask, as before. - An existing mirror is repaired on the next pass: those files have correct mtimes, so nothing else would revisit them, and the fix is a `stat` and a `chmod`, not a re-encode. - `copy()` now writes to a temporary file in the destination directory and renames it into place, so the destination has either the whole file or nothing. ## Testing Five new tests, each failing on `main` and passing here: encoded file, copied file from a `0600` source, directory traversability under a `0077` umask, repair-without-re-encode, and an interrupted copy (a patched `copy2` that writes 64 bytes and raises) leaving neither a mirror file nor a `.part` straggler. A sixth asserts `--dry-run` still changes nothing. 27 passed locally and in `docker build --target test`, which is what CI runs.

lyrathorpe added 3 commits 2026-08-24 11:36:51 +01:00

fix: make everything written into the mirror group-readable 100671da99

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.

docs: describe how the mirror handles permissions 6e48d94b32

Explain why the group bits are set explicitly rather than left to the umask,
what is deliberately not touched, and that an existing mirror is repaired in
place rather than re-encoded.

fix: copy through a temporary file so a cut-short copy is not kept

Build and publish container / build (pull_request) Successful in 6m57s

Details

a1382185a7

Copies of already-MP3 sources were written straight to their destination while
encodes went via a temporary file and a rename. A copy interrupted by a full
disk, a killed container or an I/O error therefore left a truncated MP3 in the
mirror -- and because shutil.copy2 reproduces the source's mtime along with its
bytes, staleness detection would read that fragment as up to date and never
replace it. The damage is silent and permanent until someone plays the track.

Give copy the same temporary-file-and-rename path encode already uses, so the
destination either has the whole file or has nothing.

lyrathorpe force-pushed fix/group-readable-output from 67f99e6531 to a1382185a7

2026-08-24 11:36:51 +01:00

Compare

lyrathorpe added 1 commit 2026-08-24 13:21:32 +01:00

fix: keep the mirror readable under a umask that masks the owner's read bit

Build and publish container / build (pull_request) Successful in 15m16s

Details

e9852e6c86

The umask handling added for group access cleared only the group bits and left
owner and other to the environment. A container whose umask carries 0400 then
produces mirror directories of mode 0300: writable and enterable, unreadable to
the very run that created them, and unreadable to anything serving the share.

Clear the owner read and execute bits from the umask as well. The `other` bits
stay where the environment puts them, because whether the mirror is
world-readable is a real policy question; being able to read a directory the
process itself just created is not.

Files were never exposed to this: mkstemp sets 0600 outright and copy2 takes
the source file's mode, both ignoring the umask.

lyrathorpe merged commit 0cda3fc6ea into main

2026-08-24 13:23:36 +01:00

lyrathorpe deleted branch fix/group-readable-output

2026-08-24 13:23:38 +01:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lyrathorpe/music-mirror#4