fix(agents): warn when AgentTool config_path escapes agent base directory

[adilburaksen]

Summary

resolveSubAgentFromConfigPath in ConfigAgentUtils.java resolves a sub-agent
config_path from an agent YAML. Absolute paths and relative paths containing .. are
resolved as-is, so a config_path can point outside the agent's base directory without any
signal that this is happening.

Per maintainer guidance, blocking these paths outright is a breaking change (existing tools
legitimately rely on absolute / parent-relative config_path values), so this PR keeps the
current behavior and adds an observable warning ahead of stricter enforcement in a future
major release.

Change

• Preserve backward-compatible resolution — absolute config_path values and relative
  paths containing .. are still honored.

• Log a WARN deprecation notice when the resolved path escapes the agent's base
  directory:

  AgentTool config_path '{}' accesses a path outside the agent base directory; this
  behavior is deprecated and will be disallowed in a future release.
  

Tests

ConfigAgentUtilsTest (50/50 passing):

• Absolute config_path still resolves successfully (backward compatibility).
• Relative config_path with .. that escapes the base directory still resolves
  successfully and triggers the deprecation warning.
• In-bounds relative and in-bounds .. paths continue to resolve.

Related

Same code path exists in adk-python (google/adk-python#5826) and adk-go.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[adilburaksen]

I have read the CLA Documents and I hereby sign the CLA.

[hemasekhar-p]

Hi @adilburaksen, thank you for your contribution! We appreciate you taking the time to submit this pull request. Could you please include the corresponding unit tests to verify your changes?

[adilburaksen]

Thanks for the review! I've added unit tests in ConfigAgentUtilsTest covering the change: an absolute config_path is rejected, a .. path that escapes the agent directory is rejected (with a real file present outside the dir to prove rejection happens before any read), a normal in-bounds relative path resolves successfully, and a .. that normalizes back inside the directory is still accepted (confirming the check is on the normalized path, not a naive substring). All pass locally (Tests run: 50, Failures: 0). Pushed as f077057.

[hemasekhar-p]

Thank you for your response. please ensure your PR consists of a single commit. Could you please change your commits accordingly?

[adilburaksen]

Done — squashed into a single commit (f10eceb) containing both the fix and the tests. Thanks!

[hemasekhar-p]

Thank you for the updates @adilburaksen. Currently this PR is under review by our team, we will keep you posted if any additional information is required. thank you.

[hemasekhar-p]

@sherryfox, Could you please review this PR.

[adilburaksen]

Gentle follow-up on this one. It's been about two weeks since it went into team review (Jun 3, routed to @sherryfox) — CI is green, CLA is signed, and it's squashed to a single commit. It's a small, self-contained path-traversal fix in resolve_agent_reference (config_path is accepted as an absolute path / with .. and never bounded to the agent dir). Happy to address anything if a re-look surfaces changes. Thanks for your time!

[adilburaksen]

Gentle ping for review. This is the Java side of the AgentTool config_path path-traversal fix (the Python fix already merged via #5826); it adds boundary validation in ConfigAgentUtils so absolute paths and .. escapes outside the parent agent directory are rejected, with tests. The Google OSS VRP report tracking this is waiting on the repo fixes to land, so a review here would help close it out. Happy to rebase or adjust — thanks!

[MiloszSobczyk]

Thank you for your response @adilburaksen . Could you please ensure your PR consists of a single commit?

[adilburaksen]

Done — squashed to a single commit (644299f). It rebases the config_path boundary-validation fix + tests onto current main with the merge commit removed, so the PR is now one clean commit. Thanks @MiloszSobczyk!

[MiloszSobczyk]

Hello @adilburaksen, after internal discussion we decided we cannot move forward with provided implementation, since it is a breaking change and could break tools that already operate using absolute paths or ...

Could you bring back the previous functionality and add logging when path outside the agent base directory is detected?

logger.error(
          "AgentTool config_path '{}' accesses a path outside the agent base directory; this"
              + " behavior is deprecated and will be disallowed in a future release.",
          configPath);

We will proceed with your initial implementation in the next major release. Thank you for your contribution!

[adilburaksen]

Thanks @MiloszSobczyk, that makes sense. I've reworked the PR accordingly:

• Restored the previous resolution behavior — absolute paths and relative paths containing .. are honored again, so existing tools keep working.
• Added the deprecation warning you suggested when the resolved path escapes the agent base directory:

logger.error(
    "AgentTool config_path '{}' accesses a path outside the agent base directory; this"
        + " behavior is deprecated and will be disallowed in a future release.",
    configPath);

• Updated the tests: the two containment cases now assert that absolute / .. config paths still resolve successfully (backward compatibility), alongside the existing in-bounds cases. Full ConfigAgentUtilsTest passes (50/50).

Still a single commit (14aa0699). Happy to adjust the log level or wording if you'd prefer a warn instead of error. Looking forward to the stricter enforcement in the next major release.

[MiloszSobczyk]

@adilburaksen Thanks for the change. You are right in saying that warn level log would be better suited here. Please adjust the code.

[adilburaksen]

Done — switched the log to logger.warn (commit 7a4113e0, still a single commit). ConfigAgentUtilsTest still passes 50/50. Thanks!

[MiloszSobczyk]

Summary

resolveSubAgentFromConfigPath in ConfigAgentUtils.java accepted absolute configPath values unconditionally and resolved relative paths without boundary validation. An attacker-controlled config_path field in an agent YAML could read arbitrary files.

Vulnerable pattern (before):

if (Path.of(configPath).isAbsolute()) {
    subAgentConfigPath = Path.of(configPath);   // absolute accepted
} else {
    subAgentConfigPath = configDir.resolve(configPath);  // no ".." check
}

Fix

• Reject absolute configPath values with ConfigurationException
• Normalize the resolved path and verify it stays within configDir before loading

Related

Same vulnerability exists in adk-python (PR: google/adk-python#5826) and adk-go — fix pattern is identical across all three SDKs.

@adilburaksen Could you update the description for the PR?

[adilburaksen]

Done — updated the title and description to reflect the current warn-on-escape behavior (backward-compatible resolution preserved, deprecation warning added), so it now matches the implementation. Thanks @MiloszSobczyk!