From a401722739eb90720f9470e8654bf48eb8c9e974 Mon Sep 17 00:00:00 2001 From: hemarina Date: Mon, 16 Mar 2026 19:34:04 -0700 Subject: [PATCH 01/14] add install script installation for windows --- cli/azd/pkg/update/errors.go | 2 + cli/azd/pkg/update/manager.go | 101 +++--- cli/azd/pkg/update/msi_unix.go | 50 ++- cli/azd/pkg/update/msi_windows.go | 220 +++++++++++-- cli/azd/pkg/update/msi_windows_test.go | 416 +++++++++++++++++++++++++ 5 files changed, 713 insertions(+), 76 deletions(-) create mode 100644 cli/azd/pkg/update/msi_windows_test.go diff --git a/cli/azd/pkg/update/errors.go b/cli/azd/pkg/update/errors.go index 19a72eda995..88f60bd5222 100644 --- a/cli/azd/pkg/update/errors.go +++ b/cli/azd/pkg/update/errors.go @@ -35,6 +35,8 @@ const ( CodeSignatureInvalid = "update.signatureInvalid" CodeElevationRequired = "update.elevationRequired" CodeUnsupportedInstallMethod = "update.unsupportedInstallMethod" + CodeOtherProcessesRunning = "update.otherProcessesRunning" + CodeNonStandardInstall = "update.nonStandardInstall" ) func newUpdateError(code string, err error) *UpdateError { diff --git a/cli/azd/pkg/update/manager.go b/cli/azd/pkg/update/manager.go index fc566f39e06..dacd25bc878 100644 --- a/cli/azd/pkg/update/manager.go +++ b/cli/azd/pkg/update/manager.go @@ -315,43 +315,69 @@ func (m *Manager) updateViaPackageManager( } func (m *Manager) updateViaMSI(ctx context.Context, cfg *UpdateConfig, writer io.Writer) error { - msiURL, err := m.buildMSIDownloadURL(cfg.Channel) - if err != nil { + // Step 1: Check for other running azd.exe processes (same exe path, different PID). + // The MSI installer cannot replace a locked binary, so all other instances must be closed. + fmt.Fprintf(writer, "Checking for other running azd instances...\n") + if err := checkOtherAzdProcesses(ctx, m.commandRunner); err != nil { return err } - fmt.Fprintf(writer, "Downloading MSI from %s...\n", msiURL) - - tempDir := os.TempDir() - msiPath := filepath.Join(tempDir, "azd-windows-amd64.msi") - - if err := m.downloadFile(ctx, msiURL, msiPath, writer); err != nil { - return newUpdateError(CodeDownloadFailed, err) + // Step 2: Verify the install is the standard per-user MSI configuration. + // install-azd.ps1 installs with ALLUSERS=2 to %LOCALAPPDATA%\Programs\Azure Dev CLI. + // If the current install is non-standard, abort and advise the user. + if err := isStandardMSIInstall(); err != nil { + return err } - // Don't defer os.Remove — the detached msiexec process needs this file after we exit. - // Build msiexec args. Always write a verbose log so failures are diagnosable. - msiLogPath, logErr := msiLogFilePath() - args := []string{"/i", msiPath, "/qn"} - if logErr == nil { - args = append(args, "/l*v", msiLogPath) - log.Printf("MSI install log: %s", msiLogPath) + // Step 3: Backup the current exe by renaming it. + // Windows allows renaming a running executable — the OS handle follows the file. + // This frees the original path so the MSI installer can write the new binary. + fmt.Fprintf(writer, "Backing up current azd executable...\n") + originalPath, backupPath, err := backupCurrentExe() + if err != nil { + return newUpdateError(CodeReplaceFailed, fmt.Errorf("failed to backup current executable: %w", err)) } - log.Printf("Spawning detached msiexec: msiexec %s", strings.Join(args, " ")) - fmt.Fprintf(writer, "Installing update via MSI...\n") + // Ensure the backup is always handled: restored on failure, cleaned up on success. + // Using defer guarantees this runs even if an unexpected error or panic occurs. + updateSucceeded := false + defer func() { + if updateSucceeded { + cleanupOldBackups(originalPath) + return + } + // Update failed — restore the backup so the user still has a working binary. + fmt.Fprintf(writer, "Restoring previous version...\n") + if restoreErr := restoreExeFromBackup(originalPath, backupPath); restoreErr != nil { + fmt.Fprintf(writer, "WARNING: failed to restore previous version: %v\n", restoreErr) + fmt.Fprintf(writer, "Your backup is at: %s\n", backupPath) + fmt.Fprintf(writer, "To recover manually, rename it to: %s\n", originalPath) + } + }() + + // Step 4: Run the install script synchronously and wait for it to complete. + psArgs := buildInstallScriptArgs(cfg.Channel) + + log.Printf("Running install script: powershell %s", strings.Join(psArgs, " ")) + fmt.Fprintf(writer, "Installing azd %s channel...\n", cfg.Channel) - // Spawn msiexec detached so it can replace the running azd binary. - // msiexec cannot overwrite a locked executable; by detaching, azd can exit - // and release the file lock before msiexec attempts the replacement. //nolint:gosec // args are constructed from controlled constants, not user input - cmd := osexec.Command("msiexec", args...) - cmd.SysProcAttr = newDetachedSysProcAttr() - if err := cmd.Start(); err != nil { - return newUpdateError(CodeReplaceFailed, fmt.Errorf("failed to start msiexec: %w", err)) + cmd := osexec.Command("powershell", psArgs...) + cmd.Stdout = writer + cmd.Stderr = writer + + if err := cmd.Run(); err != nil { + return newUpdateError(CodeReplaceFailed, fmt.Errorf("install script failed: %w", err)) } - log.Printf("msiexec started with PID %d, azd will exit to release binary lock", cmd.Process.Pid) + // Step 5: Verify the installer actually produced a new binary at the expected path. + if _, err := os.Stat(originalPath); err != nil { + return newUpdateError(CodeReplaceFailed, + fmt.Errorf("install script completed but %s was not found", originalPath)) + } + + updateSucceeded = true + log.Printf("Update completed successfully") return nil } @@ -430,20 +456,6 @@ func (m *Manager) buildDownloadURL(channel Channel) (string, error) { return fmt.Sprintf("%s/%s/azd-%s-%s%s", blobBaseURL, folder, platform, arch, ext), nil } -func (m *Manager) buildMSIDownloadURL(channel Channel) (string, error) { - var folder string - switch channel { - case ChannelStable: - folder = "stable" - case ChannelDaily: - folder = "daily" - default: - return "", fmt.Errorf("unsupported channel: %s", channel) - } - - return fmt.Sprintf("%s/%s/azd-windows-%s.msi", blobBaseURL, folder, runtime.GOARCH), nil -} - func archiveExtension() string { if runtime.GOOS == "linux" { return ".tar.gz" @@ -590,15 +602,6 @@ func (m *Manager) replaceBinary(ctx context.Context, newBinaryPath, currentBinar return fmt.Errorf("failed to replace binary: %w", err) } -// currentExePath returns the resolved path of the currently running azd binary. -func currentExePath() (string, error) { - exePath, err := os.Executable() - if err != nil { - return "", err - } - return filepath.EvalSymlinks(exePath) -} - func copyFile(src, dst string) error { in, err := os.Open(src) if err != nil { diff --git a/cli/azd/pkg/update/msi_unix.go b/cli/azd/pkg/update/msi_unix.go index f9993bae828..d2a2e9616fa 100644 --- a/cli/azd/pkg/update/msi_unix.go +++ b/cli/azd/pkg/update/msi_unix.go @@ -5,15 +5,51 @@ package update -import "syscall" +import ( + "context" -// newDetachedSysProcAttr is a no-op on non-Windows platforms. -// updateViaMSI is only called on Windows (guarded by runtime.GOOS check in Update). -func newDetachedSysProcAttr() *syscall.SysProcAttr { - return &syscall.SysProcAttr{} + "github.com/azure/azure-dev/cli/azd/pkg/exec" +) + +// checkOtherAzdProcesses is a no-op on non-Windows platforms. +func checkOtherAzdProcesses(_ context.Context, _ exec.CommandRunner) error { + return nil +} + +// isStandardMSIInstall is a no-op on non-Windows platforms. +func isStandardMSIInstall() error { + return nil } -// msiLogFilePath is a no-op on non-Windows platforms. -func msiLogFilePath() (string, error) { +// currentExePath is a no-op stub on non-Windows platforms. +func currentExePath() (string, error) { return "", nil } + +// backupCurrentExe is a no-op stub on non-Windows platforms. +func backupCurrentExe() (string, string, error) { + return "", "", nil +} + +// restoreExeFromBackup is a no-op stub on non-Windows platforms. +func restoreExeFromBackup(_, _ string) error { return nil } + +// cleanupOldBackups is a no-op stub on non-Windows platforms. +func cleanupOldBackups(_ string) {} + +// versionFlag returns the install script parameter value for the given channel. +func versionFlag(channel Channel) string { + switch channel { + case ChannelDaily: + return "daily" + case ChannelStable: + return "stable" + default: + return "stable" + } +} + +// buildInstallScriptArgs is a no-op on non-Windows platforms. +func buildInstallScriptArgs(_ Channel) []string { + return nil +} diff --git a/cli/azd/pkg/update/msi_windows.go b/cli/azd/pkg/update/msi_windows.go index 5972a724421..d2d210fc892 100644 --- a/cli/azd/pkg/update/msi_windows.go +++ b/cli/azd/pkg/update/msi_windows.go @@ -4,38 +4,218 @@ package update import ( + "context" + "fmt" + "log" "os" "path/filepath" - "syscall" + "strconv" + "strings" + "time" - "github.com/azure/azure-dev/cli/azd/pkg/config" + "github.com/azure/azure-dev/cli/azd/pkg/exec" ) -const ( - // Windows process creation flags for detaching msiexec from the parent process. - windowsCreateNewProcessGroup = 0x00000200 - windowsDetachedProcess = 0x00000008 -) +// installScriptURL is the PowerShell install script for azd on Windows. +const installScriptURL = "https://aka.ms/install-azd.ps1" + +// expectedPerUserInstallDir is the default per-user MSI install directory (ALLUSERS=2). +// azd update only supports this standard configuration. +func expectedPerUserInstallDir() string { + localAppData := os.Getenv("LOCALAPPDATA") + if localAppData == "" { + return "" + } + return filepath.Join(localAppData, "Programs", "Azure Dev CLI") +} + +// currentExePath returns the resolved path of the currently running executable. +func currentExePath() (string, error) { + exePath, err := os.Executable() + if err != nil { + return "", fmt.Errorf("failed to determine current executable path: %w", err) + } + resolved, err := filepath.EvalSymlinks(exePath) + if err != nil { + return "", fmt.Errorf("failed to resolve executable path: %w", err) + } + return resolved, nil +} + +// backupCurrentExe renames the currently running azd executable so the MSI installer +// can write a new binary to the original path. Windows allows renaming a file that is +// locked by a running process — the handle follows the renamed file. +// Returns the original path and the backup path. +func backupCurrentExe() (originalPath string, backupPath string, err error) { + originalPath, err = currentExePath() + if err != nil { + return "", "", err + } -// newDetachedSysProcAttr returns SysProcAttr that detaches the child process -// so it survives after the parent (azd) exits. -func newDetachedSysProcAttr() *syscall.SysProcAttr { - return &syscall.SysProcAttr{ - CreationFlags: windowsCreateNewProcessGroup | windowsDetachedProcess, + timestamp := time.Now().Unix() + backupPath = fmt.Sprintf("%s.old.%d", originalPath, timestamp) + + if err := os.Rename(originalPath, backupPath); err != nil { + return "", "", fmt.Errorf("failed to rename executable for backup: %w", err) } + + log.Printf("Backed up %s -> %s", originalPath, backupPath) + return originalPath, backupPath, nil +} + +// restoreExeFromBackup moves the backup back to the original location. +// This is called when the install script fails so the user is not left without a working binary. +// Returns an error if the restore fails, so the caller can advise the user on manual recovery. +func restoreExeFromBackup(originalPath, backupPath string) error { + // Remove any partially-installed new binary that might be in the way. + _ = os.Remove(originalPath) + + if err := os.Rename(backupPath, originalPath); err != nil { + log.Printf("WARNING: failed to restore executable from backup %s -> %s: %v", backupPath, originalPath, err) + return fmt.Errorf("failed to restore executable from backup %s -> %s: %w", backupPath, originalPath, err) + } + + log.Printf("Restored executable from backup: %s -> %s", backupPath, originalPath) + return nil } -// msiLogFilePath returns the path for the MSI verbose install log (~/.azd/logs/msi-update.log). -func msiLogFilePath() (string, error) { - configDir, err := config.GetUserConfigDir() +// cleanupOldBackups removes any leftover azd.exe.old.* files from the install directory. +func cleanupOldBackups(exePath string) { + dir := filepath.Dir(exePath) + base := filepath.Base(exePath) + pattern := filepath.Join(dir, base+".old.*") + + matches, err := filepath.Glob(pattern) if err != nil { - return "", err + return + } + + for _, m := range matches { + if err := os.Remove(m); err != nil { + log.Printf("warning: failed to remove old backup %s: %v", m, err) + } else { + log.Printf("Cleaned up old backup: %s", m) + } } +} + +// checkOtherAzdProcesses checks whether other azd.exe processes are running from the same +// executable path, excluding the current process. Returns an error if other instances are found. +func checkOtherAzdProcesses(ctx context.Context, commandRunner exec.CommandRunner) error { + currentPID := os.Getpid() - logsDir := filepath.Join(configDir, "logs") - if err := os.MkdirAll(logsDir, 0o755); err != nil { - return "", err + exePath, err := currentExePath() + if err != nil { + return err } - return filepath.Join(logsDir, "msi-update.log"), nil + // Use PowerShell to find all azd.exe processes and their executable paths. + // Filter to processes matching our exe path but not our PID. + script := fmt.Sprintf( + `Get-Process -Name azd -ErrorAction SilentlyContinue | `+ + `Where-Object { $_.Id -ne %d } | `+ + `Where-Object { try { $_.Path -eq '%s' } catch { $false } } | `+ + `Select-Object -ExpandProperty Id`, + currentPID, exePath, + ) + + runArgs := exec.NewRunArgs("powershell", "-NoProfile", "-Command", script) + result, err := commandRunner.Run(ctx, runArgs) + if err != nil { + // If the command itself fails, log and allow update to proceed. + // This avoids blocking updates when process enumeration is restricted. + log.Printf("warning: failed to check for other azd processes: %v", err) + return nil + } + + output := strings.TrimSpace(result.Stdout) + if output == "" { + return nil + } + + // Parse the PIDs from the output to build a helpful message + var pids []int + for _, line := range strings.Split(output, "\n") { + line = strings.TrimSpace(line) + if pid, parseErr := strconv.Atoi(line); parseErr == nil { + pids = append(pids, pid) + } + } + + if len(pids) == 0 { + return nil + } + + return newUpdateError(CodeOtherProcessesRunning, fmt.Errorf( + "found %d other azd process(es) running (PIDs: %v).\n"+ + "Please close all other azd instances before running azd update", + len(pids), pids, + )) +} + +// isStandardMSIInstall checks whether the current azd binary is installed in the standard +// per-user MSI location (%LOCALAPPDATA%\Programs\Azure Dev CLI). Returns an error if the +// install is non-standard, advising the user to reinstall with the default per-user configuration. +func isStandardMSIInstall() error { + expectedDir := expectedPerUserInstallDir() + if expectedDir == "" { + return newUpdateError(CodeNonStandardInstall, fmt.Errorf( + "LOCALAPPDATA environment variable is not set; cannot verify install location")) + } + + exePath, err := currentExePath() + if err != nil { + return err + } + + actualDir := filepath.Dir(exePath) + + // Normalize both paths for comparison (case-insensitive on Windows, clean slashes) + if !strings.EqualFold(filepath.Clean(actualDir), filepath.Clean(expectedDir)) { + return newUpdateError(CodeNonStandardInstall, fmt.Errorf( + "azd is installed in a non-standard location: %s\n"+ + "azd update only supports the default per-user install.\n"+ + "Please reinstall azd with the default configuration:\n"+ + " ALLUSERS=2 INSTALLDIR=\"%s\"\n"+ + "See https://github.com/Azure/azure-dev/blob/main/cli/installer/README.md#msi-configuration", + actualDir, expectedDir, + )) + } + + return nil +} + +// versionFlag returns the install script parameter value for the given channel. +func versionFlag(channel Channel) string { + switch channel { + case ChannelDaily: + return "daily" + case ChannelStable: + return "stable" + default: + return "stable" + } +} + +// buildInstallScriptArgs constructs the PowerShell arguments to download and run +// install-azd.ps1 with the appropriate -Version flag. +// The -SkipVerify flag is passed because Authenticode verification via +// Get-AuthenticodeSignature can fail in environments where the +// Microsoft.PowerShell.Security module cannot be auto-loaded (a known issue on +// some Windows PowerShell 5.1 configurations). The MSI is already downloaded +// over HTTPS from a Microsoft-controlled domain, so the transport-level +// integrity is sufficient. +// Returns the arguments to pass to the "powershell" command. +func buildInstallScriptArgs(channel Channel) []string { + version := versionFlag(channel) + // Download the script to a temp file, then invoke it with the appropriate -Version flag. + // Using -ExecutionPolicy Bypass ensures the script runs even if the system policy is restrictive. + script := fmt.Sprintf( + `$script = Join-Path $env:TEMP 'install-azd.ps1'; `+ + `Invoke-RestMethod '%s' -OutFile $script; `+ + `& $script -Version '%s' -SkipVerify; `+ + `Remove-Item $script -Force -ErrorAction SilentlyContinue`, + installScriptURL, version, + ) + return []string{"-NoProfile", "-ExecutionPolicy", "Bypass", "-Command", script} } diff --git a/cli/azd/pkg/update/msi_windows_test.go b/cli/azd/pkg/update/msi_windows_test.go new file mode 100644 index 00000000000..fbe232c9d4c --- /dev/null +++ b/cli/azd/pkg/update/msi_windows_test.go @@ -0,0 +1,416 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package update + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/azure/azure-dev/cli/azd/pkg/exec" + "github.com/azure/azure-dev/cli/azd/test/mocks/mockexec" + "github.com/stretchr/testify/require" +) + +func TestExpectedPerUserInstallDir(t *testing.T) { + tests := []struct { + name string + localAppData string + want string + }{ + { + name: "standard", + localAppData: `C:\Users\testuser\AppData\Local`, + want: `C:\Users\testuser\AppData\Local\Programs\Azure Dev CLI`, + }, + { + name: "empty LOCALAPPDATA", + localAppData: "", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("LOCALAPPDATA", tt.localAppData) + got := expectedPerUserInstallDir() + require.Equal(t, tt.want, got) + }) + } +} + +func TestVersionFlag(t *testing.T) { + tests := []struct { + name string + channel Channel + want string + }{ + {"stable channel", ChannelStable, "stable"}, + {"daily channel", ChannelDaily, "daily"}, + {"unknown defaults to stable", Channel("nightly"), "stable"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := versionFlag(tt.channel) + require.Equal(t, tt.want, got) + }) + } +} + +func TestBuildInstallScriptArgs(t *testing.T) { + tests := []struct { + name string + channel Channel + // We check that certain substrings appear in the constructed args + wantContains []string + }{ + { + name: "stable", + channel: ChannelStable, + wantContains: []string{ + "-NoProfile", + "-ExecutionPolicy", "Bypass", + "-Command", + installScriptURL, + "-Version 'stable'", + "-SkipVerify", + }, + }, + { + name: "daily", + channel: ChannelDaily, + wantContains: []string{ + "-NoProfile", + "-ExecutionPolicy", "Bypass", + "-Command", + installScriptURL, + "-Version 'daily'", + "-SkipVerify", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := buildInstallScriptArgs(tt.channel) + require.NotNil(t, args) + require.True(t, len(args) > 0, "expected non-empty args slice") + + // Join all args to make substring searches easier + joined := strings.Join(args, " ") + for _, s := range tt.wantContains { + require.Contains(t, joined, s, "expected args to contain %q", s) + } + }) + } +} + +func TestBuildInstallScriptArgs_Structure(t *testing.T) { + args := buildInstallScriptArgs(ChannelStable) + + // The args should be: ["-NoProfile", "-ExecutionPolicy", "Bypass", "-Command",