diff --git a/docs/workflows.md b/docs/workflows.md index 67ad77d..0d44c40 100644 --- a/docs/workflows.md +++ b/docs/workflows.md @@ -125,7 +125,10 @@ and placement policy differ. and explicit save path, or apply a transactional delta to an existing torrent. Set selected/skipped state, run a full recheck of the target union, and fail immediately if qBittorrent attempts to download. Successful recheck - atomically advances the placement generation and is the commit point. + atomically advances the placement generation and is the commit point. Before + qBittorrent is resumed, the client applies `a+rx` to the verified resource + directories and `a+r` to its verified files, preserving ownership, write + bits, and special mode bits so other local applications can read the data. 5. **Staging Cleanup.** Remove only job-owned staging artifacts from both endpoints. A post-commit cleanup failure produces `CLEANUP_REQUIRED`; it never rolls back or deletes the committed placement. diff --git a/pyproject.toml b/pyproject.toml index 49f9920..58364c5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "archive-clients" -version = "0.1.22" +version = "0.1.23" requires-python = ">=3.11" dependencies = ["protobuf==7.35.1", "websockets==16.0"] diff --git a/src/archive_clients/jobs.py b/src/archive_clients/jobs.py index 7a91021..91e5764 100644 --- a/src/archive_clients/jobs.py +++ b/src/archive_clients/jobs.py @@ -808,13 +808,14 @@ class ClientJobExecutor: fraction, 0, 0, "qBittorrent stopped recheck" ), ) - if should_start: - self.qbittorrent.start(qb_torrent_id) verified = self.qbittorrent.get_resource(info_hash) if verified is None: raise JobExecutionError( "verified qBittorrent resource disappeared" ) + _normalize_verified_resource_permissions(self.qb_root, verified) + if should_start: + self.qbittorrent.start(qb_torrent_id) placement = resource_pb2.Placement( client_id=self.client_id, state=resource_pb2.PLACEMENT_STATE_PRESENT, @@ -1191,6 +1192,78 @@ def _info_hash(definition: job_pb2.JobDefinition) -> str: return value +def _normalize_verified_resource_permissions( + qb_root: Path, resource: NormalizedResource +) -> None: + """Apply ``a+rx``/``a+r`` to exactly qB-verified completed content. + + qBittorrent may finish a stopped recheck with restrictive modes inherited + from source materialization. Normalize only selected, fully completed + regular files and their real parent directories; never traverse a symlink + or broaden permissions on the configured qB root itself. + """ + root_metadata = qb_root.lstat() + if not stat.S_ISDIR(root_metadata.st_mode): + raise JobExecutionError("configured qB root is not a real directory") + directories: set[Path] = set() + files: set[Path] = set() + for item in resource.files: + if not item.selected or item.completed_bytes != item.logical_bytes: + continue + relative = _resource_relative_path(item.canonical_path) + current = qb_root + for component in relative.parts[:-1]: + current = current / component + metadata = current.lstat() + if not stat.S_ISDIR(metadata.st_mode): + raise JobExecutionError( + "verified resource parent is not a real directory" + ) + directories.add(current) + candidate = current / relative.name + metadata = candidate.lstat() + if not stat.S_ISREG(metadata.st_mode): + raise JobExecutionError("verified resource file is not regular") + files.add(candidate) + for directory in sorted(directories, key=lambda value: len(value.parts)): + metadata = directory.lstat() + if not stat.S_ISDIR(metadata.st_mode): + raise JobExecutionError( + "verified resource parent changed during permission update" + ) + os.chmod( + directory, + stat.S_IMODE(metadata.st_mode) | 0o555, + follow_symlinks=False, + ) + for path in files: + metadata = path.lstat() + if not stat.S_ISREG(metadata.st_mode): + raise JobExecutionError( + "verified resource file changed during permission update" + ) + os.chmod( + path, + stat.S_IMODE(metadata.st_mode) | 0o444, + follow_symlinks=False, + ) + + +def _resource_relative_path(value: str) -> PurePosixPath: + if ( + not value + or "\x00" in value + or "\\" in value + or value.startswith("/") + or any(part in {"", ".", ".."} for part in value.split("/")) + ): + raise JobExecutionError("verified resource path is unsafe") + path = PurePosixPath(value) + if path.is_absolute(): + raise JobExecutionError("verified resource path is unsafe") + return path + + def _fingerprint_matches( current: resource_pb2.ResourceStateFingerprint, expected: resource_pb2.ResourceStateFingerprint, diff --git a/tests/test_jobs.py b/tests/test_jobs.py index dabbed4..dfa1f53 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -1,4 +1,6 @@ import hashlib +import os +import stat import tempfile import threading import unittest @@ -10,12 +12,13 @@ from archive_clients.bencode import encode from archive_clients.jobs import ( ClientJobExecutor, JobExecutionError, + _normalize_verified_resource_permissions, _resource_fingerprint, ) from archive_clients.syncthing import RouteSetupError, SyncthingTransferStatus -from archive_clients.resources import normalize_resource +from archive_clients.resources import NormalizedResource, normalize_resource from archive_clients.state import ClientStore -from archive_control.v1 import control_pb2, job_pb2 +from archive_control.v1 import control_pb2, job_pb2, resource_pb2 class CompleteSyncthing: @@ -39,6 +42,50 @@ class SlowRescanSyncthing(CompleteSyncthing): class ClientJobHappyPathTests(unittest.TestCase): + def test_verified_resource_permissions_are_readable_by_other_apps(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) / "qb" + resource_directory = root / "resource" + resource_directory.mkdir(parents=True) + completed = resource_directory / "complete.bin" + incomplete = resource_directory / "incomplete.bin" + completed.write_bytes(b"complete") + incomplete.write_bytes(b"incomplete") + os.chmod(resource_directory, 0o300) + os.chmod(completed, 0o200) + os.chmod(incomplete, 0o200) + resource = NormalizedResource( + resource_pb2.ResourceSummary(), + ( + resource_pb2.TorrentFile( + file_index=0, + canonical_path="resource/complete.bin", + logical_bytes=len(b"complete"), + completed_bytes=len(b"complete"), + selected=True, + ), + resource_pb2.TorrentFile( + file_index=1, + canonical_path="resource/incomplete.bin", + logical_bytes=len(b"incomplete"), + completed_bytes=0, + selected=True, + ), + ), + Mock(), + ) + _normalize_verified_resource_permissions(root, resource) + self.assertEqual( + stat.S_IMODE(resource_directory.stat().st_mode) & 0o555, + 0o555, + ) + self.assertEqual( + stat.S_IMODE(completed.stat().st_mode) & 0o444, 0o444 + ) + self.assertEqual( + stat.S_IMODE(incomplete.stat().st_mode), 0o200 + ) + def test_syncthing_api_outage_during_transfer_is_retried(self): """A transient local REST outage must not terminally fail the job.""" with tempfile.TemporaryDirectory() as directory: