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 893353f27..d4c8a142f 100644 --- a/core/src/main/java/com/google/adk/agents/ConfigAgentUtils.java +++ b/core/src/main/java/com/google/adk/agents/ConfigAgentUtils.java @@ -247,12 +247,18 @@ private static BaseAgent resolveSubAgentFromConfigPath( BaseAgentConfig.AgentRefConfig subAgentConfig, Path configDir) throws ConfigurationException { String configPath = subAgentConfig.configPath().trim(); - 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); + } + + Path subAgentConfigPath = configDir.resolve(configPath).normalize().toAbsolutePath(); + Path canonicalConfigDir = configDir.normalize().toAbsolutePath(); + + if (!subAgentConfigPath.startsWith(canonicalConfigDir)) { + throw new ConfigurationException( + "Path traversal detected: config_path resolves outside agent directory: " + configPath); } if (!Files.exists(subAgentConfigPath)) { 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 4f6ea6104..62f931d41 100644 --- a/core/src/test/java/com/google/adk/agents/ConfigAgentUtilsTest.java +++ b/core/src/test/java/com/google/adk/agents/ConfigAgentUtilsTest.java @@ -1381,6 +1381,176 @@ public void testCallbackRefAccessors() { assertThat(callbackRef.name()).isEqualTo("updated-name"); } + @Test + public void resolveSubAgents_withAbsoluteConfigPath_throwsConfigurationException() + throws IOException { + // A secret file outside the agent directory that an attacker might try to read. + File secretFile = tempFolder.newFile("secret.yaml"); + Files.writeString( + secretFile.toPath(), + """ + agent_class: LlmAgent + name: secret_agent + description: A secret agent that should not be loadable via absolute path + instruction: secret instruction + """); + String absoluteConfigPath = secretFile.getAbsolutePath(); + + File mainAgentFile = tempFolder.newFile("main_agent_absolute.yaml"); + Files.writeString( + mainAgentFile.toPath(), + String.format( + """ + agent_class: LlmAgent + name: main_agent + description: Main agent referencing an absolute config_path + instruction: You are a main agent + sub_agents: + - name: secret_agent + config_path: %s + """, + absoluteConfigPath)); + + ConfigurationException exception = + assertThrows( + ConfigurationException.class, + () -> ConfigAgentUtils.fromConfig(mainAgentFile.getAbsolutePath())); + + assertThat(exception).hasMessageThat().contains("Failed to create agent from config"); + StringBuilder messages = new StringBuilder(); + Throwable t = exception.getCause(); + while (t != null) { + messages.append(t.getMessage()).append("\n"); + t = t.getCause(); + } + assertThat(messages.toString()) + .contains("Absolute paths are not allowed in AgentTool config_path: " + absoluteConfigPath); + } + + @Test + public void resolveSubAgents_withTraversalConfigPath_throwsConfigurationException() + throws IOException { + // The agent config lives in a nested subdirectory so that "../../" escapes the agent + // directory. The traversal target is a real file to prove the rejection happens before any + // file read. + File secretFile = tempFolder.newFile("outside_secret.yaml"); + Files.writeString( + secretFile.toPath(), + """ + agent_class: LlmAgent + name: outside_secret_agent + description: A file outside the agent directory + instruction: secret instruction + """); + + File agentDir = tempFolder.newFolder("nested", "agents"); + File mainAgentFile = new File(agentDir, "main_agent_traversal.yaml"); + Files.writeString( + mainAgentFile.toPath(), + """ + agent_class: LlmAgent + name: main_agent + description: Main agent referencing a traversal config_path + instruction: You are a main agent + sub_agents: + - name: outside_secret_agent + config_path: ../../outside_secret.yaml + """); + + ConfigurationException exception = + assertThrows( + ConfigurationException.class, + () -> ConfigAgentUtils.fromConfig(mainAgentFile.getAbsolutePath())); + + assertThat(exception).hasMessageThat().contains("Failed to create agent from config"); + StringBuilder messages = new StringBuilder(); + Throwable t = exception.getCause(); + while (t != null) { + messages.append(t.getMessage()).append("\n"); + t = t.getCause(); + } + assertThat(messages.toString()) + .contains( + "Path traversal detected: config_path resolves outside agent directory:" + + " ../../outside_secret.yaml"); + } + + @Test + public void resolveSubAgents_withRelativeConfigPathWithinAgentDir_resolvesSuccessfully() + throws IOException, ConfigurationException { + // A normal relative config_path that stays within the agent directory must still work. + File subAgentFile = tempFolder.newFile("normal_sub_agent.yaml"); + Files.writeString( + subAgentFile.toPath(), + """ + agent_class: LlmAgent + name: normal_sub_agent + description: A normal subagent + instruction: You are a helpful subagent + """); + + File mainAgentFile = tempFolder.newFile("main_agent_relative.yaml"); + Files.writeString( + mainAgentFile.toPath(), + """ + agent_class: LlmAgent + name: main_agent + description: Main agent with a normal relative config_path + instruction: You are a main agent + sub_agents: + - name: normal_sub_agent + config_path: normal_sub_agent.yaml + """); + + BaseAgent mainAgent = ConfigAgentUtils.fromConfig(mainAgentFile.getAbsolutePath()); + + assertThat(mainAgent.name()).isEqualTo("main_agent"); + assertThat(mainAgent.subAgents()).hasSize(1); + BaseAgent subAgent = mainAgent.subAgents().get(0); + assertThat(subAgent.name()).isEqualTo("normal_sub_agent"); + assertThat(subAgent).isInstanceOf(LlmAgent.class); + } + + @Test + public void resolveSubAgents_withTraversalThatStaysWithinAgentDir_resolvesSuccessfully() + throws IOException, ConfigurationException { + // A config_path containing ".." that still normalizes to a location inside the agent + // directory must be accepted (the containment check is on the normalized path, not a naive + // ".." substring match). + File childDir = tempFolder.newFolder("child"); + File subAgentFile = new File(childDir, "nested_sub_agent.yaml"); + Files.writeString( + subAgentFile.toPath(), + """ + agent_class: LlmAgent + name: nested_sub_agent + description: A nested subagent reached via an in-bounds relative path + instruction: You are a helpful subagent + """); + + File mainAgentFile = tempFolder.newFile("main_agent_inbounds_traversal.yaml"); + // child/../child/nested_sub_agent.yaml normalizes back into the agent directory. + Files.writeString( + mainAgentFile.toPath(), + """ + agent_class: LlmAgent + name: main_agent + description: Main agent with an in-bounds traversal config_path + instruction: You are a main agent + sub_agents: + - name: nested_sub_agent + config_path: child/../child/nested_sub_agent.yaml + """); + + BaseAgent mainAgent = ConfigAgentUtils.fromConfig(mainAgentFile.getAbsolutePath()); + + assertThat(mainAgent.name()).isEqualTo("main_agent"); + assertThat(mainAgent.subAgents()).hasSize(1); + BaseAgent subAgent = mainAgent.subAgents().get(0); + assertThat(subAgent.name()).isEqualTo("nested_sub_agent"); + assertThat(subAgent).isInstanceOf(LlmAgent.class); + } + @Test public void fromConfig_validYamlLoopAgent_createsLoopAgent() throws IOException, ConfigurationException {