fix: normalize permissions after qb verification
This commit is contained in:
+4
-1
@@ -125,7 +125,10 @@ and placement policy differ.
|
|||||||
and explicit save path, or apply a transactional delta to an existing
|
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,
|
torrent. Set selected/skipped state, run a full recheck of the target union,
|
||||||
and fail immediately if qBittorrent attempts to download. Successful recheck
|
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
|
5. **Staging Cleanup.** Remove only job-owned staging artifacts from both
|
||||||
endpoints. A post-commit cleanup failure produces `CLEANUP_REQUIRED`; it
|
endpoints. A post-commit cleanup failure produces `CLEANUP_REQUIRED`; it
|
||||||
never rolls back or deletes the committed placement.
|
never rolls back or deletes the committed placement.
|
||||||
|
|||||||
+1
-1
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
|
|||||||
|
|
||||||
[project]
|
[project]
|
||||||
name = "archive-clients"
|
name = "archive-clients"
|
||||||
version = "0.1.22"
|
version = "0.1.23"
|
||||||
requires-python = ">=3.11"
|
requires-python = ">=3.11"
|
||||||
dependencies = ["protobuf==7.35.1", "websockets==16.0"]
|
dependencies = ["protobuf==7.35.1", "websockets==16.0"]
|
||||||
|
|
||||||
|
|||||||
@@ -808,13 +808,14 @@ class ClientJobExecutor:
|
|||||||
fraction, 0, 0, "qBittorrent stopped recheck"
|
fraction, 0, 0, "qBittorrent stopped recheck"
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
if should_start:
|
|
||||||
self.qbittorrent.start(qb_torrent_id)
|
|
||||||
verified = self.qbittorrent.get_resource(info_hash)
|
verified = self.qbittorrent.get_resource(info_hash)
|
||||||
if verified is None:
|
if verified is None:
|
||||||
raise JobExecutionError(
|
raise JobExecutionError(
|
||||||
"verified qBittorrent resource disappeared"
|
"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(
|
placement = resource_pb2.Placement(
|
||||||
client_id=self.client_id,
|
client_id=self.client_id,
|
||||||
state=resource_pb2.PLACEMENT_STATE_PRESENT,
|
state=resource_pb2.PLACEMENT_STATE_PRESENT,
|
||||||
@@ -1191,6 +1192,78 @@ def _info_hash(definition: job_pb2.JobDefinition) -> str:
|
|||||||
return value
|
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(
|
def _fingerprint_matches(
|
||||||
current: resource_pb2.ResourceStateFingerprint,
|
current: resource_pb2.ResourceStateFingerprint,
|
||||||
expected: resource_pb2.ResourceStateFingerprint,
|
expected: resource_pb2.ResourceStateFingerprint,
|
||||||
|
|||||||
+49
-2
@@ -1,4 +1,6 @@
|
|||||||
import hashlib
|
import hashlib
|
||||||
|
import os
|
||||||
|
import stat
|
||||||
import tempfile
|
import tempfile
|
||||||
import threading
|
import threading
|
||||||
import unittest
|
import unittest
|
||||||
@@ -10,12 +12,13 @@ from archive_clients.bencode import encode
|
|||||||
from archive_clients.jobs import (
|
from archive_clients.jobs import (
|
||||||
ClientJobExecutor,
|
ClientJobExecutor,
|
||||||
JobExecutionError,
|
JobExecutionError,
|
||||||
|
_normalize_verified_resource_permissions,
|
||||||
_resource_fingerprint,
|
_resource_fingerprint,
|
||||||
)
|
)
|
||||||
from archive_clients.syncthing import RouteSetupError, SyncthingTransferStatus
|
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_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:
|
class CompleteSyncthing:
|
||||||
@@ -39,6 +42,50 @@ class SlowRescanSyncthing(CompleteSyncthing):
|
|||||||
|
|
||||||
|
|
||||||
class ClientJobHappyPathTests(unittest.TestCase):
|
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):
|
def test_syncthing_api_outage_during_transfer_is_retried(self):
|
||||||
"""A transient local REST outage must not terminally fail the job."""
|
"""A transient local REST outage must not terminally fail the job."""
|
||||||
with tempfile.TemporaryDirectory() as directory:
|
with tempfile.TemporaryDirectory() as directory:
|
||||||
|
|||||||
Reference in New Issue
Block a user