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..7033303c3 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,18 @@ public LocalSkillSource(Path skillsBasePath) { @Override public Single> listResources(String skillName, String resourceDirectory) { - Path skillDir = skillsBasePath.resolve(skillName); + Path skillDir; + Path resourceDir; + try { + skillDir = validatePathWithinBase(skillsBasePath, skillName, SKILL_NOT_FOUND); + resourceDir = validatePathWithinBase(skillDir, resourceDirectory, RESOURCE_NOT_FOUND); + } catch (SkillSourceException e) { + return Single.error(e); + } if (!isDirectory(skillDir)) { return Single.error( new SkillSourceException("Skill not found: " + skillName, SKILL_NOT_FOUND)); } - Path resourceDir = skillDir.resolve(resourceDirectory); if (!isDirectory(resourceDir)) { return Single.error( new SkillSourceException( @@ -96,7 +102,13 @@ protected Flowable> listSkills() { @Override protected Single findResourcePath(String skillName, String resourcePath) { - Path file = skillsBasePath.resolve(skillName).resolve(resourcePath); + Path file; + try { + Path skillDir = validatePathWithinBase(skillsBasePath, skillName, SKILL_NOT_FOUND); + file = validatePathWithinBase(skillDir, resourcePath, RESOURCE_NOT_FOUND); + } catch (SkillSourceException e) { + return Single.error(e); + } if (!Files.exists(file)) { return Single.error( new SkillSourceException("Resource not found: " + file, RESOURCE_NOT_FOUND)); @@ -106,7 +118,12 @@ protected Single findResourcePath(String skillName, String resourcePath) { @Override protected Single findSkillMdPath(String skillName) { - Path skillDir = skillsBasePath.resolve(skillName); + Path skillDir; + try { + skillDir = validatePathWithinBase(skillsBasePath, skillName, SKILL_NOT_FOUND); + } catch (SkillSourceException e) { + return Single.error(e); + } if (!isDirectory(skillDir)) { return Single.error( new SkillSourceException("Skill directory not found: " + skillName, SKILL_NOT_FOUND)); @@ -122,6 +139,24 @@ protected ReadableByteChannel openChannel(Path path) throws IOException { return Files.newByteChannel(path); } + private static Path validatePathWithinBase(Path base, String component, String errorCode) + throws SkillSourceException { + // Parse the component against the base's own filesystem: LocalSkillSource accepts an arbitrary + // skillsBasePath, which may come from a non-default provider, so Path.of (which always uses + // FileSystems.getDefault()) could disagree with base.resolve below. + if (base.getFileSystem().getPath(component).isAbsolute()) { + throw new SkillSourceException("Absolute paths are not allowed: " + component, errorCode); + } + Path normalizedBase = base.normalize().toAbsolutePath(); + Path resolved = base.resolve(component).normalize().toAbsolutePath(); + if (!resolved.startsWith(normalizedBase)) { + throw new SkillSourceException( + "Path traversal detected; component must not escape its base directory: " + component, + errorCode); + } + return resolved; + } + private Optional 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..2d38de4c6 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,83 @@ 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); + assertThat(exception).hasCauseThat().hasMessageThat().contains("Path traversal detected"); + } + + @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); + assertThat(exception).hasCauseThat().hasMessageThat().contains("Path traversal detected"); + } + + @Test + public void testLoadFrontmatter_pathTraversalRejected() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + + SkillSource source = new LocalSkillSource(skillsBase); + + // loadFrontmatter routes through findSkillMdPath, which must reject a traversing skill name. + var single = source.loadFrontmatter("../.."); + 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_resourcePathTraversalRejected() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + Files.createDirectory(skillsBase.resolve("skill-1")); + // A secret file outside the individual skill directory (but under the skills base). + Files.writeString(tempFolder.getRoot().toPath().resolve("secret.txt"), "top-secret"); + + SkillSource source = new LocalSkillSource(skillsBase); + + // Valid skill name, but a resource path that escapes the skill directory via "..". + var single = source.loadResource("skill-1", "../../secret.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_absolutePathRejected() throws IOException { + Path skillsBase = tempFolder.getRoot().toPath().resolve("skills"); + Files.createDirectory(skillsBase); + Path secret = tempFolder.getRoot().toPath().resolve("secret.txt"); + Files.writeString(secret, "top-secret"); + + SkillSource source = new LocalSkillSource(skillsBase); + + // An absolute skill name must be rejected outright. + var single = source.loadResource(secret.toAbsolutePath().toString(), ""); + RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet); + assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class); + assertThat(exception) + .hasCauseThat() + .hasMessageThat() + .contains("Absolute paths are not allowed"); + } }