fix: group-readable mirror output, and atomic copies #4
@@ -63,9 +63,14 @@ Everything written into the mirror is made group-readable, and its directories
|
||||
group-traversable, so the mirror can be read back by whatever serves it. Neither
|
||||
writer does that unaided: the temporary file an encode renames into place is
|
||||
created `0600` regardless of the umask, and a straight copy of an existing MP3
|
||||
inherits the mode of a source file in a library this tool does not own. Only the
|
||||
group bits are touched; whether the mirror is world-readable stays with the
|
||||
umask, as does the ownership.
|
||||
inherits the mode of a source file in a library this tool does not own.
|
||||
|
||||
Directories are handled by clearing the owner and group read/execute bits from
|
||||
the process umask, once, at startup. Owner as well as group, because a umask
|
||||
carrying `0400` produces directories of mode `0300` — writable and enterable,
|
||||
unreadable to the very run that created them. The `other` bits are left where
|
||||
the umask puts them: whether the mirror is world-readable is a genuine policy
|
||||
question, and so is its ownership.
|
||||
|
||||
Mirror files written before this existed are topped up on the next pass. Their
|
||||
mtimes are correct, so nothing else would revisit them — and they are not
|
||||
|
||||
+13
-6
@@ -84,7 +84,12 @@ MTIME_TOLERANCE_SECONDS = 2
|
||||
# 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
|
||||
|
||||
# Cleared from the umask so directories this run creates can be listed and
|
||||
# entered. Owner as well as group: a umask carrying 0400 -- which is unusual but
|
||||
# not ours to assume away -- otherwise produces a mirror tree that not even the
|
||||
# process that built it can read back.
|
||||
DIRECTORY_ACCESS = 0o550
|
||||
|
||||
|
||||
@dataclass
|
||||
@@ -505,12 +510,14 @@ 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.
|
||||
# Directories are created with 0o777 masked by the umask, so clear the bits
|
||||
# that matter from it once here rather than chmod'ing every directory the
|
||||
# walk creates. The `other` bits are left alone, since whether the mirror is
|
||||
# world-readable is a real policy question; owner and group access is not.
|
||||
# Files cannot be handled this way -- mkstemp and copy2 both set a mode
|
||||
# outright, ignoring the umask -- so they get an explicit chmod instead.
|
||||
inherited = os.umask(0o077)
|
||||
os.umask(inherited & ~GROUP_ENTER)
|
||||
os.umask(inherited & ~DIRECTORY_ACCESS)
|
||||
|
||||
if not args.source or not args.mirror:
|
||||
logger.error("both --source and --mirror are required")
|
||||
|
||||
@@ -27,6 +27,21 @@ def tight_umask():
|
||||
os.umask(previous)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def owner_hostile_umask():
|
||||
"""Return a callable applying a umask that masks off the owner's read bit.
|
||||
|
||||
Unusual, but it is what produces a mirror tree of mode 0300 -- writable and
|
||||
enterable, unreadable to the very process that built it. Applied on demand
|
||||
rather than for the whole test, because the source library is built by
|
||||
something else entirely and the same umask would make the test's own
|
||||
fixtures unreadable before the run under test even started.
|
||||
"""
|
||||
previous = os.umask(0o022)
|
||||
yield lambda: os.umask(0o477)
|
||||
os.umask(previous)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def make_flac():
|
||||
"""Return a factory writing a short tagged FLAC file."""
|
||||
|
||||
@@ -222,6 +222,27 @@ def test_mirror_directories_are_group_traversable(tmp_path, make_flac, tight_uma
|
||||
assert mode & stat.S_IXGRP, directory
|
||||
|
||||
|
||||
def test_mirror_directories_survive_an_owner_hostile_umask(
|
||||
tmp_path, make_flac, owner_hostile_umask
|
||||
):
|
||||
"""A umask carrying 0400 otherwise builds a tree the run cannot read back."""
|
||||
source = tmp_path / "src"
|
||||
mirror = tmp_path / "dst"
|
||||
make_flac(source / "Artist" / "Album" / "a.flac")
|
||||
|
||||
# Applied only now: the library already exists, and the umask under test is
|
||||
# the one the container starts this run with.
|
||||
owner_hostile_umask()
|
||||
run(source, mirror)
|
||||
|
||||
for directory in (mirror, mirror / "Artist", mirror / "Artist" / "Album"):
|
||||
mode = directory.stat().st_mode
|
||||
assert mode & stat.S_IRUSR, directory
|
||||
assert mode & stat.S_IXUSR, directory
|
||||
assert mode & stat.S_IRGRP, directory
|
||||
assert mode & stat.S_IXGRP, directory
|
||||
|
||||
|
||||
def test_private_mirror_file_is_repaired_without_re_encoding(tmp_path, make_flac):
|
||||
"""A mirror written by an older version has a correct mtime, so nothing
|
||||
else in the pass would revisit it."""
|
||||
|
||||
Reference in New Issue
Block a user