fix(backup): skip symlinked files in zip archives (#25289)

This commit is contained in:
nguyen binh 2026-05-25 19:07:52 +07:00 committed by GitHub
parent 79799c80f5
commit 0d55315c36
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 63 additions and 16 deletions

View file

@ -85,6 +85,22 @@ def _should_exclude(rel_path: Path) -> bool:
return False
def _should_skip_backup_file(abs_path: Path, rel_path: Path, out_path: Path) -> bool:
"""Return True when a candidate file should not be written to a backup zip."""
if _should_exclude(rel_path):
return True
# zipfile.write() follows file symlinks, so skip links before any archive
# write can copy data from outside HERMES_HOME.
if abs_path.is_symlink():
return True
try:
return abs_path.resolve() == out_path.resolve()
except (OSError, ValueError):
return False
# ---------------------------------------------------------------------------
# SQLite safe copy
# ---------------------------------------------------------------------------
@ -173,16 +189,9 @@ def run_backup(args) -> None:
fpath = dp / fname
rel = fpath.relative_to(hermes_root)
if _should_exclude(rel):
if _should_skip_backup_file(fpath, rel, out_path):
continue
# Skip the output zip itself if it happens to be inside hermes root
try:
if fpath.resolve() == out_path.resolve():
continue
except (OSError, ValueError):
pass
files_to_add.append((fpath, rel))
if not files_to_add:
@ -726,16 +735,9 @@ def _write_full_zip_backup(out_path: Path, hermes_root: Path) -> Optional[Path]:
except ValueError:
continue
if _should_exclude(rel):
if _should_skip_backup_file(fpath, rel, out_path):
continue
# Skip the output zip itself if it already exists inside root.
try:
if fpath.resolve() == out_path.resolve():
continue
except (OSError, ValueError):
pass
files_to_add.append((fpath, rel))
except OSError as exc:
logger.warning("Full-zip backup: walk failed: %s", exc)

View file

@ -68,6 +68,13 @@ def _make_hermes_tree(root: Path) -> None:
(root / "logs" / "agent.log").write_text("log line\n")
def _symlink_file_or_skip(link: Path, target: Path) -> None:
try:
link.symlink_to(target)
except OSError as exc:
pytest.skip(f"symlinks unavailable in test environment: {exc}")
# ---------------------------------------------------------------------------
# _should_exclude tests
# ---------------------------------------------------------------------------
@ -257,6 +264,29 @@ class TestBackup:
zips = list(tmp_path.glob("hermes-backup-*.zip"))
assert len(zips) == 1
def test_skips_symlinked_files(self, tmp_path, monkeypatch):
"""Backup must not dereference symlinks and leak files outside HERMES_HOME."""
hermes_home = tmp_path / ".hermes"
hermes_home.mkdir()
_make_hermes_tree(hermes_home)
outside = tmp_path / "outside-secret.txt"
outside.write_text("outside secret\n")
_symlink_file_or_skip(hermes_home / "skills" / "outside-link.txt", outside)
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
monkeypatch.setattr(Path, "home", lambda: tmp_path)
out_zip = tmp_path / "backup.zip"
args = Namespace(output=str(out_zip))
from hermes_cli.backup import run_backup
run_backup(args)
with zipfile.ZipFile(out_zip, "r") as zf:
names = zf.namelist()
assert "skills/outside-link.txt" not in names
assert all(zf.read(name) != b"outside secret\n" for name in names)
# ---------------------------------------------------------------------------
# _validate_backup_zip tests
@ -1421,6 +1451,21 @@ class TestPreUpdateBackup:
f"remaining={remaining}"
)
def test_skips_symlinked_files(self, hermes_home, tmp_path):
"""Pre-update backups must not dereference symlinks outside HERMES_HOME."""
from hermes_cli.backup import create_pre_update_backup
outside = tmp_path / "outside-secret.txt"
outside.write_text("outside secret\n")
_symlink_file_or_skip(hermes_home / "skills" / "outside-link.txt", outside)
out = create_pre_update_backup(hermes_home=hermes_home)
assert out is not None
with zipfile.ZipFile(out) as zf:
names = zf.namelist()
assert "skills/outside-link.txt" not in names
assert all(zf.read(name) != b"outside secret\n" for name in names)
class TestRunPreUpdateBackup:
"""Tests for the ``_run_pre_update_backup`` wrapper in main.py —