fix: give atomic writes the umask mode

mkstemp creates its scratch 0600 and os.replace kept that mode, so
database.json, the manifests, the packs, the recipe snapshots and the
large-file cache were readable by their owner only. set_plain_mode
gives the scratch 0o666 less the umask, as open() would, before each
rename. A test walks every function that renames a mkstemp file.

scripts/artifacts.py

@@ -50,6 +50,25 @@ def write_if_changed(
     return True
 
 
+def plain_file_mode() -> int:
+    """The mode open() gives a new file: 0o666 less the process umask."""
+    # Reading the umask sets it, so it is put back at once.
+    umask = os.umask(0)
+    os.umask(umask)
+    return 0o666 & ~umask
+
+
+def set_plain_mode(path: str | os.PathLike) -> None:
+    """Give a scratch file the mode a plain open() would have.
+
+    mkstemp creates its file 0600 so no other user reads it while it is
+    written. Renamed into place, that mode stayed: database.json, the
+    manifests, the packs and the recipe snapshots came out unreadable to
+    every other account on the machine and to a web server serving them.
+    """
+    os.chmod(path, plain_file_mode())
+
+
 def write_text_atomic(path: str, content: str) -> None:
     """Write a whole file or nothing.
 
@@ -65,6 +84,7 @@ def write_text_atomic(path: str, content: str) -> None:
     try:
         with os.fdopen(handle, "w", encoding="utf-8") as f:
             f.write(content)
+        set_plain_mode(scratch)
         os.replace(scratch, path)
     except BaseException:
         with contextlib.suppress(OSError):
@@ -81,6 +101,7 @@ def write_bytes_atomic(path: str, content: bytes) -> None:
     try:
         with os.fdopen(handle, "wb") as f:
             f.write(content)
+        set_plain_mode(scratch)
         os.replace(scratch, path)
     except BaseException:
         with contextlib.suppress(OSError):

scripts/common.py

@@ -2805,6 +2805,8 @@ from artifacts import (  # noqa: E402,F401
     write_if_changed,
     write_text_atomic,
     copy_file_atomic,
+    plain_file_mode,
+    set_plain_mode,
     ArtifactLockBusy,
     artifact_lock,
     hold_artifact_lock,

scripts/generate_db.py

@@ -27,6 +27,7 @@ from common import (
     list_registered_platforms,
     load_provenance_snapshots,
     require_yaml,
+    set_plain_mode,
     source_yaml_paths,
     variant_base_name,
     write_if_changed,
@@ -309,6 +310,7 @@ def _restore(cached: str, path: str) -> None:
     os.close(fd)
     try:
         shutil.copy2(cached, scratch)
+        set_plain_mode(scratch)
         os.replace(scratch, path)
     finally:
         if os.path.exists(scratch):

scripts/generate_pack.py

@@ -14,7 +14,7 @@ from __future__ import annotations
 
 from collections.abc import Callable, Iterator
 
-from artifacts import _build_timestamp
+from artifacts import _build_timestamp, set_plain_mode
 import argparse
 import contextlib
 import functools
@@ -221,6 +221,7 @@ def _writing_pack(zip_path: str) -> Iterator[zipfile.ZipFile]:
     try:
         with zipfile.ZipFile(scratch, "w", zipfile.ZIP_DEFLATED) as zf:
             yield zf
+        set_plain_mode(scratch)
         os.replace(scratch, zip_path)
     except BaseException:
         Path(scratch).unlink(missing_ok=True)
@@ -4573,6 +4574,7 @@ def inject_manifest(zip_path: str, manifest: dict) -> None:
                         continue
                     dst.writestr(item, src.read(item.filename))
                 _write_generated_member(dst, "manifest.json", manifest_json)
+        set_plain_mode(scratch)
         os.replace(scratch, zip_path)
     except BaseException:
         Path(scratch).unlink(missing_ok=True)

scripts/largefiles.py

@@ -19,6 +19,7 @@ from collections import Counter
 from collections.abc import Iterable
 from pathlib import Path
 
+from artifacts import set_plain_mode
 from hashing import compute_hashes
 
 
@@ -219,6 +220,7 @@ def _fetch_asset(
             compute_hashes(tmp_path), expected_sha1, expected_md5
         ):
             return None
+        set_plain_mode(tmp_path)
         os.replace(tmp_path, cached)
         return cached
     finally:

scripts/refresh_data_dirs.py

@@ -25,7 +25,7 @@ import urllib.request
 import zipfile
 from collections.abc import Iterator
 from pathlib import Path
-from artifacts import file_lock as _file_lock
+from artifacts import file_lock as _file_lock, set_plain_mode
 from common import github_token, platform_arg, require_platform, yaml_load
 
 try:
@@ -133,6 +133,7 @@ def _save_versions(
         with os.fdopen(handle, "w") as f:
             json.dump(versions, f, indent=2, sort_keys=True)
             f.write("\n")
+        set_plain_mode(scratch)
         os.replace(scratch, path)
     except BaseException:
         with contextlib.suppress(OSError):

scripts/upstream.py

@@ -21,6 +21,8 @@ from dataclasses import dataclass
 from datetime import datetime
 from pathlib import Path
 
+from artifacts import set_plain_mode
+
 USER_AGENT = "retrobios-profile-sync/1.0"
 # Where fetched upstream files live, shared by every reader of this cache.
 CACHE_DIR = ".cache/upstream"
@@ -343,6 +345,7 @@ def write_cache(target: Path, text: str) -> None:
     try:
         with os.fdopen(fd, "w", encoding="utf-8") as fh:
             fh.write(text)
+        set_plain_mode(scratch)
         os.replace(scratch, target)
     except OSError:
         Path(scratch).unlink(missing_ok=True)

scripts/ziptools.py

@@ -14,6 +14,7 @@ import sys
 import tempfile
 import zipfile
 
+from artifacts import set_plain_mode
 from hashing import md5sum
 
 
@@ -141,6 +142,7 @@ def _write_member_cache(path: str, data: dict[str, list[list]]) -> None:
     try:
         with os.fdopen(fd, "w", encoding="utf-8") as fh:
             json.dump(data, fh, separators=(",", ":"))
+        set_plain_mode(scratch)
         os.replace(scratch, path)
     except OSError as exc:
         print(f"warning: could not write {path}: {exc}", file=sys.stderr)

tests/test_atomic_artifacts.py

@@ -129,5 +129,90 @@ class TheNativeIndexKeepsEveryPlatform(unittest.TestCase):
             self.assertEqual(recorded.get("batocera/file"), "https://a")
             self.assertEqual((root / "batocera" / "file").read_bytes(), b"payload")
 
+class ScratchFilesGetThePlainMode(unittest.TestCase):
+    """mkstemp makes its file 0600 and the rename kept it: database.json,
+    the manifests and the packs came out readable by their owner only."""
+
+    def setUp(self):
+        self._tmp = tempfile.TemporaryDirectory()
+        self.addCleanup(self._tmp.cleanup)
+        self.dir = Path(self._tmp.name)
+        previous = os.umask(0o022)
+        self.addCleanup(os.umask, previous)
+
+    def _mode(self, path: Path) -> int:
+        return path.stat().st_mode & 0o777
+
+    def test_text_and_bytes_writers(self):
+        artifacts.write_text_atomic(str(self.dir / "a.json"), "{}")
+        artifacts.write_bytes_atomic(str(self.dir / "b.bin"), b"x")
+        self.assertEqual(self._mode(self.dir / "a.json"), 0o644)
+        self.assertEqual(self._mode(self.dir / "b.bin"), 0o644)
+
+    def test_the_umask_decides(self):
+        os.umask(0o002)
+        artifacts.write_text_atomic(str(self.dir / "a.json"), "{}")
+        self.assertEqual(self._mode(self.dir / "a.json"), 0o664)
+
+    def test_packs_and_their_manifest(self):
+        import zipfile  # noqa: PLC0415
+
+        import generate_pack  # noqa: PLC0415
+
+        pack = self.dir / "Pack.zip"
+        with generate_pack._writing_pack(str(pack)) as zf:
+            zf.writestr("a.bin", b"x")
+        self.assertEqual(self._mode(pack), 0o644)
+        os.chmod(pack, 0o600)
+        generate_pack.inject_manifest(str(pack), {"files": []})
+        self.assertEqual(self._mode(pack), 0o644)
+        with zipfile.ZipFile(pack) as zf:
+            self.assertIn("manifest.json", zf.namelist())
+
+    def test_every_renamed_scratch_gets_the_mode(self):
+        """A function that renames a mkstemp file into place sets its mode.
+
+        copy_file_atomic is the one exception: copy2 gives the scratch the
+        source file's own mode.
+        """
+        import ast  # noqa: PLC0415
+
+        def name_of(node: ast.AST) -> str | None:
+            return node.id if isinstance(node, ast.Name) else None
+
+        def callee(call: ast.Call) -> str:
+            func = call.func
+            return func.attr if isinstance(func, ast.Attribute) else name_of(func) or ""
+
+        unset = []
+        for path in sorted((REPO_ROOT / "scripts").rglob("*.py")):
+            tree = ast.parse(path.read_text(encoding="utf-8"))
+            for func in ast.walk(tree):
+                if not isinstance(func, (ast.FunctionDef, ast.AsyncFunctionDef)):
+                    continue
+                if func.name == "copy_file_atomic":
+                    continue
+                scratches: set[str] = set()
+                for node in ast.walk(func):
+                    if (isinstance(node, ast.Assign) and isinstance(node.value, ast.Call)
+                            and callee(node.value) == "mkstemp"
+                            and isinstance(node.targets[0], ast.Tuple)):
+                        scratch = name_of(node.targets[0].elts[1])
+                        if scratch:
+                            scratches.add(scratch)
+                calls = [n for n in ast.walk(func) if isinstance(n, ast.Call)]
+                moded = {
+                    name_of(c.args[0]) for c in calls
+                    if callee(c) in ("set_plain_mode", "chmod") and c.args
+                }
+                for call in calls:
+                    if callee(call) != "replace" or not call.args:
+                        continue
+                    first = name_of(call.args[0])
+                    if first in scratches and first not in moded:
+                        unset.append(f"{path.name}:{call.lineno} {func.name}")
+        self.assertEqual(unset, [])
+
+
 if __name__ == "__main__":
     unittest.main()