| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Hi @prasanna8585, thank you for your contribution! We truly appreciate for this fix. we have noticed that Your branch is currently out of sync with the main branch, and we heve encountered some issues with the Maven build. Could you please rebase your branch and resolve these build concerns so that we can move forward with the review process? |
Sorry, something went wrong.
|
Hi @hemasekhar-p, thank you for the review. |
Sorry, something went wrong.
|
@prasanna8585 Thank you for the update. I noticed some inconsistencies in the code formatting now, could you please address those so we can proceed with the review? |
Sorry, something went wrong.
|
Hi @hemasekhar-p, thanks for catching that - pushed a formatting fix, should be consistent now. Let me know if anything else needs adjusting. |
Sorry, something went wrong.
|
@prasanna8585, could you please squash your commits into a single commit to keep the pull request history clean before we move forward with the review? |
Sorry, something went wrong.
|
@hemasekhar-p, is there any update require from my side to merge this PR? |
Sorry, something went wrong.
…t directory resolveSubAgentFromConfigPath computed whether a sub-agent's config_path resolved outside the referencing agent's own directory, but only logged a warning and continued loading the target anyway. An absolute config_path was also accepted unconditionally. This meant an imported agent bundle -- ADK's own docs describe agent YAML configs as artifacts meant to be versioned and shared -- could reference a config_path such as ../../../.env or an absolute path and have the deploying application's own process read and attempt to load an arbitrary file outside the bundle's directory as a sub-agent. When the target file fails to parse as a valid agent config, ComponentRegistry.resolveAgentClass throws an IllegalArgumentException that embeds the raw parsed value verbatim. For a plain KEY=value style file (e.g. a .env), this reliably propagates the file's actual content into the exception chain, which the standard dev-server tooling (ConfigAgentLoader) logs via logger.error(...) -- so this is not just an unintended sub-agent load, but an arbitrary file content disclosure reachable through the standard 'adk web' workflow. Fix: reject an absolute config_path outright, and reject any relative config_path whose resolved, symlink-resolved target does not stay within the referencing agent's own directory -- matching the hard rejection adk-python (171ae9e) and adk-go (604dd63) already ship for the identical AgentTool config_path resolution. Adds regression tests covering relative traversal, absolute paths, and that a target file's content can no longer reach the exception chain.
|
@prasanna8585, this PR is currently under review by our team. We will keep you updated if any changes are required. Thank you. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
ConfigAgentUtils.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:
if (Path.of(configPath).isAbsolute()) { subAgentConfigPath = Path.of(configPath); // accepted unconditionally } else { subAgentConfigPath = configDir.resolve(configPath); } Path resolvedConfigPath = subAgentConfigPath.normalize().toAbsolutePath(); Path baseDir = configDir.normalize().toAbsolutePath(); if (!resolvedConfigPath.startsWith(baseDir)) { logger.warn(...); // <-- warning only, no return/throw } if (!Files.exists(subAgentConfigPath)) { ... } return fromConfig(subAgentConfigPath.toString()); // proceeds regardlessThis is the same vulnerability class already hard-fixed with a breaking change in both other language ports:
This port (issue #1218) had instead chosen a warn-only deprecation, leaving the traversal fully exploitable today: a config_path such as ../../another_tenant/secret_agent.yaml or an absolute path is loaded and parsed as a full agent configuration, with only a log line noting the escape.
Reachability
config_path is 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
Testing
Verification performed
Maven Central is not reachable in the environment I used to develop this fix, so I could not run mvn test locally. 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):