From 3e47a35f4880977ca6b438c49b0a19ff76d7e53e Mon Sep 17 00:00:00 2001 From: Jian Wu Date: Tue, 2 Jun 2026 16:37:32 +0800 Subject: [PATCH 1/2] [azure.ai.agents] Fix init from manifest in CWD: allow copy when target is subpath of manifest dir When running `azd ai agent init` from a directory containing an agent.manifest.yaml, the auto-detect flow creates a project subdirectory via ensureProject. The subsequent copy step then finds that the target is inside the manifest directory and refuses to proceed. Fix: instead of rejecting the copy outright, skip the destination directory during WalkDir traversal to prevent infinite recursion while still copying the agent source files into the new project structure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../azure.ai.agents/internal/cmd/init_copy.go | 50 ++++++++++++++----- .../internal/cmd/init_copy_test.go | 24 ++++++--- .../azure.ai.agents/internal/cmd/init_test.go | 40 +++++++-------- 3 files changed, 74 insertions(+), 40 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy.go index e9e153a7b85..94b2182539e 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy.go @@ -42,20 +42,38 @@ func (a *InitAction) validateLocalContainerAgentCopy(ctx context.Context, manife return nil } - if isSubpath(dstAbs, srcAbs) { - return fmt.Errorf( - "cannot copy agent files: target '%s' is inside the manifest directory '%s'.\n"+ - "Move the manifest to a separate directory containing only the agent files.", - dstAbs, - srcAbs, - ) - } + // When the target is inside the manifest directory (e.g. auto-detected manifest in CWD + // with ensureProject creating a subdirectory), we allow the copy but exclude the target + // directory itself to prevent infinite recursion. The actual exclusion is handled by + // copyDirectory which skips any path under the destination when src contains dst. + // Fall through to the confirmation logic below so users with large directories + // are still prompted before a bulk copy. entries, err := os.ReadDir(srcAbs) if err != nil { return fmt.Errorf("reading manifest directory %s: %w", srcAbs, err) } - entryCount := len(entries) + + // When copying into a subdirectory of src, exclude the target directory's + // first-level ancestor from the count and preview since it will be skipped during copy. + isSubpathCopy := isSubpath(dstAbs, srcAbs) + filteredEntries := entries + if isSubpathCopy { + rel, relErr := filepath.Rel(srcAbs, dstAbs) + if relErr == nil { + firstComponent := strings.SplitN(filepath.ToSlash(rel), "/", 2)[0] + filtered := make([]os.DirEntry, 0, len(entries)) + for _, e := range entries { + if e.IsDir() && e.Name() == firstComponent { + continue + } + filtered = append(filtered, e) + } + filteredEntries = filtered + } + } + + entryCount := len(filteredEntries) if entryCount <= copyConfirmThreshold { return nil } @@ -64,7 +82,7 @@ func (a *InitAction) validateLocalContainerAgentCopy(ctx context.Context, manife return nil } - preview, err := formatDirectoryPreview(entries, previewLimit) + preview, err := formatDirectoryPreview(filteredEntries, previewLimit) if err != nil { return fmt.Errorf("formatting directory preview for %s: %w", srcAbs, err) } @@ -142,15 +160,21 @@ func copyDirectory(src, dst string) error { return nil } - if isSubpath(dstAbs, srcAbs) { - return fmt.Errorf("refusing to copy directory '%s' into its own subtree '%s'", srcAbs, dstAbs) - } + // When the destination is inside the source (e.g. auto-detected manifest in CWD + // with the project subdirectory created under it), we allow the copy but skip + // the destination subtree to prevent infinite recursion. + skipDst := isSubpath(dstAbs, srcAbs) return filepath.WalkDir(srcAbs, func(path string, d fs.DirEntry, err error) error { if err != nil { return err } + // Skip the destination directory itself to prevent recursive copy loops. + if skipDst && d.IsDir() && isSamePath(path, dstAbs) { + return filepath.SkipDir + } + // Calculate the destination path relPath, err := filepath.Rel(srcAbs, path) if err != nil { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy_test.go index 4611b331ace..596f5a09d5e 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy_test.go @@ -6,7 +6,6 @@ package cmd import ( "os" "path/filepath" - "strings" "testing" ) @@ -48,20 +47,33 @@ func TestCopyDirectory(t *testing.T) { } }) - t.Run("subpath_error", func(t *testing.T) { + t.Run("subpath_copies_with_exclusion", func(t *testing.T) { t.Parallel() src := t.TempDir() dst := filepath.Join(src, "child") if err := os.MkdirAll(dst, 0750); err != nil { t.Fatal(err) } + //nolint:gosec // test fixture file permissions are intentional + if err := os.WriteFile(filepath.Join(src, "hello.txt"), []byte("world"), 0644); err != nil { + t.Fatal(err) + } err := copyDirectory(src, dst) - if err == nil { - t.Fatal("expected error when dst is subpath of src") + if err != nil { + t.Fatalf("expected no error when dst is subpath of src (should skip dst): %v", err) + } + // Verify file was copied + content, err := os.ReadFile(filepath.Join(dst, "hello.txt")) //nolint:gosec // test fixture read + if err != nil { + t.Fatalf("expected hello.txt to be copied: %v", err) + } + if string(content) != "world" { + t.Fatalf("expected content 'world', got %q", string(content)) } - if !strings.Contains(err.Error(), "refusing to copy") { - t.Errorf("unexpected error message: %v", err) + // Verify no recursive copy + if _, err := os.Stat(filepath.Join(dst, "child")); !os.IsNotExist(err) { + t.Fatal("expected dst/child to NOT exist (should have been skipped)") } }) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go index e60ac017b9c..6886682759b 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go @@ -440,27 +440,6 @@ func TestHasAiErrorReason(t *testing.T) { } } -func TestCopyDirectory_RefusesToCopyIntoSubtree(t *testing.T) { - t.Parallel() - - root := t.TempDir() - src := filepath.Join(root, "src") - dst := filepath.Join(src, "child") - - //nolint:gosec // test fixture directory permissions are intentional - if err := os.MkdirAll(src, 0755); err != nil { - t.Fatalf("mkdir src: %v", err) - } - //nolint:gosec // test fixture file permissions are intentional - if err := os.WriteFile(filepath.Join(src, "file.txt"), []byte("hello"), 0644); err != nil { - t.Fatalf("write src file: %v", err) - } - - if err := copyDirectory(src, dst); err == nil { - t.Fatalf("expected error when destination is inside source") - } -} - func TestCopyDirectory_NoOpWhenSamePath(t *testing.T) { t.Parallel() @@ -497,6 +476,25 @@ func TestValidateLocalContainerAgentCopy_AllowsReinitInPlace(t *testing.T) { } } +func TestValidateLocalContainerAgentCopy_AllowsSubpath(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + manifestPointer := filepath.Join(dir, "agent.manifest.yaml") + //nolint:gosec // test fixture file permissions are intentional + if err := os.WriteFile(manifestPointer, []byte("name: test"), 0644); err != nil { + t.Fatalf("write manifest: %v", err) + } + + // Target is a subdirectory of the manifest directory (auto-detect in CWD scenario) + targetDir := filepath.Join(dir, "my-agent", "src", "my-agent") + + a := &InitAction{} + if err := a.validateLocalContainerAgentCopy(t.Context(), manifestPointer, targetDir); err != nil { + t.Fatalf("expected no error when target is subpath of manifest dir: %v", err) + } +} + func TestIsSubpath(t *testing.T) { t.Parallel() From 1dcd3141db7e320762f6729d50a7bccc4da0e518 Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 2 Jun 2026 14:44:43 -0400 Subject: [PATCH 2/2] fix: prevent copying directories into their own subtrees and update related tests --- .../azure.ai.agents/internal/cmd/init.go | 36 +++++++++---- .../azure.ai.agents/internal/cmd/init_copy.go | 50 +++++-------------- .../internal/cmd/init_copy_test.go | 24 +++------ .../azure.ai.agents/internal/cmd/init_test.go | 40 ++++++++------- 4 files changed, 67 insertions(+), 83 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go index 03a521f0986..89b2c270dcb 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go @@ -1064,11 +1064,28 @@ from code-deploy ZIP packaging (uses .gitignore syntax).`, if err := absolutizeRelativeManifestPaths(flags); err != nil { return err } - _, statErr := os.Stat(folderName) - newlyCreated := errors.Is(statErr, fs.ErrNotExist) - targetDir = folderName - if newlyCreated && !existingProject { - folderDisplay = filepath.ToSlash(folderName) + + // When the manifest lives in the current directory, the agent + // source code is already here — treat it like --from-code and + // initialize in-place rather than copying files into a new + // subdirectory. The existing isSamePath guard in copyDirectory + // will skip the copy when src and dst resolve to the same path. + manifestInCwd := false + if isLocalFilePath(flags.manifestPointer) { + if cwd, cwdErr := os.Getwd(); cwdErr == nil { + manifestInCwd = isSamePath(filepath.Dir(flags.manifestPointer), cwd) + } + } + + if !manifestInCwd { + _, statErr := os.Stat(folderName) + newlyCreated := errors.Is(statErr, fs.ErrNotExist) + targetDir = folderName + if newlyCreated && !existingProject { + folderDisplay = filepath.ToSlash(folderName) + } + } else if flags.src == "" { + flags.src = "." } } @@ -1969,7 +1986,8 @@ func (a *InitAction) configureModelChoice( return agentManifest, nil } -func (a *InitAction) isLocalFilePath(path string) bool { +// isLocalFilePath reports whether path refers to a local file (not an http/https URL). +func isLocalFilePath(path string) bool { // Check if it starts with http:// or https:// if strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://") { return false @@ -2142,7 +2160,7 @@ func (a *InitAction) downloadAgentYaml( useGhCli := false // Check if manifestPointer is a local file path or a URI - if a.isLocalFilePath(manifestPointer) { + if isLocalFilePath(manifestPointer) { // Guard against directories (defense in depth — the caller should // have caught this already, but check here for safety). if err := checkNotDirectory(manifestPointer); err != nil { @@ -2389,7 +2407,7 @@ func (a *InitAction) downloadAgentYaml( a.serviceNameOverride = serviceName // Safety checks for local container-based agents should happen before prompting for model SKU, etc. - if a.isLocalFilePath(manifestPointer) { + if isLocalFilePath(manifestPointer) { if _, isContainerAgent := agentManifest.Template.(agent_yaml.ContainerAgent); isContainerAgent { if err := a.validateLocalContainerAgentCopy(ctx, manifestPointer, targetDir); err != nil { return nil, "", err @@ -2403,7 +2421,7 @@ func (a *InitAction) downloadAgentYaml( return nil, "", fmt.Errorf("creating target directory %s: %w", targetDir, err) } - if a.isLocalFilePath(manifestPointer) { + if isLocalFilePath(manifestPointer) { // Check if the template is a ContainerAgent _, isHostedContainer := agentManifest.Template.(agent_yaml.ContainerAgent) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy.go index 94b2182539e..e9e153a7b85 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy.go @@ -42,38 +42,20 @@ func (a *InitAction) validateLocalContainerAgentCopy(ctx context.Context, manife return nil } - // When the target is inside the manifest directory (e.g. auto-detected manifest in CWD - // with ensureProject creating a subdirectory), we allow the copy but exclude the target - // directory itself to prevent infinite recursion. The actual exclusion is handled by - // copyDirectory which skips any path under the destination when src contains dst. - // Fall through to the confirmation logic below so users with large directories - // are still prompted before a bulk copy. + if isSubpath(dstAbs, srcAbs) { + return fmt.Errorf( + "cannot copy agent files: target '%s' is inside the manifest directory '%s'.\n"+ + "Move the manifest to a separate directory containing only the agent files.", + dstAbs, + srcAbs, + ) + } entries, err := os.ReadDir(srcAbs) if err != nil { return fmt.Errorf("reading manifest directory %s: %w", srcAbs, err) } - - // When copying into a subdirectory of src, exclude the target directory's - // first-level ancestor from the count and preview since it will be skipped during copy. - isSubpathCopy := isSubpath(dstAbs, srcAbs) - filteredEntries := entries - if isSubpathCopy { - rel, relErr := filepath.Rel(srcAbs, dstAbs) - if relErr == nil { - firstComponent := strings.SplitN(filepath.ToSlash(rel), "/", 2)[0] - filtered := make([]os.DirEntry, 0, len(entries)) - for _, e := range entries { - if e.IsDir() && e.Name() == firstComponent { - continue - } - filtered = append(filtered, e) - } - filteredEntries = filtered - } - } - - entryCount := len(filteredEntries) + entryCount := len(entries) if entryCount <= copyConfirmThreshold { return nil } @@ -82,7 +64,7 @@ func (a *InitAction) validateLocalContainerAgentCopy(ctx context.Context, manife return nil } - preview, err := formatDirectoryPreview(filteredEntries, previewLimit) + preview, err := formatDirectoryPreview(entries, previewLimit) if err != nil { return fmt.Errorf("formatting directory preview for %s: %w", srcAbs, err) } @@ -160,21 +142,15 @@ func copyDirectory(src, dst string) error { return nil } - // When the destination is inside the source (e.g. auto-detected manifest in CWD - // with the project subdirectory created under it), we allow the copy but skip - // the destination subtree to prevent infinite recursion. - skipDst := isSubpath(dstAbs, srcAbs) + if isSubpath(dstAbs, srcAbs) { + return fmt.Errorf("refusing to copy directory '%s' into its own subtree '%s'", srcAbs, dstAbs) + } return filepath.WalkDir(srcAbs, func(path string, d fs.DirEntry, err error) error { if err != nil { return err } - // Skip the destination directory itself to prevent recursive copy loops. - if skipDst && d.IsDir() && isSamePath(path, dstAbs) { - return filepath.SkipDir - } - // Calculate the destination path relPath, err := filepath.Rel(srcAbs, path) if err != nil { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy_test.go index 596f5a09d5e..4611b331ace 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy_test.go @@ -6,6 +6,7 @@ package cmd import ( "os" "path/filepath" + "strings" "testing" ) @@ -47,33 +48,20 @@ func TestCopyDirectory(t *testing.T) { } }) - t.Run("subpath_copies_with_exclusion", func(t *testing.T) { + t.Run("subpath_error", func(t *testing.T) { t.Parallel() src := t.TempDir() dst := filepath.Join(src, "child") if err := os.MkdirAll(dst, 0750); err != nil { t.Fatal(err) } - //nolint:gosec // test fixture file permissions are intentional - if err := os.WriteFile(filepath.Join(src, "hello.txt"), []byte("world"), 0644); err != nil { - t.Fatal(err) - } err := copyDirectory(src, dst) - if err != nil { - t.Fatalf("expected no error when dst is subpath of src (should skip dst): %v", err) - } - // Verify file was copied - content, err := os.ReadFile(filepath.Join(dst, "hello.txt")) //nolint:gosec // test fixture read - if err != nil { - t.Fatalf("expected hello.txt to be copied: %v", err) - } - if string(content) != "world" { - t.Fatalf("expected content 'world', got %q", string(content)) + if err == nil { + t.Fatal("expected error when dst is subpath of src") } - // Verify no recursive copy - if _, err := os.Stat(filepath.Join(dst, "child")); !os.IsNotExist(err) { - t.Fatal("expected dst/child to NOT exist (should have been skipped)") + if !strings.Contains(err.Error(), "refusing to copy") { + t.Errorf("unexpected error message: %v", err) } }) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go index 6886682759b..e60ac017b9c 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go @@ -440,6 +440,27 @@ func TestHasAiErrorReason(t *testing.T) { } } +func TestCopyDirectory_RefusesToCopyIntoSubtree(t *testing.T) { + t.Parallel() + + root := t.TempDir() + src := filepath.Join(root, "src") + dst := filepath.Join(src, "child") + + //nolint:gosec // test fixture directory permissions are intentional + if err := os.MkdirAll(src, 0755); err != nil { + t.Fatalf("mkdir src: %v", err) + } + //nolint:gosec // test fixture file permissions are intentional + if err := os.WriteFile(filepath.Join(src, "file.txt"), []byte("hello"), 0644); err != nil { + t.Fatalf("write src file: %v", err) + } + + if err := copyDirectory(src, dst); err == nil { + t.Fatalf("expected error when destination is inside source") + } +} + func TestCopyDirectory_NoOpWhenSamePath(t *testing.T) { t.Parallel() @@ -476,25 +497,6 @@ func TestValidateLocalContainerAgentCopy_AllowsReinitInPlace(t *testing.T) { } } -func TestValidateLocalContainerAgentCopy_AllowsSubpath(t *testing.T) { - t.Parallel() - - dir := t.TempDir() - manifestPointer := filepath.Join(dir, "agent.manifest.yaml") - //nolint:gosec // test fixture file permissions are intentional - if err := os.WriteFile(manifestPointer, []byte("name: test"), 0644); err != nil { - t.Fatalf("write manifest: %v", err) - } - - // Target is a subdirectory of the manifest directory (auto-detect in CWD scenario) - targetDir := filepath.Join(dir, "my-agent", "src", "my-agent") - - a := &InitAction{} - if err := a.validateLocalContainerAgentCopy(t.Context(), manifestPointer, targetDir); err != nil { - t.Fatalf("expected no error when target is subpath of manifest dir: %v", err) - } -} - func TestIsSubpath(t *testing.T) { t.Parallel()