From edad9b1ee9305f9f55da051753eed1c0c163cf5f Mon Sep 17 00:00:00 2001 From: Devansh Goswami Date: Thu, 16 Apr 2026 11:39:23 +0530 Subject: [PATCH 1/3] Improving logging and adding a prompt to regen SSH Keys --- src/Sftp/Sftp/CHANGELOG.md | 9 +++ src/Sftp/Sftp/Common/SftpBaseCmdlet.cs | 68 +++++++++++++++++-- .../Sftp/SftpCommands/ConnectAzSftpCommand.cs | 35 +++++++++- .../NewAzSftpCertificateCommand.cs | 30 ++++++++ 4 files changed, 133 insertions(+), 9 deletions(-) diff --git a/src/Sftp/Sftp/CHANGELOG.md b/src/Sftp/Sftp/CHANGELOG.md index 691b062735e1..fb2b8bce3bcc 100644 --- a/src/Sftp/Sftp/CHANGELOG.md +++ b/src/Sftp/Sftp/CHANGELOG.md @@ -19,6 +19,15 @@ --> ## Upcoming Release +* Added confirmation prompt when an SSH key pair already exists at the target location + - Both 'New-AzSftpCertificate' and 'Connect-AzSftp' now detect existing key pairs before generating new ones + - Users are prompted to choose whether to overwrite existing keys or reuse them + - Added '-Force' parameter to bypass the confirmation prompt +* Improved verbose and debug logging across all cmdlets + - Added timing information for key pair generation, certificate requests, and SFTP session duration + - Enhanced SSH client validation logging with resolved executable paths + - Added structured log prefixes (KeyPair, CertGen, SSH, SFTP, Auth, Cleanup) for easier filtering + - Improved credential cleanup logging with operation-level status messages * Fixed command injection vulnerability in file permission handling [Security] - Replaced 'powershell.exe' and 'icacls.exe' subprocess calls with direct .NET ACL APIs on Windows - Replaced 'chmod' subprocess call with native P/Invoke on Unix diff --git a/src/Sftp/Sftp/Common/SftpBaseCmdlet.cs b/src/Sftp/Sftp/Common/SftpBaseCmdlet.cs index 8e27e0b95e85..6504cb437c74 100644 --- a/src/Sftp/Sftp/Common/SftpBaseCmdlet.cs +++ b/src/Sftp/Sftp/Common/SftpBaseCmdlet.cs @@ -36,17 +36,23 @@ public abstract class SftpBaseCmdlet : AzureRMCmdlet /// protected CancellationToken CmdletCancellationToken { get; private set; } + /// + /// Forces the command to run without asking for user confirmation when overwriting existing SSH key pairs. + /// + [Parameter(Mandatory = false, HelpMessage = "Forces the command to run without asking for user confirmation when overwriting existing SSH key pairs.")] + public SwitchParameter Force { get; set; } + protected override void BeginProcessing() { CmdletCancellationToken = cancellationTokenSource.Token; base.BeginProcessing(); - WriteVerbose("Initializing SFTP cmdlet"); + WriteVerbose($"[{MyInvocation.MyCommand.Name}] Initializing (OS: {RuntimeInformation.OSDescription}, Arch: {RuntimeInformation.OSArchitecture})"); } protected override void EndProcessing() { base.EndProcessing(); - WriteVerbose("SFTP cmdlet execution completed"); + WriteVerbose($"[{MyInvocation.MyCommand.Name}] Execution completed"); } /// @@ -54,7 +60,7 @@ protected override void EndProcessing() /// protected override void StopProcessing() { - WriteVerbose("SFTP cmdlet cancellation requested"); + WriteVerbose($"[{MyInvocation.MyCommand.Name}] Cancellation requested by user"); cancellationTokenSource.Cancel(); base.StopProcessing(); } @@ -71,19 +77,67 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + /// + /// Checks if SSH key pair files exist at the specified paths and prompts the user + /// to confirm whether to overwrite or reuse existing keys. + /// Returns true if new keys should be generated (no existing keys or user confirmed overwrite), + /// false if existing keys should be reused. + /// + /// Path to the private key file + /// Path to the public key file + /// True to generate new keys, false to use existing keys + protected bool ShouldRegenerateKeyPair(string privateKeyFile, string publicKeyFile) + { + bool privateKeyExists = !string.IsNullOrEmpty(privateKeyFile) && File.Exists(privateKeyFile); + bool publicKeyExists = !string.IsNullOrEmpty(publicKeyFile) && File.Exists(publicKeyFile); + + if (!privateKeyExists && !publicKeyExists) + { + return true; // No existing keys, safe to generate + } + + string keysFolder = Path.GetDirectoryName(privateKeyFile ?? publicKeyFile); + string existingFiles = privateKeyExists && publicKeyExists + ? "private key and public key" + : privateKeyExists ? "private key only" : "public key only"; + + WriteVerbose($"[KeyPair] Existing SSH key pair detected in '{keysFolder}' ({existingFiles})"); + + if (Force.IsPresent) + { + WriteVerbose("[KeyPair] -Force specified, will overwrite existing key pair"); + return true; + } + + bool shouldOverwrite = ShouldContinue( + $"An existing SSH key pair was found in '{keysFolder}' ({existingFiles}). " + + "Selecting 'Yes' will generate a new key pair and overwrite the existing files. " + + "Selecting 'No' will use the existing key pair for certificate generation.", + "Existing SSH key pair detected"); + + WriteVerbose(shouldOverwrite + ? "[KeyPair] User confirmed overwrite of existing key pair" + : "[KeyPair] User chose to use existing key pair"); + + return shouldOverwrite; + } + /// /// Validate SSH client availability /// /// Optional folder containing SSH executables protected void ValidateSshClient(string sshClientFolder = null) { - WriteVerbose("Validating SSH client availability"); + WriteVerbose(string.IsNullOrEmpty(sshClientFolder) + ? "[SSH] Validating SSH client availability from system PATH" + : $"[SSH] Validating SSH client availability in folder: '{sshClientFolder}'"); try { - GetSshClientPath("ssh", sshClientFolder); - GetSshClientPath("sftp", sshClientFolder); - GetSshClientPath("ssh-keygen", sshClientFolder); + string sshPath = GetSshClientPath("ssh", sshClientFolder); + string sftpPath = GetSshClientPath("sftp", sshClientFolder); + string keygenPath = GetSshClientPath("ssh-keygen", sshClientFolder); + WriteVerbose($"[SSH] Found ssh: '{sshPath}', sftp: '{sftpPath}', ssh-keygen: '{keygenPath}'"); } catch (Exception ex) { diff --git a/src/Sftp/Sftp/SftpCommands/ConnectAzSftpCommand.cs b/src/Sftp/Sftp/SftpCommands/ConnectAzSftpCommand.cs index ed58a323e837..998ec83636ad 100644 --- a/src/Sftp/Sftp/SftpCommands/ConnectAzSftpCommand.cs +++ b/src/Sftp/Sftp/SftpCommands/ConnectAzSftpCommand.cs @@ -262,10 +262,31 @@ protected override void ProcessRecord() else if (autoGenerateCert) { WriteVerbose("[CertGen] Generating SSH key pair and certificate..."); + + // Check for existing key pair at target location before generation + string targetPrivateKey = !string.IsNullOrEmpty(PrivateKeyFile) ? PrivateKeyFile + : (!string.IsNullOrEmpty(credentialsFolder) ? Path.Combine(credentialsFolder, SftpConstants.SshPrivateKeyName) : null); + string targetPublicKey = !string.IsNullOrEmpty(PublicKeyFile) ? PublicKeyFile + : (targetPrivateKey != null ? targetPrivateKey + ".pub" : null); + + if (targetPrivateKey != null && !ShouldRegenerateKeyPair(targetPrivateKey, targetPublicKey)) + { + WriteVerbose("[Auth] Using existing SSH key pair for certificate generation"); + } + else if (targetPrivateKey != null && (File.Exists(targetPrivateKey) || (targetPublicKey != null && File.Exists(targetPublicKey)))) + { + WriteVerbose("[Auth] Overwriting existing SSH key pair per user confirmation"); + if (File.Exists(targetPrivateKey)) File.Delete(targetPrivateKey); + if (targetPublicKey != null && File.Exists(targetPublicKey)) File.Delete(targetPublicKey); + } + + var keyGenStartTime = DateTime.UtcNow; var (pubKey, privKey, delKeys) = FileUtils.CheckOrCreatePublicPrivateFiles( PublicKeyFile, PrivateKeyFile, credentialsFolder, SshClientFolder); PublicKeyFile = pubKey; PrivateKeyFile = privKey; + var keyGenElapsed = DateTime.UtcNow - keyGenStartTime; + WriteVerbose($"[CertGen] SSH key pair ready in {keyGenElapsed.TotalSeconds:F1} seconds"); WriteDebug($"[CertGen] SSH keys ready - Public: '{PublicKeyFile}', Private: '{PrivateKeyFile}'"); @@ -291,9 +312,13 @@ protected override void ProcessRecord() WriteDebug($"[CertGen] Certificate will be created at: '{certPath}'"); WriteVerbose("[CertGen] Requesting certificate from Microsoft Entra..."); + var certRequestStartTime = DateTime.UtcNow; var (cert, certUsername) = FileUtils.GetAndWriteCertificate(DefaultContext, PublicKeyFile, certPath, SshClientFolder, CmdletCancellationToken); + var certRequestElapsed = DateTime.UtcNow - certRequestStartTime; + WriteVerbose($"[CertGen] Certificate obtained from Microsoft Entra in {certRequestElapsed.TotalSeconds:F1} seconds"); + CertificateFile = cert; user = certUsername; @@ -419,7 +444,9 @@ protected override void ProcessRecord() } // Start SFTP operation - WriteVerbose("[SFTP] Starting SFTP client process..."); + WriteVerbose($"[SFTP] Starting SFTP client process to '{hostname}'..."); + WriteVerbose($"[SFTP] Authentication: user='{sftpSession.Username}', port={sftpSession.Port}, buffer={BufferSizeInBytes} bytes"); + var sftpStartTime = DateTime.UtcNow; var process = DoSftpOp(sftpSession, SftpUtils.StartSftpConnection); // Wait for the SFTP process to complete before cleaning up credentials if (process != null) @@ -438,7 +465,9 @@ protected override void ProcessRecord() } else { + var sftpElapsed = DateTime.UtcNow - sftpStartTime; WriteDebug($"[SFTP] SFTP process exited with code: {process.ExitCode}"); + WriteVerbose($"[SFTP] Session ended after {sftpElapsed.TotalSeconds:F0} seconds (exit code: {process.ExitCode})"); WriteVerbose("[Cleanup] SFTP session ended, cleaning up temporary credentials..."); } CleanupCredentials(deleteKeys, deleteCert, credentialsFolder, CertificateFile, PrivateKeyFile, PublicKeyFile); @@ -476,6 +505,7 @@ private void CleanupCredentials(bool deleteKeys, bool deleteCert, string credent { WriteDebug($"[Cleanup] Deleting temporary certificate: '{certFile}'"); FileUtils.DeleteFile(certFile); + WriteVerbose("[Cleanup] Temporary certificate deleted"); } if (deleteKeys) @@ -500,9 +530,10 @@ private void CleanupCredentials(bool deleteKeys, bool deleteCert, string credent { WriteDebug($"[Cleanup] Removing temporary credentials folder: '{credentialsFolder}'"); Directory.Delete(credentialsFolder, true); + WriteVerbose("[Cleanup] Temporary credentials folder removed"); } - WriteDebug("[Cleanup] Credential cleanup completed"); + WriteVerbose("[Cleanup] Credential cleanup completed successfully"); } catch (IOException ex) { diff --git a/src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs b/src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs index f2c92ce3b3ee..f0ccf998f6a5 100644 --- a/src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs +++ b/src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs @@ -173,6 +173,26 @@ protected override void ProcessRecord() actualPrivateKeyFile = Path.Combine(keysFolder, "id_rsa"); actualPublicKeyFile = Path.Combine(keysFolder, "id_rsa.pub"); WriteDebug($"[CertGen] Key files will be: private='{actualPrivateKeyFile}', public='{actualPublicKeyFile}'"); + + // Check if key pair already exists and prompt user for overwrite confirmation + if (ShouldRegenerateKeyPair(actualPrivateKeyFile, actualPublicKeyFile)) + { + // User wants new keys (or no existing keys found) - remove existing files if present + if (File.Exists(actualPrivateKeyFile)) + { + WriteVerbose($"[CertGen] Removing existing private key: '{actualPrivateKeyFile}'"); + File.Delete(actualPrivateKeyFile); + } + if (File.Exists(actualPublicKeyFile)) + { + WriteVerbose($"[CertGen] Removing existing public key: '{actualPublicKeyFile}'"); + File.Delete(actualPublicKeyFile); + } + } + else + { + WriteVerbose("[CertGen] Using existing SSH key pair for certificate generation"); + } } else if (!string.IsNullOrEmpty(PrivateKeyFile) && string.IsNullOrEmpty(PublicKeyFile)) { @@ -206,9 +226,13 @@ protected override void ProcessRecord() // Check for cancellation before starting CmdletCancellationToken.ThrowIfCancellationRequested(); + WriteVerbose("[CertGen] Preparing SSH key pair..."); + var keyPrepStartTime = DateTime.UtcNow; var (pubKeyFile, _, _) = FileUtils.CheckOrCreatePublicPrivateFiles( actualPublicKeyFile, actualPrivateKeyFile, keysFolder, SshClientFolder); actualPublicKeyFile = pubKeyFile; + var keyPrepElapsed = DateTime.UtcNow - keyPrepStartTime; + WriteVerbose($"[CertGen] SSH key pair ready in {keyPrepElapsed.TotalSeconds:F1} seconds (public key: '{actualPublicKeyFile}')"); // Check for cancellation before authentication CmdletCancellationToken.ThrowIfCancellationRequested(); @@ -217,6 +241,9 @@ protected override void ProcessRecord() string certFile; string username; + WriteVerbose("[CertGen] Requesting SSH certificate from Microsoft Entra..."); + var certRequestStartTime = DateTime.UtcNow; + if (isLocalUser) { // For local user, use a different authentication flow or mock the certificate @@ -235,6 +262,9 @@ protected override void ProcessRecord() username = un; } + var certRequestElapsed = DateTime.UtcNow - certRequestStartTime; + WriteVerbose($"[CertGen] SSH certificate obtained in {certRequestElapsed.TotalSeconds:F1} seconds"); + // Output success message try { From 31950f7b63b638371eff93de3a8a93fb7a11a5e2 Mon Sep 17 00:00:00 2001 From: Devansh Goswami Date: Thu, 16 Apr 2026 12:21:18 +0530 Subject: [PATCH 2/3] Adding tests --- .../KeyPairOverwritePromptTests.cs | 374 ++++++++++++++++++ src/Sftp/Sftp/Common/SftpBaseCmdlet.cs | 4 +- 2 files changed, 376 insertions(+), 2 deletions(-) create mode 100644 src/Sftp/Sftp.Test/ScenarioTests/KeyPairOverwritePromptTests.cs diff --git a/src/Sftp/Sftp.Test/ScenarioTests/KeyPairOverwritePromptTests.cs b/src/Sftp/Sftp.Test/ScenarioTests/KeyPairOverwritePromptTests.cs new file mode 100644 index 000000000000..7352c0055bfe --- /dev/null +++ b/src/Sftp/Sftp.Test/ScenarioTests/KeyPairOverwritePromptTests.cs @@ -0,0 +1,374 @@ +using System; +using System.IO; +using System.Linq; +using System.Management.Automation; +using Microsoft.Azure.PowerShell.Cmdlets.Sftp.Common; +using Microsoft.Azure.PowerShell.Cmdlets.Sftp.SftpCommands; +using Xunit; + +namespace Microsoft.Azure.Commands.Sftp.Test.ScenarioTests +{ + /// + /// Tests for the SSH key pair overwrite prompt behavior. + /// Validates that both New-AzSftpCertificate and Connect-AzSftp + /// have a Force parameter and support ShouldContinue confirmation + /// when existing SSH key pairs are detected. + /// + public class KeyPairOverwritePromptTests + { + #region Force Parameter Existence Tests + + [Fact] + public void TestNewAzSftpCertificateHasForceParameter() + { + // Force parameter is inherited from SftpBaseCmdlet + var command = new NewAzSftpCertificateCommand(); + + var forceProp = command.GetType().GetProperty("Force"); + Assert.NotNull(forceProp); + Assert.Equal(typeof(SwitchParameter), forceProp.PropertyType); + } + + [Fact] + public void TestConnectAzSftpHasForceParameter() + { + var command = new ConnectAzSftpCommand(); + + var forceProp = command.GetType().GetProperty("Force"); + Assert.NotNull(forceProp); + Assert.Equal(typeof(SwitchParameter), forceProp.PropertyType); + } + + [Fact] + public void TestForceParameterIsOptional() + { + var command = new NewAzSftpCertificateCommand(); + + var forceProp = command.GetType().GetProperty("Force"); + var paramAttrs = forceProp?.GetCustomAttributes(typeof(ParameterAttribute), true); + Assert.NotNull(paramAttrs); + Assert.True(paramAttrs.Length > 0); + + var paramAttr = paramAttrs[0] as ParameterAttribute; + Assert.NotNull(paramAttr); + Assert.False(paramAttr.Mandatory); + } + + [Fact] + public void TestForceParameterHasHelpMessage() + { + var command = new ConnectAzSftpCommand(); + + var forceProp = command.GetType().GetProperty("Force"); + var paramAttrs = forceProp?.GetCustomAttributes(typeof(ParameterAttribute), true); + Assert.NotNull(paramAttrs); + Assert.True(paramAttrs.Length > 0); + + var paramAttr = paramAttrs[0] as ParameterAttribute; + Assert.NotNull(paramAttr); + Assert.False(string.IsNullOrEmpty(paramAttr.HelpMessage)); + Assert.Contains("overwrite", paramAttr.HelpMessage.ToLower()); + } + + [Fact] + public void TestForceParameterDefaultsToFalse() + { + var command = new NewAzSftpCertificateCommand(); + + Assert.False(command.Force.IsPresent); + } + + #endregion + + #region ShouldProcess/ShouldContinue Support Tests + + [Fact] + public void TestNewAzSftpCertificateSupportsShouldProcess() + { + // ShouldProcess is needed for ShouldContinue to work + var cmdletAttr = typeof(NewAzSftpCertificateCommand) + .GetCustomAttributes(typeof(CmdletAttribute), false) + .OfType() + .FirstOrDefault(); + + Assert.NotNull(cmdletAttr); + Assert.True(cmdletAttr.SupportsShouldProcess, + "New-AzSftpCertificate must support ShouldProcess for the overwrite prompt to work"); + } + + [Fact] + public void TestConnectAzSftpSupportsShouldProcess() + { + var cmdletAttr = typeof(ConnectAzSftpCommand) + .GetCustomAttributes(typeof(CmdletAttribute), false) + .OfType() + .FirstOrDefault(); + + Assert.NotNull(cmdletAttr); + Assert.True(cmdletAttr.SupportsShouldProcess, + "Connect-AzSftp must support ShouldProcess for the overwrite prompt to work"); + } + + #endregion + + #region Key File Detection Logic Tests + + [Fact] + public void TestDefaultKeyNamesMatchConstants() + { + // Verify that the key file names used for detection match the constants + Assert.Equal("id_rsa", SftpConstants.SshPrivateKeyName); + Assert.Equal("id_rsa.pub", SftpConstants.SshPublicKeyName); + Assert.Equal("-cert.pub", SftpConstants.SshCertificateSuffix); + } + + [Fact] + public void TestNewAzSftpCertificateDefaultKeyPathsUseConstants() + { + // When CertificatePath is specified as a directory, + // the cmdlet should place keys using the constant names + var command = new NewAzSftpCertificateCommand(); + + // Verify the cmdlet uses the id_rsa naming convention + string expectedPrivateKeyName = SftpConstants.SshPrivateKeyName; + string expectedPublicKeyName = SftpConstants.SshPublicKeyName; + + Assert.Equal("id_rsa", expectedPrivateKeyName); + Assert.Equal("id_rsa.pub", expectedPublicKeyName); + } + + [Fact] + public void TestExistingKeyDetection_BothKeysExist() + { + // Test that existing key files can be detected + var tempDir = Path.Combine(Path.GetTempPath(), $"sftp_keypair_test_{Guid.NewGuid():N}"); + try + { + Directory.CreateDirectory(tempDir); + string privateKey = Path.Combine(tempDir, "id_rsa"); + string publicKey = Path.Combine(tempDir, "id_rsa.pub"); + + File.WriteAllText(privateKey, "fake-private-key"); + File.WriteAllText(publicKey, "fake-public-key"); + + Assert.True(File.Exists(privateKey), "Private key should exist for detection"); + Assert.True(File.Exists(publicKey), "Public key should exist for detection"); + } + finally + { + if (Directory.Exists(tempDir)) + Directory.Delete(tempDir, true); + } + } + + [Fact] + public void TestExistingKeyDetection_OnlyPrivateKeyExists() + { + var tempDir = Path.Combine(Path.GetTempPath(), $"sftp_keypair_test_{Guid.NewGuid():N}"); + try + { + Directory.CreateDirectory(tempDir); + string privateKey = Path.Combine(tempDir, "id_rsa"); + string publicKey = Path.Combine(tempDir, "id_rsa.pub"); + + File.WriteAllText(privateKey, "fake-private-key"); + + Assert.True(File.Exists(privateKey), "Private key should exist"); + Assert.False(File.Exists(publicKey), "Public key should not exist"); + } + finally + { + if (Directory.Exists(tempDir)) + Directory.Delete(tempDir, true); + } + } + + [Fact] + public void TestExistingKeyDetection_OnlyPublicKeyExists() + { + var tempDir = Path.Combine(Path.GetTempPath(), $"sftp_keypair_test_{Guid.NewGuid():N}"); + try + { + Directory.CreateDirectory(tempDir); + string privateKey = Path.Combine(tempDir, "id_rsa"); + string publicKey = Path.Combine(tempDir, "id_rsa.pub"); + + File.WriteAllText(publicKey, "fake-public-key"); + + Assert.False(File.Exists(privateKey), "Private key should not exist"); + Assert.True(File.Exists(publicKey), "Public key should exist"); + } + finally + { + if (Directory.Exists(tempDir)) + Directory.Delete(tempDir, true); + } + } + + [Fact] + public void TestExistingKeyDetection_NoKeysExist() + { + var tempDir = Path.Combine(Path.GetTempPath(), $"sftp_keypair_test_{Guid.NewGuid():N}"); + try + { + Directory.CreateDirectory(tempDir); + string privateKey = Path.Combine(tempDir, "id_rsa"); + string publicKey = Path.Combine(tempDir, "id_rsa.pub"); + + Assert.False(File.Exists(privateKey), "Private key should not exist"); + Assert.False(File.Exists(publicKey), "Public key should not exist"); + } + finally + { + if (Directory.Exists(tempDir)) + Directory.Delete(tempDir, true); + } + } + + #endregion + + #region Force Parameter Behavior Tests + + [Fact] + public void TestForceParameterCanBeSetToTrue() + { + var command = new NewAzSftpCertificateCommand(); + + command.Force = new SwitchParameter(true); + Assert.True(command.Force.IsPresent); + } + + [Fact] + public void TestForceParameterCanBeSetToFalse() + { + var command = new NewAzSftpCertificateCommand(); + + command.Force = new SwitchParameter(false); + Assert.False(command.Force.IsPresent); + } + + [Fact] + public void TestForceParameterInheritedByBothCmdlets() + { + // Both cmdlets inherit Force from SftpBaseCmdlet + var newCert = new NewAzSftpCertificateCommand(); + var connect = new ConnectAzSftpCommand(); + + var newCertForceProp = newCert.GetType().GetProperty("Force"); + var connectForceProp = connect.GetType().GetProperty("Force"); + + Assert.NotNull(newCertForceProp); + Assert.NotNull(connectForceProp); + + // Both should be declared on the same base type + Assert.Equal(newCertForceProp.DeclaringType, connectForceProp.DeclaringType); + Assert.Equal(typeof(SftpBaseCmdlet), newCertForceProp.DeclaringType); + } + + #endregion + + #region Overwrite File Integrity Tests + + [Fact] + public void TestOverwriteRemovesBothKeyFiles() + { + // Simulates what the cmdlet does when user confirms overwrite: + // both key files should be deleted before regeneration + var tempDir = Path.Combine(Path.GetTempPath(), $"sftp_overwrite_test_{Guid.NewGuid():N}"); + try + { + Directory.CreateDirectory(tempDir); + string privateKey = Path.Combine(tempDir, "id_rsa"); + string publicKey = Path.Combine(tempDir, "id_rsa.pub"); + + File.WriteAllText(privateKey, "old-private-key"); + File.WriteAllText(publicKey, "old-public-key"); + + // Simulate overwrite behavior + if (File.Exists(privateKey)) File.Delete(privateKey); + if (File.Exists(publicKey)) File.Delete(publicKey); + + Assert.False(File.Exists(privateKey), "Private key should be deleted for overwrite"); + Assert.False(File.Exists(publicKey), "Public key should be deleted for overwrite"); + } + finally + { + if (Directory.Exists(tempDir)) + Directory.Delete(tempDir, true); + } + } + + [Fact] + public void TestReusePreservesExistingKeyFiles() + { + // Simulates what the cmdlet does when user declines overwrite: + // existing key files should be preserved + var tempDir = Path.Combine(Path.GetTempPath(), $"sftp_reuse_test_{Guid.NewGuid():N}"); + try + { + Directory.CreateDirectory(tempDir); + string privateKey = Path.Combine(tempDir, "id_rsa"); + string publicKey = Path.Combine(tempDir, "id_rsa.pub"); + + string originalPrivateContent = "original-private-key-content"; + string originalPublicContent = "original-public-key-content"; + File.WriteAllText(privateKey, originalPrivateContent); + File.WriteAllText(publicKey, originalPublicContent); + + // Simulate reuse behavior (no deletion) + Assert.True(File.Exists(privateKey), "Private key should be preserved"); + Assert.True(File.Exists(publicKey), "Public key should be preserved"); + Assert.Equal(originalPrivateContent, File.ReadAllText(privateKey)); + Assert.Equal(originalPublicContent, File.ReadAllText(publicKey)); + } + finally + { + if (Directory.Exists(tempDir)) + Directory.Delete(tempDir, true); + } + } + + #endregion + + #region Parameter Availability in All Parameter Sets + + [Fact] + public void TestForceAvailableInNewAzSftpCertificateAllParameterSets() + { + // Force is defined once on the base class without ParameterSetName, + // so it should be available in all parameter sets + var forceProp = typeof(NewAzSftpCertificateCommand).GetProperty("Force"); + var paramAttrs = forceProp?.GetCustomAttributes(typeof(ParameterAttribute), true) + .OfType() + .ToList(); + + Assert.NotNull(paramAttrs); + Assert.True(paramAttrs.Count > 0); + + // No specific ParameterSetName means it applies to all sets + var firstAttr = paramAttrs[0]; + Assert.True(string.IsNullOrEmpty(firstAttr.ParameterSetName) || + firstAttr.ParameterSetName == "__AllParameterSets", + "Force should be available in all parameter sets"); + } + + [Fact] + public void TestForceAvailableInConnectAzSftpAllParameterSets() + { + var forceProp = typeof(ConnectAzSftpCommand).GetProperty("Force"); + var paramAttrs = forceProp?.GetCustomAttributes(typeof(ParameterAttribute), true) + .OfType() + .ToList(); + + Assert.NotNull(paramAttrs); + Assert.True(paramAttrs.Count > 0); + + var firstAttr = paramAttrs[0]; + Assert.True(string.IsNullOrEmpty(firstAttr.ParameterSetName) || + firstAttr.ParameterSetName == "__AllParameterSets", + "Force should be available in all parameter sets"); + } + + #endregion + } +} diff --git a/src/Sftp/Sftp/Common/SftpBaseCmdlet.cs b/src/Sftp/Sftp/Common/SftpBaseCmdlet.cs index 6504cb437c74..c9e4582c7203 100644 --- a/src/Sftp/Sftp/Common/SftpBaseCmdlet.cs +++ b/src/Sftp/Sftp/Common/SftpBaseCmdlet.cs @@ -37,9 +37,9 @@ public abstract class SftpBaseCmdlet : AzureRMCmdlet protected CancellationToken CmdletCancellationToken { get; private set; } /// - /// Forces the command to run without asking for user confirmation when overwriting existing SSH key pairs. + /// Forces the command to run without asking for user confirmation to overwrite existing SSH key pairs. /// - [Parameter(Mandatory = false, HelpMessage = "Forces the command to run without asking for user confirmation when overwriting existing SSH key pairs.")] + [Parameter(Mandatory = false, HelpMessage = "Forces the command to run without asking for user confirmation to overwrite existing SSH key pairs.")] public SwitchParameter Force { get; set; } protected override void BeginProcessing() From 5a4e51e77d000a69ca4084bd9a810acc72a54ed2 Mon Sep 17 00:00:00 2001 From: DevanshG1 <108400981+DevanshG1@users.noreply.github.com> Date: Thu, 16 Apr 2026 12:33:43 +0530 Subject: [PATCH 3/3] Update src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs b/src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs index f0ccf998f6a5..43a72a813904 100644 --- a/src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs +++ b/src/Sftp/Sftp/SftpCommands/NewAzSftpCertificateCommand.cs @@ -181,12 +181,12 @@ protected override void ProcessRecord() if (File.Exists(actualPrivateKeyFile)) { WriteVerbose($"[CertGen] Removing existing private key: '{actualPrivateKeyFile}'"); - File.Delete(actualPrivateKeyFile); + FileUtils.DeleteFile(actualPrivateKeyFile); } if (File.Exists(actualPublicKeyFile)) { WriteVerbose($"[CertGen] Removing existing public key: '{actualPublicKeyFile}'"); - File.Delete(actualPublicKeyFile); + FileUtils.DeleteFile(actualPublicKeyFile); } } else