Skip to content

Commit 525172c

Browse files
committed
fix(skill_loader): add conditional loading of execute_command based on env var
This change introduces an environment variable `ALLOW_EXECUTE_COMMAND` that controls whether the `execute_command` tool is loaded into the CommonToolSet. By default, the tool is included unless explicitly disabled via the environment variable being set to `"false"` (case-insensitive). This provides better control over security-sensitive functionality. Tests have been added to verify all scenarios including default behavior, true/false values, and case insensitivity. Co-developed-by: Aone Copilot <noreply@alibaba-inc.com> Signed-off-by: Sodawyx <sodawyx@126.com>
1 parent ca41adb commit 525172c

File tree

2 files changed

+165
-46
lines changed

2 files changed

+165
-46
lines changed

agentrun/integration/utils/skill_loader.py

Lines changed: 66 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -600,10 +600,32 @@ def _execute_command_func(
600600
ensure_ascii=False,
601601
)
602602

603+
@staticmethod
604+
def _is_execute_command_allowed() -> bool:
605+
"""检查环境变量是否允许加载 execute_command 工具
606+
607+
Check whether the ALLOW_EXECUTE_COMMAND environment variable permits
608+
loading the execute_command tool.
609+
610+
The variable is read from ``os.environ``. When it is absent or set to
611+
any value other than a case-insensitive ``"false"``, the tool is
612+
allowed (default **True**).
613+
614+
Returns:
615+
True 表示允许 / True means allowed
616+
"""
617+
value = os.environ.get("ALLOW_EXECUTE_COMMAND", "true")
618+
return value.lower() != "false"
619+
603620
def to_common_toolset(self) -> CommonToolSet:
604-
"""构造包含 load_skills、read_skill_file、execute_command 工具的 CommonToolSet
621+
"""构造包含 load_skills、read_skill_file 以及可选的 execute_command 工具的 CommonToolSet
622+
623+
Construct CommonToolSet with load_skills, read_skill_file, and
624+
optionally execute_command tools.
605625
606-
Construct CommonToolSet with load_skills, read_skill_file, and execute_command tools.
626+
The execute_command tool is included only when the environment variable
627+
``ALLOW_EXECUTE_COMMAND`` is not set to ``"false"`` (case-insensitive).
628+
When the variable is absent, it defaults to ``"true"`` (included).
607629
608630
Returns:
609631
CommonToolSet 实例 / CommonToolSet instance
@@ -656,54 +678,52 @@ def to_common_toolset(self) -> CommonToolSet:
656678
func=self._read_skill_file_func,
657679
)
658680

659-
execute_command_tool = Tool(
660-
name="execute_command",
661-
description=(
662-
"Execute a shell command on the local machine. "
663-
"Use this to run scripts, install dependencies, or perform "
664-
"file operations as instructed by skill documentation. "
665-
"Returns stdout, stderr, exit_code, and timeout status.\n\n"
666-
"⚠️ IMPORTANT: Before calling this tool, you MUST first "
667-
"display the exact command to the user and ask for explicit "
668-
"confirmation. Only proceed if the user approves. "
669-
"Never execute commands without user approval."
670-
),
671-
parameters=[
672-
ToolParameter(
673-
name="command",
674-
param_type="string",
675-
description="The shell command to execute.",
676-
required=True,
681+
tools_list: List[Tool] = [load_skills_tool, read_skill_file_tool]
682+
683+
if self._is_execute_command_allowed():
684+
execute_command_tool = Tool(
685+
name="execute_command",
686+
description=(
687+
"Execute a shell command on the local machine. Use this to"
688+
" run scripts, install dependencies, or perform file"
689+
" operations as instructed by skill documentation. Returns"
690+
" stdout, stderr, exit_code, and timeout status.\n\n⚠️"
691+
" IMPORTANT: Before calling this tool, you MUST first"
692+
" display the exact command to the user and ask for"
693+
" explicit confirmation. Only proceed if the user approves."
694+
" Never execute commands without user approval."
677695
),
678-
ToolParameter(
679-
name="cwd",
680-
param_type="string",
681-
description=(
682-
"Working directory for the command. "
683-
"Defaults to the skills directory if not specified."
696+
parameters=[
697+
ToolParameter(
698+
name="command",
699+
param_type="string",
700+
description="The shell command to execute.",
701+
required=True,
684702
),
685-
required=False,
686-
),
687-
ToolParameter(
688-
name="timeout",
689-
param_type="integer",
690-
description=(
691-
"Maximum execution time in seconds. "
692-
f"Defaults to {self._command_timeout}."
703+
ToolParameter(
704+
name="cwd",
705+
param_type="string",
706+
description=(
707+
"Working directory for the command. "
708+
"Defaults to the skills directory if not specified."
709+
),
710+
required=False,
693711
),
694-
required=False,
695-
),
696-
],
697-
func=self._execute_command_func,
698-
)
712+
ToolParameter(
713+
name="timeout",
714+
param_type="integer",
715+
description=(
716+
"Maximum execution time in seconds. "
717+
f"Defaults to {self._command_timeout}."
718+
),
719+
required=False,
720+
),
721+
],
722+
func=self._execute_command_func,
723+
)
724+
tools_list.append(execute_command_tool)
699725

700-
return CommonToolSet(
701-
tools_list=[
702-
load_skills_tool,
703-
read_skill_file_tool,
704-
execute_command_tool,
705-
]
706-
)
726+
return CommonToolSet(tools_list=tools_list)
707727

708728

709729
def skill_tools(

tests/unittests/integration/test_skill_loader.py

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1343,3 +1343,102 @@ def test_execute_command_description_has_safety_warning(
13431343
exec_tool = tool_map["execute_command"]
13441344
assert "IMPORTANT" in exec_tool.description
13451345
assert "approval" in exec_tool.description.lower()
1346+
1347+
1348+
# =============================================================================
1349+
# 17. ALLOW_EXECUTE_COMMAND 环境变量控制测试
1350+
# =============================================================================
1351+
1352+
1353+
class TestAllowExecuteCommandEnvVar:
1354+
"""测试 ALLOW_EXECUTE_COMMAND 环境变量对 execute_command 工具加载的控制"""
1355+
1356+
def test_default_includes_execute_command(self, tmp_path: Any) -> None:
1357+
"""未设置环境变量时,默认包含 execute_command 工具"""
1358+
skills_dir = str(tmp_path / "skills")
1359+
os.makedirs(skills_dir)
1360+
with patch.dict(os.environ, {}, clear=False):
1361+
os.environ.pop("ALLOW_EXECUTE_COMMAND", None)
1362+
loader = SkillLoader(skills_dir=skills_dir)
1363+
toolset = loader.to_common_toolset()
1364+
tool_names = [t.name for t in toolset.tools()]
1365+
assert "execute_command" in tool_names
1366+
assert len(toolset.tools()) == 3
1367+
1368+
def test_env_true_includes_execute_command(self, tmp_path: Any) -> None:
1369+
"""ALLOW_EXECUTE_COMMAND=true 时,包含 execute_command 工具"""
1370+
skills_dir = str(tmp_path / "skills")
1371+
os.makedirs(skills_dir)
1372+
with patch.dict(os.environ, {"ALLOW_EXECUTE_COMMAND": "true"}):
1373+
loader = SkillLoader(skills_dir=skills_dir)
1374+
toolset = loader.to_common_toolset()
1375+
tool_names = [t.name for t in toolset.tools()]
1376+
assert "execute_command" in tool_names
1377+
assert len(toolset.tools()) == 3
1378+
1379+
def test_env_false_excludes_execute_command(self, tmp_path: Any) -> None:
1380+
"""ALLOW_EXECUTE_COMMAND=false 时,不包含 execute_command 工具"""
1381+
skills_dir = str(tmp_path / "skills")
1382+
os.makedirs(skills_dir)
1383+
with patch.dict(os.environ, {"ALLOW_EXECUTE_COMMAND": "false"}):
1384+
loader = SkillLoader(skills_dir=skills_dir)
1385+
toolset = loader.to_common_toolset()
1386+
tool_names = [t.name for t in toolset.tools()]
1387+
assert "execute_command" not in tool_names
1388+
assert len(toolset.tools()) == 2
1389+
assert "load_skills" in tool_names
1390+
assert "read_skill_file" in tool_names
1391+
1392+
def test_env_false_case_insensitive(self, tmp_path: Any) -> None:
1393+
"""ALLOW_EXECUTE_COMMAND=False / FALSE 等大小写变体均生效"""
1394+
skills_dir = str(tmp_path / "skills")
1395+
os.makedirs(skills_dir)
1396+
for value in ("False", "FALSE", "fAlSe"):
1397+
with patch.dict(os.environ, {"ALLOW_EXECUTE_COMMAND": value}):
1398+
loader = SkillLoader(skills_dir=skills_dir)
1399+
toolset = loader.to_common_toolset()
1400+
tool_names = [t.name for t in toolset.tools()]
1401+
assert (
1402+
"execute_command" not in tool_names
1403+
), f"execute_command should be excluded for value={value!r}"
1404+
1405+
def test_env_non_false_includes_execute_command(
1406+
self, tmp_path: Any
1407+
) -> None:
1408+
"""ALLOW_EXECUTE_COMMAND 设置为非 false 的值时,包含 execute_command"""
1409+
skills_dir = str(tmp_path / "skills")
1410+
os.makedirs(skills_dir)
1411+
for value in ("True", "TRUE", "1", "yes", "anything"):
1412+
with patch.dict(os.environ, {"ALLOW_EXECUTE_COMMAND": value}):
1413+
loader = SkillLoader(skills_dir=skills_dir)
1414+
toolset = loader.to_common_toolset()
1415+
tool_names = [t.name for t in toolset.tools()]
1416+
assert (
1417+
"execute_command" in tool_names
1418+
), f"execute_command should be included for value={value!r}"
1419+
1420+
def test_is_execute_command_allowed_static_method(self) -> None:
1421+
"""直接测试 _is_execute_command_allowed 静态方法"""
1422+
with patch.dict(os.environ, {}, clear=False):
1423+
os.environ.pop("ALLOW_EXECUTE_COMMAND", None)
1424+
assert SkillLoader._is_execute_command_allowed() is True
1425+
1426+
with patch.dict(os.environ, {"ALLOW_EXECUTE_COMMAND": "true"}):
1427+
assert SkillLoader._is_execute_command_allowed() is True
1428+
1429+
with patch.dict(os.environ, {"ALLOW_EXECUTE_COMMAND": "false"}):
1430+
assert SkillLoader._is_execute_command_allowed() is False
1431+
1432+
with patch.dict(os.environ, {"ALLOW_EXECUTE_COMMAND": "False"}):
1433+
assert SkillLoader._is_execute_command_allowed() is False
1434+
1435+
def test_skill_tools_func_respects_env_var(self, tmp_path: Any) -> None:
1436+
"""测试顶层 skill_tools() 函数也受环境变量控制"""
1437+
skills_dir = str(tmp_path / "skills")
1438+
os.makedirs(skills_dir)
1439+
with patch.dict(os.environ, {"ALLOW_EXECUTE_COMMAND": "false"}):
1440+
toolset = skill_tools(skills_dir=skills_dir)
1441+
tool_names = [t.name for t in toolset.tools()]
1442+
assert "execute_command" not in tool_names
1443+
assert "load_skills" in tool_names
1444+
assert "read_skill_file" in tool_names

0 commit comments

Comments
 (0)