From 57acc9036312e5b1cbe5658897f4224d833fefeb Mon Sep 17 00:00:00 2001 From: Cabbagec Date: Fri, 24 Jul 2026 16:05:20 +0000 Subject: [PATCH] fix: honor hardlinks across client mount topology --- deploy/production/lithium/compose.yaml | 2 +- deploy/production/x1/client.toml | 3 ++ deploy/production/x1/compose.yaml | 3 +- deploy/production/x2/compose.yaml | 2 +- docs/deployment-and-usage.md | 7 ++- pyproject.toml | 2 +- src/archive_clients/config.py | 69 ++++++++++++++++++++++---- src/archive_clients/jobs.py | 40 +++++++++++++++ src/archive_clients/routes.py | 4 +- src/archive_clients/syncthing.py | 4 +- tests/test_config.py | 17 ++++++- tests/test_jobs.py | 19 +++++++ tests/test_routes.py | 26 ++++++++++ 13 files changed, 180 insertions(+), 18 deletions(-) diff --git a/deploy/production/lithium/compose.yaml b/deploy/production/lithium/compose.yaml index c6a50f4..6a0d049 100644 --- a/deploy/production/lithium/compose.yaml +++ b/deploy/production/lithium/compose.yaml @@ -2,7 +2,7 @@ name: archive-control-archive services: archive-client: - image: sodium/archive-clients:v0.1.5 + image: sodium/archive-clients:v0.1.6 user: "1000:1000" restart: unless-stopped command: ["--config", "/etc/archive-control/client.toml"] diff --git a/deploy/production/x1/client.toml b/deploy/production/x1/client.toml index e35a521..9f11110 100644 --- a/deploy/production/x1/client.toml +++ b/deploy/production/x1/client.toml @@ -39,4 +39,7 @@ endpoint = "http://127.0.0.1:8384" api_key_file = "/run/secrets/syncthing_api_key" api_root = "/var/syncthing" local_root = "/data/sync" +# This existing folder is physically inside the qB data tree. Map it through +# that same client bind mount so source staging can use hardlinks. +local_path_overrides = { "/var/syncthing/DownloadsSync" = "/data/qb/Sync" } advertised_addresses = ["dynamic"] diff --git a/deploy/production/x1/compose.yaml b/deploy/production/x1/compose.yaml index fd48d24..8cea0e4 100644 --- a/deploy/production/x1/compose.yaml +++ b/deploy/production/x1/compose.yaml @@ -2,7 +2,7 @@ name: archive-control-cache services: archive-client: - image: sodium/archive-clients:v0.1.5 + image: sodium/archive-clients:v0.1.6 user: "1001:1001" restart: unless-stopped network_mode: host @@ -16,4 +16,3 @@ services: - ./backups:/var/backups/archive-control - /home/ubuntu/Downloads:/data/qb - /home/ubuntu/compose/syncthing/st_home:/data/sync - - /home/ubuntu/Downloads/Sync:/data/sync/DownloadsSync diff --git a/deploy/production/x2/compose.yaml b/deploy/production/x2/compose.yaml index e08e881..b198cb1 100644 --- a/deploy/production/x2/compose.yaml +++ b/deploy/production/x2/compose.yaml @@ -2,7 +2,7 @@ name: archive-control-cache services: archive-client: - image: sodium/archive-clients:v0.1.5 + image: sodium/archive-clients:v0.1.6 user: "1001:1001" restart: unless-stopped network_mode: host diff --git a/docs/deployment-and-usage.md b/docs/deployment-and-usage.md index f4e241c..edc9bca 100644 --- a/docs/deployment-and-usage.md +++ b/docs/deployment-and-usage.md @@ -125,6 +125,11 @@ local_root = "/data/sync" advertised_addresses = ["dynamic"] ``` +When an existing Syncthing folder is physically nested in the qB data root, +use a `local_path_overrides` entry to map that exact Syncthing API path through +the same client bind mount. This enables hardlinks without creating two Docker +mount boundaries for the same host files. + The remaining node examples omit optional `[connection]`, `[jobs]`, and `[backup]` tables and therefore use these same defaults; deployments may override them per node. @@ -213,7 +218,7 @@ cache/archive routes according to policy. ```yaml services: archive-client: - image: sodium/archive-clients:v0.1.5 + image: sodium/archive-clients:v0.1.6 user: "1001:1001" restart: unless-stopped command: ["archive-client", "--config", "/etc/archive-control/client.toml"] diff --git a/pyproject.toml b/pyproject.toml index a911323..a8d995d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "archive-clients" -version = "0.1.5" +version = "0.1.6" requires-python = ">=3.11" dependencies = ["protobuf==7.35.1", "websockets==16.0"] diff --git a/src/archive_clients/config.py b/src/archive_clients/config.py index dbbbc1f..442f275 100644 --- a/src/archive_clients/config.py +++ b/src/archive_clients/config.py @@ -27,17 +27,28 @@ _ENV = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") class RootMapping: api_root: PurePosixPath local_root: Path + local_path_overrides: tuple[tuple[PurePosixPath, Path], ...] = () def api_to_local(self, api_path: str) -> Path: candidate = PurePosixPath(api_path) - try: - relative = candidate.relative_to(self.api_root) - except ValueError as exc: - raise ConfigError("API path is outside its configured root") from exc - if any(part in {"", ".", ".."} for part in relative.parts): - raise ConfigError("API path contains an unsafe component") + for api_root, local_root in self.local_path_overrides: + relative = _safe_relative(candidate, api_root) + if relative is not None: + return local_root.joinpath(*relative.parts) + relative = _safe_relative(candidate, self.api_root) + if relative is None: + raise ConfigError("API path is outside its configured root") return self.local_root.joinpath(*relative.parts) + def local_root_for_api(self, api_path: str) -> Path: + candidate = PurePosixPath(api_path) + for api_root, local_root in self.local_path_overrides: + if _safe_relative(candidate, api_root) is not None: + return local_root + if _safe_relative(candidate, self.api_root) is None: + raise ConfigError("API path is outside its configured root") + return self.local_root + @dataclass(frozen=True) class ServiceConfig: @@ -48,10 +59,13 @@ class ServiceConfig: password_file: Path | None = None api_key_file: Path | None = None advertised_addresses: tuple[str, ...] = () + local_path_overrides: tuple[tuple[PurePosixPath, Path], ...] = () @property def roots(self) -> RootMapping: - return RootMapping(self.api_root, self.local_root) + return RootMapping( + self.api_root, self.local_root, self.local_path_overrides + ) def read_password(self) -> str | None: return ( @@ -160,7 +174,7 @@ def _service(value: Any, name: str) -> ServiceConfig: raise ConfigError(f"{name} must be a table") allowed = { "endpoint", "api_root", "local_root", "username", "password_file", - "api_key_file", "advertised_addresses", + "api_key_file", "advertised_addresses", "local_path_overrides", } _keys(value, allowed, name) api_root = PurePosixPath(_string(value, "api_root")) @@ -182,6 +196,7 @@ def _service(value: Any, name: str) -> ServiceConfig: raise ConfigError("qbittorrent username and password_file are required") if name == "syncthing" and "api_key_file" not in value: raise ConfigError("syncthing api_key_file is required") + overrides = _local_path_overrides(value, api_root, name) return ServiceConfig( _endpoint(value, "endpoint", {"http", "https"}), api_root, _absolute_path(value, "local_root"), username, @@ -189,10 +204,46 @@ def _service(value: Any, name: str) -> ServiceConfig: if "password_file" in value else None, _absolute_path(value, "api_key_file") if "api_key_file" in value else None, - tuple(addresses), + tuple(addresses), overrides, ) +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]] = [] + for raw_api_path, raw_local_path in raw.items(): + if not isinstance(raw_api_path, str) or not isinstance(raw_local_path, str): + raise ConfigError(f"{name}.local_path_overrides entries must be strings") + candidate = PurePosixPath(raw_api_path) + if not candidate.is_absolute() or ".." in candidate.parts: + raise ConfigError(f"{name}.local_path_overrides API path is invalid") + if _safe_relative(candidate, api_root) is None: + raise ConfigError(f"{name}.local_path_overrides API path is outside root") + local = Path(raw_local_path) + if not local.is_absolute(): + raise ConfigError(f"{name}.local_path_overrides local path is invalid") + parsed.append((candidate, local)) + return tuple(sorted(parsed, key=lambda item: len(item[0].parts), reverse=True)) + + +def _safe_relative( + candidate: PurePosixPath, root: PurePosixPath +) -> PurePosixPath | None: + try: + relative = candidate.relative_to(root) + except ValueError: + return None + if any(part in {"", ".", ".."} for part in relative.parts): + raise ConfigError("API path contains an unsafe component") + return relative + + def _connection(value: Any) -> ConnectionConfig: if not isinstance(value, dict): raise ConfigError("connection must be a table") diff --git a/src/archive_clients/jobs.py b/src/archive_clients/jobs.py index e7d0a85..d1dba66 100644 --- a/src/archive_clients/jobs.py +++ b/src/archive_clients/jobs.py @@ -4,6 +4,7 @@ from __future__ import annotations import hashlib import json +import os import shutil import stat import threading @@ -893,6 +894,7 @@ class ClientJobExecutor: if ( not stat.S_ISREG(metadata.st_mode) or metadata.st_dev != destination_device + or _mount_id(source) != _mount_id(destination_root) ): required += logical_bytes return required @@ -1150,3 +1152,41 @@ def _job_error_code(error: Exception) -> int: if isinstance(error, EvictionError): return common_pb2.ERROR_CODE_PRECONDITION_FAILED return common_pb2.ERROR_CODE_INTERNAL + + +def _mount_id(path: Path) -> str | None: + """Return Linux's effective mount ID for a path when procfs is available. + + Bind mounts can share ``st_dev`` while still rejecting ``link(2)`` with + ``EXDEV``. Mount IDs distinguish that case without creating probe files + inside a Syncthing folder. + """ + + try: + target = os.path.realpath(path) + best: tuple[int, str] | None = None + with open("/proc/self/mountinfo", encoding="utf-8") as source: + for line in source: + fields = line.rstrip("\n").split(" ") + if len(fields) < 5: + continue + mountpoint = _unescape_mount_path(fields[4]) + if target != mountpoint and not target.startswith( + mountpoint.rstrip("/") + "/" + ): + continue + candidate = (len(mountpoint), fields[0]) + if best is None or candidate[0] > best[0]: + best = candidate + return None if best is None else best[1] + except OSError: + return None + + +def _unescape_mount_path(value: str) -> str: + return ( + value.replace("\\040", " ") + .replace("\\011", "\t") + .replace("\\012", "\n") + .replace("\\134", "\\") + ) diff --git a/src/archive_clients/routes.py b/src/archive_clients/routes.py index 825e088..234ca59 100644 --- a/src/archive_clients/routes.py +++ b/src/archive_clients/routes.py @@ -37,7 +37,9 @@ def discover_routes( relative = normalized_api_path.relative_to(roots.api_root) except (ConfigError, ValueError): continue - if not _lexically_within(local_path, roots.local_root): + if not _lexically_within( + local_path, roots.local_root_for_api(normalized_api_path.as_posix()) + ): continue if relative == PurePosixPath("."): continue diff --git a/src/archive_clients/syncthing.py b/src/archive_clients/syncthing.py index 01c6be1..4896b15 100644 --- a/src/archive_clients/syncthing.py +++ b/src/archive_clients/syncthing.py @@ -356,7 +356,9 @@ class SyncthingRouteManager: local_path = self.config.roots.api_to_local(api_path) except ConfigError as exc: raise RoutePathConflict("route path is outside the sync root") from exc - resolved_root = self.config.local_root.resolve(strict=False) + resolved_root = self.config.roots.local_root_for_api(api_path).resolve( + strict=False + ) try: local_path.resolve(strict=False).relative_to(resolved_root) except ValueError as exc: diff --git a/tests/test_config.py b/tests/test_config.py index 0bb6562..a806b7b 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,7 +1,7 @@ import os import tempfile import unittest -from pathlib import Path +from pathlib import Path, PurePosixPath from unittest.mock import patch from archive_clients.config import ClientConfig, ConfigError, RootMapping @@ -63,6 +63,21 @@ class ConfigTests(unittest.TestCase): with self.assertRaises(ConfigError): mapping.api_to_local("/elsewhere/file") + def test_mapping_can_override_one_syncthing_folder_locally(self): + mapping = RootMapping( + PurePosixPath("/sync"), + Path("/local/sync"), + ((PurePosixPath("/sync/DownloadsSync"), Path("/local/qb/Sync")),), + ) + self.assertEqual( + mapping.api_to_local("/sync/DownloadsSync/job/ready.json"), + Path("/local/qb/Sync/job/ready.json"), + ) + self.assertEqual( + mapping.local_root_for_api("/sync/DownloadsSync"), + Path("/local/qb/Sync"), + ) + def test_endpoint_scheme_and_job_keys_are_strict(self): with tempfile.TemporaryDirectory() as directory: root = Path(directory) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 49af0e4..a2da00b 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -78,6 +78,25 @@ class ClientJobHappyPathTests(unittest.TestCase): content=b"x" * 4096, ) + def test_mount_boundary_requires_copy_space_even_with_same_device(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + source_root = root / "source" + destination_root = root / "destination" + source_root.mkdir() + destination_root.mkdir() + (source_root / "fixture.bin").write_bytes(b"fixture") + with patch( + "archive_clients.jobs._mount_id", + side_effect=("source-mount", "destination-mount"), + ): + required = ClientJobExecutor._copy_required_bytes( + source_root, + destination_root, + (("fixture.bin", 7),), + ) + self.assertEqual(required, 7) + def _run_transfer( self, root: Path, diff --git a/tests/test_routes.py b/tests/test_routes.py index 4fa28be..2c4f7ab 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -62,6 +62,32 @@ class RouteDiscoveryTests(unittest.TestCase): self.assertEqual(routes[0].local_relative_path, "DownloadsSync") self.assertEqual(routes[0].state, route_pb2.ROUTE_STATE_DISCOVERED) + def test_discovery_accepts_a_safe_local_folder_override(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + override = root / "qb/Sync" + override.mkdir(parents=True) + roots = RootMapping( + PurePosixPath("/var/syncthing"), + root / "sync", + ((PurePosixPath("/var/syncthing/DownloadsSync"), override),), + ) + routes = discover_routes( + {"folders": [{ + "id": "DownloadsSync", + "path": "~/DownloadsSync", + "type": "sendreceive", + "devices": [ + {"deviceID": "LOCAL"}, + {"deviceID": "ARCHIVE"}, + ], + }]}, + "LOCAL", + roots, + True, + ) + self.assertEqual([route.route_id for route in routes], ["DownloadsSync"]) + if __name__ == "__main__": unittest.main()