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..52bf2342a 100644 --- a/core/src/main/java/com/google/adk/skills/LocalSkillSource.java +++ b/core/src/main/java/com/google/adk/skills/LocalSkillSource.java @@ -45,12 +45,21 @@ public LocalSkillSource(Path skillsBasePath) { @Override public Single> listResources(String skillName, String resourceDirectory) { - Path skillDir = skillsBasePath.resolve(skillName); + Path skillDir = skillsBasePath.resolve(skillName).normalize(); if (!isDirectory(skillDir)) { return Single.error( new SkillSourceException("Skill not found: " + skillName, SKILL_NOT_FOUND)); } - Path resourceDir = skillDir.resolve(resourceDirectory); + Path resourceDir = skillDir.resolve(resourceDirectory).normalize(); + // Prevent path traversal: the resolved resource directory must remain inside + // the skill's own directory. A raw string prefix check on the input is not + // sufficient because "references/../../../../etc" bypasses it. + if (!resourceDir.startsWith(skillDir)) { + return Single.error( + new SkillSourceException( + "Path traversal detected in resource directory: " + resourceDirectory, + RESOURCE_NOT_FOUND)); + } if (!isDirectory(resourceDir)) { return Single.error( new SkillSourceException( @@ -96,7 +105,16 @@ protected Flowable> listSkills() { @Override protected Single findResourcePath(String skillName, String resourcePath) { - Path file = skillsBasePath.resolve(skillName).resolve(resourcePath); + Path base = skillsBasePath.resolve(skillName).normalize(); + Path file = base.resolve(resourcePath).normalize(); + // Enforce boundary: the resolved path must remain inside the skill's base + // directory. Without this check, a payload like + // "references/../../../../etc/passwd" passes the startsWith("references/") + // prefix check in LoadSkillResourceTool but resolves outside skillsBasePath. + if (!file.startsWith(base)) { + return Single.error( + new SkillSourceException("Path traversal detected: " + resourcePath, RESOURCE_NOT_FOUND)); + } if (!Files.exists(file)) { return Single.error( new SkillSourceException("Resource not found: " + file, RESOURCE_NOT_FOUND)); 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..d827a8a5a 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,91 @@ public void testLoadInstructions_emptyFile() throws IOException { .hasMessageThat() .contains("Skill file must start with ---"); } + + @Test + public void testLoadResource_pathTraversalBlocked() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + + Path skillDir = skillsBase.resolve("my-skill"); + Files.createDirectory(skillDir); + Files.createDirectory(skillDir.resolve("references")); + + SkillSource source = new LocalSkillSource(skillsBase); + // "references/../../../../etc/passwd" passes startsWith("references/") but escapes skillsBase + var single = source.loadResource("my-skill", "references/../../../../etc/passwd"); + RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); + assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); + SkillSourceException cause = (SkillSourceException) exception.getCause(); + assertThat(cause.getErrorCode()).isEqualTo(SkillSourceException.RESOURCE_NOT_FOUND); + assertThat(cause).hasMessageThat().contains("Path traversal detected"); + } + + @Test + public void testLoadResource_pathTraversalWithDoubleDotOnly() 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", "../../outside.txt"); + RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); + assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); + SkillSourceException cause = (SkillSourceException) exception.getCause(); + assertThat(cause.getErrorCode()).isEqualTo(SkillSourceException.RESOURCE_NOT_FOUND); + assertThat(cause).hasMessageThat().contains("Path traversal detected"); + } + + @Test + public void testLoadResource_legitimatePathNotBlocked() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + + Path skillDir = skillsBase.resolve("my-skill"); + Files.createDirectory(skillDir); + Path referencesDir = skillDir.resolve("references"); + Files.createDirectory(referencesDir); + Files.writeString(referencesDir.resolve("readme.md"), "legitimate content"); + + SkillSource source = new LocalSkillSource(skillsBase); + ByteSource resource = source.loadResource("my-skill", "references/readme.md").blockingGet(); + assertThat(new String(resource.read(), UTF_8)).isEqualTo("legitimate content"); + } + + @Test + public void testListResources_pathTraversalInResourceDirectoryBlocked() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + + Path skillDir = skillsBase.resolve("my-skill"); + Files.createDirectory(skillDir); + Files.createDirectory(skillDir.resolve("references")); + + SkillSource source = new LocalSkillSource(skillsBase); + var single = source.listResources("my-skill", "references/../../../../etc"); + RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); + assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); + SkillSourceException cause = (SkillSourceException) exception.getCause(); + assertThat(cause.getErrorCode()).isEqualTo(SkillSourceException.RESOURCE_NOT_FOUND); + assertThat(cause).hasMessageThat().contains("Path traversal detected"); + } + + @Test + public void testListResources_dotDotResourceDirectoryBlocked() 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); + SkillSourceException cause = (SkillSourceException) exception.getCause(); + assertThat(cause.getErrorCode()).isEqualTo(SkillSourceException.RESOURCE_NOT_FOUND); + assertThat(cause).hasMessageThat().contains("Path traversal detected"); + } }