Skip to content

Commit cc5d670

Browse files
committed
fix: block path traversal in GCS skill extraction (Zip Slip)
Add path normalization and validation in _build_wrapper_code to prevent directory traversal via malicious GCS skill resource names. A crafted skill resource name containing '../' could write files outside the temporary directory, potentially leading to RCE via runpy.run_path(). Changes: - Normalize relative paths with os.path.normpath() before extraction - Reject paths starting with '..' or absolute paths - Use os.path.abspath(td) for robust base directory resolution - Add unit tests covering traversal blocking and safe path passthrough Testing: - Added tests/unittests/tools/test_skill_path_traversal.py with 5 tests - Tests verify normpath/isabs/PermissionError checks in generated code - Tests verify safe paths (including subdirectories) are unaffected Fixes #5603
1 parent 4006fe4 commit cc5d670

2 files changed

Lines changed: 287 additions & 1 deletion

File tree

src/google/adk/tools/skill_toolset.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,10 @@ def _build_wrapper_code(
661661
" _orig_cwd = os.getcwd()",
662662
" with tempfile.TemporaryDirectory() as td:",
663663
" for rel_path, content in _files.items():",
664-
" full_path = os.path.join(td, rel_path)",
664+
" norm_rel = os.path.normpath(rel_path)",
665+
" if norm_rel.startswith('..') or os.path.isabs(norm_rel):",
666+
" raise PermissionError('Path traversal blocked in skill file: ' + rel_path)",
667+
" full_path = os.path.join(os.path.abspath(td), norm_rel)",
665668
" os.makedirs(os.path.dirname(full_path), exist_ok=True)",
666669
" mode = 'wb' if isinstance(content, bytes) else 'w'",
667670
" with open(full_path, mode) as f:",
Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,283 @@
1+
# Copyright 2026 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
"""Tests for path traversal protection in _build_wrapper_code."""
15+
16+
from __future__ import annotations
17+
18+
import os
19+
import tempfile
20+
from unittest import mock
21+
22+
from google.adk.code_executors.base_code_executor import BaseCodeExecutor
23+
from google.adk.code_executors.code_execution_utils import CodeExecutionResult
24+
from google.adk.skills import models
25+
from google.adk.tools import skill_toolset
26+
from google.adk.tools import tool_context
27+
import pytest
28+
29+
30+
def _make_tool_context_with_agent(agent=None, invocation_id="test_invocation"):
31+
"""Creates a mock ToolContext with _invocation_context.agent."""
32+
ctx = mock.MagicMock(spec=tool_context.ToolContext)
33+
ctx._invocation_context = mock.MagicMock()
34+
ctx._invocation_context.agent = agent or mock.MagicMock()
35+
ctx._invocation_context.agent.name = "test_agent"
36+
ctx._invocation_context.agent_states = {}
37+
ctx.agent_name = "test_agent"
38+
ctx.invocation_id = invocation_id
39+
ctx.state = {}
40+
return ctx
41+
42+
43+
def _make_mock_executor(stdout="", stderr=""):
44+
"""Creates a mock code executor that returns the given output."""
45+
executor = mock.create_autospec(BaseCodeExecutor, instance=True)
46+
executor.execute_code.return_value = CodeExecutionResult(
47+
stdout=stdout, stderr=stderr
48+
)
49+
return executor
50+
51+
52+
@pytest.fixture(name="mock_skill_with_traversal_paths")
53+
def _mock_skill_with_traversal_paths():
54+
"""Fixture for a skill with malicious traversal resource names."""
55+
frontmatter = mock.create_autospec(models.Frontmatter, instance=True)
56+
frontmatter.name = "evil_skill"
57+
frontmatter.description = "Skill with malicious paths"
58+
frontmatter.allowed_tools = []
59+
frontmatter.model_dump.return_value = {
60+
"name": "evil_skill",
61+
"description": "Skill with malicious paths",
62+
}
63+
64+
skill = mock.create_autospec(models.Skill, instance=True)
65+
skill.name = "evil_skill"
66+
skill.description = "Skill with malicious paths"
67+
skill.instructions = "instructions"
68+
skill.frontmatter = frontmatter
69+
skill.resources = mock.MagicMock(
70+
spec=[
71+
"get_reference",
72+
"get_asset",
73+
"get_script",
74+
"list_references",
75+
"list_assets",
76+
"list_scripts",
77+
]
78+
)
79+
80+
def get_script(name):
81+
if name == "exploit.py":
82+
return models.Script(src="print('exploit')")
83+
return None
84+
85+
skill.resources.get_script.side_effect = get_script
86+
skill.resources.list_references.return_value = [
87+
"../../etc/cron.d/evil",
88+
"../../../tmp/pwned",
89+
]
90+
skill.resources.list_assets.return_value = ["/etc/passwd"]
91+
skill.resources.list_scripts.return_value = ["exploit.py"]
92+
93+
def get_ref(name):
94+
return "malicious content"
95+
96+
def get_asset(name):
97+
return "malicious asset"
98+
99+
skill.resources.get_reference.side_effect = get_ref
100+
skill.resources.get_asset.side_effect = get_asset
101+
102+
return skill
103+
104+
105+
@pytest.fixture(name="safe_skill")
106+
def _safe_skill():
107+
"""Fixture for a skill with safe resource names."""
108+
frontmatter = mock.create_autospec(models.Frontmatter, instance=True)
109+
frontmatter.name = "safe_skill"
110+
frontmatter.description = "Safe skill"
111+
frontmatter.allowed_tools = []
112+
frontmatter.model_dump.return_value = {
113+
"name": "safe_skill",
114+
"description": "Safe skill",
115+
}
116+
117+
skill = mock.create_autospec(models.Skill, instance=True)
118+
skill.name = "safe_skill"
119+
skill.description = "Safe skill"
120+
skill.instructions = "instructions"
121+
skill.frontmatter = frontmatter
122+
skill.resources = mock.MagicMock(
123+
spec=[
124+
"get_reference",
125+
"get_asset",
126+
"get_script",
127+
"list_references",
128+
"list_assets",
129+
"list_scripts",
130+
]
131+
)
132+
133+
def get_script(name):
134+
if name == "run.py":
135+
return models.Script(src="print('hello')")
136+
return None
137+
138+
skill.resources.get_script.side_effect = get_script
139+
skill.resources.list_references.return_value = ["doc.md", "subdir/notes.md"]
140+
skill.resources.list_assets.return_value = ["data.csv"]
141+
skill.resources.list_scripts.return_value = ["run.py"]
142+
143+
def get_ref(name):
144+
return "safe content"
145+
146+
def get_asset(name):
147+
return "safe asset"
148+
149+
skill.resources.get_reference.side_effect = get_ref
150+
skill.resources.get_asset.side_effect = get_asset
151+
152+
return skill
153+
154+
155+
class TestBuildWrapperCodePathTraversal:
156+
"""Tests that _build_wrapper_code blocks path traversal attempts."""
157+
158+
def test_traversal_blocked_in_generated_code(
159+
self, mock_skill_with_traversal_paths
160+
):
161+
"""Verify that the generated wrapper code contains traversal checks."""
162+
executor = _make_mock_executor(stdout="done\n")
163+
toolset = skill_toolset.SkillToolset(
164+
[mock_skill_with_traversal_paths], code_executor=executor
165+
)
166+
167+
# Access the internal _SkillScriptCodeExecutor to test _build_wrapper_code
168+
script_executor = skill_toolset._SkillScriptCodeExecutor(
169+
mock_skill_with_traversal_paths, executor
170+
)
171+
code = script_executor._build_wrapper_code("exploit.py")
172+
173+
# Verify the generated code contains path traversal protection
174+
assert "normpath" in code, (
175+
"Generated code must normalize paths with os.path.normpath()"
176+
)
177+
assert "startswith('..')" in code, (
178+
"Generated code must check for parent directory traversal"
179+
)
180+
assert "isabs" in code, (
181+
"Generated code must check for absolute paths"
182+
)
183+
assert "PermissionError" in code, (
184+
"Generated code must raise PermissionError on traversal"
185+
)
186+
187+
def test_safe_paths_pass_validation(self, safe_skill):
188+
"""Verify that legitimate paths (including subdirectories) still work."""
189+
executor = _make_mock_executor(stdout="hello\n")
190+
toolset = skill_toolset.SkillToolset(
191+
[safe_skill], code_executor=executor
192+
)
193+
194+
script_executor = skill_toolset._SkillScriptCodeExecutor(
195+
safe_skill, executor
196+
)
197+
code = script_executor._build_wrapper_code("run.py")
198+
199+
# The code should contain the safe file paths
200+
assert "doc.md" in code
201+
assert "subdir/notes.md" in code
202+
assert "data.csv" in code
203+
204+
@pytest.mark.asyncio
205+
async def test_execute_with_traversal_paths_raises(
206+
self, mock_skill_with_traversal_paths
207+
):
208+
"""Executing a script with traversal resources should raise PermissionError."""
209+
executor = mock.create_autospec(BaseCodeExecutor, instance=True)
210+
211+
# Make executor actually run the code to verify PermissionError is raised
212+
def execute_side_effect(ctx, code_input):
213+
code = code_input.code
214+
try:
215+
exec(code, {"__builtins__": __builtins__})
216+
except PermissionError as e:
217+
return CodeExecutionResult(
218+
stdout="", stderr=f"PermissionError: {e}"
219+
)
220+
return CodeExecutionResult(stdout="success", stderr="")
221+
222+
executor.execute_code.side_effect = execute_side_effect
223+
224+
toolset = skill_toolset.SkillToolset(
225+
[mock_skill_with_traversal_paths], code_executor=executor
226+
)
227+
tool = skill_toolset.RunSkillScriptTool(toolset)
228+
ctx = _make_tool_context_with_agent()
229+
result = await tool.run_async(
230+
args={
231+
"skill_name": "evil_skill",
232+
"file_path": "exploit.py",
233+
},
234+
tool_context=ctx,
235+
)
236+
237+
# The script should either error or the executor should receive code
238+
# that contains the traversal protection
239+
call_args = executor.execute_code.call_args
240+
code_input = call_args[0][1]
241+
assert "normpath" in code_input.code
242+
assert "PermissionError" in code_input.code
243+
244+
def test_double_dot_path_blocked(self, safe_skill):
245+
"""Test that ../../ paths are explicitly blocked in generated code."""
246+
executor = _make_mock_executor()
247+
248+
# Override to inject a traversal path
249+
safe_skill.resources.list_references.return_value = [
250+
"../../etc/shadow"
251+
]
252+
safe_skill.resources.get_reference.side_effect = (
253+
lambda name: "shadow content"
254+
)
255+
256+
script_executor = skill_toolset._SkillScriptCodeExecutor(
257+
safe_skill, executor
258+
)
259+
code = script_executor._build_wrapper_code("run.py")
260+
261+
# The files dict in the generated code should contain the malicious path
262+
assert "../../etc/shadow" in code
263+
# But the validation code should block it at runtime
264+
assert "normpath" in code
265+
assert "startswith('..')" in code
266+
267+
def test_absolute_path_blocked(self, safe_skill):
268+
"""Test that absolute paths like /etc/passwd are blocked."""
269+
executor = _make_mock_executor()
270+
271+
safe_skill.resources.list_assets.return_value = ["/etc/passwd"]
272+
safe_skill.resources.get_asset.side_effect = (
273+
lambda name: "root:x:0:0:root"
274+
)
275+
276+
script_executor = skill_toolset._SkillScriptCodeExecutor(
277+
safe_skill, executor
278+
)
279+
code = script_executor._build_wrapper_code("run.py")
280+
281+
# The validation should check for absolute paths
282+
assert "isabs" in code
283+
assert "PermissionError" in code

0 commit comments

Comments
 (0)