Skip to content

Commit 0cac4e9

Browse files
committed
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.
1 parent ec93f50 commit 0cac4e9

2 files changed

Lines changed: 60 additions & 0 deletions

File tree

core/src/main/java/com/google/adk/skills/LocalSkillSource.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ public LocalSkillSource(Path skillsBasePath) {
4545

4646
@Override
4747
public Single<ImmutableList<String>> listResources(String skillName, String resourceDirectory) {
48+
try {
49+
validatePathWithinBase(skillsBasePath, skillName);
50+
validatePathWithinBase(skillsBasePath.resolve(skillName), resourceDirectory);
51+
} catch (SkillSourceException e) {
52+
return Single.error(e);
53+
}
4854
Path skillDir = skillsBasePath.resolve(skillName);
4955
if (!isDirectory(skillDir)) {
5056
return Single.error(
@@ -96,6 +102,12 @@ protected Flowable<SkillMdPath<Path>> listSkills() {
96102

97103
@Override
98104
protected Single<Path> findResourcePath(String skillName, String resourcePath) {
105+
try {
106+
validatePathWithinBase(skillsBasePath, skillName);
107+
validatePathWithinBase(skillsBasePath.resolve(skillName), resourcePath);
108+
} catch (SkillSourceException e) {
109+
return Single.error(e);
110+
}
99111
Path file = skillsBasePath.resolve(skillName).resolve(resourcePath);
100112
if (!Files.exists(file)) {
101113
return Single.error(
@@ -106,6 +118,11 @@ protected Single<Path> findResourcePath(String skillName, String resourcePath) {
106118

107119
@Override
108120
protected Single<Path> findSkillMdPath(String skillName) {
121+
try {
122+
validatePathWithinBase(skillsBasePath, skillName);
123+
} catch (SkillSourceException e) {
124+
return Single.error(e);
125+
}
109126
Path skillDir = skillsBasePath.resolve(skillName);
110127
if (!isDirectory(skillDir)) {
111128
return Single.error(
@@ -122,6 +139,22 @@ protected ReadableByteChannel openChannel(Path path) throws IOException {
122139
return Files.newByteChannel(path);
123140
}
124141

142+
private static void validatePathWithinBase(Path base, String component)
143+
throws SkillSourceException {
144+
if (Path.of(component).isAbsolute()) {
145+
throw new SkillSourceException(
146+
"Absolute paths are not allowed: " + component, SKILL_NOT_FOUND);
147+
}
148+
Path normalizedBase = base.normalize().toAbsolutePath();
149+
Path resolved = base.resolve(component).normalize().toAbsolutePath();
150+
if (!resolved.startsWith(normalizedBase)) {
151+
throw new SkillSourceException(
152+
"Path traversal detected; component must remain within its parent directory: "
153+
+ component,
154+
SKILL_NOT_FOUND);
155+
}
156+
}
157+
125158
private Optional<Path> findSkillMd(Path dir) {
126159
return Optional.of(dir.resolve("SKILL.md"))
127160
.filter(Files::exists)

core/src/test/java/com/google/adk/skills/LocalSkillSourceTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,4 +374,31 @@ public void testLoadInstructions_emptyFile() throws IOException {
374374
.hasMessageThat()
375375
.contains("Skill file must start with ---");
376376
}
377+
378+
@Test
379+
public void testLoadResource_pathTraversalRejected() throws IOException {
380+
Path skillsBase = tempFolder.getRoot().toPath().resolve("skills");
381+
Files.createDirectory(skillsBase);
382+
// A secret file outside the skills base directory.
383+
Files.writeString(tempFolder.getRoot().toPath().resolve("secret.txt"), "top-secret");
384+
385+
SkillSource source = new LocalSkillSource(skillsBase);
386+
387+
// A skill name that escapes the skills base via "..".
388+
var single = source.loadResource("..", "secret.txt");
389+
RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet);
390+
assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class);
391+
}
392+
393+
@Test
394+
public void testListResources_pathTraversalRejected() throws IOException {
395+
Path skillsBase = tempFolder.getRoot().toPath().resolve("skills");
396+
Files.createDirectory(skillsBase);
397+
398+
SkillSource source = new LocalSkillSource(skillsBase);
399+
400+
var single = source.listResources("../../etc", "");
401+
RuntimeException exception = assertThrows(RuntimeException.class, single::blockingGet);
402+
assertThat(exception).hasCauseThat().isInstanceOf(SkillSourceException.class);
403+
}
377404
}

0 commit comments

Comments
 (0)