From a1bf3e4315c8a04d62f7f8dc444fbfc5643dfcd7 Mon Sep 17 00:00:00 2001 From: Cabbagec Date: Fri, 14 Aug 2026 11:56:06 +0000 Subject: [PATCH] fix: resolve per-torrent qb save paths --- docs/deployment-and-usage.md | 10 ++++ docs/deployment-preflight.md | 15 ++++- docs/storage-safety.md | 8 ++- scripts/preflight-deployment.py | 23 +++++++- src/archive_clients/cli.py | 13 ++++- src/archive_clients/config.py | 2 - src/archive_clients/daemon.py | 8 ++- src/archive_clients/eviction.py | 52 ++++++++++++----- src/archive_clients/jobs.py | 94 +++++++++++++++++++++++++----- src/archive_clients/resources.py | 22 ++++++- tests/test_cli.py | 59 +++++++++++++++++++ tests/test_config.py | 29 +++++++++ tests/test_deployment_preflight.py | 20 +++++++ tests/test_eviction.py | 54 ++++++++++++++--- tests/test_jobs.py | 67 ++++++++++++++++++++- tests/test_qbittorrent.py | 9 ++- tests/test_resources.py | 23 +++++++- 17 files changed, 453 insertions(+), 55 deletions(-) create mode 100644 tests/test_cli.py diff --git a/docs/deployment-and-usage.md b/docs/deployment-and-usage.md index ae83a67..dd751d2 100644 --- a/docs/deployment-and-usage.md +++ b/docs/deployment-and-usage.md @@ -120,6 +120,8 @@ username = "${QB_USER}" password_file = "/run/secrets/qb_password" api_root = "/downloads" local_root = "/data/qb" +# Optional only when a nested qB path uses a different client-visible mount. +# local_path_overrides = { "/downloads/fast" = "/data/qb-fast" } [syncthing] endpoint = "http://syncthing:8384" @@ -139,6 +141,14 @@ accounting. The override key is the normalized Syncthing folder path beneath `api_root`, not its folder ID. See the production deployment README for the required compose and override pattern. +qBittorrent content is resolved differently: every existing torrent keeps its +qB-reported `save_path`. The client maps that path beneath `qbittorrent.api_root` +to its own mount before a source, existing-target, permission, or eviction +operation. Thus `/downloads/Downloading` naturally maps below `/data/qb`; +there is no migration or per-resource configuration. Add a qB +`local_path_overrides` entry only when that nested API prefix is a separate +client mount. + The remaining node examples omit optional `[connection]`, `[jobs]`, and `[backup]` tables and therefore use these same defaults; deployments may override them per node. diff --git a/docs/deployment-preflight.md b/docs/deployment-preflight.md index 6a116b9..a954e91 100644 --- a/docs/deployment-preflight.md +++ b/docs/deployment-preflight.md @@ -86,6 +86,9 @@ python3 scripts/preflight-deployment.py ... \ - That future route path and qBittorrent content root use one client bind mount, so hard-link staging remains possible rather than silently falling back to a space-consuming copy. +- Every explicit `qbittorrent.local_path_overrides` entry maps the same host + path in the qBittorrent and client containers. This protects nested qB save + paths that use a dedicated bind mount. - A real `link(2)` operation between a unique zero-byte file in the qB root and one in the future automatic-route root. The probe verifies that both names refer to the same inode and removes them unconditionally. @@ -96,7 +99,8 @@ python3 scripts/preflight-deployment.py ... \ - The token, qB password, and Syncthing API-key files are non-empty regular files with no group/world permissions. - The client image can read its configuration and reports usable permissions, - sparse-file support, and filesystem capabilities for both roots. + sparse-file support, and filesystem capabilities for both primary roots and + every configured qBittorrent local-path override. - qBittorrent authentication/version compatibility and Syncthing authentication/device identity are healthy from the client container. @@ -106,6 +110,15 @@ root and add a `local_path_overrides` mapping for that exact API path. This prevents automatic route folders being created on a small configuration filesystem while the client expects to hardlink from the qB data mount. +qBittorrent resources may use any `save_path` below `qbittorrent.api_root`. +At job preflight the client maps that qB API path to its local mount and uses +it as the resource root; data is never moved to fit Archive Control. A resource +outside the configured qB API root, or whose resolved directory is unavailable +or not a real directory in the client container, fails that job before staging +or eviction. Use `qbittorrent.local_path_overrides` only for a nested qB API +prefix backed by a distinct client mount; this preflight verifies the Docker +bind topology for each such override. + When converting an existing node, stop its client and Syncthing containers, move each existing `routes/` directory from the old Syncthing config tree into the new qB-backed route-root directory, then recreate Syncthing and diff --git a/docs/storage-safety.md b/docs/storage-safety.md index 8851a71..89ff268 100644 --- a/docs/storage-safety.md +++ b/docs/storage-safety.md @@ -14,6 +14,13 @@ client: a regular file or an explicitly created directory; 5. verifies every operation remains beneath the local configured root. +For qBittorrent content, the root is resolved per resource: qB's authoritative +API-visible `save_path` is mapped beneath `qbittorrent.api_root` (or a more +specific configured local override) into the client namespace. This permits +existing nested save paths without moving data, but does not permit paths +outside the configured boundary. The resolved path remains local client state +and is never sent to the control daemon. + Sockets, devices, FIFOs, symlinks, and other special entries fail preflight. Permission or ownership mismatch is fail-fast. Archive Control never changes source ownership or mode to make a job pass. @@ -150,4 +157,3 @@ Offline tooling provides list, verify, and restore. Restore requires stopped daemon access, verifies the chosen backup, preserves the suspect database under a timestamped name, installs the replacement atomically, and runs integrity and schema checks before normal startup. Backups contain no configured secrets. - diff --git a/scripts/preflight-deployment.py b/scripts/preflight-deployment.py index c1f5bf4..e8ccc41 100755 --- a/scripts/preflight-deployment.py +++ b/scripts/preflight-deployment.py @@ -134,7 +134,7 @@ from pathlib import Path from archive_clients.config import ClientConfig config=ClientConfig.load(Path(sys.argv[1])) value={"shared_token_file":str(config.shared_token_file)} -value["qbittorrent"]={"api_root":str(config.qbittorrent.api_root),"local_root":str(config.qbittorrent.local_root),"password_file":str(config.qbittorrent.password_file)} +value["qbittorrent"]={"api_root":str(config.qbittorrent.api_root),"local_root":str(config.qbittorrent.local_root),"password_file":str(config.qbittorrent.password_file),"local_path_overrides":{str(api):str(local) for api,local in config.qbittorrent.local_path_overrides}} value["syncthing"]={"api_root":str(config.syncthing.api_root),"local_root":str(config.syncthing.local_root),"api_key_file":str(config.syncthing.api_key_file),"local_path_overrides":{str(api):str(local) for api,local in config.syncthing.local_path_overrides}} print(json.dumps(value,sort_keys=True))''' result = subprocess.run( @@ -174,6 +174,26 @@ raise SystemExit(0 if all(p.state == 1 for p in probes) else 1)''' print(result.stdout.strip()) +def require_qb_override_mappings( + client: dict[str, Any], qbittorrent: dict[str, Any], qb: dict[str, Any], +) -> None: + """Prove every explicit qB API/local override sees the same host bytes.""" + + overrides = qb.get("local_path_overrides", {}) + if not isinstance(overrides, dict): + raise CheckFailure("qbittorrent.local_path_overrides must be a table") + for api_path, local_path in overrides.items(): + if not isinstance(api_path, str) or not isinstance(local_path, str): + raise CheckFailure( + "qbittorrent.local_path_overrides entries are invalid" + ) + require_same_path( + f"qBittorrent override {api_path}/local mapping", + map_path(qbittorrent, api_path), + map_path(client, local_path), + ) + + def run_hardlink_probe( container: str, qb_root: str, route_root: str, user: str | None = None, ) -> None: @@ -298,6 +318,7 @@ def main(argv: list[str] | None = None) -> int: syncthing_route, client_route, ) require_same_path("qBittorrent api_root/local_root", qb_api, client_qb) + require_qb_override_mappings(client, qbittorrent, qb) if client_qb.destination != client_route.destination: raise CheckFailure( "qBittorrent and future route roots use separate client bind " diff --git a/src/archive_clients/cli.py b/src/archive_clients/cli.py index 12cec10..5496bda 100644 --- a/src/archive_clients/cli.py +++ b/src/archive_clients/cli.py @@ -27,10 +27,16 @@ def main(argv: Sequence[str] | None = None) -> int: parser.add_argument("--check-config", action="store_true") arguments = parser.parse_args(argv) config = ClientConfig.load(arguments.config, arguments.mode) + qb_extra_roots = tuple(sorted({ + root + for _, root in config.qbittorrent.local_path_overrides + if root != config.qbittorrent.local_root + })) probes = [ probe_root(config.qbittorrent.local_root), probe_root(config.syncthing.local_root), ] + qb_extra_probes = tuple(probe_root(root) for root in qb_extra_roots) probe_writable_directory(config.state_db.parent) probe_writable_directory(config.backup_dir) shared_token = config.read_shared_token() @@ -40,7 +46,10 @@ def main(argv: Sequence[str] | None = None) -> int: print(json.dumps({ "client_id": config.client_id, "role": config.role, - "filesystems": [probe.__dict__ | {"root": str(probe.root)} for probe in probes], + "filesystems": [ + probe.__dict__ | {"root": str(probe.root)} + for probe in (*probes[:1], *qb_extra_probes, *probes[1:]) + ], }, sort_keys=True)) return 0 configure_logging( @@ -68,7 +77,7 @@ def main(argv: Sequence[str] | None = None) -> int: }, ) asyncio.run(ArchiveClientDaemon( - config, probes, service_probes, + config, probes, service_probes, qb_extra_probes=qb_extra_probes, resource_reader=QBittorrentReader(config.qbittorrent), ).run()) except KeyboardInterrupt: diff --git a/src/archive_clients/config.py b/src/archive_clients/config.py index b748707..cc7c5a6 100644 --- a/src/archive_clients/config.py +++ b/src/archive_clients/config.py @@ -213,8 +213,6 @@ def _local_path_overrides( value: dict[str, Any], api_root: PurePosixPath, name: str ) -> tuple[tuple[PurePosixPath, Path], ...]: raw = value.get("local_path_overrides", {}) - if name != "syncthing" and raw: - raise ConfigError(f"{name}.local_path_overrides is unsupported") if not isinstance(raw, dict): raise ConfigError(f"{name}.local_path_overrides must be a table") parsed: list[tuple[PurePosixPath, Path]] = [] diff --git a/src/archive_clients/daemon.py b/src/archive_clients/daemon.py index d561994..af45deb 100644 --- a/src/archive_clients/daemon.py +++ b/src/archive_clients/daemon.py @@ -64,6 +64,7 @@ class ArchiveClientDaemon: service_probes: list[ServiceProbe], resource_reader: QBittorrentReader | None = None, route_manager: SyncthingRouteManager | None = None, + qb_extra_probes: tuple[FilesystemProbe, ...] = (), ): if len(probes) != 2: raise ValueError( @@ -71,6 +72,7 @@ class ArchiveClientDaemon: ) self.config = config self.probes = probes + self.qb_probes = (probes[0], *qb_extra_probes) self.service_probes = service_probes self.inventory = ( InventoryService(resource_reader, config.client_id) @@ -119,9 +121,13 @@ class ArchiveClientDaemon: store=self.store, qb_root=config.qbittorrent.local_root, qb_api_root=config.qbittorrent.api_root, + qb_roots=config.qbittorrent.roots, route_path=self._route_path, syncthing_transport=self.routes.transport, - sparse_supported=all(probe.sparse_files for probe in probes), + sparse_supported=all( + probe.sparse_files + for probe in (*self.qb_probes, probes[1]) + ), poll_interval=config.jobs.poll_interval, verification_timeout=config.jobs.verification_timeout, free_space_reserve_bytes=( diff --git a/src/archive_clients/eviction.py b/src/archive_clients/eviction.py index e9ca22e..e89098f 100644 --- a/src/archive_clients/eviction.py +++ b/src/archive_clients/eviction.py @@ -7,7 +7,7 @@ import hashlib import os import stat from pathlib import Path, PurePosixPath -from typing import Iterable +from typing import Callable, Iterable from archive_clients.qbittorrent import QBittorrentReader from archive_clients.resources import NormalizedResource @@ -23,7 +23,7 @@ def verify_and_snapshot( job_id: str, resource: NormalizedResource, selected_indices: Iterable[int], - qb_root: Path, + content_root: Path, store: ClientStore, ) -> dict[str, object]: selected = set(selected_indices) @@ -38,7 +38,7 @@ def verify_and_snapshot( f"cache file {index} is not selected and complete" ) relative = _relative(item.canonical_path) - path = qb_root.joinpath(*relative.parts) + path = content_root.joinpath(*relative.parts) try: metadata = path.lstat() except FileNotFoundError as exc: @@ -65,7 +65,7 @@ def verify_and_snapshot( "sha256": _sha256(path), } ) - snapshot = {"files": files} + snapshot = {"content_root": str(content_root), "files": files} return store.put_job_artifact(job_id, "eviction-snapshot", snapshot)["value"] @@ -90,9 +90,9 @@ def remove_qb_entry( def safe_unlink( *, job_id: str, - qb_root: Path, qbittorrent: QBittorrentReader, store: ClientStore, + resource_root: Callable[[NormalizedResource], Path], ) -> dict[str, object]: completed = store.get_job_artifact(job_id, "eviction-unlinked") if completed is not None: @@ -101,11 +101,19 @@ def safe_unlink( if snapshot_row is None: raise EvictionError("eviction snapshot is missing") snapshot = snapshot_row["value"] + if not isinstance(snapshot, dict): + raise EvictionError("eviction snapshot is invalid") + content_root_value = snapshot.get("content_root") + if not isinstance(content_root_value, str): + raise EvictionError("eviction snapshot content root is missing") + content_root = Path(content_root_value) + if not content_root.is_absolute() or content_root.is_symlink(): + raise EvictionError("eviction snapshot content root is unsafe") files = snapshot.get("files") if not isinstance(files, list): raise EvictionError("eviction snapshot is invalid") - referenced = _remaining_paths(qbittorrent.list_resources()) + referenced = _remaining_paths(qbittorrent.list_resources(), resource_root) removed: list[str] = [] retained: list[dict[str, str]] = [] directories: set[Path] = set() @@ -113,10 +121,10 @@ def safe_unlink( if not isinstance(record, dict) or not isinstance(record.get("path"), str): raise EvictionError("eviction file record is invalid") relative = _relative(record["path"]) - if relative.as_posix() in referenced: + path = content_root.joinpath(*relative.parts) + if path in referenced: retained.append({"path": relative.as_posix(), "reason": "shared"}) continue - path = qb_root.joinpath(*relative.parts) try: metadata = path.lstat() except FileNotFoundError: @@ -142,7 +150,7 @@ def safe_unlink( path.unlink() removed.append(relative.as_posix()) parent = path.parent - while parent != qb_root: + while parent != content_root: directories.add(parent) parent = parent.parent @@ -153,7 +161,7 @@ def safe_unlink( try: directory.rmdir() removed_directories.append( - directory.relative_to(qb_root).as_posix() + directory.relative_to(content_root).as_posix() ) except OSError as exc: if exc.errno not in {errno.ENOTEMPTY, errno.ENOENT}: @@ -214,12 +222,24 @@ def compensate_materialized_files( return removed -def _remaining_paths(resources: Iterable[NormalizedResource]) -> set[str]: - return { - item.canonical_path - for resource in resources - for item in resource.files - } +def _remaining_paths( + resources: Iterable[NormalizedResource], + resource_root: Callable[[NormalizedResource], Path], +) -> set[Path]: + result: set[Path] = set() + for resource in resources: + try: + root = resource_root(resource) + except (OSError, RuntimeError, ValueError): + # A malformed unrelated qB entry must not stop an otherwise + # safe eviction. Its unresolvable path cannot be considered a + # shared path under the verified eviction root. + continue + result.update( + root.joinpath(*_relative(item.canonical_path).parts) + for item in resource.files + ) + return result def _relative(value: str) -> PurePosixPath: diff --git a/src/archive_clients/jobs.py b/src/archive_clients/jobs.py index 91e5764..645350f 100644 --- a/src/archive_clients/jobs.py +++ b/src/archive_clients/jobs.py @@ -14,6 +14,7 @@ import uuid from pathlib import Path, PurePosixPath from typing import Callable, Iterable +from archive_clients.config import ConfigError, RootMapping from archive_clients.protocol import decode_message, encode_message from archive_clients.eviction import ( EvictionError, @@ -67,6 +68,7 @@ class ClientJobExecutor: store: ClientStore, qb_root: Path, qb_api_root: PurePosixPath, + qb_roots: RootMapping | None = None, route_path: Callable[[str], Path], syncthing_transport: object, sparse_supported: bool, @@ -78,7 +80,10 @@ class ClientJobExecutor: self.qbittorrent = qbittorrent self.store = store self.qb_root = qb_root - self.qb_api_root = qb_api_root + self.qb_api_root = PurePosixPath(qb_api_root) + self.qb_roots = qb_roots or RootMapping( + self.qb_api_root, self.qb_root + ) self.route_path = route_path self.syncthing_transport = syncthing_transport self.sparse_supported = sparse_supported @@ -479,6 +484,7 @@ class ClientJobExecutor: "archive coverage no longer covers the cache selection" ) resource = self._resource(definition) + resource_root = self._resource_root(resource) current = _resource_fingerprint(resource, self.client_id) if not _fingerprint_matches( current, definition.eviction.cache_fingerprint @@ -490,7 +496,7 @@ class ClientJobExecutor: job_id=definition.job_id, resource=resource, selected_indices=requested, - qb_root=self.qb_root, + content_root=resource_root, store=self.store, ) return None @@ -505,9 +511,9 @@ class ClientJobExecutor: if step == job_pb2.JOB_STEP_KIND_SAFE_FILE_UNLINK: safe_unlink( job_id=definition.job_id, - qb_root=self.qb_root, qbittorrent=self.qbittorrent, store=self.store, + resource_root=self._resource_root, ) placement = resource_pb2.Placement( client_id=self.client_id, @@ -538,7 +544,8 @@ class ClientJobExecutor: raise JobExecutionError( "source resource changed after job confirmation" ) - self._reject_unsafe_partfile(definition, resource) + source_root = self._resource_root(resource) + self._reject_unsafe_partfile(definition, resource, source_root) indices = _selection_indices(definition.transfer.transfer_delta_files) by_index = {item.file_index: item for item in resource.files} if not indices or any(index not in by_index for index in indices): @@ -602,7 +609,7 @@ class ClientJobExecutor: self._require_space( route_root, self._copy_required_bytes( - self.qb_root, + source_root, route_root, ( (entry.target_canonical_path, entry.logical_bytes) @@ -613,7 +620,7 @@ class ClientJobExecutor: ) stage_transfer( manifest, - source_root=self.qb_root, + source_root=source_root, sync_root=route_root, store=self.store, artifact_sources={"metainfo/source.torrent": metainfo_path}, @@ -691,23 +698,27 @@ class ClientJobExecutor: "target materialization was sent to the wrong client" ) published = load_published_transfer(self._job_directory(definition)) + info_hash = _info_hash(definition) + resource = self.qbittorrent.get_resource(info_hash) + target_root = ( + self._resource_root(resource) + if resource is not None else self.qb_root + ) # The target can likewise hardlink an arrived Syncthing payload into # qB's content root when those directories share a filesystem. self._require_space( - self.qb_root, + target_root, self._copy_required_bytes( published.job_directory, - self.qb_root, + target_root, ( (entry.payload_relative_path, entry.logical_bytes) for entry in published.manifest.files ), ), ) - info_hash = _info_hash(definition) - resource = self.qbittorrent.get_resource(info_hash) if resource is not None: - self._reject_unsafe_partfile(definition, resource) + self._reject_unsafe_partfile(definition, resource, target_root) if definition.transfer.HasField("target_baseline_fingerprint"): if resource is None or not _fingerprint_matches( _resource_fingerprint(resource, self.client_id), @@ -732,13 +743,14 @@ class ClientJobExecutor: == resource_pb2.TORRENT_RUNTIME_STATE_STOPPED ), "total_file_count": len(published.manifest.files), + "content_root": str(target_root), } self.store.put_job_artifact( definition.job_id, "target-baseline", baseline ) materialize_transfer( published, - target_root=self.qb_root, + target_root=target_root, store=self.store, sparse_supported=self.sparse_supported, cancel_check=lambda: self._raise_if_cancelled( @@ -813,7 +825,9 @@ class ClientJobExecutor: raise JobExecutionError( "verified qBittorrent resource disappeared" ) - _normalize_verified_resource_permissions(self.qb_root, verified) + _normalize_verified_resource_permissions( + self._resource_root(verified), verified + ) if should_start: self.qbittorrent.start(qb_torrent_id) placement = resource_pb2.Placement( @@ -852,6 +866,7 @@ class ClientJobExecutor: ) return baseline = baseline_row["value"] + content_root = self._artifact_content_root(baseline) info_hash = _info_hash(definition) present = self.qbittorrent.get_resource(info_hash) if present is not None: @@ -870,7 +885,7 @@ class ClientJobExecutor: self.qbittorrent.delete_entry(qb_torrent_id) compensate_materialized_files( job_id=definition.job_id, - qb_root=self.qb_root, + qb_root=content_root, store=self.store, ) @@ -906,10 +921,11 @@ class ClientJobExecutor: self, definition: job_pb2.JobDefinition, resource: NormalizedResource, + content_root: Path, ) -> None: - roots = {self.qb_root} + roots = {content_root} for item in resource.files: - candidate = self.qb_root / PurePosixPath(item.canonical_path).parts[0] + candidate = content_root / PurePosixPath(item.canonical_path).parts[0] roots.add(candidate if candidate.is_dir() else candidate.parent) hashes = { value.lower() for value in ( @@ -928,6 +944,52 @@ class ClientJobExecutor: "safely transferred by this client" ) + def _resource_root(self, resource: NormalizedResource) -> Path: + """Resolve a qB resource's save path within this client's mounts. + + qB returns paths in its own API/container namespace. The configured + root mapping translates those paths into the client namespace and + supports nested save paths and explicit prefix overrides. A missing + save path is retained only for older in-process test fixtures; every + normalized qB API resource has one. + """ + + if resource.save_path is None: + return self.qb_root + try: + root = self.qb_roots.api_to_local(resource.save_path.as_posix()) + except ConfigError as exc: + raise JobExecutionError( + "qBittorrent resource save path is outside the configured " + "API root or has no local path mapping" + ) from exc + try: + metadata = root.lstat() + except FileNotFoundError as exc: + raise JobExecutionError( + "qBittorrent resource save path is not visible in the " + "client container" + ) from exc + if not stat.S_ISDIR(metadata.st_mode): + raise JobExecutionError( + "qBittorrent resource save path is not a real directory in " + "the client container" + ) + return root + + def _artifact_content_root(self, baseline: object) -> Path: + if not isinstance(baseline, dict): + raise JobExecutionError("target baseline is invalid") + value = baseline.get("content_root") + if not isinstance(value, str): + # A pre-existing durable baseline predates per-resource roots. + # It can only have materialized to the configured target root. + return self.qb_root + root = Path(value) + if not root.is_absolute() or root.is_symlink(): + raise JobExecutionError("target baseline content root is unsafe") + return root + def _raise_if_cancelled(self, job_id: str) -> None: event = self._cancel_events.get(job_id) if event is not None and event.is_set(): diff --git a/src/archive_clients/resources.py b/src/archive_clients/resources.py index 9e6131f..f15f54b 100644 --- a/src/archive_clients/resources.py +++ b/src/archive_clients/resources.py @@ -23,6 +23,9 @@ class NormalizedResource: files: tuple[resource_pb2.TorrentFile, ...] metainfo: Metainfo metainfo_bytes: bytes = b"" + # qBittorrent's API-visible save path is intentionally local-only. It + # must never become part of inventory or placement protocol messages. + save_path: PurePosixPath | None = None def build_content_tree( @@ -137,6 +140,7 @@ def normalize_resource( character not in "0123456789abcdef" for character in qb_torrent_id ): raise ResourceError("torrent hash is invalid") + save_path = _save_path(torrent.get("save_path")) summary = resource_pb2.ResourceSummary( qb_torrent_id=qb_torrent_id, display_name=_string(torrent.get("name"), "torrent name"), @@ -172,7 +176,9 @@ def normalize_resource( revision_data, sort_keys=True, separators=(",", ":"), ).encode("utf-8")).hexdigest() summary.observed_at.FromDatetime(observed_at) - return NormalizedResource(summary, tuple(files), metainfo, metainfo_bytes) + return NormalizedResource( + summary, tuple(files), metainfo, metainfo_bytes, save_path + ) def _set_selection(target: Any, indices: list[int]) -> None: @@ -217,6 +223,20 @@ def _path(value: Any) -> str: return candidate.as_posix() +def _save_path(value: Any) -> PurePosixPath: + """Validate qBittorrent's API-visible per-torrent content root.""" + + path = _string(value, "torrent save path") + candidate = PurePosixPath(path) + if ( + not candidate.is_absolute() + or ".." in candidate.parts + or "." in candidate.parts + ): + raise ResourceError("qBittorrent torrent save path is unsafe") + return candidate + + def _integer(value: Any, name: str) -> int: if isinstance(value, bool) or not isinstance(value, int) or value < 0: raise ResourceError(f"{name} is invalid") diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..8537afd --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,59 @@ +import io +import json +import os +import tempfile +import unittest +from contextlib import redirect_stdout +from pathlib import Path + +from archive_clients.cli import main + + +class ClientCliTests(unittest.TestCase): + def test_check_config_probes_every_qb_override_root(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + for name in ("token", "qb-password", "syncthing-key"): + path = root / name + path.write_text(name, encoding="utf-8") + os.chmod(path, 0o600) + for name in ("qb", "qb-fast", "sync", "backups"): + (root / name).mkdir() + config = root / "client.toml" + config.write_text( + f'''client_id = "cache-1" +display_name = "Cache 1" +role = "cache" +control_endpoint = "ws://control/archive_control" +shared_token_file = "{root / "token"}" +state_db = "{root / "state.db"}" +backup_dir = "{root / "backups"}" + +[qbittorrent] +endpoint = "http://qb" +username = "admin" +password_file = "{root / "qb-password"}" +api_root = "/downloads" +local_root = "{root / "qb"}" +local_path_overrides = {{ "/downloads/fast" = "{root / "qb-fast"}" }} + +[syncthing] +endpoint = "http://syncthing" +api_key_file = "{root / "syncthing-key"}" +api_root = "/sync" +local_root = "{root / "sync"}" +''', + encoding="utf-8", + ) + output = io.StringIO() + with redirect_stdout(output): + self.assertEqual(main(["--config", str(config), "--check-config"]), 0) + reported = json.loads(output.getvalue()) + self.assertEqual( + [item["root"] for item in reported["filesystems"]], + [str(root / "qb"), str(root / "qb-fast"), str(root / "sync")], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_config.py b/tests/test_config.py index a806b7b..da88144 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -78,6 +78,35 @@ class ConfigTests(unittest.TestCase): Path("/local/qb/Sync"), ) + def test_qbittorrent_can_override_a_nested_save_path(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + for name in ("token", "qb-password", "syncthing-key"): + path = root / name + path.write_text(name, encoding="utf-8") + os.chmod(path, 0o600) + (root / "qb").mkdir() + (root / "fast").mkdir() + (root / "sync").mkdir() + config_path = root / "client.toml" + config_path.write_text( + _config(root).replace( + f'local_root = "{root / "qb"}"', + f'local_root = "{root / "qb"}"\n' + "local_path_overrides = { \"/downloads/fast\" = " + f'"{root / "fast"}" }}', + 1, + ), + encoding="utf-8", + ) + config = ClientConfig.load(config_path) + self.assertEqual( + config.qbittorrent.roots.api_to_local( + "/downloads/fast/resource/file.bin" + ), + root / "fast/resource/file.bin", + ) + def test_endpoint_scheme_and_job_keys_are_strict(self): with tempfile.TemporaryDirectory() as directory: root = Path(directory) diff --git a/tests/test_deployment_preflight.py b/tests/test_deployment_preflight.py index 55d3698..33fa6dc 100644 --- a/tests/test_deployment_preflight.py +++ b/tests/test_deployment_preflight.py @@ -50,6 +50,26 @@ class DeploymentPreflightTests(unittest.TestCase): "/data/qb/.archive-control-routes", ) + def test_qb_override_must_map_to_the_same_host_path(self): + client = {"Mounts": [ + {"Type": "bind", "Source": "/srv/fast", "Destination": "/data/fast"}, + ]} + qbittorrent = {"Mounts": [ + {"Type": "bind", "Source": "/srv/fast", "Destination": "/downloads/fast"}, + ]} + preflight.require_qb_override_mappings( + client, qbittorrent, + {"local_path_overrides": {"/downloads/fast": "/data/fast"}}, + ) + with self.assertRaisesRegex(preflight.CheckFailure, "host paths differ"): + preflight.require_qb_override_mappings( + client, + {"Mounts": [{ + "Type": "bind", "Source": "/other", "Destination": "/downloads/fast", + }]}, + {"local_path_overrides": {"/downloads/fast": "/data/fast"}}, + ) + def test_runs_hardlink_probe_with_qb_and_route_roots(self): completed = __import__("subprocess").CompletedProcess( args=[], returncode=0, stdout="hard-link staging probe passed\n", stderr="" diff --git a/tests/test_eviction.py b/tests/test_eviction.py index 138d5f1..92ce62c 100644 --- a/tests/test_eviction.py +++ b/tests/test_eviction.py @@ -1,5 +1,7 @@ import tempfile import unittest +from dataclasses import replace +from pathlib import PurePosixPath from pathlib import Path from archive_clients.eviction import ( @@ -86,7 +88,7 @@ class EvictionTests(unittest.TestCase): job_id="job-1", resource=evicted, selected_indices=[0, 1], - qb_root=self.root, + content_root=self.root, store=self.store, ) remove_qb_entry( @@ -97,9 +99,9 @@ class EvictionTests(unittest.TestCase): ) result = safe_unlink( job_id="job-1", - qb_root=self.root, qbittorrent=qb, store=self.store, + resource_root=lambda _: self.root, ) self.assertTrue((self.root / "tree/shared.bin").exists()) self.assertFalse((self.root / "tree/owned.bin").exists()) @@ -121,7 +123,7 @@ class EvictionTests(unittest.TestCase): job_id="job-1", resource=evicted, selected_indices=[0], - qb_root=self.root, + content_root=self.root, store=self.store, ) path.unlink() @@ -134,9 +136,9 @@ class EvictionTests(unittest.TestCase): ) result = safe_unlink( job_id="job-1", - qb_root=self.root, qbittorrent=qb, store=self.store, + resource_root=lambda _: self.root, ) self.assertTrue(path.exists()) self.assertEqual( @@ -152,7 +154,7 @@ class EvictionTests(unittest.TestCase): job_id="job-1", resource=evicted, selected_indices=[0], - qb_root=self.root, + content_root=self.root, store=self.store, ) remove_qb_entry( @@ -163,19 +165,57 @@ class EvictionTests(unittest.TestCase): ) first = safe_unlink( job_id="job-1", - qb_root=self.root, qbittorrent=qb, store=self.store, + resource_root=lambda _: self.root, ) second = safe_unlink( job_id="job-1", - qb_root=self.root, qbittorrent=qb, store=self.store, + resource_root=lambda _: self.root, ) self.assertEqual(first, second) self.assertEqual(qb.deleted, ["a" * 40]) + def test_nested_save_path_uses_its_own_root_and_not_a_same_named_peer(self): + nested = self.root / "Downloading" + nested.mkdir() + evicted = replace( + normalized("a" * 40, ["resource/file.bin"]), + save_path=PurePosixPath("/downloads/Downloading"), + ) + peer = replace( + normalized("b" * 40, ["resource/file.bin"]), + save_path=PurePosixPath("/downloads"), + ) + nested_file = nested / "resource/file.bin" + nested_file.parent.mkdir() + nested_file.write_bytes(b"x" * evicted.files[0].logical_bytes) + root_file = self.root / "resource/file.bin" + root_file.parent.mkdir() + root_file.write_bytes(b"x" * peer.files[0].logical_bytes) + qb = FakeQB(evicted, [peer]) + + verify_and_snapshot( + job_id="job-1", resource=evicted, selected_indices=[0], + content_root=nested, store=self.store, + ) + remove_qb_entry( + job_id="job-1", torrent_hash="a" * 40, + qbittorrent=qb, store=self.store, + ) + safe_unlink( + job_id="job-1", qbittorrent=qb, store=self.store, + resource_root=lambda item: ( + nested + if item.save_path == PurePosixPath("/downloads/Downloading") + else self.root + ), + ) + self.assertFalse(nested_file.exists()) + self.assertTrue(root_file.exists()) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_jobs.py b/tests/test_jobs.py index dfa1f53..6fc315b 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -4,11 +4,13 @@ import stat import tempfile import threading import unittest -from pathlib import Path +from dataclasses import replace +from pathlib import Path, PurePosixPath from unittest.mock import Mock, patch from uuid import uuid4 from archive_clients.bencode import encode +from archive_clients.config import RootMapping from archive_clients.jobs import ( ClientJobExecutor, JobExecutionError, @@ -246,6 +248,55 @@ class ClientJobHappyPathTests(unittest.TestCase): content=b"x" * 4096, ) + def test_nested_qb_save_path_stages_from_mapped_subdirectory(self): + with tempfile.TemporaryDirectory() as directory: + self._run_transfer( + Path(directory), + job_pb2.JOB_OPERATION_UNARCHIVE, + source_save_path="/downloads/Downloading", + ) + + def test_qb_save_path_preflight_rejects_unmapped_path(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + executor = ClientJobExecutor( + client_id="cache-1", qbittorrent=Mock(), + store=ClientStore(root / "state.db"), qb_root=root, + qb_api_root=PurePosixPath("/downloads"), + route_path=lambda _: root, syncthing_transport=Mock(), + sparse_supported=True, + ) + resource = NormalizedResource( + resource_pb2.ResourceSummary(), (), Mock(), + save_path=PurePosixPath("/outside"), + ) + with self.assertRaisesRegex(JobExecutionError, "outside"): + executor._resource_root(resource) + + def test_qb_save_path_override_uses_its_dedicated_local_mount(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + primary = root / "primary" + override = root / "override" + primary.mkdir() + override.mkdir() + executor = ClientJobExecutor( + client_id="cache-1", qbittorrent=Mock(), + store=ClientStore(root / "state.db"), qb_root=primary, + qb_api_root=PurePosixPath("/downloads"), + qb_roots=RootMapping( + PurePosixPath("/downloads"), primary, + ((PurePosixPath("/downloads/slow"), override),), + ), + route_path=lambda _: root, syncthing_transport=Mock(), + sparse_supported=True, + ) + resource = NormalizedResource( + resource_pb2.ResourceSummary(), (), Mock(), + save_path=PurePosixPath("/downloads/slow"), + ) + self.assertEqual(executor._resource_root(resource), override) + def test_mount_boundary_requires_copy_space_even_with_same_device(self): with tempfile.TemporaryDirectory() as directory: root = Path(directory) @@ -390,6 +441,7 @@ class ClientJobHappyPathTests(unittest.TestCase): source_stage_free_bytes: int | None = None, content: bytes = b"archive-control-happy-path", syncthing: CompleteSyncthing | None = None, + source_save_path: str = "/downloads", ): source_root = root / "source" target_root = root / "target" @@ -397,7 +449,12 @@ class ClientJobHappyPathTests(unittest.TestCase): source_root.mkdir() target_root.mkdir() route_root.mkdir() - (source_root / "fixture.bin").write_bytes(content) + save_relative = PurePosixPath(source_save_path).relative_to( + PurePosixPath("/downloads") + ) + source_content_root = source_root.joinpath(*save_relative.parts) + source_content_root.mkdir(parents=True, exist_ok=True) + (source_content_root / "fixture.bin").write_bytes(content) info = { b"length": len(content), b"name": b"fixture.bin", @@ -411,6 +468,7 @@ class ClientJobHappyPathTests(unittest.TestCase): "hash": torrent_hash, "name": "fixture.bin", "state": "uploading", + "save_path": source_save_path, }, [{ "index": 0, @@ -454,8 +512,11 @@ class ClientJobHappyPathTests(unittest.TestCase): source_qb = Mock() source_qb.get_resource.return_value = resource target_qb = Mock() + target_resource = replace( + resource, save_path=PurePosixPath("/downloads") + ) target_qb.get_resource.side_effect = [ - None, None, resource, resource, + None, None, target_resource, target_resource, ] syncthing = syncthing or CompleteSyncthing() source = ClientJobExecutor( diff --git a/tests/test_qbittorrent.py b/tests/test_qbittorrent.py index 0744479..87c72ea 100644 --- a/tests/test_qbittorrent.py +++ b/tests/test_qbittorrent.py @@ -55,6 +55,7 @@ class QBittorrentReaderTests(unittest.TestCase): b"Ok.", json.dumps([{ "hash": torrent_hash, "name": "a.txt", "state": "uploading", + "save_path": "/downloads", }]).encode(), b'[{"index":0,"name":"a.txt","size":3,"progress":1,"priority":1}]', torrent_bytes, @@ -102,6 +103,7 @@ class QBittorrentReaderTests(unittest.TestCase): b"Ok.", json.dumps([{ "hash": qb_hash, "name": "a.txt", "state": "uploading", + "save_path": "/downloads", }]).encode(), b'[{"index":0,"name":"a.txt","size":3,"progress":1,"priority":1}]', torrent_bytes, @@ -157,6 +159,7 @@ class QBittorrentReaderTests(unittest.TestCase): "infohash_v2": v2_hash, "name": "a.txt", "state": "uploading", + "save_path": "/downloads", }]).encode(), b'[{"index":0,"name":"a.txt","size":3,"progress":1,"priority":1}]', torrent_bytes, @@ -217,9 +220,9 @@ class QBittorrentReaderTests(unittest.TestCase): malformed_bytes, malformed_hash = torrent("broken.txt", b"bad") second_bytes, second_hash = torrent("second.txt", b"two") torrents = [ - {"hash": first_hash, "name": "first.txt", "state": "uploading"}, - {"hash": malformed_hash, "name": "broken.txt", "state": "stalledUP"}, - {"hash": second_hash, "name": "second.txt", "state": "uploading"}, + {"hash": first_hash, "name": "first.txt", "state": "uploading", "save_path": "/downloads"}, + {"hash": malformed_hash, "name": "broken.txt", "state": "stalledUP", "save_path": "/downloads"}, + {"hash": second_hash, "name": "second.txt", "state": "uploading", "save_path": "/downloads"}, ] responses = [ b"Ok.", json.dumps(torrents).encode(), diff --git a/tests/test_resources.py b/tests/test_resources.py index f0fd3f3..07c9c07 100644 --- a/tests/test_resources.py +++ b/tests/test_resources.py @@ -40,6 +40,7 @@ class ResourceTests(unittest.TestCase): "hash": decoded.info_hash_v2_hex, "name": "v2.bin", "state": "stoppedUP", + "save_path": "/downloads", }, [{ "index": 0, @@ -91,6 +92,7 @@ class ResourceTests(unittest.TestCase): "hash": decoded.info_hash_v1_hex, "name": "resource", "state": "stoppedUP", + "save_path": "/downloads/nested", }, [ { @@ -106,6 +108,7 @@ class ResourceTests(unittest.TestCase): observed, ) summary = normalized.summary + self.assertEqual(normalized.save_path.as_posix(), "/downloads/nested") self.assertEqual( summary.runtime_state, resource_pb2.TORRENT_RUNTIME_STATE_STOPPED ) @@ -139,6 +142,7 @@ class ResourceTests(unittest.TestCase): "hash": torrent_hash, "name": "with-padding", "state": "stalledUP", + "save_path": "/downloads", }, [ { @@ -169,7 +173,7 @@ class ResourceTests(unittest.TestCase): metainfo = encode({b"info": info}) torrent = { "hash": hashlib.sha1(encode(info)).hexdigest(), - "name": "renamed", "state": "uploading", + "name": "renamed", "state": "uploading", "save_path": "/downloads", } renamed = normalize_resource(torrent, [{ "index": 0, "name": "renamed.txt", "size": 3, @@ -182,6 +186,23 @@ class ResourceTests(unittest.TestCase): "progress": 1.0, "priority": 1, }], metainfo) + def test_missing_or_out_of_shape_save_path_is_rejected(self): + info = { + b"length": 3, b"name": b"a.txt", b"piece length": 16384, + b"pieces": b"x" * 20, + } + metainfo = encode({b"info": info}) + torrent = { + "hash": hashlib.sha1(encode(info)).hexdigest(), + "name": "a.txt", "state": "uploading", "save_path": "relative", + } + with self.assertRaisesRegex(ResourceError, "save path is unsafe"): + normalize_resource(torrent, [{ + "index": 0, "name": "a.txt", "size": 3, + "completed": 3, "priority": 1, + }], metainfo) + + def test_noncanonical_bencode_is_rejected(self): with self.assertRaisesRegex(BencodeError, "unsorted"): decode_metainfo(b"d4:infod1:b1:x1:a1:yee")