diff --git a/src/pullbox/utilities/comicinfo.py b/src/pullbox/utilities/comicinfo.py index ccf13884..cf5eae0e 100644 --- a/src/pullbox/utilities/comicinfo.py +++ b/src/pullbox/utilities/comicinfo.py @@ -9,7 +9,10 @@ from __future__ import annotations +import contextlib +import os import shutil +import stat import tempfile import xml.etree.ElementTree as ET import zipfile @@ -278,6 +281,26 @@ def _find_comicinfo_member(source: zipfile.ZipFile) -> zipfile.ZipInfo | None: return None +def _copy_permission_bits(reference: Path, target: Path) -> None: + """Give a rewritten archive the permission bits of the file it stands in for. + + Rewrites go through ``tempfile.mkstemp``, which creates the file as 0600. Left + alone, an archive that was readable by a media server before the rewrite is + owner-only afterwards. Best effort: filesystems that reject chmod, such as some + SMB/CIFS mounts, must not fail the rewrite, but the failure is logged so an + owner-only archive can be traced back to it. + """ + try: + os.chmod(target, stat.S_IMODE(reference.stat().st_mode)) + except OSError as exc: + logger.warning( + "comicinfo_permission_bits_not_restored", + reference=str(reference), + target=str(target), + error=str(exc), + ) + + def embed_comicinfo_in_cbz( cbz_path: Path, data: dict[str, Any], @@ -371,6 +394,7 @@ def embed_comicinfo_in_cbz( ) # Atomic replace + _copy_permission_bits(cbz_path, tmp_path) shutil.move(str(tmp_path), str(cbz_path)) logger.info( @@ -388,9 +412,6 @@ def embed_comicinfo_in_cbz( tmp_path.unlink() raise finally: - import contextlib - import os - # Close the file descriptor if still open if tmp_fd >= 0: with contextlib.suppress(OSError): @@ -481,6 +502,8 @@ def materialize_cbz_with_comicinfo( "entries", ) + # A plain move or copy would carry the source's mode to the target. + _copy_permission_bits(source_path, temp_output_path) shutil.move(str(temp_output_path), str(target_path)) if transfer_method == "move" and source_path.resolve(strict=False) != target_path.resolve( strict=False @@ -516,9 +539,6 @@ def materialize_cbz_with_comicinfo( temp_output_path.unlink() raise finally: - import contextlib - import os - if tmp_fd >= 0: with contextlib.suppress(OSError): os.close(tmp_fd) diff --git a/tests/unit/test_comicinfo_embed.py b/tests/unit/test_comicinfo_embed.py index 284daa18..a116844f 100644 --- a/tests/unit/test_comicinfo_embed.py +++ b/tests/unit/test_comicinfo_embed.py @@ -6,6 +6,9 @@ import zipfile from typing import TYPE_CHECKING +import structlog +from structlog.testing import capture_logs + from pullbox.utilities.comicinfo import embed_comicinfo_in_cbz, materialize_cbz_with_comicinfo if TYPE_CHECKING: @@ -286,3 +289,77 @@ def test_materialize_cbz_with_comicinfo_move_deletes_source_after_success( assert not source.exists() assert target.exists() assert _read_comicinfo(target)["Series"] == "Moved Series" + + +def _mode(path: Path) -> int: + return path.stat().st_mode & 0o777 + + +def test_embed_comicinfo_keeps_the_archive_permission_bits(tmp_path: Path) -> None: + archive_path = tmp_path / "issue.cbz" + _write_cbz(archive_path, "Old Series") + archive_path.chmod(0o664) + + assert embed_comicinfo_in_cbz(archive_path, {"Series": "New Series"}) is True + + assert _read_comicinfo(archive_path)["Series"] == "New Series" + assert _mode(archive_path) == 0o664 + + +def test_embed_comicinfo_keeps_permission_bits_with_caller_temp_path(tmp_path: Path) -> None: + archive_path = tmp_path / "issue.cbz" + _write_cbz(archive_path, "Old Series") + archive_path.chmod(0o600) + + embed_comicinfo_in_cbz( + archive_path, + {"Series": "New Series"}, + temp_path=tmp_path / "work" / "issue.tmp.cbz", + ) + + assert _mode(archive_path) == 0o600 + + +def test_materialize_cbz_with_comicinfo_carries_source_permission_bits(tmp_path: Path) -> None: + source_path = tmp_path / "downloads" / "issue.cbz" + source_path.parent.mkdir() + _write_cbz(source_path, "Old Series") + source_path.chmod(0o644) + target_path = tmp_path / "library" / "Series" / "issue.cbz" + + materialize_cbz_with_comicinfo( + source_path, + target_path, + {"Series": "New Series"}, + transfer_method="copy", + ) + + assert _read_comicinfo(target_path)["Series"] == "New Series" + assert _mode(target_path) == 0o644 + + +def test_embed_comicinfo_survives_a_filesystem_that_rejects_chmod( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + archive_path = tmp_path / "issue.cbz" + _write_cbz(archive_path, "Old Series") + + def reject_chmod(*_args: object, **_kwargs: object) -> None: + raise PermissionError("chmod is not permitted on this mount") + + monkeypatch.setattr("pullbox.utilities.comicinfo.os.chmod", reject_chmod) + # A logger cached before app reconfiguration would bypass capture_logs. + monkeypatch.setattr( + "pullbox.utilities.comicinfo.logger", + structlog.wrap_logger(None, cache_logger_on_first_use=False), + ) + + with capture_logs() as logs: + assert embed_comicinfo_in_cbz(archive_path, {"Series": "New Series"}) is True + assert _read_comicinfo(archive_path)["Series"] == "New Series" + warnings = [log for log in logs if log["event"] == "comicinfo_permission_bits_not_restored"] + assert len(warnings) == 1 + assert warnings[0]["log_level"] == "warning" + assert warnings[0]["reference"] == str(archive_path) + assert "chmod is not permitted" in warnings[0]["error"]