From 4d4cffd118ac0addf1c3b42bbf0395b1fb0b9436 Mon Sep 17 00:00:00 2001
From: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com>
Date: Sat, 29 Aug 2026 13:26:53 +0300
Subject: [PATCH] fix(profiles): make_targz writes to a temp file and renames,
not the destination directly
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
tarfile.open(archive_path, "w:gz") truncates the destination the instant
it opens. If tf.add() fails partway (disk full, permission loss,
interruption), whatever was previously at that path is gone — including
an existing profile or board export the caller chose to overwrite. This
is the same failure shape a7e7de6407 just fixed for the desktop gateway
file-save path, one commit earlier in the same window, but it was never
propagated to this shared archive-writing primitive even though board
export gained a new caller into it in that same window.
make_targz now writes into a sibling temp file (mkstemp, same directory
as the destination so the final step is a same-volume rename) and only
replaces the destination via os.replace() after the archive is fully
written and closed, mirroring the mkstemp+os.replace pattern already
used throughout this codebase (agent/secret_sources/_cache.py,
cron/jobs.py, gateway/status.py, etc). The temp file is unlinked on any
failure.
---
hermes_cli/archive_safe.py | 28 ++++++++--
tests/hermes_cli/test_archive_safe.py | 77 +++++++++++++++++++++++++++
2 files changed, 102 insertions(+), 3 deletions(-)
create mode 100644 tests/hermes_cli/test_archive_safe.py
diff --git a/hermes_cli/archive_safe.py b/hermes_cli/archive_safe.py
index 140f96bd95..b73a057081 100644
--- a/hermes_cli/archive_safe.py
+++ b/hermes_cli/archive_safe.py
@@ -21,6 +21,7 @@ from __future__ import annotations
import os
import shutil
import tarfile
+import tempfile
from pathlib import Path, PurePosixPath, PureWindowsPath
@@ -51,10 +52,31 @@ def normalize_archive_parts(member_name: str) -> list[str]:
def make_targz(base: str, root_dir: str, base_dir: str) -> str:
- """Create ``.tar.gz`` of ``root_dir/base_dir`` in GNU tar format."""
+ """Create ``.tar.gz`` of ``root_dir/base_dir`` in GNU tar format.
+
+ Writes to a sibling temp file and renames onto ``archive_path`` only
+ after the archive is fully written. ``tarfile.open`` on a path truncates
+ the destination the instant it opens, so writing there directly means a
+ failure partway through ``tf.add`` (disk full, permission loss,
+ interruption) destroys whatever was already at that path — including an
+ existing export the caller chose to overwrite.
+ """
archive_path = f"{base}.tar.gz"
- with tarfile.open(archive_path, "w:gz", format=tarfile.GNU_FORMAT) as tf:
- tf.add(str(Path(root_dir) / base_dir), arcname=base_dir)
+ dest_dir = os.path.dirname(archive_path) or "."
+ fd, tmp_path = tempfile.mkstemp(
+ dir=dest_dir, prefix=".archive_", suffix=".tar.gz.tmp"
+ )
+ try:
+ with os.fdopen(fd, "wb") as f:
+ with tarfile.open(fileobj=f, mode="w:gz", format=tarfile.GNU_FORMAT) as tf:
+ tf.add(str(Path(root_dir) / base_dir), arcname=base_dir)
+ os.replace(tmp_path, archive_path)
+ except BaseException:
+ try:
+ os.unlink(tmp_path)
+ except OSError:
+ pass
+ raise
return archive_path
diff --git a/tests/hermes_cli/test_archive_safe.py b/tests/hermes_cli/test_archive_safe.py
new file mode 100644
index 0000000000..39dfb39d74
--- /dev/null
+++ b/tests/hermes_cli/test_archive_safe.py
@@ -0,0 +1,77 @@
+"""Tests for the shared tar.gz writer (``hermes_cli.archive_safe.make_targz``).
+
+``make_targz`` backs both ``hermes profile export`` and ``hermes kanban
+export``. The contract pinned down here: a failure partway through writing
+the archive (disk full, permission loss, interruption) must never destroy a
+pre-existing file at the destination path — the same failure-atomicity
+guarantee the desktop gateway-download path already has.
+"""
+
+from __future__ import annotations
+
+import sys
+import tarfile
+from pathlib import Path
+
+import pytest
+
+_WORKTREE = Path(__file__).resolve().parents[2]
+if str(_WORKTREE) not in sys.path:
+ sys.path.insert(0, str(_WORKTREE))
+
+from hermes_cli.archive_safe import make_targz
+
+
+def _stage_source(tmp_path: Path) -> None:
+ payload = tmp_path / "src" / "inner"
+ payload.mkdir(parents=True)
+ (payload / "file.txt").write_text("hello")
+
+
+def test_make_targz_preserves_existing_file_on_mid_write_failure(tmp_path, monkeypatch):
+ _stage_source(tmp_path)
+ base = str(tmp_path / "out")
+ archive_path = Path(f"{base}.tar.gz")
+ sentinel = b"PRE-EXISTING ARCHIVE THAT MUST SURVIVE A FAILED RE-EXPORT"
+ archive_path.write_bytes(sentinel)
+
+ def _boom(self, *a, **k):
+ raise RuntimeError("simulated failure mid-add (disk full / permission loss)")
+
+ monkeypatch.setattr(tarfile.TarFile, "add", _boom)
+
+ with pytest.raises(RuntimeError):
+ make_targz(base, str(tmp_path), "src")
+
+ assert archive_path.read_bytes() == sentinel, (
+ "a mid-write failure must not truncate/replace a pre-existing archive"
+ )
+ leftovers = [p for p in tmp_path.iterdir() if p.name.startswith(".archive_")]
+ assert leftovers == [], f"temp file was not cleaned up on failure: {leftovers}"
+
+
+def test_make_targz_round_trips_content(tmp_path):
+ _stage_source(tmp_path)
+ base = str(tmp_path / "out")
+
+ result = make_targz(base, str(tmp_path), "src")
+
+ assert result == f"{base}.tar.gz"
+ with tarfile.open(result, "r:gz") as tf:
+ names = sorted(m.name for m in tf.getmembers())
+ assert "src/inner/file.txt" in names
+ member = tf.extractfile("src/inner/file.txt")
+ assert member is not None
+ assert member.read() == b"hello"
+
+
+def test_make_targz_overwrites_existing_file_on_success(tmp_path):
+ _stage_source(tmp_path)
+ base = str(tmp_path / "out")
+ archive_path = Path(f"{base}.tar.gz")
+ archive_path.write_bytes(b"stale archive from a previous export")
+
+ make_targz(base, str(tmp_path), "src")
+
+ with tarfile.open(archive_path, "r:gz") as tf:
+ assert "src/inner/file.txt" in {m.name for m in tf.getmembers()}