fix: Fix path traversal in GCS skill extraction (Zip Slip)

Fix for #5603 - Validate normalized relative paths in skill extraction code to prevent directory traversal via malicious GCS skill resource names.

## Problem

The _build_wrapper_code method in skill_toolset.py generates Python code that extracts skill files into a temporary directory. The relative paths from GCS blob names are used directly with os.path.join(td, rel_path) without validation.

A malicious skill resource name containing ../ (e.g., references/../../etc/cron.d/evil) would resolve outside the temporary directory, enabling arbitrary file writes. Combined with runpy.run_path(), this creates a path-traversal-to-RCE chain (CWE-22 / Zip Slip variant).

## Fix

Added path normalization and validation before file extraction in the generated wrapper code:
1. Normalize the relative path with os.path.normpath()
2. Reject paths starting with .. (parent directory traversal)
3. Reject absolute paths via os.path.isabs()
4. Use os.path.abspath(td) for the base directory to prevent bypasses

## Testing

### Unit Tests Added

Added tests/unittests/tools/test_skill_path_traversal.py with 5 tests.

Fixes #5603

Merge #5927

Change-Id: I7dc2f92e863785ccb1d6ea0ed65b0b01c537fabc

Author: Ashutosh0x

src/google/adk/tools/skill_toolset.py

@@ -75,7 +75,7 @@ def _build_skill_system_instruction(prefix: str | None = None) -> str:
       "- **scripts/** (Optional): Executable scripts that can be run via "
       "bash.\n\n"
       "This is very important:\n\n"
-      f"1. If a skill seems relevant to the current user query, you MUST use "
+      "1. If a skill seems relevant to the current user query, you MUST use "
       f'the `{p}load_skill` tool with `skill_name="<SKILL_NAME>"` to read '
       "its full instructions before proceeding.\n"
       "2. Once you have read the instructions, follow them exactly as "
@@ -302,16 +302,15 @@ def _get_declaration(self) -> types.FunctionDeclaration | None:
   async def run_async(
       self, *, args: dict[str, Any], tool_context: ToolContext
   ) -> Any:
-    skill_name = args.get("skill_name")
-    file_path = args.get("file_path")
-
-    errors = []
-    if not skill_name:
-      errors.append("Argument 'skill_name' is required.")
-    if not file_path:
-      errors.append("Argument 'file_path' is required.")
-
-    if errors:
+    skill_name: str | None = args.get("skill_name")
+    file_path: str | None = args.get("file_path")
+
+    if not skill_name or not file_path:
+      errors = []
+      if not skill_name:
+        errors.append("Argument 'skill_name' is required.")
+      if not file_path:
+        errors.append("Argument 'file_path' is required.")
       return {
           "error": "\n".join(errors),
           "error_code": "INVALID_ARGUMENTS",
@@ -661,7 +660,13 @@ 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:",
@@ -815,18 +820,24 @@ async def run_async(
       self, *, args: dict[str, Any], tool_context: ToolContext
   ) -> Any:
     # Standardized arguments: skill_name and file_path.
-    skill_name = args.get("skill_name")
-    file_path = args.get("file_path")
+    skill_name: str | None = args.get("skill_name")
+    file_path: str | None = args.get("file_path")
     script_args = args.get("args")
     short_options = args.get("short_options")
     positional_args = args.get("positional_args")
 
-    errors = []
-    if not skill_name:
-      errors.append("Argument 'skill_name' is required.")
-    if not file_path:
-      errors.append("Argument 'file_path' is required.")
+    if not skill_name or not file_path:
+      errors = []
+      if not skill_name:
+        errors.append("Argument 'skill_name' is required.")
+      if not file_path:
+        errors.append("Argument 'file_path' is required.")
+      return {
+          "error": "\n".join(errors),
+          "error_code": "INVALID_ARGUMENTS",
+      }
 
+    errors = []
     if script_args is not None and not isinstance(script_args, (dict, list)):
       errors.append(
           "'args' must be a JSON object (dict) or a list of strings,"

tests/unittests/tools/test_skill_path_traversal.py

@@ -0,0 +1,280 @@
+# 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 typing import Any
+from typing import cast
+from unittest import mock
+
+from google.adk.agents.base_agent import BaseAgent
+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: BaseAgent | None = None, invocation_id: str = "test_invocation"
+) -> tool_context.ToolContext:
+  """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: str = "", stderr: str = "") -> mock.MagicMock:
+  """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 cast(mock.MagicMock, executor)
+
+
+@pytest.fixture(name="mock_skill_with_traversal_paths")  # type: ignore[untyped-decorator]
+def _mock_skill_with_traversal_paths() -> models.Skill:
+  """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: str) -> models.Script | None:
+    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: str) -> str:
+    return "malicious content"
+
+  def get_asset(name: str) -> str:
+    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")  # type: ignore[untyped-decorator]
+def _safe_skill() -> models.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: str) -> models.Script | None:
+    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: str) -> str:
+    return "safe content"
+
+  def get_asset(name: str) -> str:
+    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: models.Skill
+  ) -> None:
+    """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(
+        mock_skill_with_traversal_paths, "exploit.py", None
+    )
+
+    # 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: models.Skill) -> None:
+    """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(safe_skill, "run.py", None)
+
+    # 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  # type: ignore[untyped-decorator]
+  async def test_execute_with_traversal_paths_raises(
+      self, mock_skill_with_traversal_paths: models.Skill
+  ) -> None:
+    """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: Any, code_input: Any) -> CodeExecutionResult:
+      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: models.Skill) -> None:
+    """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(safe_skill, "run.py", None)
+
+    # 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: models.Skill) -> None:
+    """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(safe_skill, "run.py", None)
+
+    # The validation should check for absolute paths
+    assert "isabs" in code
+    assert "PermissionError" in code