fix(skills): guard _rmtree_writable against escaping SKILLS_DIR

[jackjin1997]

What does this PR do?

Adds a scope guard to _rmtree_writable() in tools/skills_sync.py so a skill sync can never rmtree SKILLS_DIR itself or any path outside it. This is defense-in-depth for the data-loss class reported in #48200 (hardening recommendations 4 and 6).

Related Issue

Related to #48200

Scope note (intentionally not "Fixes"): #48200 reports a full ~/.hermes/ wipe whose exact mechanism the reporter could not pin down (logs lost). This PR does not claim to be the sole root cause fix — it closes one concrete escalation path the reporter explicitly flagged (recs 4/6) and is valuable hardening on its own. The other recommendations (default pre_update_backup, an update timeout, post-update validation) are policy/feature decisions better left to maintainers.

Root Cause (of the guarded path)

_rmtree_writable() ran a bare shutil.rmtree() on whatever it was handed. All callers pass a skill directory or its .bak sibling computed via _compute_relative_dest() → SKILLS_DIR / rel. A ..-traversal can't occur (Path.relative_to forbids it), but if rel ever collapses to . then dest == SKILLS_DIR, and:

• _rmtree_writable(dest) would wipe the entire skills root, and
• backup = dest.with_suffix(".bak") becomes HERMES_HOME/skills.bak, so a stale-backup cleanup rmtree would target a path inside HERMES_HOME but outside skills/.

There was no boundary check preventing either.

Changes Made

• tools/skills_sync.py: resolve the target and raise ValueError if it equals SKILLS_DIR or is not strictly inside it, before any rmtree.
• tests/tools/test_skills_sync.py: 5 tests — removes a skill dir and a .bak sibling inside SKILLS_DIR (still works); refuses SKILLS_DIR itself, a skills.bak sibling in HERMES_HOME, and the HERMES_HOME root.

How to Test

python -m pytest tests/tools/test_skills_sync.py -q

Type of Change

• Bug fix / safety hardening (non-breaking)

Checklist

• Branch from main
• pytest tests/tools/test_skills_sync.py passes (64: 59 existing + 5 new), no regressions
• Tests isolated (patch SKILLS_DIR to a tmp dir, no writes to real ~/.hermes)
• Single logical change, conventional commit
• No behavior change for legitimate callers (all operate strictly inside SKILLS_DIR)

AI Disclosure

This issue was investigated and the fix written with AI assistance.

[jackjin1997]

Closing this — it's been superseded. Current main already carries an equivalent scope guard in _rmtree_writable, landed the same day I opened this PR in 25c590ccd ("fix(skills): refuse SKILLS_DIR root in rmtree guard, not just outside-tree"), which addresses #48200 with the same defense-in-depth intent — and does it more cleanly than my version: a single if skills_root not in target.parents refuses both the root itself and any outside-tree path in one check, where mine used two conditions.

No point carrying a redundant second guard, so closing. Thanks @teknium1 — glad the direction was right.