From 44e023052d68fd9bef9ed578eaa43dbe5bd72b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Tue, 29 Sep 2026 14:57:36 +0200 Subject: [PATCH 1/9] Merge shell history saves Merge pending per-process history entries with the on-disk file under an exclusive file lock so concurrent shell instances do not overwrite each other. Keep clear-history best-effort and add focused coverage for merge, de-duplication, trimming, clear, and multi-line entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Runtime/SerializedExecutionTests.cs | 150 ++++++++++++++++++ .../ShellInterpreter.cs | 145 +++++++++++++++-- CosmosDBShell/Program.cs | 5 +- docs/navigation.md | 2 +- 4 files changed, 282 insertions(+), 20 deletions(-) diff --git a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs index 3927b0f1..9563b1ce 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -67,6 +67,156 @@ 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 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 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() { diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs index 50e56321..370fddd0 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs @@ -38,6 +38,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 +51,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 +67,8 @@ public partial class ShellInterpreter : IDisposable private readonly object historyLock = new(); + private readonly List pendingHistoryEntries = []; + private readonly SemaphoreSlim executionGate = new(1, 1); private long stateVersion; @@ -125,7 +131,7 @@ internal ShellInterpreter(string? configPath = null) foreach (var line in lines) { var decoded = DecodeHistoryLine(line); - this.RecordHistoryEntry(decoded); + this.RecordHistoryEntry(decoded, persist: false); } } @@ -2306,44 +2312,153 @@ 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() + { + try + { + lock (this.historyLock) + { + this.history.Clear(); + this.pendingHistoryEntries.Clear(); + + lock (HistoryFileLock) + { + using var stream = this.OpenHistoryFileWithExclusiveLock(); + stream.SetLength(0); + RestrictHistoryFileToOwner(this.HistoryFile); + } + } + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + System.Diagnostics.Debug.WriteLine(ex); + } + } + + private void SaveHistoryCore() { lock (this.historyLock) { - if (this.history.Count > MAXHISTORYITEMS) + TrimHistoryEntries(this.history); + + 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()) + using var stream = this.OpenHistoryFileWithExclusiveLock(); + RestrictHistoryFileToOwner(this.HistoryFile); + + var mergedHistory = ReadHistoryEntries(stream); + foreach (var entry in this.pendingHistoryEntries) { - // 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); - } + mergedHistory.Remove(entry); + mergedHistory.Add(entry); } - using var writer = new StreamWriter(this.HistoryFile, new System.Text.UTF8Encoding(encoderShouldEmitUTF8Identifier: false), options); - foreach (var line in this.history) + TrimHistoryEntries(mergedHistory); + + stream.SetLength(0); + stream.Position = 0; + + using (var writer = new StreamWriter( + stream, + new System.Text.UTF8Encoding(encoderShouldEmitUTF8Identifier: false), + bufferSize: 1024, + leaveOpen: true)) { - writer.WriteLine(EncodeHistoryLine(line)); + foreach (var line in mergedHistory) + { + writer.WriteLine(EncodeHistoryLine(line)); + } } + + this.pendingHistoryEntries.Clear(); + } + } + } + + private FileStream OpenHistoryFileWithExclusiveLock() + { + var options = new FileStreamOptions { Mode = FileMode.OpenOrCreate, Access = FileAccess.ReadWrite, Share = FileShare.None }; + if (!OperatingSystem.IsWindows()) + { + options.UnixCreateMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; + } + + for (var attempt = 0; ; attempt++) + { + try + { + return new FileStream(this.HistoryFile, 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..5b8693f7 100644 --- a/CosmosDBShell/Program.cs +++ b/CosmosDBShell/Program.cs @@ -256,10 +256,7 @@ void WriteStartupError(string message) if (o.ClearHistory) { - if (File.Exists(ShellInterpreter.Instance.HistoryFile)) - { - File.Delete(ShellInterpreter.Instance.HistoryFile); - } + ShellInterpreter.Instance.ClearHistory(); if (!startupMachineMode) { diff --git a/docs/navigation.md b/docs/navigation.md index 2af408a0..07d22216 100644 --- a/docs/navigation.md +++ b/docs/navigation.md @@ -234,7 +234,7 @@ 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. 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. From 41a112183d7ee90e7e9450713642d714b7abdc0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Wed, 30 Sep 2026 10:44:00 +0200 Subject: [PATCH 2/9] Test concurrent history persistence across processes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Runtime/SerializedExecutionTests.cs | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs index 9563b1ce..acf529b0 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -1,5 +1,6 @@ namespace CosmosShell.Tests.Runtime; +using System.Diagnostics; using Azure.Data.Cosmos.Shell.Core; public class SerializedExecutionTests @@ -94,6 +95,66 @@ public void PrintCommand_MergesHistorySavedByMultipleInterpreters() } } + [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); + 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 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 void PrintCommand_DeduplicatesMergedHistoryAcrossInterpreters() { @@ -395,4 +456,42 @@ public async Task RunSerializedAsync_CancelledWaiterDoesNotExecuteOrReleaseAnoth Assert.Equal(3, await shell.RunSerializedAsync(() => Task.FromResult(3), TestContext.Current.CancellationToken)); } + + 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 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 From 381001f55d3ae01b8db0403933503ba773e072ea Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 30 Sep 2026 08:56:57 +0000 Subject: [PATCH 3/9] Retry locked history during startup Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- .../Runtime/SerializedExecutionTests.cs | 26 +++++++++++++++++++ .../ShellInterpreter.cs | 19 +++++++++----- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs index acf529b0..93f88633 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -155,6 +155,32 @@ await Task.WhenAll( } } + [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 lockedStream = new FileStream(historyFile, FileMode.Open, FileAccess.ReadWrite, FileShare.None)) + { + constructor = Task.Run(() => new ShellInterpreter(configPath), TestContext.Current.CancellationToken); + await Task.Delay(TimeSpan.FromMilliseconds(75), TestContext.Current.CancellationToken); + Assert.False(constructor.IsCompleted); + } + + using var shell = await constructor; + Assert.Equal(["echo existing"], shell.History); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + [Fact] public void PrintCommand_DeduplicatesMergedHistoryAcrossInterpreters() { diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs index 370fddd0..138392f1 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs @@ -120,20 +120,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(); + entries = ReadHistoryEntries(stream); + } } - foreach (var line in lines) + foreach (var entry in entries) { - var decoded = DecodeHistoryLine(line); - this.RecordHistoryEntry(decoded, persist: false); + 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(); From cbc415c8c9997bf4a8a97e5c32a72faaf7122e24 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 30 Sep 2026 09:03:33 +0000 Subject: [PATCH 4/9] Report clear history failures Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- .../Runtime/SerializedExecutionTests.cs | 24 +++++++++++++++++++ .../ShellInterpreter.cs | 23 +++++++----------- CosmosDBShell/Program.cs | 11 ++++++++- 3 files changed, 42 insertions(+), 16 deletions(-) diff --git a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs index 93f88633..12e13eab 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -275,6 +275,30 @@ public void PrintCommand_DoesNotResurrectSavedHistoryAfterClear() } } + [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 existing"); + + using (var lockedStream = new FileStream(historyFile, FileMode.Open, FileAccess.ReadWrite, FileShare.None)) + { + Assert.Throws(() => shell.ClearHistory()); + } + + Assert.Equal(["echo existing"], shell.History); + Assert.NotEmpty(File.ReadAllLines(historyFile)); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + [Fact] public void PrintCommand_PreservesMultilineHistoryWhenMergingInterpreters() { diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs index 138392f1..a54df710 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs @@ -2332,24 +2332,17 @@ private void SaveHistory() internal void ClearHistory() { - try + lock (this.historyLock) { - lock (this.historyLock) + lock (HistoryFileLock) { - this.history.Clear(); - this.pendingHistoryEntries.Clear(); - - lock (HistoryFileLock) - { - using var stream = this.OpenHistoryFileWithExclusiveLock(); - stream.SetLength(0); - RestrictHistoryFileToOwner(this.HistoryFile); - } + using var stream = this.OpenHistoryFileWithExclusiveLock(); + stream.SetLength(0); + RestrictHistoryFileToOwner(this.HistoryFile); } - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - System.Diagnostics.Debug.WriteLine(ex); + + this.history.Clear(); + this.pendingHistoryEntries.Clear(); } } diff --git a/CosmosDBShell/Program.cs b/CosmosDBShell/Program.cs index 5b8693f7..0f4bf949 100644 --- a/CosmosDBShell/Program.cs +++ b/CosmosDBShell/Program.cs @@ -256,7 +256,16 @@ void WriteStartupError(string message) if (o.ClearHistory) { - ShellInterpreter.Instance.ClearHistory(); + try + { + ShellInterpreter.Instance.ClearHistory(); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + WriteStartupError(ex.Message); + Environment.ExitCode = ShellExitCode.FromException(ex); + return; + } if (!startupMachineMode) { From af4ba85e22dfbf66d52a1342adb52f2261753a58 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 30 Sep 2026 09:06:52 +0000 Subject: [PATCH 5/9] Validate permissions before clearing history Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs index a54df710..b6fb215b 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs @@ -2337,8 +2337,8 @@ internal void ClearHistory() lock (HistoryFileLock) { using var stream = this.OpenHistoryFileWithExclusiveLock(); - stream.SetLength(0); RestrictHistoryFileToOwner(this.HistoryFile); + stream.SetLength(0); } this.history.Clear(); From fe724bba49a4ae0b3972814b756152e82dd5cb9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Wed, 30 Sep 2026 11:14:40 +0200 Subject: [PATCH 6/9] Report failures when clearing shell history Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Integration/ShellProcessTests.cs | 54 ++++++++++++++++--- .../Runtime/SerializedExecutionTests.cs | 24 +++++++++ .../ShellInterpreter.cs | 23 +++----- CosmosDBShell/Program.cs | 11 +++- CosmosDBShell/lang/en.ftl | 1 + l10n/CosmosDBShell.json | 1 + 6 files changed, 90 insertions(+), 24 deletions(-) diff --git a/CosmosDBShell.Tests/Integration/ShellProcessTests.cs b/CosmosDBShell.Tests/Integration/ShellProcessTests.cs index 26288aef..259225b7 100644 --- a/CosmosDBShell.Tests/Integration/ShellProcessTests.cs +++ b/CosmosDBShell.Tests/Integration/ShellProcessTests.cs @@ -422,6 +422,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) @@ -432,7 +464,8 @@ private static async Task RunShellAsync( private static async Task RunShellAsync( string? stdinScript, IEnumerable? extraArgs, - CancellationToken cancellationToken) + CancellationToken cancellationToken, + string? configDirectory = null) { var argsList = extraArgs?.ToList(); var requiresOwnedStdin = stdinScript != null @@ -450,7 +483,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 @@ -555,13 +590,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 93f88633..8f041955 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -275,6 +275,30 @@ public void PrintCommand_DoesNotResurrectSavedHistoryAfterClear() } } + [Fact] + public void ClearHistory_WhenHistoryFileIsLocked_PreservesLoadedHistory() + { + var configPath = Path.Join(Path.GetTempPath(), $"cosmosshell-history-{Guid.NewGuid():N}"); + try + { + using var shell = new ShellInterpreter(configPath); + shell.PrintCommand("echo retained"); + + using var lockedStream = new FileStream( + shell.HistoryFile, + FileMode.Open, + FileAccess.ReadWrite, + FileShare.None); + + Assert.Throws(shell.ClearHistory); + Assert.Equal(["echo retained"], shell.History); + } + finally + { + Directory.Delete(configPath, recursive: true); + } + } + [Fact] public void PrintCommand_PreservesMultilineHistoryWhenMergingInterpreters() { diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs index 138392f1..a54df710 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs @@ -2332,24 +2332,17 @@ private void SaveHistory() internal void ClearHistory() { - try + lock (this.historyLock) { - lock (this.historyLock) + lock (HistoryFileLock) { - this.history.Clear(); - this.pendingHistoryEntries.Clear(); - - lock (HistoryFileLock) - { - using var stream = this.OpenHistoryFileWithExclusiveLock(); - stream.SetLength(0); - RestrictHistoryFileToOwner(this.HistoryFile); - } + using var stream = this.OpenHistoryFileWithExclusiveLock(); + stream.SetLength(0); + RestrictHistoryFileToOwner(this.HistoryFile); } - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - System.Diagnostics.Debug.WriteLine(ex); + + this.history.Clear(); + this.pendingHistoryEntries.Clear(); } } diff --git a/CosmosDBShell/Program.cs b/CosmosDBShell/Program.cs index 5b8693f7..05ec3f6a 100644 --- a/CosmosDBShell/Program.cs +++ b/CosmosDBShell/Program.cs @@ -256,7 +256,16 @@ void WriteStartupError(string message) if (o.ClearHistory) { - ShellInterpreter.Instance.ClearHistory(); + try + { + ShellInterpreter.Instance.ClearHistory(); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + WriteStartupError(MessageService.GetArgsString("shell-history-clear-error", "message", ex.Message)); + Environment.ExitCode = ShellExitCode.GeneralFailure; + return; + } if (!startupMachineMode) { diff --git a/CosmosDBShell/lang/en.ftl b/CosmosDBShell/lang/en.ftl index 75b83eda..e20b4d04 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/l10n/CosmosDBShell.json b/l10n/CosmosDBShell.json index e60f4e98..a0717ef2 100644 --- a/l10n/CosmosDBShell.json +++ b/l10n/CosmosDBShell.json @@ -1423,6 +1423,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", From 2a9d4548ba69173002aeb2a1a623f7f5f79c8c45 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 30 Sep 2026 09:35:58 +0000 Subject: [PATCH 7/9] Load read-only shell history files Co-authored-by: mkrueger <341098+mkrueger@users.noreply.github.com> --- .../Runtime/SerializedExecutionTests.cs | 50 +++++++++++++++++++ .../ShellInterpreter.cs | 13 +++-- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs index 4a935330..fbd22801 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -103,6 +103,13 @@ public async Task PrintCommand_MergesHistorySavedByConcurrentProcesses() 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}"); @@ -131,6 +138,10 @@ public async Task PrintCommand_MergesHistorySavedByConcurrentProcesses() 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)); @@ -181,6 +192,37 @@ public async Task Constructor_RetriesWhenHistoryFileIsLocked() } } + [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() { @@ -528,6 +570,14 @@ private static Process CreateHistoryWriterProcess(string configPath, string pref 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"); diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs index b6fb215b..14af7710 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs @@ -127,7 +127,7 @@ internal ShellInterpreter(string? configPath = null) { if (File.Exists(this.HistoryFile)) { - using var stream = this.OpenHistoryFileWithExclusiveLock(); + using var stream = this.OpenHistoryFileWithExclusiveLock(FileAccess.Read); entries = ReadHistoryEntries(stream); } } @@ -2393,10 +2393,15 @@ private void SaveHistoryCore() } } - private FileStream OpenHistoryFileWithExclusiveLock() + private FileStream OpenHistoryFileWithExclusiveLock(FileAccess access = FileAccess.ReadWrite) { - var options = new FileStreamOptions { Mode = FileMode.OpenOrCreate, Access = FileAccess.ReadWrite, Share = FileShare.None }; - if (!OperatingSystem.IsWindows()) + 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; } From bff2534a83ccfca46d3dbc8af51f717821debecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Wed, 30 Sep 2026 11:55:37 +0200 Subject: [PATCH 8/9] Stabilize locked history constructor test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Runtime/SerializedExecutionTests.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs index 4a935330..3af42fc6 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -165,10 +165,18 @@ public async Task Constructor_RetriesWhenHistoryFileIsLocked() try { Task constructor; + using var constructorStarted = new ManualResetEventSlim(); using (var lockedStream = new FileStream(historyFile, FileMode.Open, FileAccess.ReadWrite, FileShare.None)) { - constructor = Task.Run(() => new ShellInterpreter(configPath), TestContext.Current.CancellationToken); - await Task.Delay(TimeSpan.FromMilliseconds(75), TestContext.Current.CancellationToken); + 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); } From ab208893a680ffe9949c19c445711a0cb5838f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Fri, 2 Oct 2026 10:44:46 +0200 Subject: [PATCH 9/9] Preserve Windows history access rules during atomic replacement (#228) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Runtime/SerializedExecutionTests.cs | 99 +++++++++++++++++++ .../ShellInterpreter.cs | 48 ++++++--- docs/navigation.md | 2 +- 3 files changed, 136 insertions(+), 13 deletions(-) diff --git a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs index 51af4008..b4266a9b 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -1,6 +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 @@ -476,6 +478,85 @@ public void WriteHistoryAtomically_PublishesOnlyAfterWritingCompleteContent() } } + [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() { @@ -678,6 +759,24 @@ 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; diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs index a3e54eda..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; @@ -2460,21 +2462,10 @@ private void SaveHistoryCore() internal static void WriteHistoryAtomically(string historyFile, Action write) { var temporary = historyFile + "." + Guid.NewGuid().ToString("N") + ".tmp"; - var options = new FileStreamOptions - { - Mode = FileMode.CreateNew, - Access = FileAccess.Write, - Share = FileShare.None, - }; - if (!OperatingSystem.IsWindows()) - { - options.UnixCreateMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; - } - var created = false; try { - using (var stream = new FileStream(temporary, options)) + using (var stream = CreateHistoryTemporaryFile(temporary, historyFile)) { created = true; write(stream); @@ -2492,6 +2483,39 @@ internal static void WriteHistoryAtomically(string historyFile, Action w } } + [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; diff --git a/docs/navigation.md b/docs/navigation.md index cfcac246..28075b76 100644 --- a/docs/navigation.md +++ b/docs/navigation.md @@ -236,7 +236,7 @@ There is no separate "enter multi-line mode" command — the shell enters and le 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