Skip to content

fix(update): refuse to mutate a venv containing foreign-owned files (#83529) - #99542

Merged
teknium1 merged 2 commits into
mainfrom
fix/p1-update-venv-ownership-preflight
Aug 31, 2026
Merged

teknium1 merged 2 commits into
mainfrom
fix/p1-update-venv-ownership-preflight

Conversation

@teknium1

Copy link
Copy Markdown
Collaborator

Root cause

Issue #83529 ("hermes update - destroys hermes", Debian, many +1s): at some point the user ran hermes/pip under sudo, leaving root-owned files inside the shared venv — classically site-packages/hermes_agent-*.dist-info/INSTALLER. A later normal-user hermes update pulls code fine, then uv pip install -e . fails:

failed to remove file `.../dist-info/INSTALLER`: Permission denied (os error 13)

…mid-mutation. By that point venv/bin/hermes has already been deleted, so the CLI is bricked (No such file or directory). The recovery is sudo chown -R user:user ~/.hermes/hermes-agent, but users have no way to know that — @eabase spent days diagnosing it and documented the fix in the issue.

Fix: refuse before mutate

Same philosophy as the contended-venv gate (#87331): a venv we cannot safely mutate is never mutated at all.

  • New _venv_foreign_owned_paths(venv_root, limit=5): a bounded, POSIX-only ownership scan — the venv root, direct entries of venv/bin, top-level site-packages entries, and each *.dist-info's direct children. Hard-capped at ~2000 stat() calls, per-entry OSError swallowed, returns [] on any structural surprise — never raises, never slow. Pure os.stat/os.scandir, no subprocess calls (update tests mock subprocess.run with sequenced side effects). Small _path_uid() seam so tests can simulate root-owned files without needing actual root.
  • New _refuse_update_if_venv_foreign_owned(project_root) called in _cmd_update_impl after the code pull (pulling code is safe) and immediately before the dependency-install block (the first venv mutation).
  • Windows (no os.geteuid) and root (euid == 0) skip the preflight entirely.

What users see now vs before

Before: update dies halfway with a cryptic uv permission error and a deleted venv/bin/hermes — Hermes won't start at all.

Now, with the venv still fully intact:

✗ Update stopped: this install's venv contains files owned by another user.
  Updating now would fail midway (Permission denied) and leave Hermes broken.
  This usually happens after running hermes or pip with sudo. Offending paths:
    - /home/user/.hermes/hermes-agent/venv/lib/python3.12/site-packages/hermes_agent-1.0.0.dist-info/INSTALLER (owner uid 0)

  Fix ownership, then re-run the update:
    sudo chown -R $(id -un): /home/user/.hermes/hermes-agent
    hermes update

  Nothing in the venv was modified.

Tests

New tests/hermes_cli/test_update_venv_ownership_preflight.py (10 tests, all passing): all-owned → no-op; foreign-owned dist-info child detected; refusal message includes path, uid, chown command, "Nothing in the venv was modified"; limit cap; no-geteuid (Windows) skip; root skip; never raises on PermissionError from scandir or a missing venv. Existing update-path suites exercised locally; the handful of failures observed reproduce identically on pristine origin/main (local-env preexisting, unrelated to this change).

Fixes #83529

Credit: @eabase for the diagnosis and the documented chown recovery that this preflight now surfaces automatically.

Infographic

refuse-before-mutate ownership preflight

…83529)

A venv ever touched by sudo pip / sudo hermes contains root-owned files
(classically site-packages/*.dist-info/INSTALLER). A later normal-user
'hermes update' pulls code fine, then 'uv pip install -e .' dies with
'Permission denied (os error 13)' mid-mutation — venv/bin/hermes already
deleted, CLI bricked.

Add a bounded, pure-stat ownership preflight (_venv_foreign_owned_paths)
that runs after the code pull and immediately before the dependency
install. If foreign-owned paths are found it refuses up front, names the
offending paths + owner uid, prints the exact recovery command
(sudo chown -R $(id -un): <root>), and confirms the venv is untouched.
Windows (no os.geteuid) and root skip entirely. Never raises, capped at
~2000 stat calls, no subprocess use (update tests mock subprocess.run).

Same refuse-before-mutate philosophy as the contended-venv gate (#87331).

Fixes #83529
Diagnosis and documented recovery by @eabase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: hermes update - destroys hermes

1 participant