security(gateway): re-resolve skills_sync path constants per call

[srojk34]

Summary

tools/skills_sync.py's HERMES_HOME/SKILLS_DIR/MANIFEST_FILE are resolved once at import time via get_hermes_home(), which reads a context-local ContextVar (_HERMES_HOME_OVERRIDE) set per-request under the multiplexed gateway (multiple profiles served from one process, e.g. the desktop tui_gateway). gateway/run.py calls sync_skills() directly in-process on every profile's startup:

# gateway/run.py
from tools.skills_sync import sync_skills
sync_skills(quiet=True)

Freezing the resolved path at import time pins every later profile's sync to whichever profile's HERMES_HOME happened to be active the first time this module was imported in the process — a later-starting profile silently syncs bundled skills into the FIRST profile's directory instead of its own, and never gets its own bundled skills synced at all. The module's own comment (visible elsewhere in the codebase, hermes_cli/profiles.py) already flags this for the CLI/hermes update path — "Uses subprocess because sync_skills() caches HERMES_HOME at module level" — which routes around it via subprocess isolation, but gateway/run.py's in-process call has no such isolation.

_rmtree_writable's safety guard (SKILLS_DIR.resolve() as the scope floor for a destructive rmtree, guarding against a catastrophic wipe of ~/.hermes/) also benefits from this fix: a frozen wrong-profile skills_root could desync the guard from the actual skills directory being operated on.

Fix

Adopt tools/skills_hub.py's already-established fix for this exact bug class: a module __getattr__ (PEP 562) that resolves HERMES_HOME/SKILLS_DIR/MANIFEST_FILE dynamically per access, honoring the active profile override, while a test's patch("tools.skills_sync.SKILLS_DIR", ...) still sets a real module attribute that shadows dynamic resolution entirely — preserving every existing test seam in tests/tools/test_skills_sync.py (dozens of call sites) unmodified. All ~20 internal call sites in the module now resolve fresh via _hermes_home()/_skills_dir()/_manifest_file() instead of the frozen names.

Test plan

• pytest tests/tools/test_skills_sync.py tests/tools/test_skills_list_modified_diff.py -q — 75 passed
• pytest tests/test_profile_isolation_runtime.py -q — 14 passed (10 pre-existing + 4 new)
• pytest tests/hermes_cli/test_skills_hub.py -q — 28 passed
• Running these three files together in one session surfaces 3 pre-existing, unrelated TestSkillsHubPathResolution failures (a test-order-dependent state leak in tools/skills_hub.py's own test suite) — confirmed via git stash that this happens identically without this change; each file passes cleanly run on its own.
• ruff check on all changed files — clean

Note on file co-tenancy

Open PR #34577 ("keep bundled skill copies writable on Nix store") also touches tools/skills_sync.py, but for an unrelated concern (shutil.copytree write-permission handling on read-only source filesystems, via new _copy_file_writable/_make_tree_owner_writable helpers) — no functional overlap with this profile-isolation fix, though whichever merges second will need a small rebase around the constant-block region.

[rodriguez46p-ui]

Hourly triage from Hermes Agent:

• Reviewed the diff for tools/skills_sync.py and tests/test_profile_isolation_runtime.py at head 76cd535359129ad750763593103ceaa211e06b05. The change follows the existing skills_hub pattern: dynamic per-access path resolution, while preserving monkeypatch/test seams for real module attributes.
• Added a local ad-hoc profile-isolation probe: import/touch tools.skills_sync under profile A, then run sync_skills(quiet=True) under profile B with a temp bundled-skill source. Result: skill was copied only into profile B; no leak into profile A.
• Local focused checks passed on this Windows host:
  • /c/Users/yolop/.hermes/hermes-agent/venv/Scripts/python.exe -m pytest tests/test_profile_isolation_runtime.py::TestSkillsSyncPathResolution -q — 4 passed
  • /c/Users/yolop/.hermes/hermes-agent/venv/Scripts/python.exe -m pytest tests/tools/test_skills_list_modified_diff.py -q — 6 passed
  • py_compile tools/skills_sync.py tests/test_profile_isolation_runtime.py — passed
  • git diff --check origin/main...HEAD — passed
  • tracked worktree status — clean
• I also tried the broader tests/test_profile_isolation_runtime.py::TestSkillsSyncPathResolution tests/tools/test_skills_sync.py tests/tools/test_skills_list_modified_diff.py -q; it exposed an existing Windows-only assertion in tests/tools/test_skills_sync.py::TestComputeRelativeDest::test_preserves_category_structure that expects a POSIX slash suffix (mlops/axolotl) even though Windows Path stringification uses backslash separators. That assertion exists unchanged on origin/main, so I am treating it as a pre-existing local test portability issue rather than a regression from this PR.
• Hosted CI currently has All required checks pass green. The two Docker build jobs were still in progress when I checked, so merge state was still UNSTABLE rather than fully settled.

No blocking code issue found in the reviewed path.

[teknium1]

Thanks for carrying the established dynamic-path pattern from tools/skills_hub.py into tools/skills_sync.py.

Problems

• The stated gateway-startup mechanism is not present on current main: gateway/run.py:20812-20817 calls sync_skills() once before the secondary-profile loop starts at gateway/run.py:8616. _start_one_profile_adapters() scopes each secondary profile (gateway/run.py:8657) but does not call sync_skills(). The resolver change alone therefore cannot make gateway startup seed every served profile.
• The added tests exercise resolvers only. They do not invoke sync_skills() under a second profile override and verify the copied skill/manifest destination.

Suggested changes

• Either add the intended per-profile startup sync at the actual scoped startup path, or narrow the PR claim to generic protection for scoped in-process callers.
• Add a two-profile end-to-end sync_skills() regression with a temporary bundled source.

This is an automated hermes-sweeper review.