diff --git a/cli/azd/.vscode/cspell-azd-dictionary.txt b/cli/azd/.vscode/cspell-azd-dictionary.txt index 307c698fcaa..7eb6124caec 100644 --- a/cli/azd/.vscode/cspell-azd-dictionary.txt +++ b/cli/azd/.vscode/cspell-azd-dictionary.txt @@ -1,6 +1,7 @@ AADSTS ABRT ACCESSTOKEN +ALLUSERS AZCLI AZURECLI AZURESUBSCRIPTION @@ -176,6 +177,7 @@ ldflags lechnerc77 libc llms +INSTALLDIR localtools maml mcptools diff --git a/cli/azd/cmd/update.go b/cli/azd/cmd/update.go index a45090ca75b..f6d2c65766b 100644 --- a/cli/azd/cmd/update.go +++ b/cli/azd/cmd/update.go @@ -305,7 +305,7 @@ func (a *updateAction) Run(ctx context.Context) (*actions.ActionResult, error) { return &actions.ActionResult{ Message: &actions.ResultMessage{ Header: fmt.Sprintf( - "Successfully updated azd to version %s. Changes take effect on next invocation.", + "Updated azd to version %s. Changes take effect on next invocation.", versionInfo.Version, ), }, diff --git a/cli/azd/pkg/update/errors.go b/cli/azd/pkg/update/errors.go index 19a72eda995..8a17ae21b51 100644 --- a/cli/azd/pkg/update/errors.go +++ b/cli/azd/pkg/update/errors.go @@ -35,6 +35,7 @@ const ( CodeSignatureInvalid = "update.signatureInvalid" CodeElevationRequired = "update.elevationRequired" CodeUnsupportedInstallMethod = "update.unsupportedInstallMethod" + 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..1d5f0d3e4b3 100644 --- a/cli/azd/pkg/update/manager.go +++ b/cli/azd/pkg/update/manager.go @@ -315,43 +315,81 @@ 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 { + // 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 } - fmt.Fprintf(writer, "Downloading MSI from %s...\n", msiURL) + // 1. Rename the running exe to temp (frees the path; process continues via the OS handle) + // 2. Copy it back as an unlocked safety net (if killed at any point, azd.exe still exists) + // 3. The MSI will overwrite the unlocked safety copy with the new version + 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)) + } - tempDir := os.TempDir() - msiPath := filepath.Join(tempDir, "azd-windows-amd64.msi") + // Track whether the install succeeded so we know whether to restore or clean up. + updateSucceeded := false + defer func() { + if updateSucceeded { + // Remove the temp backup directory. If this fails, the OS + // will clean it up eventually since it lives under %TEMP%. + _ = os.RemoveAll(filepath.Dir(backupPath)) + return + } + // Update failed — restore the backup so the user has the original 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, copy it to: %s\n", originalPath) + } + }() - if err := m.downloadFile(ctx, msiURL, msiPath, writer); err != nil { - return newUpdateError(CodeDownloadFailed, err) + // Run the install script synchronously. The MSI overwrites the unlocked + // safety copy at the original path with the new version. + psArgs := buildInstallScriptArgs(cfg.Channel) + + // Snapshot the safety copy's mod time before the install so we can detect + // whether the MSI actually replaced the file. A plain os.Stat after install + // would always succeed because the safety copy already exists at originalPath. + preInfo, statErr := os.Stat(originalPath) + if statErr != nil { + return newUpdateError(CodeReplaceFailed, + fmt.Errorf("failed to stat safety copy before install: %w", statErr)) } - // 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) + log.Printf("Running install script: powershell %s", strings.Join(psArgs, " ")) + fmt.Fprintf(writer, "Installing azd %s channel...\n", cfg.Channel) + + runArgs := exec.NewRunArgs("powershell", psArgs...). + WithStdOut(writer). + WithStdErr(writer) + + if _, err := m.commandRunner.Run(ctx, runArgs); err != nil { + return newUpdateError(CodeReplaceFailed, fmt.Errorf("install script failed: %w", err)) } - log.Printf("Spawning detached msiexec: msiexec %s", strings.Join(args, " ")) - fmt.Fprintf(writer, "Installing update via MSI...\n") + // Verify the MSI actually replaced the binary by comparing mod time and + // size against the pre-install safety copy. If both are identical the MSI + // did not write a new file (silent failure). + postInfo, statErr := os.Stat(originalPath) + if statErr != nil { + return newUpdateError(CodeReplaceFailed, + fmt.Errorf("install script completed but %s was not found", originalPath)) + } - // 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)) + if postInfo.ModTime().Equal(preInfo.ModTime()) && postInfo.Size() == preInfo.Size() { + return newUpdateError(CodeReplaceFailed, + fmt.Errorf("install script completed but the binary at %s was not updated "+ + "(file unchanged); the MSI may have failed silently", originalPath)) } - log.Printf("msiexec started with PID %d, azd will exit to release binary lock", cmd.Process.Pid) + updateSucceeded = true + log.Printf("Update completed successfully") return nil } @@ -430,20 +468,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,13 +614,17 @@ 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. +// currentExePath returns the resolved path of the currently running executable. func currentExePath() (string, error) { exePath, err := os.Executable() if err != nil { - return "", err + 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 filepath.EvalSymlinks(exePath) + return resolved, nil } func copyFile(src, dst string) error { diff --git a/cli/azd/pkg/update/msi_unix.go b/cli/azd/pkg/update/msi_unix.go index f9993bae828..7caca921251 100644 --- a/cli/azd/pkg/update/msi_unix.go +++ b/cli/azd/pkg/update/msi_unix.go @@ -5,15 +5,20 @@ package update -import "syscall" +// isStandardMSIInstall is a no-op on non-Windows platforms. +func isStandardMSIInstall() error { + return nil +} -// 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{} +// backupCurrentExe is a no-op stub on non-Windows platforms. +func backupCurrentExe() (string, string, error) { + return "", "", nil } -// msiLogFilePath is a no-op on non-Windows platforms. -func msiLogFilePath() (string, error) { - return "", nil +// restoreExeFromBackup is a no-op stub on non-Windows platforms. +func restoreExeFromBackup(_, _ string) error { return nil } + +// 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..4b3ff1425e8 100644 --- a/cli/azd/pkg/update/msi_windows.go +++ b/cli/azd/pkg/update/msi_windows.go @@ -4,38 +4,181 @@ package update import ( + "fmt" + "io" + "log" "os" "path/filepath" - "syscall" - - "github.com/azure/azure-dev/cli/azd/pkg/config" + "strings" ) -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") +} + +// backupCurrentExe prepares the install directory for the MSI to write a new binary. +// +// On Windows a running executable is locked — it cannot be overwritten or deleted. +// However, it CAN be renamed/moved. After a rename the OS handle follows the file, +// so the running process continues from the new path without issues. +// +// Strategy ("rename + safety copy"): +// 1. Rename azd.exe → %TEMP%/azd-update-backup-XXXX/azd.exe +// This frees the original path AND keeps the running process alive. +// 2. Copy the backup back to the original path (azd.exe). +// This is an unlocked copy that acts as a safety net: if the process is +// killed at any point after this (Ctrl+C, power loss, ect), the user +// still has a working azd.exe. +// 3. The MSI installer later overwrites the unlocked safety copy with the new version. +// +// Returns the original path and the backup path (in the temp directory). +func backupCurrentExe() (originalPath string, backupPath string, err error) { + originalPath, err = currentExePath() + if err != nil { + return "", "", err + } + + // Create a dedicated temp directory for the backup. + tmpDir, err := os.MkdirTemp("", "azd-update-backup") + if err != nil { + return "", "", fmt.Errorf("failed to create temp directory for backup: %w", err) + } + + backupPath = filepath.Join(tmpDir, filepath.Base(originalPath)) -// 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, + // Step 1: Rename the running exe out of the way. + // The OS handle follows the renamed file — the running process is unaffected. + if err := os.Rename(originalPath, backupPath); err != nil { + _ = os.Remove(tmpDir) + return "", "", fmt.Errorf("failed to rename executable for backup: %w", err) } + + // Step 2: Copy the backup back as an unlocked safety copy. + // If the process is killed before the MSI finishes, this file ensures the + // user still has a working azd.exe at the original path. + if err := copyFileWindows(backupPath, originalPath); err != nil { + // Copy failed — restore the rename so we don't leave a broken state. + _ = os.Rename(backupPath, originalPath) + _ = os.Remove(tmpDir) + return "", "", fmt.Errorf("failed to create safety copy of executable: %w", err) + } + + log.Printf("Backed up %s -> %s", originalPath, backupPath) + return originalPath, backupPath, nil } -// msiLogFilePath returns the path for the MSI verbose install log (~/.azd/logs/msi-update.log). -func msiLogFilePath() (string, error) { - configDir, err := config.GetUserConfigDir() +// copyFileWindows copies src to dst. +func copyFileWindows(src, dst string) error { + in, err := os.Open(src) if err != nil { - return "", err + return err } + defer in.Close() - logsDir := filepath.Join(configDir, "logs") - if err := os.MkdirAll(logsDir, 0o755); err != nil { - return "", err + out, err := os.Create(dst) + if err != nil { + return err + } + + if _, err := io.Copy(out, in); err != nil { + out.Close() + return err } - return filepath.Join(logsDir, "msi-update.log"), nil + return out.Close() +} + +// restoreExeFromBackup overwrites the original path with the backup copy. +// This is called when the install script fails so the user has the same +// binary they started with (rather than a partially-installed one). +// Returns an error if the restore fails, so the caller can advise the user on manual recovery. +func restoreExeFromBackup(originalPath, backupPath string) error { + // The safety copy at originalPath may be the old version or a partially-installed + // new binary. Overwrite it with the known-good backup. + _ = os.Remove(originalPath) + + if err := copyFileWindows(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) + } + + // Clean up the backup directory. + _ = os.RemoveAll(filepath.Dir(backupPath)) + + log.Printf("Restored executable from backup: %s -> %s", backupPath, originalPath) + return nil +} + +// 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 failed. +// 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..9516908e503 --- /dev/null +++ b/cli/azd/pkg/update/msi_windows_test.go @@ -0,0 +1,245 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package update + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "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",