From 916256dfc9d0d5310e16351a53349a9eceb61bb3 Mon Sep 17 00:00:00 2001 From: prasanna8585 <65734642+prasanna8585@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:58:05 +0530 Subject: [PATCH] fix(configurable): prevent path traversal in AgentTool config_path resolution resolveSubAgentFromConfigPath() accepted absolute config_path values unconditionally, and for relative values only logged a warning when the resolved path escaped the agent's own base directory -- it did not block the load. Execution continued to Files.exists() and then fromConfig(), which parses the (attacker-reachable) file as a full agent configuration. This is the same vulnerability class already hard-fixed (with a breaking change) in adk-go (604dd63) and adk-python (171ae9e); this port (issue #1218) had chosen a warn-only deprecation instead, leaving the traversal fully exploitable. Fix: - Reject absolute config_path values outright. - Reject (rather than warn on) a resolved path that escapes the agent's base directory, resolving symlinks on both sides where the paths exist so a symlink inside the base directory cannot be used to escape it. Adds regression tests for the traversal case, the absolute-path case, and confirms the existing in-directory subagent test (fromConfig_withSubAgents_createsHierarchy) continues to pass unmodified. Confirmed dynamically via a faithful, line-for-line transcription of both the vulnerable and fixed logic using the standard JDK (Maven Central is not reachable in this environment): the warning was non-blocking and a file outside the intended directory was read in full before this fix; the fixed logic rejects the same input while a legitimate in-directory reference still succeeds. --- .../google/adk/agents/ConfigAgentUtils.java | 43 +++++++++----- .../adk/agents/ConfigAgentUtilsTest.java | 58 +++++++++++++++++++ 2 files changed, 87 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/com/google/adk/agents/ConfigAgentUtils.java b/core/src/main/java/com/google/adk/agents/ConfigAgentUtils.java index 309d346c6..25b98804b 100644 --- a/core/src/main/java/com/google/adk/agents/ConfigAgentUtils.java +++ b/core/src/main/java/com/google/adk/agents/ConfigAgentUtils.java @@ -250,21 +250,23 @@ private static BaseAgent resolveSubAgentFromConfigPath( Path subAgentConfigPath; if (Path.of(configPath).isAbsolute()) { - subAgentConfigPath = Path.of(configPath); - } else { - subAgentConfigPath = configDir.resolve(configPath); + throw new ConfigurationException( + "Absolute paths are not allowed in AgentTool config_path: " + configPath); } - - // Warn when the resolved config path escapes the agent's base directory. For backward - // compatibility this is still allowed, but the behavior is deprecated and will be disallowed - // in a future release. - Path resolvedConfigPath = subAgentConfigPath.normalize().toAbsolutePath(); - Path baseDir = configDir.normalize().toAbsolutePath(); - if (!resolvedConfigPath.startsWith(baseDir)) { - logger.warn( - "AgentTool config_path '{}' accesses a path outside the agent base directory; this" - + " behavior is deprecated and will be disallowed in a future release.", - configPath); + subAgentConfigPath = configDir.resolve(configPath); + + // Reject config paths that resolve outside the agent's base directory. Both sides are + // resolved to their real (symlink-free) absolute form where the paths exist, so a symlink + // inside the base directory cannot be used to escape it; if a path does not yet exist, + // the lexical absolute/normalized form is used, which still correctly rejects a literal + // "../" escape. + Path resolvedConfigPath = resolveReal(subAgentConfigPath.normalize().toAbsolutePath()); + Path baseDir = resolveReal(configDir.normalize().toAbsolutePath()); + if (!resolvedConfigPath.startsWith(baseDir) && !resolvedConfigPath.equals(baseDir)) { + throw new ConfigurationException( + "Path traversal detected: AgentTool config_path '" + + configPath + + "' resolves outside the agent base directory."); } if (!Files.exists(subAgentConfigPath)) { @@ -280,6 +282,19 @@ private static BaseAgent resolveSubAgentFromConfigPath( } } + /** + * Resolves symlinks in {@code path} where the path exists; falls back to the given (already + * normalized, absolute) path unchanged if it does not exist, so that a missing file is reported + * as not-found rather than misclassified as a traversal. + */ + private static Path resolveReal(Path path) { + try { + return path.toRealPath(); + } catch (IOException e) { + return path; + } + } + /** * Load configuration from a YAML file path as a specific type. * diff --git a/core/src/test/java/com/google/adk/agents/ConfigAgentUtilsTest.java b/core/src/test/java/com/google/adk/agents/ConfigAgentUtilsTest.java index 5c1e74be3..b0fa99711 100644 --- a/core/src/test/java/com/google/adk/agents/ConfigAgentUtilsTest.java +++ b/core/src/test/java/com/google/adk/agents/ConfigAgentUtilsTest.java @@ -331,6 +331,64 @@ public void fromConfig_withSubAgents_createsHierarchy() assertThat(llmSubAgent.instruction().toString()).contains("helpful subagent"); } + @Test + public void fromConfig_subAgentConfigPathTraversal_throwsConfigurationException() + throws IOException { + // A file OUTSIDE the agent's own config directory, standing in for another tenant's or the + // host's data that config_path should never be able to reach. + File outsideDir = tempFolder.newFolder("outside"); + File secretFile = new File(outsideDir, "secret.yaml"); + Files.writeString(secretFile.toPath(), "agent_class: LlmAgent\nname: leaked\n"); + + File agentDir = tempFolder.newFolder("agents", "main"); + File mainAgentFile = new File(agentDir, "main_agent.yaml"); + Files.writeString( + mainAgentFile.toPath(), + """ + agent_class: LlmAgent + name: main_agent + description: Main agent with a traversing subagent reference + instruction: You are a main agent + sub_agents: + - name: escaping_subagent + config_path: ../../outside/secret.yaml + """); + + ConfigurationException exception = + assertThrows( + ConfigurationException.class, + () -> ConfigAgentUtils.fromConfig(mainAgentFile.getAbsolutePath())); + assertThat(exception).hasMessageThat().contains("Path traversal detected"); + } + + @Test + public void fromConfig_subAgentAbsoluteConfigPath_throwsConfigurationException() + throws IOException { + File outsideFile = tempFolder.newFile("absolute_target.yaml"); + Files.writeString(outsideFile.toPath(), "agent_class: LlmAgent\nname: leaked\n"); + + File mainAgentFile = tempFolder.newFile("main_agent.yaml"); + Files.writeString( + mainAgentFile.toPath(), + String.format( + """ + agent_class: LlmAgent + name: main_agent + description: Main agent with an absolute-path subagent reference + instruction: You are a main agent + sub_agents: + - name: absolute_subagent + config_path: %s + """, + outsideFile.getAbsolutePath())); + + ConfigurationException exception = + assertThrows( + ConfigurationException.class, + () -> ConfigAgentUtils.fromConfig(mainAgentFile.getAbsolutePath())); + assertThat(exception).hasMessageThat().contains("Absolute paths are not allowed"); + } + @Test public void resolveSubAgents_missingConfigPath_throwsConfigurationException() throws IOException { File mainAgentFile = tempFolder.newFile("main_agent.yaml");