fix(security): harden curator rollback tar extraction (symlink/abs-path escape)

[zapabob]

Summary

agent/curator_backup.py::rollback() extracts a snapshot tarball back into
~/.hermes/skills/. Before extraction it validated only member.name with a
POSIX-absolute / .. check, then fell back to a plain tf.extractall() on
Python < 3.12 (where filter="data" is unavailable). That leaves two escape
vectors if an attacker can plant a crafted skills.tar.gz in
.curator_backups/:

1. Symlink / hardlink members — the traversal lives in member.linkname,
   which was never inspected. The pre-3.12 fallback creates the link, and a
   later member can then be written through it, outside skills/.
2. Windows absolute / drive-letter / UNC paths (C:/x, C:\x,
   \\server\share) — the POSIX-only startswith("/") check lets these
   through, and Path(name).parts shows no .. for them.

Fix

• Add _assert_safe_member() (with helper _is_unsafe_archive_path()) that
  rejects unsafe member names (POSIX abs, Windows abs/drive/UNC, ..)
  and unsafe link targets, mirroring the secure extraction pattern
  already used in hermes_cli/profiles.py::_safe_extract_profile_archive.
• The filter="data" path on Python 3.12+ is unchanged.
• Relative, in-tree symlinks still extract, so legitimate snapshots are
  unaffected (no functional degradation).

Test plan

• tests/agent/test_curator_backup.py — new tests:
  • test_is_unsafe_archive_path_variants (POSIX abs, Windows abs/drive/UNC, ..)
  • test_rollback_rejects_symlink_escape (symlink with absolute target is refused)
  • test_assert_safe_member_allows_in_tree_symlink (relative in-tree link allowed)
• Existing test_rollback_rejects_unsafe_tarball still passes.
• All 28 tests in the file pass locally.

[tonydwb]

Code Review Summary

Verdict: Approved

Security: Hardened curator rollback tar extraction (symlink/abs-path escape)

• Clean fix — adds _is_unsafe_archive_path() covering POSIX absolute, Windows absolute/drive/UNC paths, and .. traversal. Previously only name.startswith("/") was checked, missing Windows paths on POSIX hosts and link targets entirely.
• Defense-in-depth — _assert_safe_member() now checks member.linkname for symlink/hardlink escapes, addressing the pre-3.12 filter="data" fallback gap where a malicious tarball could symlink to /etc/cron.d and write through it on a later member.
• Tests cover the full threat surface — 8 unsafe variants including all OS path types, symlink escape, and a safe-relative-symlink sanity check.
• No regression risk: the existing Python 3.12 filter="data" path is unchanged; only the fallback branch gets the stronger guard.

---

Reviewed by Hermes Agent

[liuhao1024]

Verification Review — looks solid ✅

Reviewed the curator rollback tar extraction hardening. Key observations:

1. Symlink/hardlink target validation — _assert_safe_member() checks member.linkname for both symlink and hardlink members, closing the gap where a malicious archive could create a link pointing outside the extraction root and write through it on the fallback (pre-3.12) path.

2. Cross-platform path coverage — _is_unsafe_archive_path() checks POSIX absolute paths, Windows absolute/drive-letter/UNC paths, and .. traversal using both PurePosixPath and PureWindowsPath. This correctly handles the case where a Windows-crafted archive is extracted on a POSIX host.

3. Empty linkname rejection — if not link or _is_unsafe_archive_path(link) correctly rejects members with empty or missing link targets.

4. Test coverage — Tests cover POSIX abs, Windows abs/drive/UNC, .. traversal, symlink escape, and in-tree symlink (allowed). The test_rollback_rejects_symlink_escape test creates a real malicious tarball and verifies rollback failure.

No findings.

[teknium1]

Thanks for the focused hardening. The premise remains valid on current main: agent/curator_backup.py:624-629 validates only member.name, while agent/curator_backup.py:631-634 falls back to unfiltered extractall() when filter="data" is unavailable. Python 3.11 remains supported by pyproject.toml:20.

The proposed _assert_safe_member() covers both member names and symlink/hardlink targets, including Windows path forms; the regression tests use the existing temporary HERMES_HOME fixture and invoke rollback() for the malicious-symlink case. This preserves the existing Python 3.12+ data-filter path.

GitHub currently reports this PR as conflicting, but the current-main changes to agent/curator_backup.py are adjacent rollback/cron maintenance rather than a replacement of the extraction guard, so this appears mechanically salvageable.

Automated hermes-sweeper review.

[egilewski]

suggesting changes

_assert_safe_member() rejects every symlink target containing a .. component,
even when it resolves inside the extraction root. A normal snapshot can preserve
alpha/link -> ../beta/SKILL.md, but rollback rejects that legitimate archive
because _is_unsafe_archive_path("../beta/SKILL.md") returns true. This conflicts
with the PR's stated invariant that relative in-tree symlinks remain usable.
Please resolve symlink targets lexically relative to the member's parent and
reject only normalized destinations that leave the extraction root, while
continuing to treat hardlink targets as archive-root-relative; add a regression
for a sibling-target symlink.

The absolute, drive/UNC, and escaping-target checks work for the covered cases,
and the focused tests plus fallback probe show that the demonstrated outside-write
exploit is stopped.

Security evidence:

• trust boundary: rollback extracts snapshot tar members supplied by the local snapshot store.
• source/sink/invariant: every member and link target must remain inside the extraction root without rejecting legitimate in-tree links preserved by snapshots.
• current-main reproduction: an absolute symlink followed by a file member wrote outside the skills directory.
• PR-head or patch-replay validation: the patch rejects that exploit, but its shared link-target predicate also rejects the safe in-tree target ../beta/SKILL.md for a member at alpha/link.
• positive/negative cases: absolute and traversal escapes are rejected and same-directory relative links are accepted, while the untested sibling-directory relative link is incorrectly rejected.
• residual bypass search: no covered escape bypass was found, but destination-unaware .. rejection introduces this in-scope rollback compatibility regression.
• reviewer validation: source review, focused-test evidence, the fallback exploit probe, and lexical resolution of the sibling-link case support this conclusion.

Not checked:

• Full test suite
• CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex