mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-04-30 14:56:01 +00:00
security: prevent exec tool from leaking process env vars to LLM
The exec tool previously passed the full parent process environment to child processes, which meant LLM-generated commands could access secrets stored in env vars (e.g. API keys from EnvironmentFile=). Switch from subprocess_shell with inherited env to bash login shell with a minimal environment (HOME, LANG, TERM only). The login shell sources the user's profile for PATH setup, making the pathAppend config option a fallback rather than the primary PATH mechanism.
This commit is contained in:
parent
84b1c6a0d7
commit
be6063a142
@ -3,6 +3,7 @@
|
|||||||
import asyncio
|
import asyncio
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
|
import shutil
|
||||||
import sys
|
import sys
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any
|
from typing import Any
|
||||||
@ -93,13 +94,13 @@ class ExecTool(Tool):
|
|||||||
|
|
||||||
effective_timeout = min(timeout or self.timeout, self._MAX_TIMEOUT)
|
effective_timeout = min(timeout or self.timeout, self._MAX_TIMEOUT)
|
||||||
|
|
||||||
env = os.environ.copy()
|
env = self._build_env()
|
||||||
if self.path_append:
|
|
||||||
env["PATH"] = env.get("PATH", "") + os.pathsep + self.path_append
|
bash = shutil.which("bash") or "/bin/bash"
|
||||||
|
|
||||||
try:
|
try:
|
||||||
process = await asyncio.create_subprocess_shell(
|
process = await asyncio.create_subprocess_exec(
|
||||||
command,
|
bash, "-l", "-c", command,
|
||||||
stdout=asyncio.subprocess.PIPE,
|
stdout=asyncio.subprocess.PIPE,
|
||||||
stderr=asyncio.subprocess.PIPE,
|
stderr=asyncio.subprocess.PIPE,
|
||||||
cwd=cwd,
|
cwd=cwd,
|
||||||
@ -154,6 +155,25 @@ class ExecTool(Tool):
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
return f"Error executing command: {str(e)}"
|
return f"Error executing command: {str(e)}"
|
||||||
|
|
||||||
|
def _build_env(self) -> dict[str, str]:
|
||||||
|
"""Build a minimal environment for subprocess execution.
|
||||||
|
|
||||||
|
Uses HOME so that ``bash -l`` sources the user's profile (which sets
|
||||||
|
PATH and other essentials). Only PATH is extended with *path_append*;
|
||||||
|
the parent process's environment is **not** inherited, preventing
|
||||||
|
secrets in env vars from leaking to LLM-generated commands.
|
||||||
|
"""
|
||||||
|
home = os.environ.get("HOME", "/tmp")
|
||||||
|
env: dict[str, str] = {
|
||||||
|
"HOME": home,
|
||||||
|
"LANG": os.environ.get("LANG", "C.UTF-8"),
|
||||||
|
"TERM": os.environ.get("TERM", "dumb"),
|
||||||
|
}
|
||||||
|
if self.path_append:
|
||||||
|
# Seed PATH so the login shell can append to it.
|
||||||
|
env["PATH"] = self.path_append
|
||||||
|
return env
|
||||||
|
|
||||||
def _guard_command(self, command: str, cwd: str) -> str | None:
|
def _guard_command(self, command: str, cwd: str) -> str | None:
|
||||||
"""Best-effort safety guard for potentially destructive commands."""
|
"""Best-effort safety guard for potentially destructive commands."""
|
||||||
cmd = command.strip()
|
cmd = command.strip()
|
||||||
|
|||||||
30
tests/tools/test_exec_env.py
Normal file
30
tests/tools/test_exec_env.py
Normal file
@ -0,0 +1,30 @@
|
|||||||
|
"""Tests for exec tool environment isolation."""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from nanobot.agent.tools.shell import ExecTool
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_exec_does_not_leak_parent_env(monkeypatch):
|
||||||
|
"""Env vars from the parent process must not be visible to commands."""
|
||||||
|
monkeypatch.setenv("NANOBOT_SECRET_TOKEN", "super-secret-value")
|
||||||
|
tool = ExecTool()
|
||||||
|
result = await tool.execute(command="printenv NANOBOT_SECRET_TOKEN")
|
||||||
|
assert "super-secret-value" not in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_exec_has_working_path():
|
||||||
|
"""Basic commands should be available via the login shell's PATH."""
|
||||||
|
tool = ExecTool()
|
||||||
|
result = await tool.execute(command="echo hello")
|
||||||
|
assert "hello" in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_exec_path_append():
|
||||||
|
"""The pathAppend config should be available in the command's PATH."""
|
||||||
|
tool = ExecTool(path_append="/opt/custom/bin")
|
||||||
|
result = await tool.execute(command="echo $PATH")
|
||||||
|
assert "/opt/custom/bin" in result
|
||||||
Loading…
x
Reference in New Issue
Block a user