diff --git a/CosmosDBShell.Tests/Integration/ShellProcessTests.cs b/CosmosDBShell.Tests/Integration/ShellProcessTests.cs index c55bc72f..4f3e65a7 100644 --- a/CosmosDBShell.Tests/Integration/ShellProcessTests.cs +++ b/CosmosDBShell.Tests/Integration/ShellProcessTests.cs @@ -439,6 +439,38 @@ public async Task ClearHistory_WithOutputJson_DoesNotWriteInformationalStdOut() Assert.Empty(result.StdOut.Trim()); } + [Fact] + public async Task ClearHistory_WhenHistoryFileIsLocked_ReturnsFailure() + { + var configDir = Path.Join(Path.GetTempPath(), $"cosmosshell-clear-history-{Guid.NewGuid():N}"); + Directory.CreateDirectory(configDir); + var historyFile = Path.Join(configDir, "cmd_history"); + await File.WriteAllTextAsync(historyFile, "echo retained", TestContext.Current.CancellationToken); + try + { + using var lockedStream = new FileStream( + historyFile, + FileMode.Open, + FileAccess.ReadWrite, + FileShare.None); + + var result = await RunShellAsync( + stdinScript: null, + extraArgs: ["--output", "json", "--clear-history"], + cancellationToken: TestContext.Current.CancellationToken, + configDirectory: configDir); + + Assert.Equal(1, result.ExitCode); + Assert.Empty(result.StdOut.Trim()); + Assert.Contains("\"status\":\"error\"", result.StdErr, StringComparison.OrdinalIgnoreCase); + Assert.Contains("Failed to clear history", result.StdErr); + } + finally + { + Directory.Delete(configDir, recursive: true); + } + } + private static async Task RunShellAsync( string stdinScript, CancellationToken cancellationToken) @@ -450,6 +482,7 @@ private static async Task RunShellAsync( string? stdinScript, IEnumerable? extraArgs, CancellationToken cancellationToken, + string? configDirectory = null, IReadOnlyDictionary? environment = null) { var argsList = extraArgs?.ToList(); @@ -468,7 +501,9 @@ private static async Task RunShellAsync( // Isolate the shell's config directory so process-level tests (for example // --clear-history) never touch the developer's real command history under // %LocalAppData%\CosmosDBShell. - var isolatedConfigDir = Path.Join(Path.GetTempPath(), $"cosmosshell-test-{Guid.NewGuid():N}"); + var ownsConfigDirectory = configDirectory == null; + var isolatedConfigDir = configDirectory + ?? Path.Join(Path.GetTempPath(), $"cosmosshell-test-{Guid.NewGuid():N}"); Directory.CreateDirectory(isolatedConfigDir); try @@ -587,13 +622,16 @@ private static async Task RunShellAsync( } finally { - try - { - Directory.Delete(isolatedConfigDir, recursive: true); - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + if (ownsConfigDirectory) { - // Best-effort cleanup; the OS temp directory is reclaimed eventually. + try + { + Directory.Delete(isolatedConfigDir, recursive: true); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // Best-effort cleanup; the OS temp directory is reclaimed eventually. + } } } } diff --git a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs index 3927b0f1..b4266a9b 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -1,5 +1,8 @@ namespace CosmosShell.Tests.Runtime; +using System.Diagnostics; +using System.Security.AccessControl; +using System.Security.Principal; using Azure.Data.Cosmos.Shell.Core; public class SerializedExecutionTests @@ -67,6 +70,316 @@ public void PrintCommand_PersistsBoundedHistory() } } + [Fact] + public void PrintCommand_MergesHistorySavedByMultipleInterpreters() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + try + { + using var first = new ShellInterpreter(configPath); + using var second = new ShellInterpreter(configPath); + + first.PrintCommand("echo from-first"); + second.PrintCommand("echo from-second"); + + var persisted = File.ReadAllLines(Path.Join(configPath, "cmd_history")); + Assert.Equal(["echo from-first", "echo from-second"], persisted); + + using var restarted = new ShellInterpreter(configPath); + Assert.Equal(["echo from-first", "echo from-second"], restarted.History); + } + finally + { + if (Directory.Exists(configPath)) + { + Directory.Delete(configPath, recursive: true); + } + } + } + + [Fact] + public async Task PrintCommand_MergesHistorySavedByConcurrentProcesses() + { + var childConfigPath = Environment.GetEnvironmentVariable("COSMOSDBSHELL_TEST_HISTORY_CONFIG"); + var childPrefix = Environment.GetEnvironmentVariable("COSMOSDBSHELL_TEST_HISTORY_PREFIX"); + if (!string.IsNullOrEmpty(childConfigPath) && !string.IsNullOrEmpty(childPrefix)) + { + using var shell = new ShellInterpreter(childConfigPath); + File.WriteAllText(Path.Join(childConfigPath, $"{childPrefix}.ready"), string.Empty); + var startFile = Path.Join(childConfigPath, "start"); + while (!File.Exists(startFile)) + { + await Task.Delay(TimeSpan.FromMilliseconds(10), TestContext.Current.CancellationToken); + } + + for (var index = 0; index < 20; index++) + { + shell.PrintCommand($"echo {childPrefix}-{index}"); + } + + return; + } + + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-process-{Guid.NewGuid():N}"); + Directory.CreateDirectory(configPath); + using var first = CreateHistoryWriterProcess(configPath, "first"); + using var second = CreateHistoryWriterProcess(configPath, "second"); + var firstStarted = false; + var secondStarted = false; + try + { + firstStarted = first.Start(); + secondStarted = second.Start(); + Assert.True(firstStarted); + Assert.True(secondStarted); + + var firstOutput = first.StandardOutput.ReadToEndAsync(TestContext.Current.CancellationToken); + var firstError = first.StandardError.ReadToEndAsync(TestContext.Current.CancellationToken); + var secondOutput = second.StandardOutput.ReadToEndAsync(TestContext.Current.CancellationToken); + var secondError = second.StandardError.ReadToEndAsync(TestContext.Current.CancellationToken); + + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + timeout.CancelAfter(TimeSpan.FromSeconds(45)); + await WaitForFileAsync(Path.Join(configPath, "first.ready"), timeout.Token); + await WaitForFileAsync(Path.Join(configPath, "second.ready"), timeout.Token); + File.WriteAllText(Path.Join(configPath, "start"), string.Empty); + + await Task.WhenAll( + first.WaitForExitAsync(timeout.Token), + second.WaitForExitAsync(timeout.Token)); + + var output = await Task.WhenAll(firstOutput, firstError, secondOutput, secondError); + Assert.True(first.ExitCode == 0, $"First process exited with {first.ExitCode}: {output[0]} {output[1]}"); + Assert.True(second.ExitCode == 0, $"Second process exited with {second.ExitCode}: {output[2]} {output[3]}"); + + using var restarted = new ShellInterpreter(configPath); + Assert.Equal(40, restarted.History.Count); + for (var index = 0; index < 20; index++) + { + Assert.Contains($"echo first-{index}", restarted.History); + Assert.Contains($"echo second-{index}", restarted.History); + } + } + finally + { + StopProcess(first, firstStarted); + StopProcess(second, secondStarted); + Directory.Delete(configPath, recursive: true); + } + } + + [Fact] + public async Task Constructor_RetriesWhenHistoryFileIsLocked() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + Directory.CreateDirectory(configPath); + var historyFile = Path.Join(configPath, "cmd_history"); + await File.WriteAllLinesAsync(historyFile, ["echo existing"], TestContext.Current.CancellationToken); + try + { + Task constructor; + using var constructorStarted = new ManualResetEventSlim(); + using (var lockedStream = new FileStream(historyFile, FileMode.Open, FileAccess.ReadWrite, FileShare.None)) + { + constructor = Task.Run( + () => + { + constructorStarted.Set(); + return new ShellInterpreter(configPath); + }, + TestContext.Current.CancellationToken); + Assert.True(constructorStarted.Wait(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken)); + Thread.Sleep(TimeSpan.FromMilliseconds(25)); + Assert.False(constructor.IsCompleted); + } + + using var shell = await constructor; + Assert.Equal(["echo existing"], shell.History); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + + [Fact] + public void Constructor_LoadsReadableHistoryFileWithoutWritePermission() + { + if (OperatingSystem.IsWindows()) + { + Assert.Skip("Unix file modes do not apply on Windows."); + return; + } + + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + Directory.CreateDirectory(configPath); + var historyFile = Path.Join(configPath, "cmd_history"); + try + { + File.WriteAllLines(historyFile, ["echo readable"]); + File.SetUnixFileMode(historyFile, UnixFileMode.UserRead); + + using var shell = new ShellInterpreter(configPath); + Assert.Equal(["echo readable"], shell.History); + } + finally + { + if (File.Exists(historyFile)) + { + File.SetUnixFileMode(historyFile, UnixFileMode.UserRead | UnixFileMode.UserWrite); + } + + Directory.Delete(configPath, recursive: true); + } + } + + [Fact] + public void PrintCommand_DeduplicatesMergedHistoryAcrossInterpreters() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + try + { + using var first = new ShellInterpreter(configPath); + using var second = new ShellInterpreter(configPath); + + first.PrintCommand("echo first-only"); + second.PrintCommand("echo second-only"); + first.PrintCommand("echo shared"); + second.PrintCommand("echo shared"); + + using var restarted = new ShellInterpreter(configPath); + Assert.Equal(["echo first-only", "echo second-only", "echo shared"], restarted.History); + } + finally + { + if (Directory.Exists(configPath)) + { + Directory.Delete(configPath, recursive: true); + } + } + } + + [Fact] + public void PrintCommand_TrimsMergedHistoryToNewestEntries() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + try + { + using var first = new ShellInterpreter(configPath); + using var second = new ShellInterpreter(configPath); + + for (var i = 0; i < 30; i++) + { + first.PrintCommand($"echo first-{i}"); + } + + for (var i = 0; i < 40; i++) + { + second.PrintCommand($"echo second-{i}"); + } + + using var restarted = new ShellInterpreter(configPath); + Assert.Equal(60, restarted.History.Count); + Assert.Equal("echo first-10", restarted.History[0]); + Assert.Equal("echo first-29", restarted.History[19]); + Assert.Equal("echo second-0", restarted.History[20]); + Assert.Equal("echo second-39", restarted.History[^1]); + Assert.DoesNotContain("echo first-0", restarted.History); + } + finally + { + if (Directory.Exists(configPath)) + { + Directory.Delete(configPath, recursive: true); + } + } + } + + [Fact] + public void PrintCommand_DoesNotResurrectSavedHistoryAfterClear() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + try + { + using var shell = new ShellInterpreter(configPath); + var historyFile = Path.Join(configPath, "cmd_history"); + + shell.PrintCommand("echo before-clear"); + shell.ClearHistory(); + Assert.Empty(File.ReadAllLines(historyFile)); + + using (var restartedAfterClear = new ShellInterpreter(configPath)) + { + Assert.Empty(restartedAfterClear.History); + } + + shell.PrintCommand("echo after-clear"); + + using var restartedAfterSave = new ShellInterpreter(configPath); + Assert.Equal(["echo after-clear"], restartedAfterSave.History); + } + finally + { + if (Directory.Exists(configPath)) + { + Directory.Delete(configPath, recursive: true); + } + } + } + + [Fact] + public void ClearHistory_WhenHistoryFileIsLocked_PreservesLoadedHistory() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + try + { + using var shell = new ShellInterpreter(configPath); + var historyFile = Path.Join(configPath, "cmd_history"); + shell.PrintCommand("echo retained"); + + using (var lockedStream = new FileStream(historyFile, FileMode.Open, FileAccess.ReadWrite, FileShare.None)) + { + Assert.Throws(() => shell.ClearHistory()); + } + + Assert.Equal(["echo retained"], shell.History); + Assert.NotEmpty(File.ReadAllLines(historyFile)); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + + [Fact] + public void PrintCommand_PreservesMultilineHistoryWhenMergingInterpreters() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + try + { + using var first = new ShellInterpreter(configPath); + using var second = new ShellInterpreter(configPath); + var multiline = "query\nselect * from c"; + + first.PrintCommand(multiline); + second.PrintCommand("echo after-multiline"); + + var persisted = File.ReadAllLines(Path.Join(configPath, "cmd_history")); + Assert.Equal(2, persisted.Length); + Assert.Equal(multiline, ShellInterpreter.DecodeHistoryLine(persisted[0])); + + using var restarted = new ShellInterpreter(configPath); + Assert.Equal([multiline, "echo after-multiline"], restarted.History); + } + finally + { + if (Directory.Exists(configPath)) + { + Directory.Delete(configPath, recursive: true); + } + } + } + [Fact] public void PrintCommand_RestrictsHistoryFileToOwnerOnUnix() { @@ -108,6 +421,206 @@ public void PrintCommand_RestrictsHistoryFileToOwnerOnUnix() } } + [Fact] + public void WriteHistoryAtomically_FailedWritePreservesDestinationAndRemovesTemporaryFile() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + Directory.CreateDirectory(configPath); + var historyFile = Path.Join(configPath, "cmd_history"); + try + { + File.WriteAllText(historyFile, "echo retained\n"); + var failure = new IOException("Injected history write failure."); + + var exception = Assert.Throws(() => ShellInterpreter.WriteHistoryAtomically(historyFile, stream => + { + using var writer = new StreamWriter(stream, leaveOpen: true); + writer.WriteLine("echo partial"); + writer.Flush(); + throw failure; + })); + + Assert.Same(failure, exception); + Assert.Equal("echo retained\n", File.ReadAllText(historyFile)); + Assert.Equal([historyFile], Directory.GetFiles(configPath)); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + + [Fact] + public void WriteHistoryAtomically_PublishesOnlyAfterWritingCompleteContent() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + Directory.CreateDirectory(configPath); + var historyFile = Path.Join(configPath, "cmd_history"); + try + { + File.WriteAllText(historyFile, "echo retained\n"); + + ShellInterpreter.WriteHistoryAtomically(historyFile, stream => + { + using var writer = new StreamWriter(stream, leaveOpen: true) { NewLine = "\n" }; + writer.WriteLine("echo first"); + writer.Flush(); + Assert.Equal("echo retained\n", File.ReadAllText(historyFile)); + writer.WriteLine("echo second"); + }); + + Assert.Equal("echo first\necho second\n", File.ReadAllText(historyFile)); + Assert.Equal([historyFile], Directory.GetFiles(configPath)); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + + [Fact] + public void WriteHistoryAtomically_PreservesWindowsDestinationDaclAtCreationAndPublication() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Skip("Windows ACLs do not apply on this platform."); + return; + } + + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + Directory.CreateDirectory(configPath); + var historyFile = Path.Join(configPath, "cmd_history"); + try + { + File.WriteAllText(historyFile, "echo retained\n"); + using var identity = WindowsIdentity.GetCurrent(); + var owner = Assert.IsType(identity.User); + var security = new FileSecurity(); + security.SetAccessRuleProtection(isProtected: true, preserveInheritance: false); + security.AddAccessRule(new FileSystemAccessRule(owner, FileSystemRights.FullControl, AccessControlType.Allow)); + security.AddAccessRule(new FileSystemAccessRule( + new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null), FileSystemRights.Read, AccessControlType.Allow)); + new FileInfo(historyFile).SetAccessControl(security); + var expected = new FileInfo(historyFile).GetAccessControl(AccessControlSections.Access) + .GetSecurityDescriptorSddlForm(AccessControlSections.Access); + + ShellInterpreter.WriteHistoryAtomically(historyFile, stream => + { + AssertWindowsHistoryDacl(Assert.Single(Directory.GetFiles(configPath, "cmd_history.*.tmp")), expected); + using var writer = new StreamWriter(stream, leaveOpen: true); + writer.WriteLine("echo replacement"); + }); + + AssertWindowsHistoryDacl(historyFile, expected); + Assert.Equal(["echo replacement"], File.ReadAllLines(historyFile)); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + + [Fact] + public void WriteHistoryAtomically_NewWindowsHistoryHasOwnerOnlyDaclAtCreationAndPublication() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Skip("Windows ACLs do not apply on this platform."); + return; + } + + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + Directory.CreateDirectory(configPath); + var historyFile = Path.Join(configPath, "cmd_history"); + try + { + using var identity = WindowsIdentity.GetCurrent(); + var owner = Assert.IsType(identity.User); + var expected = new FileSecurity(); + expected.SetAccessRuleProtection(isProtected: true, preserveInheritance: false); + expected.AddAccessRule(new FileSystemAccessRule(owner, FileSystemRights.FullControl, AccessControlType.Allow)); + var expectedDacl = expected.GetSecurityDescriptorSddlForm(AccessControlSections.Access); + + ShellInterpreter.WriteHistoryAtomically(historyFile, stream => + { + AssertWindowsHistoryDacl(Assert.Single(Directory.GetFiles(configPath, "cmd_history.*.tmp")), expectedDacl); + using var writer = new StreamWriter(stream, leaveOpen: true); + writer.WriteLine("echo private"); + }); + + AssertWindowsHistoryDacl(historyFile, expectedDacl); + Assert.Equal(["echo private"], File.ReadAllLines(historyFile)); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + + [Fact] + public void PrintCommand_FailedSavesPreserveHistoryAndBoundPendingEntries() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + try + { + using var shell = new ShellInterpreter(configPath); + shell.PrintCommand("echo retained"); + var lockFile = shell.HistoryFile + ".lock"; + File.Delete(lockFile); + Directory.CreateDirectory(lockFile); + + for (var index = 0; index < 70; index++) + { + shell.PrintCommand($"echo pending-{index}"); + } + + Assert.Equal(60, shell.PendingHistoryCount); + Assert.Equal(60, shell.History.Count); + Assert.Equal(["echo retained"], File.ReadAllLines(shell.HistoryFile)); + + Directory.Delete(lockFile); + shell.PrintCommand("echo recovered"); + + Assert.Equal(0, shell.PendingHistoryCount); + using var restarted = new ShellInterpreter(configPath); + Assert.Equal(60, restarted.History.Count); + Assert.Equal("echo pending-11", restarted.History[0]); + Assert.Equal("echo recovered", restarted.History[^1]); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + + [Fact] + public void ClearHistory_UsesSharedLockAndPreservesPendingEntriesOnFailure() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + try + { + using var shell = new ShellInterpreter(configPath); + shell.PrintCommand("echo retained"); + using (var locked = new FileStream(shell.HistoryFile + ".lock", FileMode.Open, FileAccess.ReadWrite, FileShare.None)) + { + shell.PrintCommand("echo pending"); + Assert.Throws(() => shell.ClearHistory()); + Assert.Equal(1, shell.PendingHistoryCount); + Assert.Equal(["echo retained", "echo pending"], shell.History); + Assert.Equal(["echo retained"], File.ReadAllLines(shell.HistoryFile)); + } + + shell.ClearHistory(); + Assert.Equal(0, shell.PendingHistoryCount); + Assert.Empty(shell.History); + Assert.Empty(File.ReadAllLines(shell.HistoryFile)); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + [Fact] public async Task Dispose_ReleasesExecutionGateAndIsIdempotent() { @@ -245,4 +758,68 @@ public async Task RunSerializedAsync_CancelledWaiterDoesNotExecuteOrReleaseAnoth Assert.Equal(3, await shell.RunSerializedAsync(() => Task.FromResult(3), TestContext.Current.CancellationToken)); } + + private static void AssertWindowsHistoryDacl(string path, string expectedDacl) + { + if (!OperatingSystem.IsWindows()) + { + throw new PlatformNotSupportedException("Windows ACLs do not apply on this platform."); + } + + var security = new FileInfo(path).GetAccessControl(AccessControlSections.Access); + Assert.True(security.AreAccessRulesProtected); + var expected = Assert.IsType(new RawSecurityDescriptor(expectedDacl).DiscretionaryAcl); + var actual = Assert.IsType(new RawSecurityDescriptor(security.GetSecurityDescriptorBinaryForm(), 0).DiscretionaryAcl); + var expectedBytes = new byte[expected.BinaryLength]; + var actualBytes = new byte[actual.BinaryLength]; + expected.GetBinaryForm(expectedBytes, 0); + actual.GetBinaryForm(actualBytes, 0); + Assert.Equal(expectedBytes, actualBytes); + } + + private static Process CreateHistoryWriterProcess(string configPath, string prefix) + { + var testAssembly = typeof(SerializedExecutionTests).Assembly.Location; + var startInfo = new ProcessStartInfo + { + FileName = GetDotnetPath(), + WorkingDirectory = AppContext.BaseDirectory, + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true, + }; + startInfo.ArgumentList.Add("test"); + startInfo.ArgumentList.Add(testAssembly); + startInfo.ArgumentList.Add("--filter"); + startInfo.ArgumentList.Add($"FullyQualifiedName={typeof(SerializedExecutionTests).FullName}.PrintCommand_MergesHistorySavedByConcurrentProcesses"); + startInfo.Environment["COSMOSDBSHELL_TEST_HISTORY_CONFIG"] = configPath; + startInfo.Environment["COSMOSDBSHELL_TEST_HISTORY_PREFIX"] = prefix; + return new Process { StartInfo = startInfo }; + } + + private static async Task WaitForFileAsync(string path, CancellationToken cancellationToken) + { + while (!File.Exists(path)) + { + await Task.Delay(TimeSpan.FromMilliseconds(10), cancellationToken); + } + } + + private static string GetDotnetPath() + { + var dotnet = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH"); + return !string.IsNullOrEmpty(dotnet) && File.Exists(dotnet) + ? dotnet + : OperatingSystem.IsWindows() ? "dotnet.exe" : "dotnet"; + } + + private static void StopProcess(Process process, bool started) + { + if (started && !process.HasExited) + { + process.Kill(entireProcessTree: true); + process.WaitForExit(); + } + } } \ No newline at end of file diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs index 3be539c4..638f8140 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs @@ -6,6 +6,8 @@ namespace Azure.Data.Cosmos.Shell.Core; using System.Globalization; using System.Reflection; +using System.Security.AccessControl; +using System.Security.Principal; using System.Text.Json; using System.Text.Json.Serialization; using Azure.Data.Cosmos.Shell.Commands; @@ -38,6 +40,8 @@ public partial class ShellInterpreter : IDisposable private const int MAXHISTORYITEMS = 60; + private const int HistoryFileOpenRetryCount = 10; + private const int OptionalArmDiscoveryTimeoutSeconds = 3; internal const int MaximumCallDepth = 64; @@ -49,6 +53,8 @@ public partial class ShellInterpreter : IDisposable // user command that just happens to start with the prefix string. private const string EncodedHistoryLineMarker = "E:"; + private static readonly TimeSpan HistoryFileOpenRetryDelay = TimeSpan.FromMilliseconds(25); + private static readonly TimeSpan LocalEmulatorOperationTimeout = TimeSpan.FromSeconds(10); private static CancellationTokenSource? currentTokenSource; @@ -63,6 +69,8 @@ public partial class ShellInterpreter : IDisposable private readonly object historyLock = new(); + private readonly List pendingHistoryEntries = []; + private readonly object lineEditorLock = new(); private readonly SemaphoreSlim executionGate = new(1, 1); @@ -118,20 +126,27 @@ internal ShellInterpreter(string? configPath = null) this.HistoryFile = Path.Join(this.cfgPath, "cmd_history"); this.welcomeMarkerFile = Path.Join(this.cfgPath, "welcome_seen"); - if (File.Exists(this.HistoryFile)) + try { - string[] lines; + List entries = []; lock (HistoryFileLock) { - lines = File.ReadAllLines(this.HistoryFile); + if (File.Exists(this.HistoryFile)) + { + using var stream = this.OpenHistoryFileWithExclusiveLock(FileAccess.Read); + entries = ReadHistoryEntries(stream); + } } - foreach (var line in lines) + foreach (var entry in entries) { - var decoded = DecodeHistoryLine(line); - this.RecordHistoryEntry(decoded); + this.RecordHistoryEntry(entry, persist: false); } } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + System.Diagnostics.Debug.WriteLine(ex); + } Console.CancelKeyPress += this.Console_CancelKeyPress; this.editorCancelTokenSource = new CancellationTokenSource(); @@ -289,6 +304,17 @@ internal IReadOnlyList History } } + internal int PendingHistoryCount + { + get + { + lock (this.historyLock) + { + return this.pendingHistoryEntries.Count; + } + } + } + internal string? LastBuffer { get; set; } internal string? OriginalString { get; set; } @@ -2348,44 +2374,225 @@ private void Console_CancelKeyPress(object? sender, ConsoleCancelEventArgs e) } private void SaveHistory() + { + try + { + this.SaveHistoryCore(); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + System.Diagnostics.Debug.WriteLine(ex); + } + } + + internal void ClearHistory() + { + lock (this.historyLock) + { + lock (HistoryFileLock) + { + using var historyFileLock = this.OpenHistoryFileWithExclusiveLock(path: this.HistoryFile + ".lock"); + if (File.Exists(this.HistoryFile)) + { + RestrictHistoryFileToOwner(this.HistoryFile); + using var stream = this.OpenHistoryFileWithExclusiveLock(FileAccess.Read); + } + + WriteHistoryAtomically(this.HistoryFile, static stream => stream.SetLength(0)); + } + + this.history.Clear(); + this.pendingHistoryEntries.Clear(); + } + } + + private void SaveHistoryCore() { lock (this.historyLock) { - if (this.history.Count > MAXHISTORYITEMS) + TrimHistoryEntries(this.history); + TrimHistoryEntries(this.pendingHistoryEntries); + + if (this.pendingHistoryEntries.Count == 0) { - this.history = [.. this.history.Skip(this.history.Count - MAXHISTORYITEMS)]; + return; } // Written under the locks so concurrent interactive and MCP saves, and shells // sharing the history file, cannot interleave. lock (HistoryFileLock) { - var options = new FileStreamOptions { Mode = FileMode.Create, Access = FileAccess.Write, Share = FileShare.Read }; - if (!OperatingSystem.IsWindows()) + // The lock file remains in place when the history file is atomically replaced. + using var historyFileLock = this.OpenHistoryFileWithExclusiveLock(path: this.HistoryFile + ".lock"); + List mergedHistory = []; + if (File.Exists(this.HistoryFile)) { - // History can contain connection secrets; UnixCreateMode covers only new files. - options.UnixCreateMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; - if (File.Exists(this.HistoryFile)) - { - File.SetUnixFileMode(this.HistoryFile, UnixFileMode.UserRead | UnixFileMode.UserWrite); - } + RestrictHistoryFileToOwner(this.HistoryFile); + using var stream = this.OpenHistoryFileWithExclusiveLock(FileAccess.Read); + mergedHistory = ReadHistoryEntries(stream); } - using var writer = new StreamWriter(this.HistoryFile, new System.Text.UTF8Encoding(encoderShouldEmitUTF8Identifier: false), options); - foreach (var line in this.history) + foreach (var entry in this.pendingHistoryEntries) { - writer.WriteLine(EncodeHistoryLine(line)); + mergedHistory.Remove(entry); + mergedHistory.Add(entry); } + + TrimHistoryEntries(mergedHistory); + + WriteHistoryAtomically(this.HistoryFile, stream => + { + using var writer = new StreamWriter( + stream, + new System.Text.UTF8Encoding(encoderShouldEmitUTF8Identifier: false), + bufferSize: 1024, + leaveOpen: true); + foreach (var line in mergedHistory) + { + writer.WriteLine(EncodeHistoryLine(line)); + } + }); + + this.pendingHistoryEntries.Clear(); + } + } + } + + [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1204", Justification = "History helpers are grouped with SaveHistory for cohesion.")] + internal static void WriteHistoryAtomically(string historyFile, Action write) + { + var temporary = historyFile + "." + Guid.NewGuid().ToString("N") + ".tmp"; + var created = false; + try + { + using (var stream = CreateHistoryTemporaryFile(temporary, historyFile)) + { + created = true; + write(stream); + stream.Flush(flushToDisk: true); + } + + File.Move(temporary, historyFile, overwrite: true); + } + finally + { + if (created) + { + File.Delete(temporary); + } + } + } + + [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1204", Justification = "History helpers are grouped with SaveHistory for cohesion.")] + private static FileStream CreateHistoryTemporaryFile(string temporary, string historyFile) + { + if (OperatingSystem.IsWindows()) + { + FileSecurity security; + if (File.Exists(historyFile)) + { + security = new FileInfo(historyFile).GetAccessControl(AccessControlSections.Access); + } + else + { + using var identity = WindowsIdentity.GetCurrent(); + var owner = identity.User ?? throw new UnauthorizedAccessException("The current Windows user SID is unavailable."); + security = new FileSecurity(); + security.AddAccessRule(new FileSystemAccessRule(owner, FileSystemRights.FullControl, AccessControlType.Allow)); + } + + // Apply the DACL at creation, without inheriting broader directory permissions. + security.SetAccessRuleProtection(isProtected: true, preserveInheritance: true); + return new FileInfo(temporary).Create( + FileMode.CreateNew, FileSystemRights.Write, FileShare.None, 4096, FileOptions.None, security); + } + + return new FileStream(temporary, new FileStreamOptions + { + Mode = FileMode.CreateNew, + Access = FileAccess.Write, + Share = FileShare.None, + UnixCreateMode = UnixFileMode.UserRead | UnixFileMode.UserWrite, + }); + } + + private FileStream OpenHistoryFileWithExclusiveLock(FileAccess access = FileAccess.ReadWrite, string? path = null) + { + path ??= this.HistoryFile; + if (access != FileAccess.Read && File.Exists(path)) + { + RestrictHistoryFileToOwner(path); + } + + var options = new FileStreamOptions + { + Mode = access == FileAccess.Read ? FileMode.Open : FileMode.OpenOrCreate, + Access = access, + Share = FileShare.None, + }; + if (access != FileAccess.Read && !OperatingSystem.IsWindows()) + { + options.UnixCreateMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; + } + + for (var attempt = 0; ; attempt++) + { + try + { + return new FileStream(path, options); } + catch (IOException) when (attempt < HistoryFileOpenRetryCount) + { + Thread.Sleep(HistoryFileOpenRetryDelay); + } + } + } + + [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1204", Justification = "History helpers are grouped with SaveHistory for cohesion.")] + private static List ReadHistoryEntries(Stream stream) + { + stream.Position = 0; + using var reader = new StreamReader(stream, detectEncodingFromByteOrderMarks: true, leaveOpen: true); + var entries = new List(); + while (reader.ReadLine() is { } line) + { + var decoded = DecodeHistoryLine(line); + entries.Remove(decoded); + entries.Add(decoded); + } + + return entries; + } + + [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1204", Justification = "History helpers are grouped with SaveHistory for cohesion.")] + private static void RestrictHistoryFileToOwner(string historyFile) + { + if (!OperatingSystem.IsWindows()) + { + File.SetUnixFileMode(historyFile, UnixFileMode.UserRead | UnixFileMode.UserWrite); } } - private void RecordHistoryEntry(string entry) + [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1204", Justification = "History helpers are grouped with SaveHistory for cohesion.")] + private static void TrimHistoryEntries(List entries) + { + if (entries.Count > MAXHISTORYITEMS) + { + entries.RemoveRange(0, entries.Count - MAXHISTORYITEMS); + } + } + + private void RecordHistoryEntry(string entry, bool persist = true) { lock (this.historyLock) { this.history.Remove(entry); this.history.Add(entry); + if (persist) + { + this.pendingHistoryEntries.Remove(entry); + this.pendingHistoryEntries.Add(entry); + } } } diff --git a/CosmosDBShell/Program.cs b/CosmosDBShell/Program.cs index 4c11ce16..faf5fc6b 100644 --- a/CosmosDBShell/Program.cs +++ b/CosmosDBShell/Program.cs @@ -256,9 +256,15 @@ void WriteStartupError(string message) if (o.ClearHistory) { - if (File.Exists(ShellInterpreter.Instance.HistoryFile)) + try + { + ShellInterpreter.Instance.ClearHistory(); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - File.Delete(ShellInterpreter.Instance.HistoryFile); + WriteStartupError(MessageService.GetArgsString("shell-history-clear-error", "message", ex.Message)); + Environment.ExitCode = ShellExitCode.FromException(ex); + return; } if (!startupMachineMode) diff --git a/CosmosDBShell/lang/en.ftl b/CosmosDBShell/lang/en.ftl index aa57b094..7ff573da 100644 --- a/CosmosDBShell/lang/en.ftl +++ b/CosmosDBShell/lang/en.ftl @@ -118,6 +118,7 @@ shell-welcome-examples = Examples shell-welcome-preview = PREVIEW VERSION shell-welcome-preview-warning = Commands, output, and behavior may change before general availability. shell-hisory_file_deleted = History deleted. +shell-history-clear-error = Failed to clear history: { $message } shell-connect-browser-auth = Authenticating via browser. Please complete the login in the browser window that opens. shell-connect-devicecode-auth = Browser authentication failed. Falling back to device code authentication. shell-connect-key-auth = Connecting with account key... diff --git a/README.md b/README.md index a0b9f630..dc14d742 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ A terminal-native shell for Azure Cosmos DB — navigate databases like a filesy Exports replace their destination only after successful completion, preserving an existing file on failure or cancellation. Imports stream records; CSV exports use temporary disk storage to discover columns without retaining all documents in memory. See [import/export](docs/commands.md#export). -MCP command execution is serialized with the shell, and destructive confirmations are invalidated by connection or navigation changes. MCP invocations are echoed in the shell so their activity stays visible, and they are recorded in history alongside interactive commands. History remains fully replayable, including connection strings; treat its file as sensitive. See [MCP security](docs/mcp.md#security) and [history](docs/navigation.md#history). +MCP command execution is serialized with the shell, and destructive confirmations are invalidated by connection or navigation changes. MCP invocations are echoed in the shell so their activity stays visible, and they are recorded in history alongside interactive commands. Concurrent shells merge history under a shared lock and publish complete replacements instead of truncating the saved file. History remains fully replayable, including connection strings; treat its file as sensitive. See [MCP security](docs/mcp.md#security) and [history](docs/navigation.md#history). MCP clients supporting resource subscriptions can watch `cosmos://shell/current-location` for interactive navigation and connection changes; the resource includes the current account endpoint separately from the location. See [MCP location updates](docs/mcp.md#shell-location-updates). diff --git a/docs/navigation.md b/docs/navigation.md index 2af408a0..28075b76 100644 --- a/docs/navigation.md +++ b/docs/navigation.md @@ -234,9 +234,9 @@ There is no separate "enter multi-line mode" command — the shell enters and le ### History -Multi-line commands are saved to history as a single entry. When you recall one with `Up` / `Ctrl+P` or reverse-search (`Ctrl+R`), the full multi-line text is restored. History files written by older versions of the shell continue to load unchanged. +Multi-line commands are saved to history as a single entry. When you recall one with `Up` / `Ctrl+P` or reverse-search (`Ctrl+R`), the full multi-line text is restored. History files written by older versions of the shell continue to load unchanged. Concurrent shells merge their saved entries into the shared history file. Saves and clears coordinate through `cmd_history.lock` and replace `cmd_history` only after the complete replacement has been written and flushed, so a failed write leaves the saved history intact. History saving remains best-effort; at most the newest 60 pending entries are retained for a later retry. A failed `--clear-history` reports an error and leaves the loaded history unchanged. -Commands are stored in full so they can be executed again, including connection strings containing account keys. Treat the `cmd_history` file in the shell configuration directory as sensitive: protect it with your user account's file permissions and do not share it. Use Entra ID to avoid storing account keys, or `--clear-history` to clear the saved history. MCP tool invocations are echoed as command lines and recorded in the same history. +Commands are stored in full so they can be executed again, including connection strings containing account keys. Treat the `cmd_history` file in the shell configuration directory as sensitive: protect it with your user account's file permissions and do not share it. Temporary history files have owner-only permissions on Unix. On Windows, they are created with the existing history file's access rules, or owner-only access for a new history file, without inheriting broader directory permissions; those rules remain on the published file. Use Entra ID to avoid storing account keys, or `--clear-history` to clear the saved history. MCP tool invocations are echoed as command lines and recorded in the same history. ## Keyboard Shortcuts diff --git a/l10n/CosmosDBShell.json b/l10n/CosmosDBShell.json index bb8761b9..2663105a 100644 --- a/l10n/CosmosDBShell.json +++ b/l10n/CosmosDBShell.json @@ -1438,6 +1438,7 @@ "shell-connect-vscode-credential-auth": "Connecting with Visual Studio Code credential...", "shell-connect-vscode-credential-fallback": "Visual Studio Code credential unavailable, falling back...", "shell-hisory_file_deleted": "History deleted.", + "shell-history-clear-error": "Failed to clear history: {0}", "shell-not_connected_hint": "Not connected. Run \u0027connect \u003Cendpoint\u003E\u0027 to authenticate, or \u0027help connect\u0027 for more options.", "shell-ready": "Cosmos DB shell ready.", "shell-startup-mcp-off": "off",