mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 06:51:16 +08:00
fix(security): sanitize workdir parameter in terminal tool backends
Shell injection via unquoted workdir interpolation in docker, singularity, and SSH backends. When workdir contained shell metacharacters (e.g. ~/;id), arbitrary commands could execute. Changes: - Add shlex.quote() at each interpolation point in docker.py, singularity.py, and ssh.py with tilde-aware quoting (keep ~ unquoted for shell expansion, quote only the subpath) - Add _validate_workdir() allowlist in terminal_tool.py as defense-in-depth before workdir reaches any backend Original work by Mariano A. Nicolini (PR #5620). Salvaged with fixes for tilde expansion (shlex.quote breaks cd ~/path) and replaced incomplete deny-list with strict character allowlist. Co-authored-by: Mariano A. Nicolini <entropidelic@users.noreply.github.com>
This commit is contained in:
committed by
Teknium
parent
537a2b8bb8
commit
af9a9f773c
@@ -154,6 +154,34 @@ def _check_all_guards(command: str, env_type: str) -> dict:
|
||||
approval_callback=_approval_callback)
|
||||
|
||||
|
||||
# Allowlist: characters that can legitimately appear in directory paths.
|
||||
# Covers alphanumeric, path separators, tilde, dot, hyphen, underscore, space,
|
||||
# plus, at, equals, and comma. Everything else is rejected.
|
||||
_WORKDIR_SAFE_RE = re.compile(r'^[A-Za-z0-9/_\-.~ +@=,]+$')
|
||||
|
||||
|
||||
def _validate_workdir(workdir: str) -> str | None:
|
||||
"""Reject workdir values that don't look like a filesystem path.
|
||||
|
||||
Uses an allowlist of safe characters rather than a deny-list, so novel
|
||||
shell metacharacters can't slip through.
|
||||
|
||||
Returns None if safe, or an error message string if dangerous.
|
||||
"""
|
||||
if not workdir:
|
||||
return None
|
||||
if not _WORKDIR_SAFE_RE.match(workdir):
|
||||
# Find the first offending character for a helpful message.
|
||||
for ch in workdir:
|
||||
if not _WORKDIR_SAFE_RE.match(ch):
|
||||
return (
|
||||
f"Blocked: workdir contains disallowed character {repr(ch)}. "
|
||||
"Use a simple filesystem path without shell metacharacters."
|
||||
)
|
||||
return "Blocked: workdir contains disallowed characters."
|
||||
return None
|
||||
|
||||
|
||||
def _handle_sudo_failure(output: str, env_type: str) -> str:
|
||||
"""
|
||||
Check for sudo failure and add helpful message for messaging contexts.
|
||||
@@ -1166,6 +1194,19 @@ def terminal_tool(
|
||||
desc = approval.get("description", "flagged as dangerous")
|
||||
approval_note = f"Command was flagged ({desc}) and auto-approved by smart approval."
|
||||
|
||||
# Validate workdir against shell injection
|
||||
if workdir:
|
||||
workdir_error = _validate_workdir(workdir)
|
||||
if workdir_error:
|
||||
logger.warning("Blocked dangerous workdir: %s (command: %s)",
|
||||
workdir[:200], command[:200])
|
||||
return json.dumps({
|
||||
"output": "",
|
||||
"exit_code": -1,
|
||||
"error": workdir_error,
|
||||
"status": "blocked"
|
||||
}, ensure_ascii=False)
|
||||
|
||||
# Prepare command for execution
|
||||
if background:
|
||||
# Spawn a tracked background process via the process registry.
|
||||
|
||||
Reference in New Issue
Block a user