mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-12 15:19:16 +03:00
fix(cli): stop leaking API keys to CLI app subprocesses
Installed CLI apps were started with os.environ.copy(), so provider keys from the parent process were visible to untrusted binaries. Use a minimal allowlist env matching the shell tool. Fixes #4783
This commit is contained in:
@@ -990,6 +990,35 @@ class CliAppManager:
|
|||||||
return None
|
return None
|
||||||
raise CliAppError("this CLI app uses an unsupported install strategy")
|
raise CliAppError("this CLI app uses an unsupported install strategy")
|
||||||
|
|
||||||
|
def _subprocess_env(self) -> dict[str, str]:
|
||||||
|
"""Minimal env for CLI app subprocesses — no API keys or secrets.
|
||||||
|
|
||||||
|
Mirrors the shell tool's allowlist so installed apps cannot read
|
||||||
|
provider credentials from the parent process environment.
|
||||||
|
"""
|
||||||
|
if sys.platform == "win32":
|
||||||
|
sr = os.environ.get("SYSTEMROOT", r"C:\Windows")
|
||||||
|
env = {
|
||||||
|
"SYSTEMROOT": sr,
|
||||||
|
"COMSPEC": os.environ.get("COMSPEC", f"{sr}\\system32\\cmd.exe"),
|
||||||
|
"USERPROFILE": os.environ.get("USERPROFILE", ""),
|
||||||
|
"HOMEDRIVE": os.environ.get("HOMEDRIVE", "C:"),
|
||||||
|
"HOMEPATH": os.environ.get("HOMEPATH", "\\"),
|
||||||
|
"TEMP": os.environ.get("TEMP", f"{sr}\\Temp"),
|
||||||
|
"TMP": os.environ.get("TMP", f"{sr}\\Temp"),
|
||||||
|
"PATHEXT": os.environ.get("PATHEXT", ".COM;.EXE;.BAT;.CMD"),
|
||||||
|
"PATH": os.environ.get("PATH", f"{sr}\\system32;{sr}"),
|
||||||
|
"PYTHONUNBUFFERED": "1",
|
||||||
|
}
|
||||||
|
return {k: v for k, v in env.items() if v is not None}
|
||||||
|
return {
|
||||||
|
"HOME": os.environ.get("HOME", "/tmp"),
|
||||||
|
"LANG": os.environ.get("LANG", "C.UTF-8"),
|
||||||
|
"TERM": os.environ.get("TERM", "dumb"),
|
||||||
|
"PATH": os.environ.get("PATH", "/usr/bin:/bin"),
|
||||||
|
"PYTHONUNBUFFERED": "1",
|
||||||
|
}
|
||||||
|
|
||||||
def _run_argv(self, argv: list[str], *, timeout: int) -> subprocess.CompletedProcess[str]:
|
def _run_argv(self, argv: list[str], *, timeout: int) -> subprocess.CompletedProcess[str]:
|
||||||
command = subprocess.list2cmdline(argv)
|
command = subprocess.list2cmdline(argv)
|
||||||
logger.info("CLI Apps: running {}", command)
|
logger.info("CLI Apps: running {}", command)
|
||||||
@@ -1431,7 +1460,7 @@ Use the `run_cli_app` tool with `name="{name}"` for command execution. Do not in
|
|||||||
encoding="utf-8",
|
encoding="utf-8",
|
||||||
errors="replace",
|
errors="replace",
|
||||||
timeout=effective_timeout,
|
timeout=effective_timeout,
|
||||||
env=os.environ.copy(),
|
env=self._subprocess_env(),
|
||||||
)
|
)
|
||||||
except subprocess.TimeoutExpired:
|
except subprocess.TimeoutExpired:
|
||||||
return f"CLI app '{name}' timed out after {effective_timeout}s"
|
return f"CLI app '{name}' timed out after {effective_timeout}s"
|
||||||
|
|||||||
@@ -0,0 +1,54 @@
|
|||||||
|
"""CLI app subprocesses must not inherit API keys from the parent environ."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from nanobot.apps.cli.service import CliAppManager
|
||||||
|
|
||||||
|
|
||||||
|
def test_subprocess_env_excludes_api_keys(monkeypatch, tmp_path) -> None:
|
||||||
|
monkeypatch.setenv("OPENAI_API_KEY", "sk-should-not-leak")
|
||||||
|
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-leak")
|
||||||
|
monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-leak")
|
||||||
|
|
||||||
|
manager = CliAppManager(workspace=tmp_path, data_dir=tmp_path / "cli-apps")
|
||||||
|
env = manager._subprocess_env()
|
||||||
|
|
||||||
|
assert "OPENAI_API_KEY" not in env
|
||||||
|
assert "ANTHROPIC_API_KEY" not in env
|
||||||
|
assert "OPENROUTER_API_KEY" not in env
|
||||||
|
assert env.get("PYTHONUNBUFFERED") == "1"
|
||||||
|
assert "PATH" in env
|
||||||
|
|
||||||
|
|
||||||
|
def test_run_passes_filtered_env(monkeypatch, tmp_path) -> None:
|
||||||
|
monkeypatch.setenv("OPENAI_API_KEY", "sk-should-not-leak")
|
||||||
|
manager = CliAppManager(workspace=tmp_path, data_dir=tmp_path / "cli-apps")
|
||||||
|
captured: dict[str, object] = {}
|
||||||
|
|
||||||
|
def fake_run(*args, **kwargs):
|
||||||
|
captured.update(kwargs)
|
||||||
|
|
||||||
|
class Result:
|
||||||
|
returncode = 0
|
||||||
|
stdout = "ok"
|
||||||
|
stderr = ""
|
||||||
|
|
||||||
|
return Result()
|
||||||
|
|
||||||
|
monkeypatch.setattr("nanobot.apps.cli.service.subprocess.run", fake_run)
|
||||||
|
monkeypatch.setattr(manager, "get_app", lambda name: {"name": name, "entry_point": "echo"})
|
||||||
|
monkeypatch.setattr(
|
||||||
|
manager,
|
||||||
|
"_load_installed",
|
||||||
|
lambda: {"echo": {"entry_point": "echo"}},
|
||||||
|
)
|
||||||
|
monkeypatch.setattr("nanobot.apps.cli.service.shutil.which", lambda entry: "/bin/echo")
|
||||||
|
monkeypatch.setattr(manager, "_resolve_cwd", lambda *a, **k: tmp_path)
|
||||||
|
monkeypatch.setattr(manager, "_artifact_snapshot", lambda cwd: {})
|
||||||
|
monkeypatch.setattr(manager, "_changed_artifacts", lambda cwd, snap: [])
|
||||||
|
|
||||||
|
manager.run("echo", ["hi"])
|
||||||
|
|
||||||
|
env = captured.get("env")
|
||||||
|
assert isinstance(env, dict)
|
||||||
|
assert "OPENAI_API_KEY" not in env
|
||||||
Reference in New Issue
Block a user