From f8a798a256685006d514e05624d4198e233106c5 Mon Sep 17 00:00:00 2001 From: adilburaksen Date: Wed, 27 May 2026 01:44:43 +0300 Subject: [PATCH] fix(skills): prevent path traversal in LocalSkillSource Add input validation to LocalSkillSource to ensure skill names and resource paths cannot escape the skills base directory via path traversal sequences (e.g. "../../../etc/passwd") or absolute paths (e.g. "/etc/passwd"). The new validatePathWithinBase() helper normalizes and resolves each caller-supplied path component against its parent directory, then checks that the result still starts with that parent. This mirrors the boundary check already present in the Go implementation (filesystem_source.go). Affected methods: findResourcePath, listResources, findSkillMdPath. Corresponding tests added for all traversal and absolute-path cases. --- .../google/adk/skills/LocalSkillSource.java | 36 ++++++++++ .../adk/skills/LocalSkillSourceTest.java | 66 +++++++++++++++++++ 2 files changed, 102 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 939c30b3c..8c891e90f 100644 --- a/core/src/main/java/com/google/adk/skills/LocalSkillSource.java +++ b/core/src/main/java/com/google/adk/skills/LocalSkillSource.java @@ -41,6 +41,12 @@ public LocalSkillSource(Path skillsBasePath) { @Override public Single> 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(new SkillSourceException("Skill not found: " + skillName)); @@ -86,6 +92,12 @@ protected Flowable listSkills() { @Override protected Single 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(new SkillSourceException("Resource not found: " + file)); @@ -95,6 +107,11 @@ protected Single findResourcePath(String skillName, String resourcePath) { @Override protected Single 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(new SkillSourceException("Skill directory not found: " + skillName)); @@ -115,4 +132,23 @@ private Optional findSkillMd(Path dir) { .or(() -> Optional.of(dir.resolve("skill.md"))) .filter(Files::exists); } + + /** + * Validates that {@code component} does not escape {@code base} when resolved against it. + * + * @throws SkillSourceException if the resolved path would be outside {@code base} + */ + private static void validatePathWithinBase(Path base, String component) + throws SkillSourceException { + if (Path.of(component).isAbsolute()) { + throw new SkillSourceException("Absolute paths are not allowed: " + component); + } + 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); + } + } } 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 256f1d66a..fae408083 100644 --- a/core/src/test/java/com/google/adk/skills/LocalSkillSourceTest.java +++ b/core/src/test/java/com/google/adk/skills/LocalSkillSourceTest.java @@ -250,4 +250,70 @@ public void testListSkillMdPaths_skillSourceException() throws IOException { RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); } + + @Test + public void testLoadResource_pathTraversalInSkillName() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + + SkillSource source = new LocalSkillSource(skillsBase); + var single = source.loadResource("../other-dir", "file.txt"); + RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); + assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); + assertThat(exception).hasCauseThat().hasMessageThat().contains("Path traversal detected"); + } + + @Test + public void testLoadResource_pathTraversalInResourcePath() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + Path skillDir = skillsBase.resolve("my-skill"); + Files.createDirectory(skillDir); + + SkillSource source = new LocalSkillSource(skillsBase); + var single = source.loadResource("my-skill", "../../../etc/passwd"); + RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); + assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); + assertThat(exception).hasCauseThat().hasMessageThat().contains("Path traversal detected"); + } + + @Test + public void testLoadResource_absoluteResourcePath() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + Path skillDir = skillsBase.resolve("my-skill"); + Files.createDirectory(skillDir); + + SkillSource source = new LocalSkillSource(skillsBase); + var single = source.loadResource("my-skill", "/etc/passwd"); + RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); + assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); + assertThat(exception).hasCauseThat().hasMessageThat().contains("Absolute paths are not allowed"); + } + + @Test + public void testListResources_pathTraversalInSkillName() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + + SkillSource source = new LocalSkillSource(skillsBase); + var single = source.listResources("../../etc", "passwd"); + RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); + assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); + assertThat(exception).hasCauseThat().hasMessageThat().contains("Path traversal detected"); + } + + @Test + public void testListResources_pathTraversalInResourceDirectory() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + Path skillDir = skillsBase.resolve("my-skill"); + Files.createDirectory(skillDir); + + SkillSource source = new LocalSkillSource(skillsBase); + var single = source.listResources("my-skill", "../other-skill"); + RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); + assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); + assertThat(exception).hasCauseThat().hasMessageThat().contains("Path traversal detected"); + } }