Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions internal/configurable/configurable_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"os"
"os/exec"
"path/filepath"
"strings"
"sync"

"github.com/modelcontextprotocol/go-sdk/mcp"
Expand Down Expand Up @@ -370,17 +371,36 @@ func ResolveAgentReference(ctx context.Context, parentPath, refPath string) (age
return nil, fmt.Errorf("agent reference path cannot be empty")
}

targetPath := refPath
// Handle relative paths
if !filepath.IsAbs(refPath) {
targetPath = filepath.Join(filepath.Dir(parentPath), refPath)
if filepath.IsAbs(refPath) {
return nil, fmt.Errorf("absolute paths are not allowed in AgentTool config_path: %s", refPath)
}

targetPath := filepath.Join(filepath.Dir(parentPath), refPath)

absPath, err := filepath.Abs(targetPath)
if err != nil {
return nil, fmt.Errorf("failed to resolve absolute path: %w", err)
}

// Prevent path traversal outside the parent agent's directory. Both sides are
// made absolute before comparing, and symlinks are resolved where the paths
// exist, so a symlink inside the agent directory cannot be used to escape it.
parentDir, err := filepath.Abs(filepath.Dir(parentPath))
if err != nil {
return nil, fmt.Errorf("failed to resolve agent directory: %w", err)
}
if resolved, err := filepath.EvalSymlinks(parentDir); err == nil {
parentDir = resolved
}
checkPath := absPath
if resolved, err := filepath.EvalSymlinks(absPath); err == nil {
checkPath = resolved
}
if !strings.HasPrefix(checkPath, parentDir+string(os.PathSeparator)) && checkPath != parentDir {
return nil, fmt.Errorf(
"path traversal detected: config_path %q resolves outside agent directory", refPath)
}

registryMu.RLock()
if a, ok := agentRegistry[absPath]; ok {
registryMu.RUnlock()
Expand Down
144 changes: 144 additions & 0 deletions internal/configurable/configurable_utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package configurable

import (
"context"
"os"
"path/filepath"
"strings"
"testing"
)

const traversalError = "path traversal detected"

// newAgentDir lays out an agent directory with a sibling config, a file outside
// the directory, and a symlink inside the directory pointing at that outside
// file. It returns the base directory and the parent agent config path.
func newAgentDir(t *testing.T) (base, parentPath string) {
t.Helper()

base = t.TempDir()
// Resolve the temporary directory itself, since on some platforms it is
// reached through a symlink (for example /var -> /private/var on macOS).
if resolved, err := filepath.EvalSymlinks(base); err == nil {
base = resolved
}

agentDir := filepath.Join(base, "agents", "root")
if err := os.MkdirAll(agentDir, 0o755); err != nil {
t.Fatalf("MkdirAll(%q) failed: %v", agentDir, err)
}

parentPath = filepath.Join(agentDir, "root_agent.yaml")
if err := os.WriteFile(parentPath, []byte("agent_class: LlmAgent\n"), 0o644); err != nil {
t.Fatalf("WriteFile(%q) failed: %v", parentPath, err)
}
if err := os.WriteFile(filepath.Join(agentDir, "sub_agent.yaml"), []byte("agent_class: LlmAgent\n"), 0o644); err != nil {
t.Fatalf("WriteFile(sub_agent.yaml) failed: %v", err)
}

outside := filepath.Join(base, "outside.yaml")
if err := os.WriteFile(outside, []byte("agent_class: LlmAgent\n"), 0o644); err != nil {
t.Fatalf("WriteFile(%q) failed: %v", outside, err)
}
if err := os.Symlink(outside, filepath.Join(agentDir, "link.yaml")); err != nil {
t.Skipf("symlinks are not supported in this environment: %v", err)
}

return base, parentPath
}

func TestResolveAgentReferenceRejectsEscapingConfigPath(t *testing.T) {
_, parentPath := newAgentDir(t)

tests := []struct {
name string
refPath string
wantErr string
}{
{
name: "absolute path",
refPath: filepath.Join(string(os.PathSeparator), "etc", "passwd"),
wantErr: "absolute paths are not allowed",
},
{
name: "parent traversal",
refPath: filepath.Join("..", "..", "outside.yaml"),
wantErr: traversalError,
},
{
name: "symlink escaping the agent directory",
refPath: "link.yaml",
wantErr: traversalError,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// The reference must be rejected by the containment check itself, not
// merely fail later while the config is being loaded.
_, err := ResolveAgentReference(context.Background(), parentPath, tc.refPath)
if err == nil {
t.Fatalf("ResolveAgentReference(_, %q, %q) succeeded, want error containing %q", parentPath, tc.refPath, tc.wantErr)
}
if !strings.Contains(err.Error(), tc.wantErr) {
t.Errorf("ResolveAgentReference(_, %q, %q) = %v, want error containing %q", parentPath, tc.refPath, err, tc.wantErr)
}
})
}
}

// TestResolveAgentReferenceAllowsPathsInsideAgentDir guards against the
// containment check rejecting legitimate references. The reference is not a
// loadable agent config, so an error is expected; it must not be the traversal
// error.
func TestResolveAgentReferenceAllowsPathsInsideAgentDir(t *testing.T) {
_, parentPath := newAgentDir(t)

_, err := ResolveAgentReference(context.Background(), parentPath, "sub_agent.yaml")
if err != nil && strings.Contains(err.Error(), traversalError) {
t.Errorf("ResolveAgentReference(_, %q, %q) = %v, want no traversal rejection", parentPath, "sub_agent.yaml", err)
}
}

// TestResolveAgentReferenceRelativeParentPath covers a parent path that is not
// absolute, which the containment check must normalise before comparing.
func TestResolveAgentReferenceRelativeParentPath(t *testing.T) {
_, parentPath := newAgentDir(t)
agentDir := filepath.Dir(parentPath)

cwd, err := os.Getwd()
if err != nil {
t.Fatalf("Getwd failed: %v", err)
}
if err := os.Chdir(agentDir); err != nil {
t.Fatalf("Chdir(%q) failed: %v", agentDir, err)
}
t.Cleanup(func() {
if err := os.Chdir(cwd); err != nil {
t.Fatalf("restoring working directory failed: %v", err)
}
})

if _, err := ResolveAgentReference(context.Background(), "root_agent.yaml", "sub_agent.yaml"); err != nil &&
strings.Contains(err.Error(), traversalError) {
t.Errorf("ResolveAgentReference with a relative parent path = %v, want no traversal rejection", err)
}

if _, err := ResolveAgentReference(context.Background(), "root_agent.yaml", filepath.Join("..", "..", "outside.yaml")); err == nil {
t.Error("ResolveAgentReference with a relative parent path and an escaping reference succeeded, want an error")
}
}
Loading