From be6063a14228aeb0fda24b5eb36724f2ff6b3473 Mon Sep 17 00:00:00 2001 From: Ben Lenarts Date: Mon, 6 Apr 2026 00:21:07 +0200 Subject: [PATCH] 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. --- nanobot/agent/tools/shell.py | 30 +++++++++++++++++++++++++----- tests/tools/test_exec_env.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 tests/tools/test_exec_env.py diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index ec2f1a775..2e0b606ab 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -3,6 +3,7 @@ import asyncio import os import re +import shutil import sys from pathlib import Path from typing import Any @@ -93,13 +94,13 @@ class ExecTool(Tool): effective_timeout = min(timeout or self.timeout, self._MAX_TIMEOUT) - env = os.environ.copy() - if self.path_append: - env["PATH"] = env.get("PATH", "") + os.pathsep + self.path_append + env = self._build_env() + + bash = shutil.which("bash") or "/bin/bash" try: - process = await asyncio.create_subprocess_shell( - command, + process = await asyncio.create_subprocess_exec( + bash, "-l", "-c", command, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=cwd, @@ -154,6 +155,25 @@ class ExecTool(Tool): except Exception as 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: """Best-effort safety guard for potentially destructive commands.""" cmd = command.strip() diff --git a/tests/tools/test_exec_env.py b/tests/tools/test_exec_env.py new file mode 100644 index 000000000..30358e688 --- /dev/null +++ b/tests/tools/test_exec_env.py @@ -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