fix(configurable): prevent path traversal in AgentTool config_path resolution by adilburaksen · Pull Request #878 · google/adk-go

adilburaksen · 2026-05-23T21:39:00Z

Summary

ResolveAgentReference in configurable_utils.go accepted absolute refPath values unconditionally and joined relative paths via filepath.Join without boundary enforcement. An attacker-controlled config_path field in an agent YAML could read arbitrary files accessible to the server process.

Vulnerable pattern (before):

if !filepath.IsAbs(refPath) {
    targetPath = filepath.Join(filepath.Dir(parentPath), refPath)  // no ".." check
}
absPath, _ := filepath.Abs(targetPath)
a, err := FromConfig(ctx, absPath)  // reads file unconditionally

Fix

Reject absolute refPath values
After resolving, verify the path stays within the parent agent's directory using strings.HasPrefix on the cleaned path

…solution Absolute config_path values were accepted unconditionally, and relative paths were joined without boundary validation, allowing traversal outside the agent directory via "../../../etc/passwd" style inputs. Fix: reject absolute paths; verify the resolved path stays within the parent agent's directory using strings.HasPrefix after filepath.Clean.

…nfig_path containment check The containment check for AgentTool config_path compared an absolute target path against a parent directory that was only cleaned, not made absolute, so a relative parentPath caused legitimate in-directory references to be rejected. The comparison was also purely lexical, so a symlink inside the agent directory could still resolve outside it and be loaded. Make both sides absolute before comparing and resolve symlinks where the paths exist, falling back to the lexical path when they do not so that a missing file still reports as not found rather than as a traversal. Add regression tests covering absolute paths, parent traversal, symlink escape, references inside the agent directory, and a relative parent path.

adilburaksen · 2026-07-28T10:53:03Z

@wolo-lab @hanorik — could one of you take a look at this when you have a moment, or route it to whoever owns internal/configurable?

This closes a path traversal in ResolveAgentReference: an attacker-supplied config_path is currently used verbatim when absolute, and joined without a containment check when relative, so a sub-agent reference such as ../../../../etc/passwd is read from outside the parent agent's directory.

The same issue was fixed in the sibling implementations and both have landed:

adk-python fix(agents): prevent path traversal in AgentTool config_path resolution adk-python#5826 (merged, 171ae9e)
adk-java fix(agents): warn when AgentTool config_path escapes agent base directory adk-java#1218 (merged, 08f4fdc6c9)

adk-go is the last of the three still carrying the original pattern.

I have just pushed an update that tightens the check and adds tests:

both sides of the comparison are now made absolute. The previous revision compared an absolute target against a parent directory that was only filepath.Cleaned, so a relative parentPath rejected legitimate in-directory references.
symlinks are resolved with filepath.EvalSymlinks where the paths exist, so a symlink inside the agent directory can no longer resolve outside it. When a path does not exist the check falls back to the lexical path, so a missing file still reports as not found rather than as a traversal.
added configurable_utils_test.go covering absolute paths, parent traversal, symlink escape, a legitimate in-directory reference, and a relative parent path. The symlink and relative-path cases fail against the previous revision of this PR and pass with the update.

Status: CLA signed, check-changes green, go build, go vet, gofmt and the package tests all clean locally, and the branch is rebased on current main — it still applies to internal/configurable/configurable_utils.go after the V2 restructure. No reviewer was ever assigned to the PR, which I suspect is why it has been sitting rather than any deliberate decision.

Happy to adjust the approach to whatever you prefer, or feel free to take the change over and land it yourselves.

wolo-lab · 2026-08-06T12:31:21Z

Thank you for preparing a fix. @baptmont, please take a look at this PR.

…solution (#878) (#1277) fix(configurable): prevent path traversal in AgentTool config_path resolution Reject absolute config_path values and verify that a relative one resolves inside the referencing agent's directory, resolving symlinks where the paths exist. Matches the hard rejection adk-python landed in 171ae9e; adk-java (#1218) chose a warn-only deprecation instead. BREAKING: an absolute config_path is no longer accepted. --------- * fix(configurable): prevent path traversal in AgentTool config_path resolution Absolute config_path values were accepted unconditionally, and relative paths were joined without boundary validation, allowing traversal outside the agent directory via "../../../etc/passwd" style inputs. Fix: reject absolute paths; verify the resolved path stays within the parent agent's directory using strings.HasPrefix after filepath.Clean. * fix(configurable): compare absolute, symlink-resolved paths in the config_path containment check The containment check for AgentTool config_path compared an absolute target path against a parent directory that was only cleaned, not made absolute, so a relative parentPath caused legitimate in-directory references to be rejected. The comparison was also purely lexical, so a symlink inside the agent directory could still resolve outside it and be loaded. Make both sides absolute before comparing and resolve symlinks where the paths exist, falling back to the lexical path when they do not so that a missing file still reports as not found rather than as a traversal. Add regression tests covering absolute paths, parent traversal, symlink escape, references inside the agent directory, and a relative parent path. --------- Co-authored-by: João Westerberg <westerberg@google.com> (cherry picked from commit 604dd63) Co-authored-by: Adil Burak Şen <56400880+adilburaksen@users.noreply.github.com>

…erences #878 added a containment check to ResolveAgentReference, but the workflow loader resolves node references on a separate path that never reaches it. resolveNodeLike accepted an absolute reference and joined a relative one without validating the result, so a workflow config could name any .yaml or .yml file on disk. The FunctionNode, JoinNode and ToolNode branches of resolveNodeFromYAML all return before its fall-through call to ResolveAgentReference, so for those node classes an out-of-tree file was read, parsed and instantiated with no containment check at all. Only the agent branch was covered, and by the time it ran the read had already happened. Extract the check into resolveContainedConfigPath and call it from both resolvers, before the file is read and before the node cache is consulted. One shared copy is the point: two independent resolvers is how the gap appeared in the first place. BREAKING: an absolute workflow node reference is no longer accepted, matching the config_path behaviour #878 established.

PR google#878 added a containment check to ResolveAgentReference: an AgentTool config_path may not be absolute, and a relative one must resolve inside the referencing agent's directory. Workflow edge references were left on the pre-google#878 code — absolute refs accepted unconditionally, relative ones joined with no boundary check — and resolveNodeFromYAML reads them straight off disk with os.ReadFile before dispatching on agent_class. A workflow config could therefore load and instantiate a FunctionNode, JoinNode or ToolNode from anywhere on the filesystem. Extract the containment logic into resolveConfigReference and route both ResolveAgentReference and resolveNodeLike through it, so the rule has one implementation rather than one per reference kind. Two fixes to the check itself, both found while covering the shared helper: - The parent directory is now resolved before the reference is joined onto it, so both sides of the comparison are rooted in the same real path. Resolving only the parent side meant that a parent directory reached through a symlink plus a not-yet-existing target — which has no symlinks to resolve and so keeps its unresolved spelling — was reported as a traversal instead of as not found, the case google#878 intended to allow. - A reference carrying a volume name is rejected alongside absolute ones. On Windows a drive-relative reference such as `C:node.yaml` is not absolute yet still escapes, by resolving against the current directory of that drive; filepath.VolumeName also covers UNC paths and is empty on Unix. BREAKING: an absolute config_path is no longer accepted in workflow edge references, matching the google#878 change to agent references. Assisted-by: Claude Opus 5 <noreply@anthropic.com>

…solution (google#878) fix(configurable): prevent path traversal in AgentTool config_path resolution Reject absolute config_path values and verify that a relative one resolves inside the referencing agent's directory, resolving symlinks where the paths exist. Matches the hard rejection adk-python landed in 171ae9e; adk-java (google#1218) chose a warn-only deprecation instead. BREAKING: an absolute config_path is no longer accepted. --------- * fix(configurable): prevent path traversal in AgentTool config_path resolution Absolute config_path values were accepted unconditionally, and relative paths were joined without boundary validation, allowing traversal outside the agent directory via "../../../etc/passwd" style inputs. Fix: reject absolute paths; verify the resolved path stays within the parent agent's directory using strings.HasPrefix after filepath.Clean. * fix(configurable): compare absolute, symlink-resolved paths in the config_path containment check The containment check for AgentTool config_path compared an absolute target path against a parent directory that was only cleaned, not made absolute, so a relative parentPath caused legitimate in-directory references to be rejected. The comparison was also purely lexical, so a symlink inside the agent directory could still resolve outside it and be loaded. Make both sides absolute before comparing and resolve symlinks where the paths exist, falling back to the lexical path when they do not so that a missing file still reports as not found rather than as a traversal. Add regression tests covering absolute paths, parent traversal, symlink escape, references inside the agent directory, and a relative parent path. --------- Co-authored-by: João Westerberg <westerberg@google.com>

@tonydzi

PR #878 added a containment check to ResolveAgentReference: an AgentTool config_path may not be absolute, and a relative one must resolve inside the referencing agent's directory. Workflow edge references were left on the pre-#878 code — absolute refs accepted unconditionally, relative ones joined with no boundary check — and resolveNodeFromYAML reads them straight off disk with os.ReadFile before dispatching on agent_class. A workflow config could therefore load and instantiate a FunctionNode, JoinNode or ToolNode from anywhere on the filesystem. Extract the containment logic into resolveConfigReference and route both ResolveAgentReference and resolveNodeLike through it, so the rule has one implementation rather than one per reference kind. Two fixes to the check itself, both found while covering the shared helper: - The parent directory is now resolved before the reference is joined onto it, so both sides of the comparison are rooted in the same real path. Resolving only the parent side meant that a parent directory reached through a symlink plus a not-yet-existing target — which has no symlinks to resolve and so keeps its unresolved spelling — was reported as a traversal instead of as not found, the case #878 intended to allow. - A reference carrying a volume name is rejected alongside absolute ones. On Windows a drive-relative reference such as `C:node.yaml` is not absolute yet still escapes, by resolving against the current directory of that drive; filepath.VolumeName also covers UNC paths and is empty on Unix. BREAKING: an absolute config_path is no longer accepted in workflow edge references, matching the #878 change to agent references. Assisted-by: Claude Opus 5 <noreply@anthropic.com> fix(configurable): refuse symlinked components rather than resolving them resolveConfigReference resolved the reference's symlinks and compared the result against the parent directory. EvalSymlinks fails when the target does not exist and the fallback keeps the lexical spelling, so a symlink pointing out of the directory at a target that is not there yet passes as contained. Only the missing target stops the read, and it stops being missing the moment anything creates it. Refuse links instead of following them. A reference must satisfy filepath.IsLocal, which settles the lexical half on every platform, and no component below the resolved parent directory may be a symlink. Refusing does not depend on the link's target existing at the moment of the check, which is what the previous approach relied on. IsLocal also replaces the hand-rolled IsAbs and VolumeName pair. It rejects the same drive-relative and UNC spellings, and additionally the Windows reserved device names — NUL, com1, LPT1, CONIN$ and the trailing-space and trailing-dot variants — none of which VolumeName catches. The component walk covers Windows directory junctions too, since os.Lstat reports IO_REPARSE_TAG_MOUNT_POINT with ModeSymlink set. Rejections are sentinel errors, so the tests identify them with errors.Is rather than by matching the message text. fix(configurable): restore the strings import the merge dropped 34896ab merged main into this branch and took the import block from this side, which no longer imports strings, while keeping the test main added on the other side. That test still calls strings.Contains, so the package does not compile and no CI job on this PR can report anything but a build failure. Verified by building the test binary for linux/amd64, which is what every job in go.yml and nightly.yml runs on. Assisted-by: Claude (Anthropic) / claude-opus-5 Machine: A-2022BAYAREA Account: a Operator: robot:git-s7-fast fix(configurable): refuse junctions, not just symlinks, in the component walk The walk tests os.Lstat's ModeSymlink, and the commit that introduced it says that covers Windows directory junctions because Lstat reports IO_REPARSE_TAG_MOUNT_POINT with ModeSymlink set. That was true until Go 1.23. Since then, and by default under godebug winsymlink=1, only IO_REPARSE_TAG_SYMLINK sets ModeSymlink and every other reparse tag falls through to ModeIrregular: os/types_windows.go mode(), with the old behaviour kept beside it in modePreGo1_23. go.mod declares go 1.26, so this module gets the newer mapping. Measured on windows/amd64, go1.26.7: Lstat of a junction returns ?rw-rw-rw-, ModeSymlink false and ModeIrregular true, and a junction placed inside the agent directory pointing out of it is accepted as contained -- with the target present and, which is the shape this check exists for, with the target still missing. A junction needs no elevation to create, unlike a symlink, so it is the cheaper of the two to arrange. Refusing ModeIrregular as well keeps the rule the same for every reparse tag rather than for one of them. The predicate is split out so it can be asserted on every platform, since the junction test needs Windows to build one and this project has no Windows CI to build it on. Assisted-by: Claude (Anthropic) / claude-opus-5 Machine: A-2022BAYAREA Account: a Operator: robot:git-s7-fast fix(configurable): pin the containment semantics and correct its diagnostics Review found that the one behaviour this change deliberately chose was the one behaviour nothing asserted. Both os.Symlink calls in the package tests build links pointing outside the config directory, so an implementation that went back to resolving links and refusing only escaping ones passed the whole suite unchanged -- carrying with it the dangling-target hole that motivated refusing links in the first place. Pin it from both sides, and correct three places where the code says more or less than it does. - TestResolveConfigReferenceRefusesLinksThatStayInside asserts that a link wholly inside the config directory is refused for being a link, in its minimal form and in the Kubernetes projected-volume layout the kubelet materialises for a ConfigMap. That layout is the cost of the choice and is now visible in the suite rather than only in a review comment. - TestResolveConfigReferenceCanonicalizesRegistryKey pins the EvalSymlinks call on the parent directory. Deleting that call changes no verdict -- os.Lstat follows every component but the last, so the two sides of the walk cannot end up rooted differently -- and the whole suite passed without it. What it does is canonicalise the path callers use as the agentRegistry and nodeRegistry key, so a directory and a symlink to it share one entry instead of each building its own copy of the same agent. Its comment described the hazard of the earlier prefix-compare design. - A reference below a component that is not a directory reported as an inspection failure rather than as either rejection sentinel, giving callers a third class of error for a reference that simply cannot exist. ENOTDIR is now treated as ErrNotExist is, and left for the caller's own read to report. Note that ENOTDIR does not satisfy fs.ErrNotExist, unlike ENOENT, so it needs naming explicitly. - An empty reference lost its diagnostic when IsLocal replaced the hand-rolled checks: it rendered as the generic message with a dangling ": " and no path. An empty reference is almost always an unfilled template, so it gets its own message again, under the same sentinel. - TestResolveConfigReferenceVolumeQualifiedRefs asserted nothing on Unix, where VolumeName is empty for all three inputs and every iteration hit the continue before the assertion. With no Windows job in CI it therefore asserted nothing anywhere. The expectation is now platform-dependent rather than skipped: refused where the spelling carries a volume, accepted where it is an ordinary if odd file name. Two comments overclaimed. refuseSymlinkComponents said it refuses components that can redirect the read out of the directory; it refuses all links regardless of target, and catches neither hard links nor FIFOs, so it now says which boundary it draws and names what falls outside it. isLinkLike said every reparse tag other than IO_REPARSE_TAG_SYMLINK falls through to ModeIrregular; two do not, as IO_REPARSE_TAG_AF_UNIX maps to ModeSocket and IO_REPARSE_TAG_DEDUP is reported as an ordinary file. Neither redirects a read out of the directory, so the junction conclusion stands. Each new test was killed by a mutant rather than only observed passing. Removing the EvalSymlinks call fails the registry-key test and nothing else; reverting the walk to resolve-and-refuse-only-escaping fails the stays-inside test and nothing else; removing the empty-reference branch, removing the ENOTDIR clause, and over-rejecting a colon in a reference each fail their own test and no other. Hard links are left alone, as reviewers agreed: link counts are not portable on FileInfo, the realistic vector is archive extraction, and the behaviour is unchanged from the merge-base. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Agent-session: ses_e5fa3107d7affe0SsJobIWTTb6 test(configurable): drop the API-key dependency from the registry-key test TestResolveConfigReferenceCanonicalizesRegistryKey wrote its fixture configs as LlmAgent with a model, so ResolveAgentReference built a genai client and failed with "api key is required for Google AI backend" wherever no key was set. Reproduced on linux, darwin and windows. CI defines no API key secret, so the Go workflow would have gone red on it as soon as the fork run was approved. Worse than the red line: execution stopped at the Fatalf before the two registry assertions, so the half of the test that pins the consequence the canonical key exists for never ran in a keyless environment. SequentialAgent is registered alongside LlmAgent and needs no model. With it, removing the EvalSymlinks call now fails the test twice, on the key equality and on the registry entry, instead of once. Reported by @tonydzi from a run on windows/amd64, go1.26.7. New Approach Exp fs.go Fixes New version Added symlink support Updated tests Fixes Fixes Fix for windows Moved isLinkLike Misc fixes

PR google#878 added a containment check to ResolveAgentReference: an AgentTool config_path may not be absolute, and a relative one must resolve inside the referencing agent's directory. Workflow edge references never reached that check — resolveNodeFromYAML reads the referenced file with os.ReadFile before it dispatches on agent_class, and FunctionNode, JoinNode and ToolNode are built entirely inside that function — so a workflow config could load and instantiate a node from anywhere on the filesystem. Move the containment rule into one helper, utils.SafeSubpath, reached through resolveConfigReference by both ResolveAgentReference and resolveNodeLike, ahead of the read and the registry-cache lookup. A second copy of the rule is a second place to forget it, which is how the workflow path was left behind. The rule has two halves: - Lexical: SafeSubpath refuses a reference that is absolute, starts with "/" (or "\" on Windows), escapes its base with "..", or carries a null byte, plus the Windows-only spellings a hand-rolled IsAbs/VolumeName pair misses — drive-relative refs like C:node.yaml, UNC paths, the reserved device names (NUL, COM1, LPT1, CONIN$ …), and trailing space or dot — then confirms via filepath.Rel that the joined path stays under the base. - Links: no component below the base may be a symlink, a junction, or any other reparse point. The reference is refused for being a link, not followed to see where it points. Resolving cannot be made safe: a link inside the directory pointing out of it at a target that does not exist yet resolves to nothing and passes a containment test, then the read fails only because the target is missing — and it stops being missing the moment anything creates it. The walk tests ModeSymlink|ModeIrregular, not ModeSymlink alone, because since Go 1.23 os.Lstat reports a Windows directory junction (IO_REPARSE_TAG_MOUNT_POINT) as ModeIrregular; this module declares go 1.26, so a ModeSymlink-only test walks past a junction. Rejections are sentinel errors (ErrNotValidRelativePath, ErrSymlinkInRelativePath), identified with errors.Is rather than by message. BREAKING CHANGE: an absolute config_path is no longer accepted in workflow edge references, matching what google#878 did for agent references. A reference reached through a link anywhere inside the config directory no longer loads either, for agent and workflow references alike, wherever that link points — this refuses config directories materialised entirely from symlinks (Kubernetes ConfigMaps and projected volumes, Bazel runfiles, Nix store paths); stage such a mount into a directory of real files (e.g. cp -rL) before pointing the agent at it. Unchanged from the merge-base, named rather than fixed: a hard link inside the directory pointing outside it is indistinguishable from a regular file to os.Lstat, and the TOCTOU window between the check and the later os.ReadFile is inherent to validating a path as a string. Testing: go build, go test -race -shuffle=on, golangci-lint run and go mod tidy -diff across the workspace; GOOS=windows and GOOS=darwin go vet and test-binary builds. The deliberately chosen "refuse links that stay inside" behaviour is pinned by its own test, and each new test was shown to fail against a mutant rather than only observed passing. The repository has no Windows CI job; the Windows behaviour was measured by hand on windows/amd64, go1.26.7. Co-authored-by: Anton Dzyatkovsky <194927794+tonydzi@users.noreply.github.com> Co-authored-by: Sasha Shynkaruk <39381081+jjsasha63@users.noreply.github.com>

adilburaksen force-pushed the fix/config-path-traversal branch from 3352de5 to 2a4876e Compare May 23, 2026 21:47

Ashutosh0x mentioned this pull request May 30, 2026

fix(security): prevent arbitrary command execution in McpToolset YAML config #923

Closed

8 of 9 tasks

adilburaksen and others added 3 commits June 6, 2026 15:27

Merge branch 'main' into fix/config-path-traversal

dda67a8

Merge branch 'main' into fix/config-path-traversal

835eb24

wolo-lab requested a review from baptmont August 6, 2026 12:31

Merge branch 'main' into fix/config-path-traversal

cd39b98

baptmont approved these changes Aug 7, 2026

View reviewed changes

Merge branch 'main' into fix/config-path-traversal

0d71c1e

baptmont merged commit 604dd63 into google:main Aug 7, 2026
13 checks passed

wolo-lab mentioned this pull request Aug 10, 2026

fix(configurable): prevent path traversal in AgentTool config_path resolution (#878) #1277

Merged

tonydzi mentioned this pull request Aug 10, 2026

fix(configurable): apply config_path containment to workflow node refs #1299

Open

jjsasha63 mentioned this pull request Aug 11, 2026

fix(configurable): apply config_path containment to workflow node references #1305

Closed

Navigation Menu

Repository navigation

fix(configurable): prevent path traversal in AgentTool config_path resolution - #878

fix(configurable): prevent path traversal in AgentTool config_path resolution#878
baptmont merged 6 commits into
google:mainfrom
adilburaksen:fix/config-path-traversal

adilburaksen commented May 23, 2026

Uh oh!

google-cla Bot commented May 23, 2026

Uh oh!

adilburaksen commented Jul 28, 2026

Uh oh!

wolo-lab commented Aug 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

adilburaksen commented May 23, 2026

Summary

Fix

Related

Uh oh!

google-cla Bot commented May 23, 2026

Uh oh!

adilburaksen commented Jul 28, 2026

Uh oh!

wolo-lab commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wolo-lab commented Aug 6, 2026 •

edited

Loading