fix(profiles): validate custom alias names to prevent path traversal
`hermes profile alias <profile> --name <custom>` accepted arbitrary strings and used them verbatim as a filename under ~/.local/bin. Because normalize_profile_name only lowercases/strips (no regex gate), a value like `../../.bashrc` escaped the wrapper directory and clobbered arbitrary user-writable files. remove_wrapper_script had the same sink. Add validate_alias_name (reusing the profile-id regex, which forbids `/`, `.`, and `..`) and wire it into check_alias_collision, create_wrapper_script, remove_wrapper_script, and the CLI alias action so the rejection surfaces a clear "Invalid alias name" error instead of silently writing or unlinking outside the wrapper dir. Co-authored-by: Gutslabs <gutslabsxyz@gmail.com> Co-authored-by: Xowiek <xowiekk@gmail.com>fix/verification-admin-route-recovery
parent
27ddd8fd80
commit
11183e8332
|
|
@ -11149,7 +11149,7 @@ def cmd_profile(args):
|
|||
remove = getattr(args, "remove", False)
|
||||
custom_name = getattr(args, "alias_name", None)
|
||||
|
||||
from hermes_cli.profiles import profile_exists
|
||||
from hermes_cli.profiles import profile_exists, validate_alias_name
|
||||
|
||||
if not profile_exists(name):
|
||||
print(f"Error: Profile '{name}' does not exist.")
|
||||
|
|
@ -11157,6 +11157,12 @@ def cmd_profile(args):
|
|||
|
||||
alias_name = custom_name or name
|
||||
|
||||
try:
|
||||
validate_alias_name(alias_name)
|
||||
except ValueError as exc:
|
||||
print(f"Error: {exc}")
|
||||
sys.exit(1)
|
||||
|
||||
if remove:
|
||||
if remove_wrapper_script(alias_name):
|
||||
print(f"✓ Removed alias '{alias_name}'")
|
||||
|
|
|
|||
|
|
@ -328,6 +328,22 @@ def validate_profile_name(name: str) -> None:
|
|||
)
|
||||
|
||||
|
||||
def validate_alias_name(name: str) -> None:
|
||||
"""Raise ``ValueError`` if *name* is not a safe wrapper-alias identifier.
|
||||
|
||||
The alias is used verbatim as a filename under :func:`_get_wrapper_dir`
|
||||
(``~/.local/bin``), so it must be a single safe command name with no path
|
||||
separators or traversal segments — otherwise a value like ``../../.bashrc``
|
||||
would escape the wrapper directory and clobber arbitrary user files. We
|
||||
reuse the profile id regex, which already forbids ``/``, ``.``, and ``..``.
|
||||
"""
|
||||
if not _PROFILE_ID_RE.match(name):
|
||||
raise ValueError(
|
||||
f"Invalid alias name {name!r}. Must match "
|
||||
f"[a-z0-9][a-z0-9_-]{{0,63}}"
|
||||
)
|
||||
|
||||
|
||||
def get_profile_dir(name: str) -> Path:
|
||||
"""Resolve a profile name to its HERMES_HOME directory."""
|
||||
canon = normalize_profile_name(name)
|
||||
|
|
@ -351,9 +367,14 @@ def profile_exists(name: str) -> bool:
|
|||
def check_alias_collision(name: str) -> Optional[str]:
|
||||
"""Return a human-readable collision message, or None if the name is safe.
|
||||
|
||||
Checks: reserved names, hermes subcommands, existing binaries in PATH.
|
||||
Checks: alias-name validity, reserved names, hermes subcommands, existing
|
||||
binaries in PATH.
|
||||
"""
|
||||
canon = normalize_profile_name(name)
|
||||
try:
|
||||
validate_alias_name(canon)
|
||||
except ValueError as exc:
|
||||
return str(exc)
|
||||
if canon in _RESERVED_NAMES:
|
||||
return f"'{canon}' is a reserved name"
|
||||
if canon in _HERMES_SUBCOMMANDS:
|
||||
|
|
@ -403,6 +424,9 @@ def create_wrapper_script(name: str, target: Optional[str] = None) -> Optional[P
|
|||
"""
|
||||
canon = normalize_profile_name(name)
|
||||
profile = normalize_profile_name(target) if target else canon
|
||||
# The alias is used verbatim as a filename under the wrapper dir; reject
|
||||
# any value that isn't a single safe identifier so it can't traverse out.
|
||||
validate_alias_name(canon)
|
||||
wrapper_dir = _get_wrapper_dir()
|
||||
try:
|
||||
wrapper_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
|
@ -435,6 +459,12 @@ def remove_wrapper_script(name: str) -> bool:
|
|||
"""Remove the wrapper script for a profile. Returns True if removed."""
|
||||
wrapper_dir = _get_wrapper_dir()
|
||||
canon = normalize_profile_name(name)
|
||||
# A traversal-shaped name could point unlink() at a file outside the
|
||||
# wrapper dir; refuse it rather than acting on an arbitrary path.
|
||||
try:
|
||||
validate_alias_name(canon)
|
||||
except ValueError:
|
||||
return False
|
||||
is_windows = sys.platform == "win32"
|
||||
|
||||
# Check both the extensionless path (POSIX) and .bat (Windows)
|
||||
|
|
|
|||
|
|
@ -972,6 +972,7 @@ AUTHOR_MAP = {
|
|||
"oluwadareab12@gmail.com": "oluwadareab12",
|
||||
"simon@simonmarcus.org": "simon-marcus",
|
||||
"xowiekk@gmail.com": "Xowiek",
|
||||
"gutslabsxyz@gmail.com": "Gutslabs",
|
||||
"1243352777@qq.com": "zons-zhaozhy",
|
||||
"e.silacandmr@gmail.com": "Es1la",
|
||||
"51599529+stephen0110@users.noreply.github.com": "stephen0110",
|
||||
|
|
|
|||
|
|
@ -26,6 +26,9 @@ from hermes_cli.profiles import (
|
|||
get_active_profile_name,
|
||||
resolve_profile_env,
|
||||
check_alias_collision,
|
||||
create_wrapper_script,
|
||||
remove_wrapper_script,
|
||||
validate_alias_name,
|
||||
rename_profile,
|
||||
export_profile,
|
||||
import_profile,
|
||||
|
|
@ -769,6 +772,14 @@ class TestAliasCollision:
|
|||
result = check_alias_collision("mybot")
|
||||
assert result is None # our own wrapper, safe to overwrite
|
||||
|
||||
def test_traversal_alias_rejected_before_path_lookup(self, profile_env):
|
||||
"""A path-traversal alias is rejected without ever shelling out to which/where."""
|
||||
with patch("subprocess.run") as mock_run:
|
||||
result = check_alias_collision("../../.bashrc")
|
||||
assert result is not None
|
||||
assert "invalid alias name" in result.lower()
|
||||
mock_run.assert_not_called()
|
||||
|
||||
|
||||
# ===================================================================
|
||||
# TestWrapperScript
|
||||
|
|
@ -850,6 +861,53 @@ class TestWrapperScript:
|
|||
assert "#!/bin/sh" not in content
|
||||
|
||||
|
||||
# ===================================================================
|
||||
# TestWrapperScriptSecurity — path-traversal hardening
|
||||
# ===================================================================
|
||||
|
||||
class TestWrapperScriptSecurity:
|
||||
"""A crafted alias name must not escape the wrapper directory."""
|
||||
|
||||
def test_validate_alias_name_rejects_traversal(self):
|
||||
with pytest.raises(ValueError, match="Invalid alias name"):
|
||||
validate_alias_name("../../.bashrc")
|
||||
|
||||
def test_validate_alias_name_rejects_absolute_path(self, tmp_path):
|
||||
with pytest.raises(ValueError, match="Invalid alias name"):
|
||||
validate_alias_name(str(tmp_path / "evil"))
|
||||
|
||||
def test_validate_alias_name_accepts_safe_identifier(self):
|
||||
validate_alias_name("mybot") # does not raise
|
||||
|
||||
def test_create_wrapper_rejects_traversal(self, profile_env):
|
||||
sentinel = profile_env / ".bashrc"
|
||||
sentinel.write_text("keep", encoding="utf-8")
|
||||
with pytest.raises(ValueError, match="Invalid alias name"):
|
||||
create_wrapper_script("../../.bashrc", target="coder")
|
||||
# The traversal target was not touched.
|
||||
assert sentinel.read_text(encoding="utf-8") == "keep"
|
||||
|
||||
def test_create_wrapper_rejects_absolute_path(self, profile_env, tmp_path):
|
||||
target = tmp_path / "abs-wrapper"
|
||||
with pytest.raises(ValueError, match="Invalid alias name"):
|
||||
create_wrapper_script(str(target))
|
||||
assert not target.exists()
|
||||
|
||||
def test_remove_wrapper_rejects_traversal(self, profile_env):
|
||||
sentinel = profile_env / ".bashrc"
|
||||
sentinel.write_text("keep", encoding="utf-8")
|
||||
assert remove_wrapper_script("../../.bashrc") is False
|
||||
assert sentinel.read_text(encoding="utf-8") == "keep"
|
||||
|
||||
def test_legit_alias_stays_inside_wrapper_dir(self, profile_env, monkeypatch):
|
||||
monkeypatch.setattr("sys.platform", "darwin")
|
||||
from hermes_cli.profiles import _get_wrapper_dir
|
||||
wrapper = create_wrapper_script("mybot", target="coder")
|
||||
assert wrapper is not None
|
||||
assert wrapper.resolve().is_relative_to(_get_wrapper_dir().resolve())
|
||||
assert 'hermes -p coder "$@"' in wrapper.read_text()
|
||||
|
||||
|
||||
# ===================================================================
|
||||
# TestFindAliasForProfile — display-side reverse lookup
|
||||
# ===================================================================
|
||||
|
|
|
|||
Loading…
Reference in New Issue