From b0f373997fda14ea2dd554f24755e4682b7602ba Mon Sep 17 00:00:00 2001 From: alexeybe1kin <210597588+alexeybe1kin@users.noreply.github.com> Date: Sat, 31 Oct 2026 11:54:58 +0200 Subject: [PATCH 1/2] Backup check reads each archive once, not twice GET /backups hashed every snapshot file in full and then read every archive member's payload to the end, doubling disk reads of large model archives (Ollama, Qdrant) on each request. The hash already proves the bytes match the manifest, and the tar header walk rejects the same truncated archives, so members are now checked by their headers plus the first 16 bytes of each required SQLite store. --- CHANGELOG.md | 6 ++++++ docs/backups.md | 6 ++++-- systemgate/backups.py | 9 +++++---- tests/test_backups.py | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8db0ab9..a4dcb69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ replacing a module has to be a decision with visible consequences. ## Unreleased +- **Backup verification reads each archive once instead of twice.** `GET /backups` hashed every + snapshot file and then read every archive member's payload to the end, doubling the disk reads + of large model archives on each request. The hash already proves the bytes match the manifest, + and the tar header walk rejects the same truncated archives, so members are now checked by + their headers, plus the first 16 bytes of each required SQLite store. + - **`/processes` no longer returns command lines, and wrong keys no longer cost PBKDF2.** `/processes` listed each host process's command line, which can carry a password or a key; it now returns the same metadata as `/runtime` (pid, name, user, CPU, memory). diff --git a/docs/backups.md b/docs/backups.md index 91839a1..e66691b 100644 --- a/docs/backups.md +++ b/docs/backups.md @@ -15,8 +15,10 @@ never extracted. A snapshot changing during verification is rejected. `unavailable`, an empty root reports `empty`, and rejected candidates make the response `degraded`. An incomplete directory does not become a recovery point merely because it has a recent name. -Verification reads the snapshot files in full on each request; large model archives can make -this endpoint slow. Poll it deliberately rather than at dashboard animation frequency. Responses +Verification hashes each snapshot file in full once per request; archive members are checked by +their headers, plus the first 16 bytes of each required SQLite store. Large model archives can +still make this endpoint slow. Poll it deliberately rather than at dashboard animation frequency. +Responses include `checked_at` and per-snapshot `verified_at`; there is no stale success cache. Hashes establish consistency with the manifest, not authenticity against an attacker who can rewrite both. `restore_status: not_tested` is explicit: this telemetry does not test decryption, restore a stack, diff --git a/systemgate/backups.py b/systemgate/backups.py index caa11b9..9490200 100644 --- a/systemgate/backups.py +++ b/systemgate/backups.py @@ -76,12 +76,13 @@ def _archive(path: Path, required: str | None) -> None: seen.add(member.name) if member.name == required and not member.isfile(): _invalid("Required SQLite store is not a file; obtain an intact snapshot.") - if member.isfile(): + # The manifest hash already proved these bytes, and walking to the + # next header rejects archives cut inside a member's data, so only + # the required store's SQLite magic needs a payload read. + if member.isfile() and member.name == required: with archive.extractfile(member) as stream: - if member.name == required and stream.read(16) != b"SQLite format 3\x00": + if stream.read(16) != b"SQLite format 3\x00": _invalid("Required SQLite store is invalid; obtain an intact snapshot.") - while stream.read(1024 * 1024): - pass if required and required not in seen: _invalid("Required SQLite store is missing; obtain an intact snapshot.") diff --git a/tests/test_backups.py b/tests/test_backups.py index d291f8f..8f35662 100644 --- a/tests/test_backups.py +++ b/tests/test_backups.py @@ -131,6 +131,42 @@ def test_even_hash_matching_unsafe_archives_fail(tmp_path, member_name, kind): assert backups.backup_status(tmp_path)["latest"] is None +def test_a_truncated_archive_is_never_a_recovery_point(tmp_path): + directory = snapshot(tmp_path) + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w") as archive: + payload = b"SQLite format 3\x00" + b"x" * 5000 + member = tarfile.TarInfo("pi.db") + member.size = len(payload) + archive.addfile(member, io.BytesIO(payload)) + tail = tarfile.TarInfo("data") + tail.size = 1024 + archive.addfile(tail, io.BytesIO(b"y" * 1024)) + data = buffer.getvalue() + # pi.db's 5016 bytes occupy 5120 padded, so the cut lands 100 bytes into the + # second member's payload. + (directory / "pi.tar").write_bytes(data[:512 + 5120 + 512 + 100]) + save_manifest(directory) + result = backups.backup_status(tmp_path) + assert result["latest"] is None + assert [row["name"] for row in result["rejected"]] == ["snapshot-a"] + + +def test_only_the_required_store_payload_is_opened(tmp_path, monkeypatch): + snapshot(tmp_path) + opened = [] + original = tarfile.TarFile.extractfile + + def record(archive, member): + opened.append(member.name) + return original(archive, member) + + monkeypatch.setattr(tarfile.TarFile, "extractfile", record) + result = backups.backup_status(tmp_path) + assert result["latest"] is not None and result["latest"]["status"] == "verified" + assert sorted(opened) == ["pi.db", "toolgate.db"] + + def test_read_errors_are_unverifiable_without_leaking_payload(tmp_path, monkeypatch): snapshot(tmp_path) monkeypatch.setattr(backups, "_hash", lambda path: (_ for _ in ()).throw( From 5894b1bdc23f2b43408b8e7e0fb3ffec0a7ec794 Mon Sep 17 00:00:00 2001 From: alexeybe1kin <210597588+alexeybe1kin@users.noreply.github.com> Date: Sat, 31 Oct 2026 12:07:54 +0200 Subject: [PATCH 2/2] Test that a required store must start with the SQLite magic No test failed when the magic check was dropped, before or after the read was narrowed to that one member. Also reflow the docs paragraph. Co-Authored-By: Claude Opus 5.5 --- docs/backups.md | 3 +-- tests/test_backups.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/docs/backups.md b/docs/backups.md index e66691b..2b19ed5 100644 --- a/docs/backups.md +++ b/docs/backups.md @@ -18,8 +18,7 @@ An incomplete directory does not become a recovery point merely because it has a Verification hashes each snapshot file in full once per request; archive members are checked by their headers, plus the first 16 bytes of each required SQLite store. Large model archives can still make this endpoint slow. Poll it deliberately rather than at dashboard animation frequency. -Responses -include `checked_at` and per-snapshot `verified_at`; there is no stale success cache. Hashes establish +Responses include `checked_at` and per-snapshot `verified_at`; there is no stale success cache. Hashes establish consistency with the manifest, not authenticity against an attacker who can rewrite both. `restore_status: not_tested` is explicit: this telemetry does not test decryption, restore a stack, replay later deletions or reconcile actions. Manifest deletion-ledger and execution-journal states diff --git a/tests/test_backups.py b/tests/test_backups.py index 8f35662..b945036 100644 --- a/tests/test_backups.py +++ b/tests/test_backups.py @@ -152,6 +152,18 @@ def test_a_truncated_archive_is_never_a_recovery_point(tmp_path): assert [row["name"] for row in result["rejected"]] == ["snapshot-a"] +def test_a_required_store_that_is_not_sqlite_is_never_a_recovery_point(tmp_path): + directory = snapshot(tmp_path) + with tarfile.open(directory / "pi.tar", "w") as archive: + member = tarfile.TarInfo("pi.db") + member.size = 16 + archive.addfile(member, io.BytesIO(b"not a database!!")) + save_manifest(directory) + result = backups.backup_status(tmp_path) + assert result["latest"] is None + assert [row["name"] for row in result["rejected"]] == ["snapshot-a"] + + def test_only_the_required_store_payload_is_opened(tmp_path, monkeypatch): snapshot(tmp_path) opened = []