Give path exports the permissions open() would

news/525.bugfix.md

@@ -0,0 +1,4 @@
+Files written by `to_html(path)`, `to_svg(path)`, `write_image`, `write_images`
+and facet exports now get the same permissions as any other new file (normally
+`rw-r--r--`), and a file that already exists keeps its mode. They were created
+owner-only (`rw-------`), so a web server or another user could not read them.

python/xy/export.py

@@ -10,7 +10,9 @@
 import numbers
 import os
 import re as _re
+import secrets
 import shutil
+import stat
 import subprocess
 import sys
 import tempfile
@@ -216,6 +218,52 @@ def _base64_chunks(blob: bytes) -> list[str]:
 )
 
 
+def _open_file_mode(target: Path) -> int:
+    """The permission bits `open(target, "w")` would leave on `target`.
+
+    An existing file keeps its own bits (`open` never changes them; the kernel
+    clears setuid/setgid on write, hence the `0o777` mask). A new file gets
+    `0o666` filtered by whatever governs creation in that directory — the
+    umask, or a default ACL — which is read back from an empty probe file
+    rather than by flipping the process-wide umask.
+    """
+    try:
+        return stat.S_IMODE(os.stat(target).st_mode) & 0o777
+    except OSError:
+        pass
+    flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0)
+    for _ in range(tempfile.TMP_MAX):
+        probe = target.parent / f".{target.name}.{secrets.token_hex(4)}.mode"
+        try:
+            fd = os.open(probe, flags, 0o666)
+        except FileExistsError:
+            continue
+        try:
+            return stat.S_IMODE(os.fstat(fd).st_mode)
+        finally:
+            os.close(fd)
+            with suppress(FileNotFoundError):
+                probe.unlink()
+    raise FileExistsError(f"no free temporary file name beside {str(target)!r}")
+
+
+def _publish_mode(fd: int, target: Path) -> None:
+    """Give the finished temp file the mode `open()` would have given the export.
+
+    `tempfile.mkstemp` creates `0o600` so nothing can read the file while it is
+    being written, but `os.replace` then carries that mode onto the target and
+    every export ended up owner-only. The final mode is applied through the
+    descriptor, after the data is flushed and immediately before the replace.
+    Best effort: if the mode cannot be determined or applied (no free inode for
+    the probe, a filesystem without chmod), the finished export is still
+    published, just owner-only as before.
+    """
+    if not hasattr(os, "fchmod"):  # POSIX; Windows has no owner-only mode to undo
+        return
+    with suppress(OSError):
+        os.fchmod(fd, _open_file_mode(target))
+
+
 def _atomic_write_bytes(path: str | PathLike[str], data: bytes) -> None:
     """Write bytes through a same-directory temp file, then replace atomically."""
     target = Path(path)
@@ -227,6 +275,7 @@ def _atomic_write_bytes(path: str | PathLike[str], data: bytes) -> None:
             f.write(data)
             f.flush()
             os.fsync(f.fileno())
+            _publish_mode(f.fileno(), target)
         os.replace(tmp_path, target)
     except Exception:
         if fd != -1:
@@ -254,6 +303,7 @@ def _atomic_write_text(path: str | PathLike[str], text: str) -> None:
             f.write(text)
             f.flush()
             os.fsync(f.fileno())
+            _publish_mode(f.fileno(), target)
         os.replace(tmp_path, target)
     except Exception:
         if fd != -1:

spec/api/export.md

@@ -228,9 +228,12 @@ Two properties are the point of the API:
   browser at all.
 
 Writes are atomic per file (same-directory temp file, fsync, `os.replace`), so a
-reader never observes a partial image. Failure mid-batch is not transactional:
-files already written stay on disk. The return value is the list of written
-byte strings, in input order.
+reader never observes a partial image. The temp file stays owner-only while it
+is written; just before the replace it takes the mode a plain `open(path, "w")`
+would leave (an existing file's permission bits, or for a new file `0o666`
+filtered by the umask or the directory's default ACL), so exports are not left
+owner-only. Failure mid-batch is not transactional: files already written stay
+on disk. The return value is the list of written byte strings, in input order.
 ## 9. What styling survives which export path
 
 XY has five styling mechanisms (`spec/api/styling.md` § The five ways to style)

tests/test_figure.py

@@ -5,8 +5,11 @@
 from __future__ import annotations
 
 import datetime as dt
+import errno
 import html as _html
 import json
+import os
+import stat
 import warnings
 from pathlib import Path
 
@@ -1253,6 +1256,87 @@ def fail_replace(src: Path, dst: Path) -> None:
     assert not list(tmp_path.glob(".chart.html.*.tmp"))
 
 
+@pytest.mark.skipif(os.name != "posix", reason="POSIX permission bits")
+@pytest.mark.parametrize("name", ["chart.html", "chart.png", "chart.svg", "chart.pdf", "batch.png"])
+def test_path_exports_get_the_permissions_open_would_give(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch, name: str
+):
+    """The atomic temp file must not leak its owner-only mode onto the export:
+    a new file gets the mode `open(path, "w")` gives beside it, and an existing
+    file keeps its own mode. The temp file stays private until it is complete."""
+    fig = Figure(title="permissions").line([0.0, 1.0], [1.0, 2.0])
+
+    def export(target: Path) -> None:
+        if name.startswith("batch"):
+            export_module.write_images([fig], [target])
+        elif target.suffix == ".html":
+            fig.to_html(target)
+        elif target.suffix == ".svg":
+            fig.to_svg(target)
+        else:
+            fig.write_image(target)
+
+    written_modes: list[int] = []
+    real_fsync = export_module.os.fsync
+
+    def recording_fsync(fd: int) -> None:
+        written_modes.append(stat.S_IMODE(os.fstat(fd).st_mode))
+        real_fsync(fd)
+
+    monkeypatch.setattr(export_module.os, "fsync", recording_fsync)
+    previous = os.umask(0o022)
+    try:
+        reference = tmp_path / "reference"
+        reference.open("w").close()
+        expected = stat.S_IMODE(reference.stat().st_mode)
+
+        created = tmp_path / name
+        export(created)
+        assert stat.S_IMODE(created.stat().st_mode) == expected
+
+        existing = tmp_path / f"shared-{name}"
+        existing.write_bytes(b"old")
+        existing.chmod(0o664)
+        export(existing)
+        assert stat.S_IMODE(existing.stat().st_mode) == 0o664
+        assert existing.read_bytes() != b"old"
+    finally:
+        os.umask(previous)
+    assert written_modes and all(mode == 0o600 for mode in written_modes)
+    assert not list(tmp_path.glob(".*.tmp")) and not list(tmp_path.glob(".*.mode"))
+
+
+@pytest.mark.skipif(os.name != "posix", reason="POSIX permission bits")
+@pytest.mark.parametrize("failure", ["probe", "fchmod"])
+def test_export_still_lands_when_the_mode_cannot_be_applied(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch, failure: str
+):
+    """Applying the open()-style mode is best effort: a directory that cannot
+    take one more file (no free inode), or a filesystem without chmod, still
+    gets the finished export, owner-only as before."""
+    fig = Figure(title="probe").line([0.0, 1.0], [1.0, 2.0])
+    if failure == "probe":
+        real_open = export_module.os.open
+
+        def no_probe(path, flags, mode=0o777, *args, **kwargs):
+            if str(path).endswith(".mode"):
+                raise OSError(errno.ENOSPC, os.strerror(errno.ENOSPC))
+            return real_open(path, flags, mode, *args, **kwargs)
+
+        monkeypatch.setattr(export_module.os, "open", no_probe)
+    else:
+
+        def no_fchmod(fd, mode):
+            raise OSError(errno.EPERM, os.strerror(errno.EPERM))
+
+        monkeypatch.setattr(export_module.os, "fchmod", no_fchmod)
+    target = tmp_path / "chart.html"
+    html = fig.to_html(target)
+    assert target.read_text(encoding="utf-8") == html
+    assert stat.S_IMODE(target.stat().st_mode) == 0o600
+    assert not list(tmp_path.glob(".*.tmp")) and not list(tmp_path.glob(".*.mode"))
+
+
 def test_figure_dom_slots_are_validated_before_export():
     fig = Figure().line([0.0, 1.0], [1.0, 2.0])
     fig.class_names = {"legend": "ok", "legnd": "typo"}