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

internal/configurable/configurable_utils.go

@@ -365,40 +365,77 @@ func ResolveCallbackReference(ctx context.Context, callbackName string) (any, er
 	return nil, fmt.Errorf("callback '%s' not found", callbackName)
 }
 
-// ResolveAgentReference builds an agent from a reference config.
-func ResolveAgentReference(ctx context.Context, parentPath, refPath string) (agent.Agent, error) {
+// resolveContainedConfigPath resolves refPath against the directory holding
+// parentPath and returns the canonical result, rejecting references that
+// escape that directory.
+//
+// Every config reference naming another file on disk must go through this
+// helper, before the file is read and before any cache is consulted. A config
+// file is only as trustworthy as whoever supplied it, and the directory of the
+// referencing config is the boundary the loader promises to stay inside.
+//
+// This mirrors adk-python's resolve_agent_reference: reject an absolute
+// reference, canonicalise both the reference and the agent directory, and
+// require the first to sit under the second. The canonical path is returned, so
+// a config reached through a symlink resolves its own references relative to
+// where it really lives, as it does in Python.
+func resolveContainedConfigPath(parentPath, refPath string) (string, error) {
 	if refPath == "" {
-		return nil, fmt.Errorf("agent reference path cannot be empty")
+		return "", fmt.Errorf("config reference path cannot be empty")
 	}
 
 	if filepath.IsAbs(refPath) {
-		return nil, fmt.Errorf("absolute paths are not allowed in AgentTool config_path: %s", refPath)
+		return "", fmt.Errorf("absolute paths are not allowed in config_path: %s", refPath)
 	}
 
-	targetPath := filepath.Join(filepath.Dir(parentPath), refPath)
-
-	absPath, err := filepath.Abs(targetPath)
+	agentDir, err := filepath.Abs(filepath.Dir(parentPath))
 	if err != nil {
-		return nil, fmt.Errorf("failed to resolve absolute path: %w", err)
+		return "", fmt.Errorf("failed to resolve agent directory: %w", err)
+	}
+	// Join cleans the result, so ".." segments collapse before anything looks
+	// at the filesystem.
+	resolvedPath := realPath(filepath.Join(agentDir, refPath))
+	canonicalAgentDir := realPath(agentDir)
+
+	// Equivalent to Python's os.path.commonpath([dir, path]) != dir: the
+	// reference must be the directory itself or sit beneath it, compared by
+	// whole path elements rather than by string prefix.
+	rel, err := filepath.Rel(canonicalAgentDir, resolvedPath)
+	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
+		return "", fmt.Errorf(
+			"path traversal detected: config_path %q resolves outside agent directory", refPath)
 	}
 
-	// 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))
+	return resolvedPath, nil
+}
+
+// realPath is the Go equivalent of Python's os.path.realpath: it resolves
+// symlinks as far as the path exists and keeps the remainder as written.
+//
+// [filepath.EvalSymlinks] alone is not equivalent, because it fails outright
+// when any component is missing, and a reference to a config that does not
+// exist yet is an ordinary typo rather than a containment failure. Resolving
+// the longest existing prefix keeps a symlinked parent directory honest while
+// letting the missing tail through, so the caller reports "config file not
+// found" instead of a traversal error.
+func realPath(path string) string {
+	if resolved, err := filepath.EvalSymlinks(path); err == nil {
+		return resolved
+	}
+
+	dir, last := filepath.Split(path)
+	dir = filepath.Clean(dir)
+	if last == "" || dir == path {
+		return path // reached the root without resolving anything
+	}
+	return filepath.Join(realPath(dir), last)
+}
+
+// ResolveAgentReference builds an agent from a reference config.
+func ResolveAgentReference(ctx context.Context, parentPath, refPath string) (agent.Agent, error) {
+	absPath, err := resolveContainedConfigPath(parentPath, refPath)
 	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)
+		return nil, err
 	}
 
 	registryMu.RLock()

internal/configurable/configurable_utils_test.go

@@ -142,3 +142,151 @@ func TestResolveAgentReferenceRelativeParentPath(t *testing.T) {
 		t.Error("ResolveAgentReference with a relative parent path and an escaping reference succeeded, want an error")
 	}
 }
+
+// TestResolveAgentReferenceFollowsSymlinkInsideDir guards against the
+// containment check being too strict: a symlink that stays inside the agent
+// directory is a legitimate way to organise configs, and adk-python allows it.
+func TestResolveAgentReferenceFollowsSymlinkInsideDir(t *testing.T) {
+	_, parentPath := newAgentDir(t)
+	agentDir := filepath.Dir(parentPath)
+
+	if err := os.Symlink("sub_agent.yaml", filepath.Join(agentDir, "alias.yaml")); err != nil {
+		t.Skipf("symlinks are not supported in this environment: %v", err)
+	}
+
+	_, err := ResolveAgentReference(context.Background(), parentPath, "alias.yaml")
+	if err != nil && strings.Contains(err.Error(), traversalError) {
+		t.Errorf("ResolveAgentReference(_, %q, %q) = %v, want no traversal rejection", parentPath, "alias.yaml", err)
+	}
+}
+
+// TestResolveAgentReferenceMissingFileIsNotTraversal covers a reference to a
+// config that does not exist. Canonicalisation cannot resolve it, but a typo
+// must report the missing file rather than a containment failure, which is what
+// os.path.realpath gives adk-python for free.
+func TestResolveAgentReferenceMissingFileIsNotTraversal(t *testing.T) {
+	_, parentPath := newAgentDir(t)
+
+	_, err := ResolveAgentReference(context.Background(), parentPath, filepath.Join("nodes", "typo.yaml"))
+	if err == nil {
+		t.Fatal("ResolveAgentReference for a missing config succeeded, want an error")
+	}
+	if strings.Contains(err.Error(), traversalError) {
+		t.Errorf("ResolveAgentReference for a missing config = %v, want a not-found error", err)
+	}
+	if !strings.Contains(err.Error(), "config file not found") {
+		t.Errorf("ResolveAgentReference for a missing config = %v, want a not-found error", err)
+	}
+}
+
+// TestResolveAgentReferenceThroughSymlinkedAgentDir covers an agent directory
+// reached through a symlink. Both sides are canonicalised, so a reference
+// inside it must still resolve.
+func TestResolveAgentReferenceThroughSymlinkedAgentDir(t *testing.T) {
+	base, parentPath := newAgentDir(t)
+	agentDir := filepath.Dir(parentPath)
+
+	linkedDir := filepath.Join(base, "linked_root")
+	if err := os.Symlink(agentDir, linkedDir); err != nil {
+		t.Skipf("symlinks are not supported in this environment: %v", err)
+	}
+
+	viaLink := filepath.Join(linkedDir, "root_agent.yaml")
+	_, err := ResolveAgentReference(context.Background(), viaLink, "sub_agent.yaml")
+	if err != nil && strings.Contains(err.Error(), traversalError) {
+		t.Errorf("ResolveAgentReference(_, %q, %q) = %v, want no traversal rejection", viaLink, "sub_agent.yaml", err)
+	}
+}
+
+// TestResolveAgentReferenceRejectsSiblingPrefixDir covers a sibling directory
+// whose name begins with the agent directory's name. Containment has to compare
+// whole path elements, not raw string prefixes.
+func TestResolveAgentReferenceRejectsSiblingPrefixDir(t *testing.T) {
+	base, parentPath := newAgentDir(t)
+
+	siblingDir := filepath.Join(base, "agents", "root-evil")
+	if err := os.MkdirAll(siblingDir, 0o755); err != nil {
+		t.Fatalf("MkdirAll(%q) failed: %v", siblingDir, err)
+	}
+	if err := os.WriteFile(filepath.Join(siblingDir, "evil.yaml"), []byte("agent_class: LlmAgent\n"), 0o644); err != nil {
+		t.Fatalf("WriteFile(evil.yaml) failed: %v", err)
+	}
+
+	refPath := filepath.Join("..", "root-evil", "evil.yaml")
+	_, err := ResolveAgentReference(context.Background(), parentPath, refPath)
+	if err == nil {
+		t.Fatalf("ResolveAgentReference(_, %q, %q) succeeded, want error containing %q", parentPath, refPath, traversalError)
+	}
+	if !strings.Contains(err.Error(), traversalError) {
+		t.Errorf("ResolveAgentReference(_, %q, %q) = %v, want error containing %q", parentPath, refPath, err, traversalError)
+	}
+}
+
+// TestResolveAgentReferenceRejectsThroughDanglingSymlinkedDir covers a
+// reference through a symlinked directory whose final component does not
+// exist. realPath cannot resolve the reference in one EvalSymlinks call in
+// that case, and falls back to resolving the longest existing prefix and
+// rejoining the missing tail; that fallback must still see through the
+// directory symlink and reject the escape, rather than comparing against the
+// reference's unresolved, lexical spelling.
+func TestResolveAgentReferenceRejectsThroughDanglingSymlinkedDir(t *testing.T) {
+	base, parentPath := newAgentDir(t)
+	agentDir := filepath.Dir(parentPath)
+
+	elsewhere := filepath.Join(base, "elsewhere")
+	if err := os.MkdirAll(elsewhere, 0o755); err != nil {
+		t.Fatalf("MkdirAll(%q) failed: %v", elsewhere, err)
+	}
+	if err := os.Symlink(elsewhere, filepath.Join(agentDir, "dirlink")); err != nil {
+		t.Skipf("symlinks are not supported in this environment: %v", err)
+	}
+
+	// "missing.yaml" does not exist under elsewhere, so the reference as a
+	// whole cannot be resolved by a single EvalSymlinks call.
+	refPath := filepath.Join("dirlink", "missing.yaml")
+	_, err := ResolveAgentReference(context.Background(), parentPath, refPath)
+	if err == nil {
+		t.Fatalf("ResolveAgentReference(_, %q, %q) succeeded, want error containing %q", parentPath, refPath, traversalError)
+	}
+	if !strings.Contains(err.Error(), traversalError) {
+		t.Errorf("ResolveAgentReference(_, %q, %q) = %v, want error containing %q", parentPath, refPath, err, traversalError)
+	}
+}
+
+// TestResolveAgentReferenceCleansParentSegmentBeforeSymlinkResolution covers a
+// reference that walks into a symlinked directory and back out with ".."
+// before the final component. filepath.Join cleans "dirlink/.." away
+// textually before anything looks at the filesystem, so the reference
+// resolves as a plain sibling of the symlink inside the agent directory; the
+// symlink's target is never consulted. This matches adk-python's
+// normpath-then-realpath order and is pinned here so a future refactor does
+// not silently change which file a reference like this loads.
+func TestResolveAgentReferenceCleansParentSegmentBeforeSymlinkResolution(t *testing.T) {
+	base, parentPath := newAgentDir(t)
+	agentDir := filepath.Dir(parentPath)
+
+	elsewhere := filepath.Join(base, "elsewhere")
+	if err := os.MkdirAll(elsewhere, 0o755); err != nil {
+		t.Fatalf("MkdirAll(%q) failed: %v", elsewhere, err)
+	}
+	if err := os.Symlink(elsewhere, filepath.Join(agentDir, "dirlink")); err != nil {
+		t.Skipf("symlinks are not supported in this environment: %v", err)
+	}
+
+	refPath := filepath.Join("dirlink", "..", "sub_agent.yaml")
+	_, err := ResolveAgentReference(context.Background(), parentPath, refPath)
+	if err != nil && strings.Contains(err.Error(), traversalError) {
+		t.Errorf("ResolveAgentReference(_, %q, %q) = %v, want no traversal rejection", parentPath, refPath, err)
+	}
+}
+
+// TestResolveAgentReferenceRejectsEmptyPath covers the empty reference, which
+// must be rejected outright rather than falling through to filepath.Dir's "."
+// and silently resolving to the agent directory itself.
+func TestResolveAgentReferenceRejectsEmptyPath(t *testing.T) {
+	_, parentPath := newAgentDir(t)
+
+	if _, err := ResolveAgentReference(context.Background(), parentPath, ""); err == nil {
+		t.Error("ResolveAgentReference with an empty reference succeeded, want an error")
+	}
+}

internal/configurable/configurable_workflow.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"path/filepath"
 	"strings"
 
 	"gopkg.in/yaml.v3"
@@ -200,14 +199,16 @@ func resolveNodeLike(ctx context.Context, parentPath, ref string) (workflow.Node
 	isYAML := strings.HasSuffix(ref, ".yaml") || strings.HasSuffix(ref, ".yml")
 
 	if isYAML {
-		targetPath := ref
-		if !filepath.IsAbs(ref) {
-			targetPath = filepath.Join(filepath.Dir(parentPath), ref)
-		}
+		// Containment is checked here, before the file is read and before the
+		// node cache is consulted, so a reference escaping the parent config's
+		// directory never reaches os.ReadFile. The FunctionNode, JoinNode and
+		// ToolNode branches of resolveNodeFromYAML all return before its
+		// fall-through call to ResolveAgentReference, so this is their only
+		// containment check.
 		var err error
-		absPath, err = filepath.Abs(targetPath)
+		absPath, err = resolveContainedConfigPath(parentPath, ref)
 		if err != nil {
-			return nil, fmt.Errorf("failed to resolve absolute path: %w", err)
+			return nil, err
 		}
 		cacheKey = absPath
 	} else {