fix: Port agent loader and file path resolution fixes to v1 - #6788
Open
GWeale wants to merge 2 commits into
Open
fix: Port agent loader and file path resolution fixes to v1#6788GWeale wants to merge 2 commits into
GWeale wants to merge 2 commits into
Conversation
Port of 3f6eb1f from main. Any client of a headless `adk api_server` could load and run the special internal agents that back the dev UI, `__adk_agent_builder_assistant` among them, by naming one as the app. The agent loader accepted any `__`-prefixed name unconditionally, and the server never looked at the prefix. Loading a `__`-prefixed agent now raises `PermissionError` unless the loader was configured to allow it, and `AdkWebServer` answers 403 for those names before it asks the loader at all. `get_fast_api_app` turns both on from its existing `web` argument, so the dev UI keeps working and a headless api_server does not. Behaviour change: driving a special internal agent through `adk api_server` now returns 403 where it used to run the agent. Adapted for v1, which has a single `AdkWebServer` rather than the `ApiServer` and `DevServer` pair upstream writes the flag against. Upstream guards two `load_agent` call sites; its other four sit on `DevServer`, which is never constructed when `web=False`, so those routes do not exist there at all. v1 registers every route on one class, so all six calls go through a single `_load_agent_or_app` helper that holds the 403. That makes the four eval and graph routes answer 403 where upstream answers 404, and it is what covers a caller who supplies an agent loader of their own that ignores the loader flag.
Port of 1fa7cda from main. `resolve_file_path` returned an absolute path unchanged and joined a relative one onto an unresolved root without ever checking where the result landed, so `..` segments and absolute paths both escaped the project root. The Agent Builder's write, read and delete tools resolve every path through it, which put arbitrary files on the machine within reach of whatever the model was told to do. It now resolves the root and the candidate and raises `ValueError` when the candidate is not inside the root. Interior `..` that stays inside the root still resolves, as does an absolute path pointing inside it. Behaviour change: an Agent Builder session that wrote, read or deleted a file outside its root directory now gets a tool failure instead. Every caller of `resolve_file_path` and `resolve_file_paths` already wraps the call in a `try`, so the new `ValueError` surfaces as `{"success": False, ...}` rather than a traceback. The commit's import-isolation test comes along unchanged; it guards a separate property, that importing `adk_web_server` does not eagerly pull in the Agent Builder agent stack.
GWeale
changed the title
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
2 participants
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.
Ports two commits from
maintov1.Agent name gating in the CLI server (
3f6eb1f0)AgentLoader.load_agentraisesPermissionErrorfor a__-prefixed name; set_allow_special_agentson the loader instance to keep loading them.adk api_serveranswers 403 for those names, including the eval and graph routes.adk webstill serves them, via the existingwebargument toget_fast_api_app.Path scoping in the Agent Builder file tools (
1fa7cda9)resolve_file_pathraisesValueErrorwhen the path resolves outside the root directory; the file tools return{"success": False, ...}...and symlinks collapsed.The server-side check is re-implemented for
v1's singleAdkWebServerrather than ported.