From b59c0f8e015ac79a0da2378fb1cb79bdd9b8491f Mon Sep 17 00:00:00 2001 From: Glenn Harper Date: Thu, 23 Jul 2026 10:34:20 -0400 Subject: [PATCH 1/2] fix(ai-agents): preserve shell script permissions on init Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fed9e97b-e79b-4889-ac76-0d9a428599cd --- .../azure.ai.agents/internal/cmd/init.go | 31 +++++++-- .../azure.ai.agents/internal/cmd/init_test.go | 67 +++++++++++++++++++ 2 files changed, 94 insertions(+), 4 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 0b48d8a78fb..606a4ec2352 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go @@ -3488,8 +3488,7 @@ func downloadDirectoryContents( return fmt.Errorf("failed to download file %s: %w", itemPath, err) } - //nolint:gosec // downloaded project files are intended to be readable by project tooling - if err := os.WriteFile(itemLocalPath, []byte(fileContent), 0644); err != nil { + if err := writeDownloadedFile(itemLocalPath, []byte(fileContent)); err != nil { return fmt.Errorf("failed to write file %s: %w", itemLocalPath, err) } } else if itemType == "dir" { @@ -3600,8 +3599,7 @@ func downloadDirectoryContentsWithoutGhCli( return fmt.Errorf("failed to read file content %s: %w", itemPath, err) } - //nolint:gosec // downloaded project files are intended to be readable by project tooling - if err := os.WriteFile(itemLocalPath, fileContent, 0644); err != nil { + if err := writeDownloadedFile(itemLocalPath, fileContent); err != nil { return fmt.Errorf("failed to write file %s: %w", itemLocalPath, err) } } else if itemType == "dir" { @@ -3622,6 +3620,31 @@ func downloadDirectoryContentsWithoutGhCli( return nil } +func writeDownloadedFile(path string, content []byte) error { + permissions := downloadedFilePermissions(path) + + //nolint:gosec // downloaded project files intentionally use project-friendly permissions + if err := os.WriteFile(path, content, permissions); err != nil { + return err + } + + if permissions == osutil.PermissionExecutableFile { + // os.WriteFile does not update permissions when the file already exists. + //nolint:gosec // G703: path is the GitHub contents item already written under the selected target directory + if err := os.Chmod(path, permissions); err != nil { + return fmt.Errorf("setting executable permissions: %w", err) + } + } + return nil +} + +func downloadedFilePermissions(path string) os.FileMode { + if strings.EqualFold(filepath.Ext(path), ".sh") { + return osutil.PermissionExecutableFile + } + return osutil.PermissionFile +} + // extractToolboxAndConnectionConfigs extracts toolbox resource definitions from the agent manifest // and converts them into project.Toolbox config entries and project.ToolConnection entries. // Tools with a target/authType also produce connection entries for Bicep provisioning. 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 7d5bf3a5bdf..925315a1dc9 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 @@ -11,6 +11,7 @@ import ( "net/http" "os" "path/filepath" + "runtime" "strings" "testing" @@ -21,6 +22,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/osutil" "github.com/spf13/cobra" "github.com/stretchr/testify/require" "google.golang.org/genproto/googleapis/rpc/errdetails" @@ -897,6 +899,71 @@ func TestCopyDirectory_NoOpWhenSamePath(t *testing.T) { } } +func TestWriteDownloadedFilePermissions(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + fileName string + wantPermissions os.FileMode + }{ + { + name: "shell scripts are executable", + fileName: "postprovision.sh", + wantPermissions: osutil.PermissionExecutableFile, + }, + { + name: "shell script extension is case insensitive", + fileName: "predeploy.SH", + wantPermissions: osutil.PermissionExecutableFile, + }, + { + name: "other files remain non-executable", + fileName: "azure.yaml", + wantPermissions: osutil.PermissionFile, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), tt.fileName) + if got := downloadedFilePermissions(path); got != tt.wantPermissions { + t.Fatalf("downloadedFilePermissions() = %04o, want %04o", got, tt.wantPermissions) + } + + if err := os.WriteFile(path, []byte("old"), osutil.PermissionFile); err != nil { + t.Fatal(err) + } + + if err := writeDownloadedFile(path, []byte("new")); err != nil { + t.Fatal(err) + } + + content, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(content) != "new" { + t.Fatalf("content = %q, want %q", content, "new") + } + + if runtime.GOOS == "windows" { + return + } + + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != tt.wantPermissions { + t.Errorf("permissions = %04o, want %04o", got, tt.wantPermissions) + } + }) + } +} + func TestValidateLocalContainerAgentCopy_AllowsReinitInPlace(t *testing.T) { t.Parallel() From bb843c382569d75a6c63d170bbbe65e8b3f3350d Mon Sep 17 00:00:00 2001 From: glenn Date: Thu, 23 Jul 2026 17:32:21 -0400 Subject: [PATCH 2/2] fix(ai-agents): refuse downloaded file overwrites Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../azure.ai.agents/internal/cmd/init.go | 15 +++++------ .../azure.ai.agents/internal/cmd/init_test.go | 26 ++++++++++++++++--- 2 files changed, 29 insertions(+), 12 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 606a4ec2352..54742f40ce4 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go @@ -3624,18 +3624,17 @@ func writeDownloadedFile(path string, content []byte) error { permissions := downloadedFilePermissions(path) //nolint:gosec // downloaded project files intentionally use project-friendly permissions - if err := os.WriteFile(path, content, permissions); err != nil { + file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, permissions) + if err != nil { return err } - if permissions == osutil.PermissionExecutableFile { - // os.WriteFile does not update permissions when the file already exists. - //nolint:gosec // G703: path is the GitHub contents item already written under the selected target directory - if err := os.Chmod(path, permissions); err != nil { - return fmt.Errorf("setting executable permissions: %w", err) - } + if _, err := file.Write(content); err != nil { + _ = file.Close() + return err } - return nil + + return file.Close() } func downloadedFilePermissions(path string) os.FileMode { 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 925315a1dc9..3904b4b40e6 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 @@ -933,10 +933,6 @@ func TestWriteDownloadedFilePermissions(t *testing.T) { t.Fatalf("downloadedFilePermissions() = %04o, want %04o", got, tt.wantPermissions) } - if err := os.WriteFile(path, []byte("old"), osutil.PermissionFile); err != nil { - t.Fatal(err) - } - if err := writeDownloadedFile(path, []byte("new")); err != nil { t.Fatal(err) } @@ -964,6 +960,28 @@ func TestWriteDownloadedFilePermissions(t *testing.T) { } } +func TestWriteDownloadedFileRefusesToOverwrite(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "postprovision.sh") + if err := os.WriteFile(path, []byte("old"), osutil.PermissionFile); err != nil { + t.Fatal(err) + } + + err := writeDownloadedFile(path, []byte("new")) + if !errors.Is(err, fs.ErrExist) { + t.Fatalf("writeDownloadedFile() error = %v, want fs.ErrExist", err) + } + + content, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(content) != "old" { + t.Fatalf("content = %q, want %q", content, "old") + } +} + func TestValidateLocalContainerAgentCopy_AllowsReinitInPlace(t *testing.T) { t.Parallel()