Skip to content

fix(security): guard os.chmod(parent) against / and system dirs (#25821)#25841

Closed
vanthinh6886 wants to merge 1 commit into
NousResearch:mainfrom
vanthinh6886:fix/secure-dir-chmod-root-guard
Closed

fix(security): guard os.chmod(parent) against / and system dirs (#25821)#25841
vanthinh6886 wants to merge 1 commit into
NousResearch:mainfrom
vanthinh6886:fix/secure-dir-chmod-root-guard

Conversation

@vanthinh6886

Copy link
Copy Markdown
Contributor

Summary

Prevents os.chmod(path.parent, 0o700) from running on / or well-known system directories, which bricks every non-root user on the host.

Fixes #25821

Changes

  • Add safe_chmod_parent() to hermes_constants.py — refuses to chmod root or system dirs (/etc, /var, /usr, /home, /root, /opt, /tmp, /proc, /sys, /dev, /run, /boot)
  • Replace all 5 unprotected os.chmod(path.parent, 0o700) call sites:
    • hermes_cli/auth.py (3 sites: _save_auth_store, _save_qwen_cli_tokens, _save_nous_shared_token)
    • tools/mcp_oauth.py (_write_json)
    • agent/google_oauth.py (save_credentials)
  • Add startup warning in get_hermes_home() when HERMES_HOME resolves to a protected directory

Why

Without this guard, a malformed HERMES_HOME=/ causes chmod("/", 0o700) which breaks systemd-resolved, journald, Docker containers, and every non-root service. See issue for production incident report.

…Research#25821)

Add safe_chmod_parent() to hermes_constants.py that refuses to chmod
root or well-known system directories (/etc, /var, /usr, /home, etc.).

Replace all 5 unprotected os.chmod(path.parent, 0o700) call sites:
- hermes_cli/auth.py (3 sites: _save_auth_store, _save_qwen_cli_tokens,
  _save_nous_shared_token)
- tools/mcp_oauth.py (_write_json)
- agent/google_oauth.py (save_credentials)

Also add a startup warning in get_hermes_home() when HERMES_HOME
resolves to a protected directory.

Without this guard, a malformed HERMES_HOME=/ causes chmod("/",
0o700) which bricks every non-root user on the host (systemd-resolved,
journald, Docker containers, etc.).
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround comp/agent Core agent loop, run_agent.py, prompt builder area/auth Authentication, OAuth, credential pools labels May 14, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing fix for #25821 — same issue also addressed by #25831. Both PRs add a guard against os.chmod(path.parent, 0o700) on root/system directories.

@YonganZhang

Copy link
Copy Markdown

Drop-by +1 from #27384 (which I just closed as duplicate) — quick observation while implementing the same fix independently:

The blocklist in your safe_chmod_parent() covers the headline brick-the-host cases (/, /etc, /var, /usr, /home, /root, /opt, /tmp, /proc, /sys, /dev, /run, /boot). Three additional dirs worth considering for completeness:

  • /lib and /lib64 — chmod 0o700 here breaks ld.so resolution for every non-root user, even harder to recover than /.
  • /srv, /mnt, /media — less catastrophic, but the mnt case can lock out shared volumes used by Docker compose stacks, which is a non-obvious failure mode for ops debugging.

Also worth a d.resolve() (or os.path.realpath) before the comparison — otherwise a symlink under ~/.hermes pointing to / slips past a string-based check. The issue body specifically mentions symlink-induced resolution as a trigger.

Either way, the fix shape is sound; happy to see it land with regression coverage on the canonical PR.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #29670 — we went with @liuhao1024's #25831 as the base since it was submitted first (16:29 vs 16:54) and shipped with 5 unit tests. Your version's explicit _PROTECTED_DIRS blocklist and startup warning were thoughtful additions; the depth-3 guard in the merged version achieves the same protection without an enumeration to maintain. Thanks for the catch on this one.

@teknium1 teknium1 closed this May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/agent Core agent loop, run_agent.py, prompt builder P1 High — major feature broken, no workaround type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: os.chmod(path.parent, 0o700) bricks host when path.parent resolves to /

4 participants