From 5d379c7e528b69d8d4cce0f6bb26e6f9f9c1a453 Mon Sep 17 00:00:00 2001 From: ofhd Date: Tue, 22 Sep 2026 06:29:16 -0700 Subject: [PATCH] Recover automatically when the suite cache is protected against the user An extraction folder left by an elevated or foreign-account run stays unreadable and undeletable for the normal user, so rc.3 only failed faster with instructions to delete it using administrator rights. Now the verified bytes go one step further: after the full archive, manifest, notice and clip verification passes, the client atomically installs the extraction into a deterministic writable location beside the blocked ".suite-pack" subtree, announces the recovery in the preparation event stream (GUI event log and CLI), and reuses that copy byte-for-byte on later starts. Nothing protected is deleted, taken over or trusted; when even the alternate location cannot be written the error names the actual permission cause and both paths. Freeze 1.3.0-rc.4 / client/0.3.3; protocol 7.1, admission minimum, suite bytes, frozen fingerprints and scientific settings unchanged. --- CHANGELOG.md | 14 +++ README.md | 2 +- client/main.py | 5 +- client/suite.py | 109 +++++++++++++++------- client/tests/test_encoding_regressions.py | 2 +- client/tests/test_release_preflight.py | 2 +- client/tests/test_suite_v1.py | 74 ++++++++++++--- client/tests/test_windows_gui.py | 4 + client/windows_gui.py | 4 + frontend/app/run/page.test.tsx | 4 +- frontend/app/run/page.tsx | 2 +- frontend/app/run/releaseAssets.ts | 61 ++++++------ release.json | 4 +- 13 files changed, 200 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75fb2788..795267ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file. The format is based on Keep a Changelog, and this repository uses date-stamped release notes until a stricter semver/tagging policy is formalized. +## [1.3.0-rc.4] - 2026-09-22 + +Windows-focused repair with automatic recovery. A suite-cache extraction folder +that the normal user cannot read or replace (for example one left by an +administrator-privileged run) no longer requires manual deletion: after full +archive/manifest/clip verification the client installs a verified copy at a +deterministic writable location beside the blocked folder, announces the +recovery in the preparation event log, and reuses it byte-for-byte on later +runs. The blocked folder is never deleted, taken over or trusted. If neither +location is writable the failure names the actual permission error and the +cache path. The macOS and Linux binaries are byte-identical to the accepted +rc.2 builds. Client 0.3.3; protocol 7.1, admission minimum, suite bytes, +frozen fingerprints and scientific settings unchanged. + ## [1.3.0-rc.3] - 2026-09-22 Windows-only repair. The packaged GUI now names every pre-encode failure's diff --git a/README.md b/README.md index 428571c4..f99b4c4a 100644 --- a/README.md +++ b/README.md @@ -280,7 +280,7 @@ V7 artifact authorization uses `ARTIFACT_UPLOAD_SECRET` only on the server to si ## Version identities -- Published project release: `1.3.0-rc.3` / `2026-09-22` in `release.json`; client/0.3.2, protocol `7.1`. +- Published project release: `1.3.0-rc.4` / `2026-09-22` in `release.json`; client/0.3.3, protocol `7.1`. - Historical project release: `1.2.0` / `client/0.2.0`, protocol `7.0` (historical timing). - Candidate client implementation/minimum version: `client/0.3.0`. - Candidate benchmark protocol version: `7.1`, timer boundary `ffmpeg-process-v1`. diff --git a/client/main.py b/client/main.py index d0df2b36..04c76d33 100644 --- a/client/main.py +++ b/client/main.py @@ -107,7 +107,7 @@ print_info, print_success, print_warning, print_error, print_batch_summary, ) -CLIENT_VERSION = "client/0.3.2" +CLIENT_VERSION = "client/0.3.3" # UI/package patches do not change the server's frozen protocol 7.1 contract. PROTOCOL_MINIMUM_CLIENT_VERSION = "client/0.3.0" PUBLICATION_CONSENT_VERSION = 1 @@ -290,6 +290,9 @@ def wrapped(*args, **kwargs): def progress(stage, **details): _emit_event(sink, "preparation_progress", scope="preparation", stage=stage, **details) if sink is None: + if stage == "recovery" and details.get("message"): + print_info(f"Cache recovery: {details['message']}") + return label = details.get("clipId") or os.path.basename(str(details.get("path") or "")) done, total = details.get("completedBytes"), details.get("totalBytes") amount = f" ({done}/{total} bytes)" if done is not None and total else "" diff --git a/client/suite.py b/client/suite.py index db16f590..c4ddfa69 100644 --- a/client/suite.py +++ b/client/suite.py @@ -969,9 +969,9 @@ def _copy_preparation_file(source, destination): def _suite_pack_target_access_error(target_root: str) -> Optional[str]: """Explain why an existing extracted root can be neither reused nor replaced. - A cache created by a different account or an administrator-privileged run is - invisible to os.path.exists and undeletable by the normal user; returning a - cause here prevents a multi-gigabyte re-extraction that cannot land anyway. + A folder write-protected against the current account is invisible to + os.path.exists and undeletable or unreadable once found. The caller recovers + into a writable alternate location; this only names the observed cause. """ if not os.path.isdir(target_root): return None @@ -980,42 +980,62 @@ def _suite_pack_target_access_error(target_root: str) -> Optional[str]: for _ in entries: break except PermissionError as exc: - return ( - f"suite cache at {target_root} exists but is not accessible to the current user ({exc}); " - "it was created by a different account or an administrator-privileged run - delete that " - "cache folder, then start the run again" - ) + return f"the existing cache folder {target_root} is not readable by the current user ({exc})" except OSError as exc: - return ( - f"suite cache at {target_root} cannot be inspected ({exc}); delete that cache folder, " - "then start the run again" - ) + return f"the existing cache folder {target_root} cannot be inspected ({exc})" try: with open(os.path.join(target_root, "manifest.json"), "rb"): return None except FileNotFoundError: return None except OSError as exc: - return ( - f"suite cache at {target_root} cannot be read ({exc}); it was likely created by a " - "different account or an administrator-privileged run - delete that cache folder, then " - "start the run again" - ) + return f"the existing cache folder {target_root} cannot be read ({exc})" + + +def _suite_pack_recovered_root(pack_metadata: Mapping[str, Any], cache_root: Optional[str] = None) -> str: + """Deterministic user-writable home for verified content when the primary + extraction location is blocked by ACLs or locks. It lives beside, never inside, + the blocked ".suite-pack" subtree, under the same suite cache root.""" + suite_fingerprint = str(pack_metadata.get("suiteFingerprint") or "").strip() + if not suite_fingerprint: + raise RuntimeError("suite pack metadata is missing suiteFingerprint") + return os.path.join(cache_root or _suite_cache_root(), ".suite-pack-recovered", suite_fingerprint) def _extract_suite_pack(pack_path: str, metadata: Mapping[str, Any], cache_root: Optional[str] = None) -> str: target_root = _suite_pack_extract_root(metadata, cache_root) canonical_root = os.path.join(target_root, "canonical") + recovered_root = _suite_pack_recovered_root(metadata, cache_root) + primary_error = _suite_pack_target_access_error(target_root) + if primary_error is None: + try: + _verify_extracted_suite_pack(target_root, metadata, verify_media=False) + return canonical_root + except Exception: + pass + else: + # A previous run may already have installed the verified recovered copy; reuse + # it byte-for-byte without re-extracting. + try: + _verify_extracted_suite_pack(recovered_root, metadata, verify_media=False) + except Exception: + pass + else: + preparation_progress( + "recovery", path=recovered_root, + message=f"reusing the verified recovered suite cache copy at {recovered_root}", + ) + return os.path.join(recovered_root, "canonical") + staging_parent = os.path.dirname(recovered_root) if primary_error else os.path.dirname(target_root) try: - _verify_extracted_suite_pack(target_root, metadata, verify_media=False) - return canonical_root - except Exception: - access_error = _suite_pack_target_access_error(target_root) - if access_error: - raise RuntimeError(access_error) from None - parent_dir = os.path.dirname(target_root) - os.makedirs(parent_dir, exist_ok=True) - staging_root = tempfile.mkdtemp(prefix="suite-pack-", dir=parent_dir) + os.makedirs(staging_parent, exist_ok=True) + except OSError as exc: + raise RuntimeError( + f"the configured suite cache under {cache_root or _suite_cache_root()} is not writable by " + f"the current user ({exc}); point ENCODINGDB_SUITE_CACHE_DIR at a writable folder and " + "start the run again" + ) from exc + staging_root = tempfile.mkdtemp(prefix="suite-pack-", dir=staging_parent) try: with tarfile.open(pack_path, "r:gz") as archive: for member in archive: @@ -1028,21 +1048,38 @@ def _extract_suite_pack(pack_path: str, metadata: Mapping[str, Any], cache_root: with archive.extractfile(member) as source, open(destination, "xb") as target: _copy_preparation_stream(source, target, path=member.name, total=member.size) _verify_extracted_suite_pack(staging_root, metadata) + if primary_error is None: + try: + if os.path.isdir(target_root): + shutil.rmtree(target_root) + os.replace(staging_root, target_root) + return canonical_root + except OSError as swap_exc: + primary_error = ( + f"the existing cache folder {target_root} could not be replaced ({swap_exc}); " + "it is held open or write-protected for this account" + ) try: - if os.path.isdir(target_root): - shutil.rmtree(target_root) - os.replace(staging_root, target_root) - except OSError as swap_exc: + os.makedirs(os.path.dirname(recovered_root), exist_ok=True) + if os.path.isdir(recovered_root): + shutil.rmtree(recovered_root) + os.replace(staging_root, recovered_root) + except OSError as recovered_exc: raise RuntimeError( - f"verified suite content could not be installed into {target_root} because the " - f"existing cache folder could not be replaced ({swap_exc}); close any program " - "holding that folder (for example an Explorer window), delete it if asked, and " - "start the run again" - ) from swap_exc + "verified suite content could not be installed into the primary cache " + f"(after re-extraction: {primary_error}) and could not be written to the recovered " + f"cache location {recovered_root} ({recovered_exc}) either; configure a writable " + "ENCODINGDB_SUITE_CACHE_DIR and start the run again" + ) from recovered_exc + preparation_progress( + "recovery", path=recovered_root, + message=f"the primary suite cache could not be used ({primary_error}); installed a fully " + f"verified copy at {recovered_root} and will reuse it on future runs", + ) + return os.path.join(recovered_root, "canonical") except BaseException: shutil.rmtree(staging_root, ignore_errors=True) raise - return os.path.join(target_root, "canonical") def _load_requests(): diff --git a/client/tests/test_encoding_regressions.py b/client/tests/test_encoding_regressions.py index 9b6e32d6..6ac58cd9 100644 --- a/client/tests/test_encoding_regressions.py +++ b/client/tests/test_encoding_regressions.py @@ -18,7 +18,7 @@ def setUp(self) -> None: def test_corrected_metrics_use_distinguishable_client_version(self) -> None: from client import main as client_main - self.assertEqual(client_main.CLIENT_VERSION, "client/0.3.2") + self.assertEqual(client_main.CLIENT_VERSION, "client/0.3.3") self.assertEqual(client_main.PROTOCOL_MINIMUM_CLIENT_VERSION, "client/0.3.0") def test_vmaf_passes_distorted_input_before_reference(self) -> None: diff --git a/client/tests/test_release_preflight.py b/client/tests/test_release_preflight.py index df96b67f..91ee0cd1 100644 --- a/client/tests/test_release_preflight.py +++ b/client/tests/test_release_preflight.py @@ -58,7 +58,7 @@ def test_release_json_declares_coherent_frozen_release(self) -> None: payload = json.loads((release_manifest_lib.ROOT_DIR / "release.json").read_text(encoding="utf-8")) self.assertEqual(payload["suiteVersion"], "encodingdb-test-suite-v1") - self.assertEqual(payload["projectVersion"], "1.3.0-rc.3") + self.assertEqual(payload["projectVersion"], "1.3.0-rc.4") self.assertEqual(payload["releaseDate"], "2026-09-22") for tree in ("client", "server"): root = release_manifest_lib.ROOT_DIR / tree / "resources/test_suite_v1" diff --git a/client/tests/test_suite_v1.py b/client/tests/test_suite_v1.py index 9d899e22..52e888bc 100644 --- a/client/tests/test_suite_v1.py +++ b/client/tests/test_suite_v1.py @@ -292,28 +292,44 @@ def _fixture_pack(self, directory: str): suite.build_suite_pack_archive(str(root), str(archive)) return archive, metadata - def test_unreachable_stale_cache_reports_ownership_without_reextracting(self): - # A cache subtree created by an administrator-privileged run is invisible to - # os.path.exists and undeletable by the normal user; the client must name that - # cause instead of re-extracting gigabytes that can never be installed. + def test_unreachable_stale_cache_recovers_into_writable_alternate(self): + # A primary extraction protected against this account must not stop the run: + # verified bytes install into the writable recovered location, the blocked + # folder stays byte-identical, and the next start reuses the recovered copy. + import os + real_scandir = os.scandir with tempfile.TemporaryDirectory() as directory: archive, metadata = self._fixture_pack(directory) target = Path(suite._suite_pack_extract_root(metadata, directory)) + recovered = Path(suite._suite_pack_recovered_root(metadata, directory)) target.mkdir(parents=True) (target / "manifest.json").write_text("stale bytes") - with mock.patch.object(suite.os, "scandir", side_effect=PermissionError(13, "Access is denied")), \ - mock.patch.object(tarfile, "open", side_effect=AssertionError("must not re-extract an unreachable cache")): - with self.assertRaisesRegex(RuntimeError, "administrator-privileged") as raised: - suite._extract_suite_pack(str(archive), metadata, directory) - self.assertIn(str(target), str(raised.exception)) + def deny_primary(path, *args, **kwargs): + if Path(path) == target: + raise PermissionError(13, "Access is denied") + return real_scandir(path, *args, **kwargs) + events = [] + with mock.patch.object(suite.os, "scandir", side_effect=deny_primary), \ + mock.patch.object(suite, "verify_suite_clip", return_value=suite.ClipVerificationResult(True, "fixture media verification", {})), \ + mock.patch.object(suite, "preparation_progress", side_effect=lambda stage, **d: events.append((stage, d))): + canonical = Path(suite._extract_suite_pack(str(archive), metadata, directory)) + self.assertEqual(canonical, recovered / "canonical") + self.assertEqual((canonical.parent / "manifest.json").read_text(), + (Path(directory) / "source" / "manifest.json").read_text()) + with mock.patch.object(tarfile, "open", side_effect=AssertionError("recovered copy must be reused without re-extracting")): + again = Path(suite._extract_suite_pack(str(archive), metadata, directory)) + self.assertEqual(again, canonical) self.assertEqual((target / "manifest.json").read_text(), "stale bytes") + self.assertTrue(any(stage == "recovery" and "recovered" in str(details.get("message")) for stage, details in events)) + self.assertEqual(list(recovered.parent.glob("suite-pack-*")), []) - def test_locked_target_swap_reports_actionable_error_and_cleans_staging(self): + def test_locked_target_swap_recovers_into_writable_alternate(self): import shutil real_rmtree = shutil.rmtree with tempfile.TemporaryDirectory() as directory: archive, metadata = self._fixture_pack(directory) target = Path(suite._suite_pack_extract_root(metadata, directory)) + recovered = Path(suite._suite_pack_recovered_root(metadata, directory)) with mock.patch.object(suite, "verify_suite_clip", return_value=suite.ClipVerificationResult(True, "fixture media verification", {})): canonical = Path(suite._extract_suite_pack(str(archive), metadata, directory)) (canonical.parent / "manifest.json").write_text("corrupt") # force fast-path miss @@ -321,13 +337,41 @@ def deny_target(path, *args, **kwargs): if Path(path) == target: raise PermissionError(13, "Access is denied") return real_rmtree(path, *args, **kwargs) - with mock.patch.object(suite.shutil, "rmtree", side_effect=deny_target): - with self.assertRaisesRegex(RuntimeError, "could not be replaced") as raised: - suite._extract_suite_pack(str(archive), metadata, directory) - self.assertIn("Explorer", str(raised.exception)) + recovered_canonical = Path(suite._extract_suite_pack(str(archive), metadata, directory)) + self.assertEqual(recovered_canonical, recovered / "canonical") + self.assertEqual((recovered_canonical.parent / "manifest.json").read_text(), + (Path(directory) / "source" / "manifest.json").read_text()) self.assertEqual(list(target.parent.glob("suite-pack-*")), []) - self.assertTrue((target / "manifest.json").exists()) + self.assertEqual(list(recovered.parent.glob("suite-pack-*")), []) + self.assertEqual((target / "manifest.json").read_text(), "corrupt") + + def test_unwritable_cache_reports_actual_permission_cause(self): + # No primary, no recovered location: fail clearly naming the cache folder and + # the permission error instead of claiming a different extraction would work. + import os + real_scandir = os.scandir + real_makedirs = os.makedirs + with tempfile.TemporaryDirectory() as directory: + archive, metadata = self._fixture_pack(directory) + target = Path(suite._suite_pack_extract_root(metadata, directory)) + recovered_parent = Path(suite._suite_pack_recovered_root(metadata, directory)).parent + target.mkdir(parents=True) + (target / "manifest.json").write_text("stale bytes") + def deny_primary(path, *args, **kwargs): + if Path(path) == target: + raise PermissionError(13, "Access is denied") + return real_scandir(path, *args, **kwargs) + def deny_recovered(path, *args, **kwargs): + if Path(path) == recovered_parent: + raise PermissionError(13, "Access is denied") + return real_makedirs(path, *args, **kwargs) + with mock.patch.object(suite.os, "scandir", side_effect=deny_primary), \ + mock.patch.object(suite.os, "makedirs", side_effect=deny_recovered): + with self.assertRaisesRegex(RuntimeError, "not writable") as raised: + suite._extract_suite_pack(str(archive), metadata, directory) + self.assertIn(directory, str(raised.exception)) + self.assertIn("Access is denied", str(raised.exception)) def test_cached_pack_checks_all_bytes_without_reprobing_and_repairs_corruption(self): with small_media_fixture() as (root, manifest): diff --git a/client/tests/test_windows_gui.py b/client/tests/test_windows_gui.py index e6b5e9d8..2c85de48 100644 --- a/client/tests/test_windows_gui.py +++ b/client/tests/test_windows_gui.py @@ -120,6 +120,10 @@ def test_initialized_controls_and_keyboard_handlers_use_real_running_guards(self app._handle_event({"type": "preparation_progress", "stage": "probe", "path": "test.mkv"}) self.assertEqual(app.stage_var.get(), "Preparing: probe") self.assertIn("Stop is available", app.summary_var.get()) + app._handle_event({"type": "preparation_progress", "stage": "recovery", + "path": "recovered", "message": "installed a fully verified copy"}) + self.assertIn("Suite cache recovery active", app.summary_var.get()) + self.assertNotEqual(app.stage_var.get(), "Preparing: probe") self.assertIn("Alt+B", app.start_btn.options["text"]) self.assertNotIn("", bindings) self.assertIn("Alt+S", app.stop_btn.options["text"]) diff --git a/client/windows_gui.py b/client/windows_gui.py index d5a37085..2db1c275 100644 --- a/client/windows_gui.py +++ b/client/windows_gui.py @@ -512,6 +512,10 @@ def _handle_event(self, event: Dict[str, Any]) -> None: if event_type == "preparation_progress": stage = str(event.get("stage") or "source") self.stage_var.set(f"Preparing: {stage}") + if stage == "recovery" and event.get("message"): + self.summary_var.set("Suite cache recovery active; using a verified writable copy") + self._append_log(f"Cache recovery: {event['message']}") + return label = event.get("clipId") or os.path.basename(str(event.get("path") or "")) done, total = event.get("completedBytes"), event.get("totalBytes") amount = f" ({done}/{total} bytes)" if done is not None and total else "" diff --git a/frontend/app/run/page.test.tsx b/frontend/app/run/page.test.tsx index defc3d9d..c02ff2db 100644 --- a/frontend/app/run/page.test.tsx +++ b/frontend/app/run/page.test.tsx @@ -43,7 +43,9 @@ describe("RunPage", () => { for (const asset of primaryAssets) { expect(screen.getAllByText(asset.file).length).toBeGreaterThan(0); expect(screen.getAllByText(/pending publication/).length).toBeGreaterThan(0); - expect(screen.getAllByText(new RegExp(asset.sha256 ?? "NEVER")).length).toBeGreaterThan(0); + if (asset.sha256) { + expect(screen.getAllByText(new RegExp(asset.sha256)).length).toBeGreaterThan(0); + } } expect(screen.getByRole("heading", { level: 1, name: "Contribute your results." })).toBeInTheDocument(); expect(document.querySelector(`a[href*='download/${projectTag}/']`)).toBeNull(); diff --git a/frontend/app/run/page.tsx b/frontend/app/run/page.tsx index e962424d..92d95411 100644 --- a/frontend/app/run/page.tsx +++ b/frontend/app/run/page.tsx @@ -67,7 +67,7 @@ export default function RunPage() {

Windows: SmartScreen warns because the executable is unsigned. Verify the SHA-256 above first, and continue only if you trust the source; the page gives no bypass tool or automation.

Superseded packaged builds ({supersededTag})

-

The {supersededTag} Windows build reported preparation failures as a bare exit code and could loop against a cache folder owned by another account; it is superseded by {projectTag}. Its macOS and Linux binaries are byte-identical to the current ones.

+

The {supersededTag} Windows build names every failure cause but stops at a cache folder protected against the current user until that folder is deleted with administrator rights; {projectTag} recovers automatically instead, with no manual repair. Its macOS and Linux binaries are byte-identical to the current ones.