From 0cac4e9849ca4f85516d14ebe71a0203835b728c Mon Sep 17 00:00:00 2001
From: adilburaksen <adilburaksen@gmail.com>
Date: Tue, 9 Jun 2026 16:11:54 +0300
Subject: [PATCH] fix(skills): prevent path traversal in LocalSkillSource

LocalSkillSource resolved the caller-supplied skillName and resourcePath
directly against skillsBasePath with Path.resolve(), which does not
normalize or reject ".." segments. A skill name or resource path such as
"../../etc" / "passwd" therefore escaped skillsBasePath, allowing
arbitrary file read (e.g. /etc/passwd) and directory listing outside the
configured base.

Validate each caller-supplied component against its base directory in
findResourcePath, listResources, and findSkillMdPath: reject absolute
paths and any component whose normalized resolution leaves the base.
Adds tests covering skillName and resourceDirectory traversal.
---
 .../google/adk/skills/LocalSkillSource.java   | 33 +++++++++++++++++++
 .../adk/skills/LocalSkillSourceTest.java      | 27 +++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/core/src/main/java/com/google/adk/skills/LocalSkillSource.java b/core/src/main/java/com/google/adk/skills/LocalSkillSource.java
index 4eafa4d5f..53ecbbcc3 100644
--- a/core/src/main/java/com/google/adk/skills/LocalSkillSource.java
+++ b/core/src/main/java/com/google/adk/skills/LocalSkillSource.java
@@ -45,6 +45,12 @@ public LocalSkillSource(Path skillsBasePath) {
 
   @Override
   public Single<ImmutableList<String>> listResources(String skillName, String resourceDirectory) {
+    try {
+      validatePathWithinBase(skillsBasePath, skillName);
+      validatePathWithinBase(skillsBasePath.resolve(skillName), resourceDirectory);
+    } catch (SkillSourceException e) {
+      return Single.error(e);
+    }
     Path skillDir = skillsBasePath.resolve(skillName);
     if (!isDirectory(skillDir)) {
       return Single.error(
@@ -96,6 +102,12 @@ protected Flowable<SkillMdPath<Path>> listSkills() {
 
   @Override
   protected Single<Path> findResourcePath(String skillName, String resourcePath) {
+    try {
+      validatePathWithinBase(skillsBasePath, skillName);
+      validatePathWithinBase(skillsBasePath.resolve(skillName), resourcePath);
+    } catch (SkillSourceException e) {
+      return Single.error(e);
+    }
     Path file = skillsBasePath.resolve(skillName).resolve(resourcePath);
     if (!Files.exists(file)) {
       return Single.error(
@@ -106,6 +118,11 @@ protected Single<Path> findResourcePath(String skillName, String resourcePath) {
 
   @Override
   protected Single<Path> findSkillMdPath(String skillName) {
+    try {
+      validatePathWithinBase(skillsBasePath, skillName);
+    } catch (SkillSourceException e) {
+      return Single.error(e);
+    }
     Path skillDir = skillsBasePath.resolve(skillName);
     if (!isDirectory(skillDir)) {
       return Single.error(
@@ -122,6 +139,22 @@ protected ReadableByteChannel openChannel(Path path) throws IOException {
     return Files.newByteChannel(path);
   }
 
+  private static void validatePathWithinBase(Path base, String component)
+      throws SkillSourceException {
+    if (Path.of(component).isAbsolute()) {
+      throw new SkillSourceException(
+          "Absolute paths are not allowed: " + component, SKILL_NOT_FOUND);
+    }
+    Path normalizedBase = base.normalize().toAbsolutePath();
+    Path resolved = base.resolve(component).normalize().toAbsolutePath();
+    if (!resolved.startsWith(normalizedBase)) {
+      throw new SkillSourceException(
+          "Path traversal detected; component must remain within its parent directory: "
+              + component,
+          SKILL_NOT_FOUND);
+    }
+  }
+
   private Optional<Path> findSkillMd(Path dir) {
     return Optional.of(dir.resolve("SKILL.md"))
         .filter(Files::exists)
diff --git a/core/src/test/java/com/google/adk/skills/LocalSkillSourceTest.java b/core/src/test/java/com/google/adk/skills/LocalSkillSourceTest.java
index 5efdb4ab9..e27ec9cc9 100644
--- a/core/src/test/java/com/google/adk/skills/LocalSkillSourceTest.java
+++ b/core/src/test/java/com/google/adk/skills/LocalSkillSourceTest.java
@@ -374,4 +374,31 @@ public void testLoadInstructions_emptyFile() throws IOException {
         .hasMessageThat()
         .contains("Skill file must start with ---");
   }
+
+  @Test
+  public void testLoadResource_pathTraversalRejected() throws IOException {
+    Path skillsBase = tempFolder.getRoot().toPath().resolve("skills");
+    Files.createDirectory(skillsBase);
+    // A secret file outside the skills base directory.
+    Files.writeString(tempFolder.getRoot().toPath().resolve("secret.txt"), "top-secret");
+
+    SkillSource source = new LocalSkillSource(skillsBase);
+
+    // A skill name that escapes the skills base via "..".
+    var single = source.loadResource("..", "secret.txt");
+    RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet);
+    assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class);
+  }
+
+  @Test
+  public void testListResources_pathTraversalRejected() throws IOException {
+    Path skillsBase = tempFolder.getRoot().toPath().resolve("skills");
+    Files.createDirectory(skillsBase);
+
+    SkillSource source = new LocalSkillSource(skillsBase);
+
+    var single = source.listResources("../../etc", "");
+    RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet);
+    assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class);
+  }
 }