Give path exports the permissions open() would

Merged
BR
1d ago
+330-3

Every path export writes through _atomic_write_bytes / _atomic_write_text. Both create their temp file with tempfile.mkstemp, which always uses mode 0o600, and os.replace then moves that mode onto the target. This affects to_html(path), write_image, write_images, to_svg(path), and the facet-grid writers. As a result every exported chart is owner-only (rw-------), whatever the umask says, and it even replaces the mode of an existing 0o644 file.

This matters when the export is meant to be read by someone else. A chart written into a directory served by nginx or Apache (running as another user) returns 403, a report dropped into a shared or group directory can't be opened by the group, and a file that was already 0o644 becomes 0o600 after one re-export.

Repro (main @ 72a069f, umask 022)

import os, stat, xy
chart = xy.scatter_chart(xy.scatter(x=[1, 2, 3], y=[3, 1, 2]))
chart.to_html("c.html"); chart.write_image("c.png"); chart.write_image("c.svg")
open("plain.txt", "w").close()
for p in ("c.html", "c.png", "c.svg", "plain.txt"):
    print(p, oct(stat.S_IMODE(os.stat(p).st_mode)))

c.html 0o600
c.png 0o600
c.svg 0o600
plain.txt 0o644

Fix

The temp file is still created by mkstemp (0o600), so nothing can read it while it is being written. After fsync, _publish_mode applies the mode open(path, "w") would leave, using os.fchmod on the open descriptor, and then os.replace runs. That mode is the existing file's permission bits, or for a new file 0o666 filtered by the umask or the directory's default ACL (read back from an empty probe file, so the process-wide umask is never flipped). Atomicity, fsync, temp naming, and cleanup are unchanged. spec/api/export.md states the rule next to the atomic-write contract.

Tests

tests/test_figure.py::test_path_exports_get_the_permissions_open_would_give has five cases: to_html, write_image png, to_svg(path), write_image pdf, and write_images. Each case checks three things:

  1. A new export gets the same mode as a reference file made with open() in that directory.
  2. A pre-existing 0o664 target keeps 0o664.
  3. The temp file is 0o600 at every fsync.

The test is POSIX only. test_export_still_lands_when_the_mode_cannot_be_applied (added after review) checks that applying the mode is best effort: when the probe cannot be created (ENOSPC) or fchmod is unsupported (EPERM), the export is still published, owner-only (0o600). Results:

  • On main, 5/5 fail (0o600 vs 0o644). With the fix, 5/5 pass, and the existing *_keeps_existing_file_on_atomic_replace_failure tests still pass.
  • make check-security (security_export) passes.
  • The full suite passes on this branch (4099 passed, 113 skipped; main is 4094 passed, 113 skipped).
  • ruff check, ruff format --check, and ty check python report no new diagnostics compared with main.

Checklist

  • News fragment: news/525.bugfix.md
  • Spec updated: spec/api/export.md (atomic write paragraph)
  • make check-security run for the path-write change
  • No public API or import-boundary change

Summary by CodeRabbit

  • Bug Fixes
    • Exported files now receive permissions consistent with direct file creation. Existing files retain their permission bits and, where possible, their group and access ACL; new files follow the current umask and default ACL.
    • Temporary files remain owner-only while being written. If permission handling fails, exports remain restricted. Windows behavior is unchanged.
    • HTML, SVG, image, and facet exports follow the same permission behavior.
  • Documentation
    • Clarified permission handling for path writers, including batch exports and behavior when permission adjustments fail.