fix(tool): built-in file tools support relative tools (#747)

fang-tech · web-flow · commit b3da79a3cd40 · 2026-02-06T15:27:46.000+08:00
diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java b/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java
@@ -37,28 +37,23 @@ public class AgentSkillPromptProvider {
             ## Available Skills
 
             <usage>
-            When you need to perform tasks, check if any of the available skills below can help complete the task more effectively. Skills provide specialized capabilities, tools, and domain knowledge.
+            Skills provide specialized capabilities and domain knowledge. Use them when they match your current task.
 
             How to use skills:
             - Load skill: load_skill_through_path(skillId="<skill-id>", path="SKILL.md")
             - The skill will be activated and its documentation loaded with detailed instructions
             - Additional resources (scripts, assets, references) can be loaded using the same tool with different paths
 
-            Usage notes:
-            - Only use skills listed in <available_skills> below
-            - Loading SKILL.md activates the skill and will make its tools available
-
-            Code execution guidance:
-            - If a task requires running code, use the code execution tools (shell/read/write)
-                when they are available
-            - Code execution tools operate in workDir; skill scripts are uploaded to
-                workDir/skills/<skill-id>/scripts (for example: workDir/skills/skill-ID/scripts/shell.sh)
-            - Example (shell): execute_shell_command(command="skills/skill-ID/scripts/shell.sh")
+            Path Information:
+            When you load a skill, the response will include:
+            - Exact paths to all skill resources
+            - Code examples for accessing skill files
+            - Usage instructions specific to that skill
 
             Template fields explanation:
-            - <name>: The skill's display name. Use it along with <description> to determine if this skill is relevant to your current task
-            - <description>: Detailed description of when and how to use this skill. Read carefully to decide whether to load this skill
-            - <skill-id>: The unique identifier used to load the skill via load_skill_through_path tool
+            - <name>: The skill's display name
+            - <description>: When and how to use this skill
+            - <skill-id>: Unique identifier for load_skill_through_path tool
             </usage>
 
             <available_skills>
diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/file/FileToolUtils.java b/agentscope-core/src/main/java/io/agentscope/core/tool/file/FileToolUtils.java
@@ -35,6 +35,12 @@ private FileToolUtils() {
      * Validate that the given file path is within the base directory.
      * This prevents path traversal attacks and unauthorized file access.
      *
+     * <p>When baseDir is specified:
+     * <ul>
+     *   <li>Relative paths are resolved relative to baseDir</li>
+     *   <li>Absolute paths are validated to be within baseDir</li>
+     * </ul>
+     *
      * @param filePath The file path to validate
      * @param baseDir The base directory to restrict access to (null means no restriction)
      * @return The normalized absolute path if valid
@@ -45,7 +51,17 @@ static Path validatePath(String filePath, Path baseDir) throws IOException {
             throw new IOException("File path cannot be null or empty.");
         }
 
-        Path path = Paths.get(filePath).toAbsolutePath().normalize();
+        Path inputPath = Paths.get(filePath);
+
+        // If baseDir is specified, relative paths should be resolved relative to baseDir
+        Path path;
+        if (baseDir != null && !inputPath.isAbsolute()) {
+            // Relative path: resolve relative to baseDir
+            path = baseDir.resolve(inputPath).normalize();
+        } else {
+            // Absolute path or no baseDir: convert to absolute path
+            path = inputPath.toAbsolutePath().normalize();
+        }
 
         // If baseDir is specified, ensure the path is within it
         if (baseDir != null) {
diff --git a/agentscope-core/src/test/java/io/agentscope/core/tool/file/FileToolUtilsTest.java b/agentscope-core/src/test/java/io/agentscope/core/tool/file/FileToolUtilsTest.java
@@ -251,4 +251,165 @@ void testParseRanges_NullInput() {
                 },
                 "Should throw NullPointerException for null input");
     }
+
+    // ==================== validatePath Tests ====================
+
+    @Test
+    @DisplayName("Should validate relative path with baseDir")
+    void testValidatePath_RelativePathWithBaseDir() throws IOException {
+        Path baseDir = tempDir;
+        Path subDir = tempDir.resolve("subdir");
+        Files.createDirectories(subDir);
+
+        // Relative path should be resolved relative to baseDir
+        Path result = FileToolUtils.validatePath("subdir/test.txt", baseDir);
+
+        assertNotNull(result);
+        assertTrue(result.isAbsolute(), "Result should be absolute");
+        assertTrue(result.startsWith(baseDir), "Result should be within baseDir");
+        assertEquals(subDir.resolve("test.txt"), result, "Should resolve to correct path");
+    }
+
+    @Test
+    @DisplayName("Should validate simple relative path with baseDir")
+    void testValidatePath_SimpleRelativePathWithBaseDir() throws IOException {
+        Path baseDir = tempDir;
+
+        Path result = FileToolUtils.validatePath("test.txt", baseDir);
+
+        assertNotNull(result);
+        assertTrue(result.isAbsolute(), "Result should be absolute");
+        assertTrue(result.startsWith(baseDir), "Result should be within baseDir");
+        assertEquals(baseDir.resolve("test.txt"), result, "Should resolve to correct path");
+    }
+
+    @Test
+    @DisplayName("Should reject path traversal attack with baseDir")
+    void testValidatePath_PathTraversalAttackWithBaseDir() {
+        Path baseDir = tempDir;
+
+        // Try to escape baseDir using ../
+        IOException exception =
+                assertThrows(
+                        IOException.class,
+                        () -> FileToolUtils.validatePath("../../etc/passwd", baseDir),
+                        "Should reject path traversal attack");
+
+        assertTrue(
+                exception.getMessage().contains("Access denied"),
+                "Error message should indicate access denied");
+        assertTrue(
+                exception.getMessage().contains("outside the allowed base directory"),
+                "Error message should mention base directory restriction");
+    }
+
+    @Test
+    @DisplayName("Should validate absolute path within baseDir")
+    void testValidatePath_AbsolutePathWithinBaseDir() throws IOException {
+        Path baseDir = tempDir;
+        Path targetFile = tempDir.resolve("test.txt");
+
+        Path result = FileToolUtils.validatePath(targetFile.toString(), baseDir);
+
+        assertNotNull(result);
+        assertTrue(result.isAbsolute(), "Result should be absolute");
+        assertEquals(targetFile, result, "Should return the same absolute path");
+    }
+
+    @Test
+    @DisplayName("Should reject absolute path outside baseDir")
+    void testValidatePath_AbsolutePathOutsideBaseDir() {
+        Path baseDir = tempDir;
+        Path outsidePath = tempDir.getParent().resolve("outside.txt");
+
+        IOException exception =
+                assertThrows(
+                        IOException.class,
+                        () -> FileToolUtils.validatePath(outsidePath.toString(), baseDir),
+                        "Should reject path outside baseDir");
+
+        assertTrue(
+                exception.getMessage().contains("Access denied"),
+                "Error message should indicate access denied");
+    }
+
+    @Test
+    @DisplayName("Should validate relative path without baseDir")
+    void testValidatePath_RelativePathWithoutBaseDir() throws IOException {
+        // Without baseDir, relative path should be converted to absolute
+        Path result = FileToolUtils.validatePath("test.txt", null);
+
+        assertNotNull(result);
+        assertTrue(result.isAbsolute(), "Result should be absolute");
+        assertTrue(result.toString().endsWith("test.txt"), "Result should end with the file name");
+    }
+
+    @Test
+    @DisplayName("Should validate absolute path without baseDir")
+    void testValidatePath_AbsolutePathWithoutBaseDir() throws IOException {
+        Path absolutePath = tempDir.resolve("test.txt");
+
+        Path result = FileToolUtils.validatePath(absolutePath.toString(), null);
+
+        assertNotNull(result);
+        assertTrue(result.isAbsolute(), "Result should be absolute");
+        assertEquals(absolutePath, result, "Should return the same absolute path");
+    }
+
+    @Test
+    @DisplayName("Should reject null file path")
+    void testValidatePath_NullFilePath() {
+        IOException exception =
+                assertThrows(
+                        IOException.class,
+                        () -> FileToolUtils.validatePath(null, tempDir),
+                        "Should reject null file path");
+
+        assertTrue(
+                exception.getMessage().contains("cannot be null or empty"),
+                "Error message should mention null or empty");
+    }
+
+    @Test
+    @DisplayName("Should reject empty file path")
+    void testValidatePath_EmptyFilePath() {
+        IOException exception =
+                assertThrows(
+                        IOException.class,
+                        () -> FileToolUtils.validatePath("", tempDir),
+                        "Should reject empty file path");
+
+        assertTrue(
+                exception.getMessage().contains("cannot be null or empty"),
+                "Error message should mention null or empty");
+    }
+
+    @Test
+    @DisplayName("Should reject whitespace-only file path")
+    void testValidatePath_WhitespaceFilePath() {
+        IOException exception =
+                assertThrows(
+                        IOException.class,
+                        () -> FileToolUtils.validatePath("   ", tempDir),
+                        "Should reject whitespace-only file path");
+
+        assertTrue(
+                exception.getMessage().contains("cannot be null or empty"),
+                "Error message should mention null or empty");
+    }
+
+    @Test
+    @DisplayName("Should handle path with . and .. correctly within baseDir")
+    void testValidatePath_DotPathWithinBaseDir() throws IOException {
+        Path baseDir = tempDir;
+        Path subDir = tempDir.resolve("subdir");
+        Files.createDirectories(subDir);
+
+        // ./subdir/../test.txt should normalize to test.txt in baseDir
+        Path result = FileToolUtils.validatePath("./subdir/../test.txt", baseDir);
+
+        assertNotNull(result);
+        assertTrue(result.startsWith(baseDir), "Result should be within baseDir");
+        assertEquals(baseDir.resolve("test.txt"), result, "Should normalize correctly");
+    }
 }
diff --git a/agentscope-core/src/test/java/io/agentscope/core/tool/file/WriteFileToolTest.java b/agentscope-core/src/test/java/io/agentscope/core/tool/file/WriteFileToolTest.java
@@ -669,6 +669,68 @@ void testWriteTextFile_CreateNewFileWithinBaseDir() throws IOException {
         assertEquals("New file content", content);
     }
 
+    @Test
+    @DisplayName("Should handle relative path with baseDir correctly")
+    void testWriteTextFile_RelativePathWithBaseDir() throws IOException {
+        WriteFileTool tool = new WriteFileTool(tempDir.toString());
+
+        // Use relative path - should be resolved relative to baseDir
+        Mono<ToolResultBlock> result =
+                tool.writeTextFile("subdir/relative.txt", "Content from relative path", null);
+
+        StepVerifier.create(result)
+                .assertNext(
+                        block -> {
+                            assertNotNull(block);
+                            String text = extractText(block);
+                            assertTrue(
+                                    text.contains("Create and write")
+                                            && text.contains("successfully"),
+                                    "Should successfully create file with relative path");
+                        })
+                .verifyComplete();
+
+        // Verify file was created in the correct location (relative to baseDir)
+        Path expectedFile = tempDir.resolve("subdir/relative.txt");
+        assertTrue(
+                Files.exists(expectedFile),
+                "File should be created relative to baseDir: " + expectedFile);
+        String content = Files.readString(expectedFile);
+        assertEquals("Content from relative path", content);
+    }
+
+    @Test
+    @DisplayName("Should handle relative path when reading with baseDir")
+    void testInsertTextFile_RelativePathWithBaseDir() throws IOException {
+        // Create a test file using relative path
+        WriteFileTool tool = new WriteFileTool(tempDir.toString());
+        tool.writeTextFile("test_relative.txt", "Line 1\nLine 2\nLine 3", null).block();
+
+        // Insert content using relative path
+        Mono<ToolResultBlock> result = tool.insertTextFile("test_relative.txt", "Inserted Line", 2);
+
+        StepVerifier.create(result)
+                .assertNext(
+                        block -> {
+                            assertNotNull(block);
+                            String text = extractText(block);
+                            assertTrue(
+                                    text.contains("Insert content")
+                                            && text.contains("successfully"),
+                                    "Should successfully insert with relative path");
+                        })
+                .verifyComplete();
+
+        // Verify content was inserted correctly
+        Path expectedFile = tempDir.resolve("test_relative.txt");
+        List<String> lines = Files.readAllLines(expectedFile);
+        assertEquals(4, lines.size());
+        assertEquals("Line 1", lines.get(0));
+        assertEquals("Inserted Line", lines.get(1));
+        assertEquals("Line 2", lines.get(2));
+        assertEquals("Line 3", lines.get(3));
+    }
+
     /**
      * Extract text content from ToolResultBlock for assertion.
      */
