From cc5d6705a281ae10d02893a554837f17ccec21b7 Mon Sep 17 00:00:00 2001
From: Ashutosh0x <ashutoshkumarsingh0x@gmail.com>
Date: Mon, 1 Jun 2026 19:48:00 +0530
Subject: [PATCH] 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
---
 src/google/adk/tools/skill_toolset.py         |   5 +-
 .../tools/test_skill_path_traversal.py        | 283 ++++++++++++++++++
 2 files changed, 287 insertions(+), 1 deletion(-)
 create mode 100644 tests/unittests/tools/test_skill_path_traversal.py

diff --git a/src/google/adk/tools/skill_toolset.py b/src/google/adk/tools/skill_toolset.py
index 9b82b1a4b6..c0e7b748ff 100644
--- a/src/google/adk/tools/skill_toolset.py
+++ b/src/google/adk/tools/skill_toolset.py
@@ -661,7 +661,10 @@ def _build_wrapper_code(
         "  _orig_cwd = os.getcwd()",
         "  with tempfile.TemporaryDirectory() as td:",
         "    for rel_path, content in _files.items():",
-        "      full_path = os.path.join(td, rel_path)",
+        "      norm_rel = os.path.normpath(rel_path)",
+        "      if norm_rel.startswith('..') or os.path.isabs(norm_rel):",
+        "        raise PermissionError('Path traversal blocked in skill file: ' + rel_path)",
+        "      full_path = os.path.join(os.path.abspath(td), norm_rel)",
         "      os.makedirs(os.path.dirname(full_path), exist_ok=True)",
         "      mode = 'wb' if isinstance(content, bytes) else 'w'",
         "      with open(full_path, mode) as f:",
diff --git a/tests/unittests/tools/test_skill_path_traversal.py b/tests/unittests/tools/test_skill_path_traversal.py
new file mode 100644
index 0000000000..8bbc0f5a6e
--- /dev/null
+++ b/tests/unittests/tools/test_skill_path_traversal.py
@@ -0,0 +1,283 @@
+# Copyright 2026 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Tests for path traversal protection in _build_wrapper_code."""
+
+from __future__ import annotations
+
+import os
+import tempfile
+from unittest import mock
+
+from google.adk.code_executors.base_code_executor import BaseCodeExecutor
+from google.adk.code_executors.code_execution_utils import CodeExecutionResult
+from google.adk.skills import models
+from google.adk.tools import skill_toolset
+from google.adk.tools import tool_context
+import pytest
+
+
+def _make_tool_context_with_agent(agent=None, invocation_id="test_invocation"):
+  """Creates a mock ToolContext with _invocation_context.agent."""
+  ctx = mock.MagicMock(spec=tool_context.ToolContext)
+  ctx._invocation_context = mock.MagicMock()
+  ctx._invocation_context.agent = agent or mock.MagicMock()
+  ctx._invocation_context.agent.name = "test_agent"
+  ctx._invocation_context.agent_states = {}
+  ctx.agent_name = "test_agent"
+  ctx.invocation_id = invocation_id
+  ctx.state = {}
+  return ctx
+
+
+def _make_mock_executor(stdout="", stderr=""):
+  """Creates a mock code executor that returns the given output."""
+  executor = mock.create_autospec(BaseCodeExecutor, instance=True)
+  executor.execute_code.return_value = CodeExecutionResult(
+      stdout=stdout, stderr=stderr
+  )
+  return executor
+
+
+@pytest.fixture(name="mock_skill_with_traversal_paths")
+def _mock_skill_with_traversal_paths():
+  """Fixture for a skill with malicious traversal resource names."""
+  frontmatter = mock.create_autospec(models.Frontmatter, instance=True)
+  frontmatter.name = "evil_skill"
+  frontmatter.description = "Skill with malicious paths"
+  frontmatter.allowed_tools = []
+  frontmatter.model_dump.return_value = {
+      "name": "evil_skill",
+      "description": "Skill with malicious paths",
+  }
+
+  skill = mock.create_autospec(models.Skill, instance=True)
+  skill.name = "evil_skill"
+  skill.description = "Skill with malicious paths"
+  skill.instructions = "instructions"
+  skill.frontmatter = frontmatter
+  skill.resources = mock.MagicMock(
+      spec=[
+          "get_reference",
+          "get_asset",
+          "get_script",
+          "list_references",
+          "list_assets",
+          "list_scripts",
+      ]
+  )
+
+  def get_script(name):
+    if name == "exploit.py":
+      return models.Script(src="print('exploit')")
+    return None
+
+  skill.resources.get_script.side_effect = get_script
+  skill.resources.list_references.return_value = [
+      "../../etc/cron.d/evil",
+      "../../../tmp/pwned",
+  ]
+  skill.resources.list_assets.return_value = ["/etc/passwd"]
+  skill.resources.list_scripts.return_value = ["exploit.py"]
+
+  def get_ref(name):
+    return "malicious content"
+
+  def get_asset(name):
+    return "malicious asset"
+
+  skill.resources.get_reference.side_effect = get_ref
+  skill.resources.get_asset.side_effect = get_asset
+
+  return skill
+
+
+@pytest.fixture(name="safe_skill")
+def _safe_skill():
+  """Fixture for a skill with safe resource names."""
+  frontmatter = mock.create_autospec(models.Frontmatter, instance=True)
+  frontmatter.name = "safe_skill"
+  frontmatter.description = "Safe skill"
+  frontmatter.allowed_tools = []
+  frontmatter.model_dump.return_value = {
+      "name": "safe_skill",
+      "description": "Safe skill",
+  }
+
+  skill = mock.create_autospec(models.Skill, instance=True)
+  skill.name = "safe_skill"
+  skill.description = "Safe skill"
+  skill.instructions = "instructions"
+  skill.frontmatter = frontmatter
+  skill.resources = mock.MagicMock(
+      spec=[
+          "get_reference",
+          "get_asset",
+          "get_script",
+          "list_references",
+          "list_assets",
+          "list_scripts",
+      ]
+  )
+
+  def get_script(name):
+    if name == "run.py":
+      return models.Script(src="print('hello')")
+    return None
+
+  skill.resources.get_script.side_effect = get_script
+  skill.resources.list_references.return_value = ["doc.md", "subdir/notes.md"]
+  skill.resources.list_assets.return_value = ["data.csv"]
+  skill.resources.list_scripts.return_value = ["run.py"]
+
+  def get_ref(name):
+    return "safe content"
+
+  def get_asset(name):
+    return "safe asset"
+
+  skill.resources.get_reference.side_effect = get_ref
+  skill.resources.get_asset.side_effect = get_asset
+
+  return skill
+
+
+class TestBuildWrapperCodePathTraversal:
+  """Tests that _build_wrapper_code blocks path traversal attempts."""
+
+  def test_traversal_blocked_in_generated_code(
+      self, mock_skill_with_traversal_paths
+  ):
+    """Verify that the generated wrapper code contains traversal checks."""
+    executor = _make_mock_executor(stdout="done\n")
+    toolset = skill_toolset.SkillToolset(
+        [mock_skill_with_traversal_paths], code_executor=executor
+    )
+
+    # Access the internal _SkillScriptCodeExecutor to test _build_wrapper_code
+    script_executor = skill_toolset._SkillScriptCodeExecutor(
+        mock_skill_with_traversal_paths, executor
+    )
+    code = script_executor._build_wrapper_code("exploit.py")
+
+    # Verify the generated code contains path traversal protection
+    assert "normpath" in code, (
+        "Generated code must normalize paths with os.path.normpath()"
+    )
+    assert "startswith('..')" in code, (
+        "Generated code must check for parent directory traversal"
+    )
+    assert "isabs" in code, (
+        "Generated code must check for absolute paths"
+    )
+    assert "PermissionError" in code, (
+        "Generated code must raise PermissionError on traversal"
+    )
+
+  def test_safe_paths_pass_validation(self, safe_skill):
+    """Verify that legitimate paths (including subdirectories) still work."""
+    executor = _make_mock_executor(stdout="hello\n")
+    toolset = skill_toolset.SkillToolset(
+        [safe_skill], code_executor=executor
+    )
+
+    script_executor = skill_toolset._SkillScriptCodeExecutor(
+        safe_skill, executor
+    )
+    code = script_executor._build_wrapper_code("run.py")
+
+    # The code should contain the safe file paths
+    assert "doc.md" in code
+    assert "subdir/notes.md" in code
+    assert "data.csv" in code
+
+  @pytest.mark.asyncio
+  async def test_execute_with_traversal_paths_raises(
+      self, mock_skill_with_traversal_paths
+  ):
+    """Executing a script with traversal resources should raise PermissionError."""
+    executor = mock.create_autospec(BaseCodeExecutor, instance=True)
+
+    # Make executor actually run the code to verify PermissionError is raised
+    def execute_side_effect(ctx, code_input):
+      code = code_input.code
+      try:
+        exec(code, {"__builtins__": __builtins__})
+      except PermissionError as e:
+        return CodeExecutionResult(
+            stdout="", stderr=f"PermissionError: {e}"
+        )
+      return CodeExecutionResult(stdout="success", stderr="")
+
+    executor.execute_code.side_effect = execute_side_effect
+
+    toolset = skill_toolset.SkillToolset(
+        [mock_skill_with_traversal_paths], code_executor=executor
+    )
+    tool = skill_toolset.RunSkillScriptTool(toolset)
+    ctx = _make_tool_context_with_agent()
+    result = await tool.run_async(
+        args={
+            "skill_name": "evil_skill",
+            "file_path": "exploit.py",
+        },
+        tool_context=ctx,
+    )
+
+    # The script should either error or the executor should receive code
+    # that contains the traversal protection
+    call_args = executor.execute_code.call_args
+    code_input = call_args[0][1]
+    assert "normpath" in code_input.code
+    assert "PermissionError" in code_input.code
+
+  def test_double_dot_path_blocked(self, safe_skill):
+    """Test that ../../ paths are explicitly blocked in generated code."""
+    executor = _make_mock_executor()
+
+    # Override to inject a traversal path
+    safe_skill.resources.list_references.return_value = [
+        "../../etc/shadow"
+    ]
+    safe_skill.resources.get_reference.side_effect = (
+        lambda name: "shadow content"
+    )
+
+    script_executor = skill_toolset._SkillScriptCodeExecutor(
+        safe_skill, executor
+    )
+    code = script_executor._build_wrapper_code("run.py")
+
+    # The files dict in the generated code should contain the malicious path
+    assert "../../etc/shadow" in code
+    # But the validation code should block it at runtime
+    assert "normpath" in code
+    assert "startswith('..')" in code
+
+  def test_absolute_path_blocked(self, safe_skill):
+    """Test that absolute paths like /etc/passwd are blocked."""
+    executor = _make_mock_executor()
+
+    safe_skill.resources.list_assets.return_value = ["/etc/passwd"]
+    safe_skill.resources.get_asset.side_effect = (
+        lambda name: "root:x:0:0:root"
+    )
+
+    script_executor = skill_toolset._SkillScriptCodeExecutor(
+        safe_skill, executor
+    )
+    code = script_executor._build_wrapper_code("run.py")
+
+    # The validation should check for absolute paths
+    assert "isabs" in code
+    assert "PermissionError" in code