fix(configurable): prevent path traversal in AgentTool config_path resolution - #1407
Open
prasanna8585 wants to merge 1 commit into
Open
fix(configurable): prevent path traversal in AgentTool config_path resolution#1407prasanna8585 wants to merge 1 commit into
prasanna8585 wants to merge 1 commit into
Conversation
…solution 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 google#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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ConfigAgentUtils.resolveSubAgentFromConfigPath()accepted absoluteconfig_pathvalues 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:This is the same vulnerability class already hard-fixed with a breaking change in both other language ports:
adk-go: google/adk-go@604dd63 ("BREAKING: an absolute config_path is no longer accepted")adk-python: commit171ae9eThis port (issue #1218) had instead chosen a warn-only deprecation, leaving the traversal fully exploitable today: a
config_pathsuch as../../another_tenant/secret_agent.yamlor an absolute path is loaded and parsed as a full agent configuration, with only a log line noting the escape.Reachability
config_pathis a field in a subagent reference within an agent's own YAML config (sub_agents: - config_path: ...). In any deployment where different trust domains' agent configs are hosted under a shared root (e.g. a multi-tenant agent-hosting platform, or any scenario where config content can be influenced by a less-trusted party), this allows reading and loading arbitrary files reachable by the process as agent configuration -- outside the intended per-agent containment directory.Fix
config_pathvalues outright.toRealPath(), falling back to the lexical path when the target doesn't yet exist, so a missing file is still reported as not-found rather than misclassified as a traversal) -- matching the symlink-escape protection inadk-go's second commit for the same fix.Testing
fromConfig_subAgentConfigPathTraversal_throwsConfigurationException-- the exact../escape case, confirmed rejected.fromConfig_subAgentAbsoluteConfigPath_throwsConfigurationException-- the absolute-path case, confirmed rejected.fromConfig_withSubAgents_createsHierarchytest (a legitimate in-directory subagent reference) is unmodified and continues to pass, confirming no regression.Verification performed
Maven Central is not reachable in the environment I used to develop this fix, so I could not run
mvn testlocally. I instead dynamically confirmed both the vulnerability and the fix using a faithful, line-for-line transcription of the real method (standard JDK only, no external dependencies needed to exercise this specific logic):../secret_dir/victim_secret.yamlpath logged the deprecation warning, then proceeded to read the target file's full content regardless."Path traversal detected: ..."; an absolute path is rejected with"Absolute paths are not allowed..."; a legitimate in-directory reference still succeeds.