Skip to content

Harden root FHS PATH guard against HOME privilege crossing#673

Open
badMade wants to merge 4 commits into
mainfrom
badmade/fix-root-path-probe-vulnerability
Open

Harden root FHS PATH guard against HOME privilege crossing#673
badMade wants to merge 4 commits into
mainfrom
badmade/fix-root-path-probe-vulnerability

Conversation

@badMade
Copy link
Copy Markdown
Owner

@badMade badMade commented Jun 1, 2026

Motivation

  • Prevent privilege-boundary crossing during root install/update where an attacker-controlled HOME or startup file could be executed or modified as root.
  • Restore safe semantics from before the regression: resolve root's real home from the system account and avoid launching an interactive shell that sources user startup files.

Description

  • scripts/install.sh: resolve root home via getent passwd root (fallback /root), probe with a non-interactive bash -c under a minimal env (explicit PATH) so user ~/.bashrc is not sourced, operate on ROOT_HOME instead of environment HOME, and restrict edits to startup files that are regular files (no symlink), root-owned, and not group/world-writable before appending PATH guard lines.
  • hermes_cli/main.py: resolve root home via pwd.getpwuid(0) (fallback /root), run the probe using bash -c with an explicit PATH to avoid sourcing ~/.bashrc, and refuse to modify candidate startup files if they are symlinks or fail ownership/permission checks (must be owned by root and not group/world-writable) before appending the PATH guard.
  • Both changes target the same unsafe behaviors: trusting HOME and launching an interactive bash -i that can execute attacker-controlled startup files, and appending to arbitrary files without ownership/mode/symlink checks.

Testing

  • Ran python -m py_compile hermes_cli/main.py and the file compiled successfully.
  • Ran bash -n scripts/install.sh (shell syntax check) and the script passed -n validation.
  • No new unit tests added; changes are minimal and validated via syntax/type checks listed above.

Codex Task

Copilot AI review requested due to automatic review settings June 1, 2026 00:33
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves security and robustness when probing and modifying shell configuration files for the root user. It resolves the root user's home directory directly from the system database instead of relying on the HOME environment variable. Additionally, it restricts the bash probe to a non-interactive shell with an explicit PATH, and introduces strict ownership and permission checks on .bashrc and .bash_profile to prevent modifying files that are symlinks, not owned by root, or group/world-writable. There are no review comments, so no feedback is provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 1, 2026

🔎 Lint report: badmade/fix-root-path-probe-vulnerability vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8254 on HEAD, 8258 on base (✅ -4)

🆕 New issues (3):

Rule Count
invalid-argument-type 3
First entries
run_agent.py:7311: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
run_agent.py:13565: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
run_agent.py:13568: [invalid-argument-type] invalid-argument-type: Argument to function `len` is incorrect: Expected `Sized`, found `(str & ~AlwaysFalsy) | (dict[Unknown, Unknown] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 3 union elements`

✅ Fixed issues (4):

Rule Count
invalid-argument-type 3
unresolved-reference 1
First entries
run_agent.py:13565: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown | str, Unknown | str | dict[str, str]] | Any | ... omitted 3 union elements`
run_agent.py:13568: [invalid-argument-type] invalid-argument-type: Argument to function `len` is incorrect: Expected `Sized`, found `(str & ~AlwaysFalsy) | (dict[Unknown | str, Unknown | str | dict[str, str]] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 3 union elements`
gateway/run.py:5544: [unresolved-reference] unresolved-reference: Name `team_id` used when not defined
run_agent.py:7311: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown | str, Unknown | str | dict[str, str]] | Any | ... omitted 3 union elements`

Unchanged: 4355 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 1, 2026

Auto-merge: checks failing

The following checks did not pass:

  • test (failure)

Please fix the failing checks before this PR can be merged.

View workflow run

@badMade
Copy link
Copy Markdown
Owner Author

badMade commented Jun 1, 2026

@copilot, fix all failing checks.

Copy link
Copy Markdown

Copilot AI commented Jun 1, 2026

@copilot, fix all failing checks.

Fixed and pushed in d0b367b.

I addressed the failing checks by fixing the NameError in gateway authorization (team_id was referenced before assignment). The new PR-triggered runs are now in action_required with zero jobs, so a maintainer approval/rerun is needed for checks to execute on this commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants