fix: Port skill resource path and skill name validation fixes to v1 (#6805)

src/google/adk/integrations/skill_registry/gcp_skill_registry.py

@@ -56,7 +56,22 @@ class GCPSkillRegistry(SkillRegistry):
 
     Returns:
       A Skill object.
+
+    Raises:
+      ValueError: If the name is not a valid skill name.
     """
+    # The name reaches here straight from a model-issued tool call, so it must
+    # be a single path segment before it is interpolated into the resource
+    # path. Accept the same character set skill names are already held to; the
+    # snake-or-kebab pattern is the superset of the two accepted spellings.
+    # pylint: disable-next=protected-access
+    if not models._SNAKE_OR_KEBAB_NAME_PATTERN.match(name):
+      raise ValueError(
+          f"Invalid skill name {name!r}: name must be lowercase kebab-case"
+          " (a-z, 0-9, hyphens) or snake_case (a-z, 0-9, underscores), with"
+          " no leading, trailing, or consecutive delimiters."
+      )
+
     full_name = (
         f"projects/{self.project_id}/locations/{self.location}/skills/{name}"
     )

src/google/adk/tools/skill_toolset.py

@@ -626,7 +626,13 @@ class _SkillScriptCodeExecutor:
         "  _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:",

tests/unittests/integrations/skill_registry/test_gcp_skill_registry.py

@@ -56,8 +56,9 @@ def _create_fake_zip_bytes():
   return zip_buffer.getvalue()
 
 
+@pytest.mark.parametrize("valid_name", ["my-skill", "my_skill", "skill2"])
 @pytest.mark.asyncio
-async def test_get_skill_success(mock_vertex_client):
+async def test_get_skill_success(mock_vertex_client, valid_name):
   """Verifies that get_skill successfully fetches and loads a skill in memory."""
   registry = GCPSkillRegistry()
 
@@ -71,13 +72,13 @@ async def test_get_skill_success(mock_vertex_client):
       return_value=mock_skill_resource
   )
 
-  skill = await registry.get_skill(name="my-skill")
+  skill = await registry.get_skill(name=valid_name)
 
   assert skill.frontmatter.name == "my-skill"
   assert skill.frontmatter.description == "test"
   assert skill.instructions == "# My Skill"
   mock_vertex_client.aio.skills.get.assert_called_once_with(
-      name="projects/test-project/locations/us-central1/skills/my-skill"
+      name=f"projects/test-project/locations/us-central1/skills/{valid_name}"
   )
 
 
@@ -182,3 +183,29 @@ async def test_get_skill_raises_on_invalid_skill_name(mock_vertex_client):
 
   with pytest.raises(ValueError, match="Invalid skill name in SKILL.md"):
     await registry.get_skill(name="my-skill")
+
+
+@pytest.mark.parametrize(
+    "unsafe_name",
+    [
+        "../../../projects/victim/locations/us-central1/skills/secret",
+        "my-skill/../other-skill",
+        "..%2f..%2fsecret",
+        "my-skill?alt=media",
+        "my-skill#fragment",
+        "my-skill/revisions/rev-123",
+        "My-Skill",
+        "",
+    ],
+)
+@pytest.mark.asyncio
+async def test_get_skill_rejects_unsafe_name_before_any_request(
+    mock_vertex_client, unsafe_name
+):
+  """Verifies that a name that is not a single safe path segment is rejected."""
+  registry = GCPSkillRegistry()
+
+  with pytest.raises(ValueError, match="Invalid skill name"):
+    await registry.get_skill(name=unsafe_name)
+
+  mock_vertex_client.aio.skills.get.assert_not_called()

tests/unittests/tools/test_skill_toolset.py

@@ -1031,8 +1031,9 @@ async def test_execute_script_extensionless_unsupported(mock_skill1):
 # ── Integration tests using real UnsafeLocalCodeExecutor ──
 
 
-def _make_skill_with_script(skill_name, script_name, script):
+def _make_skill_with_script(skill_name, script_name, script, references=None):
   """Creates a minimal mock Skill with a single script."""
+  references = references or {}
   skill = mock.create_autospec(models.Skill, instance=True)
   skill.name = skill_name
   skill.description = f"Test skill {skill_name}"
@@ -1058,9 +1059,9 @@ def _make_skill_with_script(skill_name, script_name, script):
     return None
 
   skill.resources.get_script.side_effect = get_script
-  skill.resources.get_reference.return_value = None
+  skill.resources.get_reference.side_effect = references.get
   skill.resources.get_asset.return_value = None
-  skill.resources.list_references.return_value = []
+  skill.resources.list_references.return_value = list(references)
   skill.resources.list_assets.return_value = []
   skill.resources.list_scripts.return_value = [script_name]
   return skill
@@ -1326,6 +1327,64 @@ async def test_integration_shell_nonzero_exit():
   assert "42" in result["stderr"]
 
 
+# ── Integration: skill resource paths stay inside the extraction dir ──
+
+
+@pytest.mark.asyncio
+async def test_integration_traversing_resource_name_is_refused(
+    tmp_path, monkeypatch
+):
+  """Real executor: a resource name that escapes the temp dir is refused."""
+  monkeypatch.setenv("TMPDIR", str(tmp_path))
+  script = models.Script(src="print('ran')")
+  skill = _make_skill_with_script(
+      "test_skill",
+      "hello.py",
+      script,
+      references={"../../pwned": "owned"},
+  )
+  toolset = _make_real_executor_toolset([skill])
+  tool = skill_toolset.RunSkillScriptTool(toolset)
+  ctx = _make_tool_context_with_agent()
+  result = await tool.run_async(
+      args={
+          "skill_name": "test_skill",
+          "file_path": "hello.py",
+      },
+      tool_context=ctx,
+  )
+  assert "status" in result, f"Result missing status: {result}"
+  assert result["status"] == "error"
+  assert "PermissionError" in result["stderr"]
+  assert result["stdout"] == ""
+  assert not (tmp_path / "pwned").exists()
+
+
+@pytest.mark.asyncio
+async def test_integration_nested_resource_still_materializes():
+  """Real executor: a nested resource path is still extracted."""
+  script = models.Script(src="print(open('references/subdir/notes.md').read())")
+  skill = _make_skill_with_script(
+      "test_skill",
+      "hello.py",
+      script,
+      references={"subdir/notes.md": "nested content"},
+  )
+  toolset = _make_real_executor_toolset([skill])
+  tool = skill_toolset.RunSkillScriptTool(toolset)
+  ctx = _make_tool_context_with_agent()
+  result = await tool.run_async(
+      args={
+          "skill_name": "test_skill",
+          "file_path": "hello.py",
+      },
+      tool_context=ctx,
+  )
+  assert "status" in result, f"Result missing status: {result}"
+  assert result["status"] == "success"
+  assert "nested content" in result["stdout"]
+
+
 # ── Finding 1: system instruction references correct tool name ──