From bdfddf4e1dcdddda7cc62ec6c01ae9511f1f5abe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 07:50:07 +0000 Subject: [PATCH 01/16] Initial plan From 9222e944a566c97eb45eaf5d3ad4c7136b6d79c6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 08:02:19 +0000 Subject: [PATCH 02/16] Fix SecretsStore concurrent access issue with locking mechanism Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- src/Shared/SecretsStore.cs | 123 ++++++++++++++---- .../UserSecretsParameterDefaultTests.cs | 72 ++++++++++ 2 files changed, 169 insertions(+), 26 deletions(-) diff --git a/src/Shared/SecretsStore.cs b/src/Shared/SecretsStore.cs index 5c8f550002a..90fd46bc555 100644 --- a/src/Shared/SecretsStore.cs +++ b/src/Shared/SecretsStore.cs @@ -16,13 +16,19 @@ namespace Microsoft.Extensions.SecretManager.Tools.Internal; /// internal sealed class SecretsStore { + // Static lock dictionary to synchronize access per userSecretsId + private static readonly Dictionary s_locks = new(); + private static readonly object s_locksLock = new(); + private readonly string _secretsFilePath; private readonly Dictionary _secrets; + private readonly string _userSecretsId; public SecretsStore(string userSecretsId) { ArgumentNullException.ThrowIfNull(userSecretsId); + _userSecretsId = userSecretsId; _secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); EnsureUserSecretsDirectory(); @@ -30,6 +36,19 @@ public SecretsStore(string userSecretsId) _secrets = Load(_secretsFilePath); } + private static SemaphoreSlim GetLock(string userSecretsId) + { + lock (s_locksLock) + { + if (!s_locks.TryGetValue(userSecretsId, out var semaphore)) + { + semaphore = new SemaphoreSlim(1, 1); + s_locks[userSecretsId] = semaphore; + } + return semaphore; + } + } + public string? this[string key] => _secrets[key]; public int Count => _secrets.Count; @@ -49,39 +68,50 @@ public SecretsStore(string userSecretsId) public void Save() { - EnsureUserSecretsDirectory(); - - var contents = new JsonObject(); - if (_secrets is not null) + var semaphore = GetLock(_userSecretsId); + semaphore.Wait(); + try { - foreach (var secret in _secrets.AsEnumerable()) + // Reload from disk to merge with any concurrent changes + var currentSecrets = Load(_secretsFilePath); + + // Merge our changes with what's on disk + foreach (var kvp in _secrets) + { + currentSecrets[kvp.Key] = kvp.Value; + } + + EnsureUserSecretsDirectory(); + + var contents = new JsonObject(); + foreach (var secret in currentSecrets) { contents[secret.Key] = secret.Value; } - } - // Create a temp file with the correct Unix file mode before moving it to the expected _filePath. - if (!OperatingSystem.IsWindows()) - { - var tempFilename = Path.GetTempFileName(); - File.Move(tempFilename, _secretsFilePath, overwrite: true); - } + // Create a temp file with the correct Unix file mode before moving it to the expected _filePath. + if (!OperatingSystem.IsWindows()) + { + var tempFilename = Path.GetTempFileName(); + File.Move(tempFilename, _secretsFilePath, overwrite: true); + } - var json = contents.ToJsonString(new() - { - WriteIndented = true - }); + var json = contents.ToJsonString(new() + { + WriteIndented = true + }); - File.WriteAllText(_secretsFilePath, json, Encoding.UTF8); + File.WriteAllText(_secretsFilePath, json, Encoding.UTF8); + } + finally + { + semaphore.Release(); + } } private void EnsureUserSecretsDirectory() { - var directoryName = Path.GetDirectoryName(_secretsFilePath); - if (!string.IsNullOrEmpty(directoryName) && !Directory.Exists(directoryName)) - { - Directory.CreateDirectory(directoryName); - } + EnsureUserSecretsDirectory(_secretsFilePath); } private static Dictionary Load(string secretsFilePath) @@ -130,14 +160,55 @@ public static bool TrySetUserSecret(Assembly? assembly, string name, string valu // Save the value to the secret store try { - var secretsStore = new SecretsStore(userSecretsId); - secretsStore.Set(name, value); - secretsStore.Save(); - return true; + var semaphore = GetLock(userSecretsId); + semaphore.Wait(); + try + { + // Load, set, and save in one atomic operation to ensure thread safety + var secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); + EnsureUserSecretsDirectory(secretsFilePath); + + var secrets = Load(secretsFilePath); + secrets[name] = value; + + var contents = new JsonObject(); + foreach (var secret in secrets) + { + contents[secret.Key] = secret.Value; + } + + // Create a temp file with the correct Unix file mode before moving it to the expected _filePath. + if (!OperatingSystem.IsWindows()) + { + var tempFilename = Path.GetTempFileName(); + File.Move(tempFilename, secretsFilePath, overwrite: true); + } + + var json = contents.ToJsonString(new() + { + WriteIndented = true + }); + + File.WriteAllText(secretsFilePath, json, Encoding.UTF8); + return true; + } + finally + { + semaphore.Release(); + } } catch (Exception) { } // Ignore user secret store errors } return false; } + + private static void EnsureUserSecretsDirectory(string secretsFilePath) + { + var directoryName = Path.GetDirectoryName(secretsFilePath); + if (!string.IsNullOrEmpty(directoryName) && !Directory.Exists(directoryName)) + { + Directory.CreateDirectory(directoryName); + } + } } diff --git a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs index d9d3f5d9723..e0f52f279d2 100644 --- a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs +++ b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs @@ -71,6 +71,78 @@ public void UserSecretsParameterDefault_GetDefaultValue_DoesntThrowIfSecretsFile var _ = userSecretDefault.GetDefaultValue(); } + [Fact] + public async Task TrySetUserSecret_ConcurrentWrites_PreservesAllSecrets() + { + var userSecretsId = Guid.NewGuid().ToString("N"); + ClearUsersSecrets(userSecretsId); + + var testAssembly = AssemblyBuilder.DefineDynamicAssembly( + new("TestAssembly"), AssemblyBuilderAccess.RunAndCollect, [new CustomAttributeBuilder(s_userSecretsIdAttrCtor, [userSecretsId])]); + + // Simulate concurrent writes from multiple threads (like SQL Server and RabbitMQ generating passwords) + var tasks = new List>(); + var secretsToWrite = new Dictionary + { + ["Parameters:sqlserver-password"] = "SqlPassword123!", + ["Parameters:rabbitmq-password"] = "RabbitPassword456!", + ["Parameters:redis-password"] = "RedisPassword789!", + ["Parameters:postgres-password"] = "PostgresPassword012!", + }; + + foreach (var kvp in secretsToWrite) + { + var key = kvp.Key; + var value = kvp.Value; + tasks.Add(Task.Run(() => SecretsStore.TrySetUserSecret(testAssembly, key, value))); + } + + var results = await Task.WhenAll(tasks); + + // All writes should succeed + Assert.All(results, Assert.True); + + // All secrets should be preserved + var userSecrets = GetUserSecrets(userSecretsId); + foreach (var kvp in secretsToWrite) + { + Assert.True(userSecrets.ContainsKey(kvp.Key), $"Secret '{kvp.Key}' was not found in user secrets"); + Assert.Equal(kvp.Value, userSecrets[kvp.Key]); + } + + DeleteUserSecretsFile(userSecretsId); + } + + [Fact] + public async Task TrySetUserSecret_ConcurrentWritesSameKey_LastWriteWins() + { + var userSecretsId = Guid.NewGuid().ToString("N"); + ClearUsersSecrets(userSecretsId); + + var testAssembly = AssemblyBuilder.DefineDynamicAssembly( + new("TestAssembly"), AssemblyBuilderAccess.RunAndCollect, [new CustomAttributeBuilder(s_userSecretsIdAttrCtor, [userSecretsId])]); + + // Simulate concurrent writes to the same key + var tasks = new List>(); + for (int i = 0; i < 10; i++) + { + var value = $"Value{i}"; + tasks.Add(Task.Run(() => SecretsStore.TrySetUserSecret(testAssembly, "Parameters:test-key", value))); + } + + var results = await Task.WhenAll(tasks); + + // All writes should succeed + Assert.All(results, Assert.True); + + // The key should exist with one of the values + var userSecrets = GetUserSecrets(userSecretsId); + Assert.True(userSecrets.ContainsKey("Parameters:test-key")); + Assert.NotNull(userSecrets["Parameters:test-key"]); + + DeleteUserSecretsFile(userSecretsId); + } + private static void EnsureUserSecretsDirectory(string secretsFilePath) { var directoryName = Path.GetDirectoryName(secretsFilePath); From 02de77776cb195ab4b565e7bd5fbe632539cf867 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 08:10:24 +0000 Subject: [PATCH 03/16] Add specific test case for SQL Server + RabbitMQ concurrent secret writes Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../UserSecretsParameterDefaultTests.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs index e0f52f279d2..f17df712a80 100644 --- a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs +++ b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs @@ -113,6 +113,35 @@ public async Task TrySetUserSecret_ConcurrentWrites_PreservesAllSecrets() DeleteUserSecretsFile(userSecretsId); } + [Fact] + public async Task TrySetUserSecret_SqlServerAndRabbitMQ_BothSecretsPreserved() + { + // This test specifically reproduces the issue described in the bug report + var userSecretsId = Guid.NewGuid().ToString("N"); + ClearUsersSecrets(userSecretsId); + + var testAssembly = AssemblyBuilder.DefineDynamicAssembly( + new("TestAssembly"), AssemblyBuilderAccess.RunAndCollect, [new CustomAttributeBuilder(s_userSecretsIdAttrCtor, [userSecretsId])]); + + // Simulate SQL Server and RabbitMQ generating passwords concurrently + var sqlTask = Task.Run(() => SecretsStore.TrySetUserSecret(testAssembly, "Parameters:sql-password", "SqlPassword123!")); + var rabbitTask = Task.Run(() => SecretsStore.TrySetUserSecret(testAssembly, "Parameters:rabbit-password", "RabbitPassword456!")); + + var results = await Task.WhenAll(sqlTask, rabbitTask); + + // Both writes should succeed + Assert.All(results, Assert.True); + + // Both secrets should be in the file + var userSecrets = GetUserSecrets(userSecretsId); + Assert.True(userSecrets.ContainsKey("Parameters:sql-password"), "SQL Server password was not found"); + Assert.True(userSecrets.ContainsKey("Parameters:rabbit-password"), "RabbitMQ password was not found"); + Assert.Equal("SqlPassword123!", userSecrets["Parameters:sql-password"]); + Assert.Equal("RabbitPassword456!", userSecrets["Parameters:rabbit-password"]); + + DeleteUserSecretsFile(userSecretsId); + } + [Fact] public async Task TrySetUserSecret_ConcurrentWritesSameKey_LastWriteWins() { From b852826b4534168cc0f97a319f161dc64241bf10 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 18:54:22 +0000 Subject: [PATCH 04/16] Replace SemaphoreSlim with simple object locks Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- src/Shared/SecretsStore.cs | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/Shared/SecretsStore.cs b/src/Shared/SecretsStore.cs index 90fd46bc555..bb2dbe0c3e9 100644 --- a/src/Shared/SecretsStore.cs +++ b/src/Shared/SecretsStore.cs @@ -17,7 +17,7 @@ namespace Microsoft.Extensions.SecretManager.Tools.Internal; internal sealed class SecretsStore { // Static lock dictionary to synchronize access per userSecretsId - private static readonly Dictionary s_locks = new(); + private static readonly Dictionary s_locks = new(); private static readonly object s_locksLock = new(); private readonly string _secretsFilePath; @@ -36,16 +36,16 @@ public SecretsStore(string userSecretsId) _secrets = Load(_secretsFilePath); } - private static SemaphoreSlim GetLock(string userSecretsId) + private static object GetLock(string userSecretsId) { lock (s_locksLock) { - if (!s_locks.TryGetValue(userSecretsId, out var semaphore)) + if (!s_locks.TryGetValue(userSecretsId, out var lockObj)) { - semaphore = new SemaphoreSlim(1, 1); - s_locks[userSecretsId] = semaphore; + lockObj = new object(); + s_locks[userSecretsId] = lockObj; } - return semaphore; + return lockObj; } } @@ -68,9 +68,8 @@ private static SemaphoreSlim GetLock(string userSecretsId) public void Save() { - var semaphore = GetLock(_userSecretsId); - semaphore.Wait(); - try + var lockObj = GetLock(_userSecretsId); + lock (lockObj) { // Reload from disk to merge with any concurrent changes var currentSecrets = Load(_secretsFilePath); @@ -103,10 +102,6 @@ public void Save() File.WriteAllText(_secretsFilePath, json, Encoding.UTF8); } - finally - { - semaphore.Release(); - } } private void EnsureUserSecretsDirectory() @@ -160,9 +155,8 @@ public static bool TrySetUserSecret(Assembly? assembly, string name, string valu // Save the value to the secret store try { - var semaphore = GetLock(userSecretsId); - semaphore.Wait(); - try + var lockObj = GetLock(userSecretsId); + lock (lockObj) { // Load, set, and save in one atomic operation to ensure thread safety var secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); @@ -192,10 +186,6 @@ public static bool TrySetUserSecret(Assembly? assembly, string name, string valu File.WriteAllText(secretsFilePath, json, Encoding.UTF8); return true; } - finally - { - semaphore.Release(); - } } catch (Exception) { } // Ignore user secret store errors } From ed54150b1f2ae06e11847958c00287fc1f3607b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 03:56:37 +0000 Subject: [PATCH 05/16] Share lock between SecretsStore and UserSecretsDeploymentStateManager Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- src/Aspire.Hosting/Aspire.Hosting.csproj | 1 + .../UserSecretsDeploymentStateManager.cs | 16 +++++++- src/Shared/SecretsStore.cs | 25 ++---------- src/Shared/UserSecretsFileLock.cs | 38 +++++++++++++++++++ 4 files changed, 57 insertions(+), 23 deletions(-) create mode 100644 src/Shared/UserSecretsFileLock.cs diff --git a/src/Aspire.Hosting/Aspire.Hosting.csproj b/src/Aspire.Hosting/Aspire.Hosting.csproj index 969b4d80999..9f705c1695f 100644 --- a/src/Aspire.Hosting/Aspire.Hosting.csproj +++ b/src/Aspire.Hosting/Aspire.Hosting.csproj @@ -32,6 +32,7 @@ + diff --git a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs index 2ab9594b1d7..8e6503cacfb 100644 --- a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs @@ -8,6 +8,7 @@ using System.Text.Json.Nodes; using Microsoft.Extensions.Configuration.UserSecrets; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.SecretManager.Tools.Internal; namespace Aspire.Hosting.Publishing.Internal; @@ -37,7 +38,20 @@ protected override async Task SaveStateToStorageAsync(JsonObject state, Cancella var userSecretsPath = GetStatePath() ?? throw new InvalidOperationException("User secrets path could not be determined."); var flattenedUserSecrets = DeploymentStateManagerBase.FlattenJsonObject(state); Directory.CreateDirectory(Path.GetDirectoryName(userSecretsPath)!); - await File.WriteAllTextAsync(userSecretsPath, flattenedUserSecrets.ToJsonString(s_jsonSerializerOptions), cancellationToken).ConfigureAwait(false); + + // Use the shared lock to synchronize with SecretsStore + var lockObj = UserSecretsFileLock.GetLock(userSecretsPath); + var json = flattenedUserSecrets.ToJsonString(s_jsonSerializerOptions); + + // We need to use a synchronous lock, so we'll write synchronously inside the lock + // but still return a Task to maintain the async signature + await Task.Run(() => + { + lock (lockObj) + { + File.WriteAllText(userSecretsPath, json, System.Text.Encoding.UTF8); + } + }, cancellationToken).ConfigureAwait(false); logger.LogInformation("Azure resource connection strings saved to user secrets."); } diff --git a/src/Shared/SecretsStore.cs b/src/Shared/SecretsStore.cs index bb2dbe0c3e9..95f593950e1 100644 --- a/src/Shared/SecretsStore.cs +++ b/src/Shared/SecretsStore.cs @@ -16,19 +16,13 @@ namespace Microsoft.Extensions.SecretManager.Tools.Internal; /// internal sealed class SecretsStore { - // Static lock dictionary to synchronize access per userSecretsId - private static readonly Dictionary s_locks = new(); - private static readonly object s_locksLock = new(); - private readonly string _secretsFilePath; private readonly Dictionary _secrets; - private readonly string _userSecretsId; public SecretsStore(string userSecretsId) { ArgumentNullException.ThrowIfNull(userSecretsId); - _userSecretsId = userSecretsId; _secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); EnsureUserSecretsDirectory(); @@ -36,19 +30,6 @@ public SecretsStore(string userSecretsId) _secrets = Load(_secretsFilePath); } - private static object GetLock(string userSecretsId) - { - lock (s_locksLock) - { - if (!s_locks.TryGetValue(userSecretsId, out var lockObj)) - { - lockObj = new object(); - s_locks[userSecretsId] = lockObj; - } - return lockObj; - } - } - public string? this[string key] => _secrets[key]; public int Count => _secrets.Count; @@ -68,7 +49,7 @@ private static object GetLock(string userSecretsId) public void Save() { - var lockObj = GetLock(_userSecretsId); + var lockObj = UserSecretsFileLock.GetLock(_secretsFilePath); lock (lockObj) { // Reload from disk to merge with any concurrent changes @@ -155,11 +136,11 @@ public static bool TrySetUserSecret(Assembly? assembly, string name, string valu // Save the value to the secret store try { - var lockObj = GetLock(userSecretsId); + var secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); + var lockObj = UserSecretsFileLock.GetLock(secretsFilePath); lock (lockObj) { // Load, set, and save in one atomic operation to ensure thread safety - var secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); EnsureUserSecretsDirectory(secretsFilePath); var secrets = Load(secretsFilePath); diff --git a/src/Shared/UserSecretsFileLock.cs b/src/Shared/UserSecretsFileLock.cs new file mode 100644 index 00000000000..9491d6d2afc --- /dev/null +++ b/src/Shared/UserSecretsFileLock.cs @@ -0,0 +1,38 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Extensions.SecretManager.Tools.Internal; + +/// +/// Provides synchronized access to user secrets files within the same process. +/// Both SecretsStore and UserSecretsDeploymentStateManager use this to ensure writes are serialized. +/// +internal static class UserSecretsFileLock +{ + // Static lock dictionary to synchronize access per file path + private static readonly Dictionary s_locks = new(); + private static readonly object s_locksLock = new(); + + /// + /// Gets a lock object for the specified user secrets file path. + /// + /// The full path to the user secrets file. + /// A lock object that can be used with the lock statement. + public static object GetLock(string filePath) + { + ArgumentNullException.ThrowIfNull(filePath); + + // Normalize the path to ensure consistent locking across different path representations + var normalizedPath = Path.GetFullPath(filePath); + + lock (s_locksLock) + { + if (!s_locks.TryGetValue(normalizedPath, out var lockObj)) + { + lockObj = new object(); + s_locks[normalizedPath] = lockObj; + } + return lockObj; + } + } +} From 2c9ad74175169ac978e2103c4187ba30bb6521d6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 04:21:33 +0000 Subject: [PATCH 06/16] Refactor to use SemaphoreSlim and allow DeploymentStateManagerBase override Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../Internal/DeploymentStateManagerBase.cs | 24 ++++++++++------ .../UserSecretsDeploymentStateManager.cs | 28 +++++++++++++------ src/Shared/SecretsStore.cs | 18 +++++++++--- src/Shared/UserSecretsFileLock.cs | 22 +++++++-------- 4 files changed, 60 insertions(+), 32 deletions(-) diff --git a/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs b/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs index 5d856f1acb7..ff602d22f09 100644 --- a/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs +++ b/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs @@ -46,6 +46,12 @@ private sealed class SectionMetadata(long version) private JsonObject? _state; private bool _isStateLoaded; + /// + /// Gets the semaphore used for synchronizing state operations. + /// Can be overridden by derived classes to use a different synchronization mechanism. + /// + protected virtual SemaphoreSlim StateLock => _stateLock; + /// public abstract string? StateFilePath { get; } @@ -149,7 +155,7 @@ private static void FlattenJsonObjectRecursive(JsonObject source, string prefix, /// The loaded state as a JsonObject. protected async Task LoadStateAsync(CancellationToken cancellationToken = default) { - await _stateLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await StateLock.WaitAsync(cancellationToken).ConfigureAwait(false); try { if (_isStateLoaded && _state is not null) @@ -181,21 +187,21 @@ protected async Task LoadStateAsync(CancellationToken cancellationTo } finally { - _stateLock.Release(); + StateLock.Release(); } } /// public async Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default) { - await _stateLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await StateLock.WaitAsync(cancellationToken).ConfigureAwait(false); try { await SaveStateToStorageAsync(state, cancellationToken).ConfigureAwait(false); } finally { - _stateLock.Release(); + StateLock.Release(); } } @@ -219,8 +225,8 @@ public async Task AcquireSectionAsync(string sectionName var metadata = GetSectionMetadata(sectionName); - // Protect access to _state with _stateLock to prevent concurrent modification during enumeration - await _stateLock.WaitAsync(cancellationToken).ConfigureAwait(false); + // Protect access to _state with StateLock to prevent concurrent modification during enumeration + await StateLock.WaitAsync(cancellationToken).ConfigureAwait(false); try { var sectionData = _state?.TryGetPropertyValue(sectionName, out var sectionNode) == true && sectionNode is JsonObject obj @@ -231,7 +237,7 @@ public async Task AcquireSectionAsync(string sectionName } finally { - _stateLock.Release(); + StateLock.Release(); } } @@ -268,7 +274,7 @@ public async Task SaveSectionAsync(DeploymentStateSection section, CancellationT section.Version++; // Serialize state modification and file write to prevent concurrent enumeration - await _stateLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await StateLock.WaitAsync(cancellationToken).ConfigureAwait(false); try { // Store a deep clone to ensure immutability @@ -277,7 +283,7 @@ public async Task SaveSectionAsync(DeploymentStateSection section, CancellationT } finally { - _stateLock.Release(); + StateLock.Release(); } } } diff --git a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs index 8e6503cacfb..f238d0e62c1 100644 --- a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs @@ -17,9 +17,27 @@ namespace Aspire.Hosting.Publishing.Internal; /// public sealed class UserSecretsDeploymentStateManager(ILogger logger) : DeploymentStateManagerBase(logger) { + private SemaphoreSlim? _sharedSemaphore; + /// public override string? StateFilePath => GetStatePath(); + /// + /// Gets the semaphore used for synchronizing state operations. + /// Uses the shared UserSecretsFileLock semaphore to coordinate with SecretsStore. + /// + protected override SemaphoreSlim StateLock + { + get + { + if (_sharedSemaphore == null && StateFilePath != null) + { + _sharedSemaphore = UserSecretsFileLock.GetSemaphore(StateFilePath); + } + return _sharedSemaphore ?? base.StateLock; + } + } + /// protected override string? GetStatePath() { @@ -39,18 +57,12 @@ protected override async Task SaveStateToStorageAsync(JsonObject state, Cancella var flattenedUserSecrets = DeploymentStateManagerBase.FlattenJsonObject(state); Directory.CreateDirectory(Path.GetDirectoryName(userSecretsPath)!); - // Use the shared lock to synchronize with SecretsStore - var lockObj = UserSecretsFileLock.GetLock(userSecretsPath); var json = flattenedUserSecrets.ToJsonString(s_jsonSerializerOptions); - // We need to use a synchronous lock, so we'll write synchronously inside the lock - // but still return a Task to maintain the async signature + // Write synchronously to avoid async/await in the lock that's managed by the base class await Task.Run(() => { - lock (lockObj) - { - File.WriteAllText(userSecretsPath, json, System.Text.Encoding.UTF8); - } + File.WriteAllText(userSecretsPath, json, System.Text.Encoding.UTF8); }, cancellationToken).ConfigureAwait(false); logger.LogInformation("Azure resource connection strings saved to user secrets."); diff --git a/src/Shared/SecretsStore.cs b/src/Shared/SecretsStore.cs index 95f593950e1..ea0c8761560 100644 --- a/src/Shared/SecretsStore.cs +++ b/src/Shared/SecretsStore.cs @@ -49,8 +49,9 @@ public SecretsStore(string userSecretsId) public void Save() { - var lockObj = UserSecretsFileLock.GetLock(_secretsFilePath); - lock (lockObj) + var semaphore = UserSecretsFileLock.GetSemaphore(_secretsFilePath); + semaphore.Wait(); + try { // Reload from disk to merge with any concurrent changes var currentSecrets = Load(_secretsFilePath); @@ -83,6 +84,10 @@ public void Save() File.WriteAllText(_secretsFilePath, json, Encoding.UTF8); } + finally + { + semaphore.Release(); + } } private void EnsureUserSecretsDirectory() @@ -137,8 +142,9 @@ public static bool TrySetUserSecret(Assembly? assembly, string name, string valu try { var secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); - var lockObj = UserSecretsFileLock.GetLock(secretsFilePath); - lock (lockObj) + var semaphore = UserSecretsFileLock.GetSemaphore(secretsFilePath); + semaphore.Wait(); + try { // Load, set, and save in one atomic operation to ensure thread safety EnsureUserSecretsDirectory(secretsFilePath); @@ -167,6 +173,10 @@ public static bool TrySetUserSecret(Assembly? assembly, string name, string valu File.WriteAllText(secretsFilePath, json, Encoding.UTF8); return true; } + finally + { + semaphore.Release(); + } } catch (Exception) { } // Ignore user secret store errors } diff --git a/src/Shared/UserSecretsFileLock.cs b/src/Shared/UserSecretsFileLock.cs index 9491d6d2afc..9d6d85c7226 100644 --- a/src/Shared/UserSecretsFileLock.cs +++ b/src/Shared/UserSecretsFileLock.cs @@ -9,30 +9,30 @@ namespace Microsoft.Extensions.SecretManager.Tools.Internal; /// internal static class UserSecretsFileLock { - // Static lock dictionary to synchronize access per file path - private static readonly Dictionary s_locks = new(); - private static readonly object s_locksLock = new(); + // Static semaphore dictionary to synchronize access per file path + private static readonly Dictionary s_semaphores = new(); + private static readonly object s_semaphoresLock = new(); /// - /// Gets a lock object for the specified user secrets file path. + /// Gets a semaphore for the specified user secrets file path. /// /// The full path to the user secrets file. - /// A lock object that can be used with the lock statement. - public static object GetLock(string filePath) + /// A SemaphoreSlim that can be used for synchronization. + public static SemaphoreSlim GetSemaphore(string filePath) { ArgumentNullException.ThrowIfNull(filePath); // Normalize the path to ensure consistent locking across different path representations var normalizedPath = Path.GetFullPath(filePath); - lock (s_locksLock) + lock (s_semaphoresLock) { - if (!s_locks.TryGetValue(normalizedPath, out var lockObj)) + if (!s_semaphores.TryGetValue(normalizedPath, out var semaphore)) { - lockObj = new object(); - s_locks[normalizedPath] = lockObj; + semaphore = new SemaphoreSlim(1, 1); + s_semaphores[normalizedPath] = semaphore; } - return lockObj; + return semaphore; } } } From 482e75965a258d47bbb1cc516db8a32880b163d7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 04:31:37 +0000 Subject: [PATCH 07/16] Remove unnecessary Task.Run in UserSecretsDeploymentStateManager Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../Internal/UserSecretsDeploymentStateManager.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs index f238d0e62c1..dea3044e1c5 100644 --- a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs @@ -59,11 +59,7 @@ protected override async Task SaveStateToStorageAsync(JsonObject state, Cancella var json = flattenedUserSecrets.ToJsonString(s_jsonSerializerOptions); - // Write synchronously to avoid async/await in the lock that's managed by the base class - await Task.Run(() => - { - File.WriteAllText(userSecretsPath, json, System.Text.Encoding.UTF8); - }, cancellationToken).ConfigureAwait(false); + await File.WriteAllTextAsync(userSecretsPath, json, System.Text.Encoding.UTF8, cancellationToken).ConfigureAwait(false); logger.LogInformation("Azure resource connection strings saved to user secrets."); } From 8ab3628b252bf2a90c5b17db96b68a9a65b4466b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 05:35:28 +0000 Subject: [PATCH 08/16] Addressing PR comments Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../UserSecrets/IUserSecretsManager.cs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs diff --git a/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs new file mode 100644 index 00000000000..8ea2c2d3a25 --- /dev/null +++ b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text.Json.Nodes; + +namespace Aspire.Hosting.UserSecrets; + +/// +/// Manages user secrets for an application, providing thread-safe read and write operations. +/// +internal interface IUserSecretsManager +{ + /// + /// Gets the path to the user secrets file. + /// + string? FilePath { get; } + + /// + /// Attempts to set a user secret value synchronously. + /// + /// The name of the secret. + /// The value of the secret. + /// True if the secret was set successfully; otherwise, false. + bool TrySetSecret(string name, string value); + + /// + /// Attempts to set a user secret value asynchronously. + /// + /// The name of the secret. + /// The value of the secret. + /// Cancellation token. + /// True if the secret was set successfully; otherwise, false. + Task TrySetSecretAsync(string name, string value, CancellationToken cancellationToken = default); + + /// + /// Gets a secret value if it exists, or sets it using the value generator if it doesn't. + /// + /// The name of the secret. + /// Function to generate the value if it doesn't exist. + /// The secret value. + string GetOrSetSecret(string name, Func valueGenerator); + + /// + /// Saves state to user secrets asynchronously (for deployment state manager). + /// + /// The state to save. + /// Cancellation token. + Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default); +} From c83d9cf137f9ac667f278d4a3e9ba18e4b62d97a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 06:50:04 +0000 Subject: [PATCH 09/16] Complete refactoring to use DI-based UserSecretsManager with factory pattern Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../UserSecretsParameterDefault.cs | 6 +- src/Aspire.Hosting/Aspire.Hosting.csproj | 2 - .../DistributedApplicationBuilder.cs | 14 +- .../Internal/DeploymentStateManagerBase.cs | 24 +- .../UserSecretsDeploymentStateManager.cs | 51 ++-- .../UserSecrets/IUserSecretsManager.cs | 11 +- .../UserSecrets/UserSecretsManagerFactory.cs | 257 ++++++++++++++++++ .../VersionChecking/VersionCheckService.cs | 13 +- src/Shared/SecretsStore.cs | 195 ------------- src/Shared/UserSecretsFileLock.cs | 38 --- .../Aspire.Hosting.Tests/SecretsStoreTests.cs | 34 ++- .../UserSecretsParameterDefaultTests.cs | 65 ++++- 12 files changed, 402 insertions(+), 308 deletions(-) create mode 100644 src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs delete mode 100644 src/Shared/SecretsStore.cs delete mode 100644 src/Shared/UserSecretsFileLock.cs diff --git a/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs b/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs index 0294b82b23a..9d414087dfa 100644 --- a/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs +++ b/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs @@ -4,7 +4,7 @@ using System.Diagnostics; using System.Reflection; using Aspire.Hosting.Publishing; -using Microsoft.Extensions.SecretManager.Tools.Internal; +using Aspire.Hosting.UserSecrets; namespace Aspire.Hosting.ApplicationModel; @@ -23,7 +23,9 @@ public override string GetDefaultValue() { var value = parameterDefault.GetDefaultValue(); var configurationKey = $"Parameters:{parameterName}"; - if (!SecretsStore.TrySetUserSecret(appHostAssembly, configurationKey, value)) + + var manager = UserSecretsManagerFactory.Instance.GetOrCreate(appHostAssembly); + if (manager == null || !manager.TrySetSecret(configurationKey, value)) { // This is a best-effort operation, so we don't throw if it fails. Common reason for failure is that the user secrets ID is not set // in the application's assembly. Note there's no ILogger available this early in the application lifecycle. diff --git a/src/Aspire.Hosting/Aspire.Hosting.csproj b/src/Aspire.Hosting/Aspire.Hosting.csproj index 9f705c1695f..a802959cac5 100644 --- a/src/Aspire.Hosting/Aspire.Hosting.csproj +++ b/src/Aspire.Hosting/Aspire.Hosting.csproj @@ -31,8 +31,6 @@ - - diff --git a/src/Aspire.Hosting/DistributedApplicationBuilder.cs b/src/Aspire.Hosting/DistributedApplicationBuilder.cs index 56bcc258599..e8b99fb1e23 100644 --- a/src/Aspire.Hosting/DistributedApplicationBuilder.cs +++ b/src/Aspire.Hosting/DistributedApplicationBuilder.cs @@ -23,6 +23,7 @@ using Aspire.Hosting.Orchestrator; using Aspire.Hosting.Pipelines; using Aspire.Hosting.Publishing; +using Aspire.Hosting.UserSecrets; using Aspire.Hosting.VersionChecking; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -31,7 +32,6 @@ using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Microsoft.Extensions.SecretManager.Tools.Internal; namespace Aspire.Hosting; @@ -61,6 +61,7 @@ public class DistributedApplicationBuilder : IDistributedApplicationBuilder private readonly DistributedApplicationOptions _options; private readonly HostApplicationBuilder _innerBuilder; + private readonly IUserSecretsManager? _userSecretsManager; /// public IHostEnvironment Environment => _innerBuilder.Environment; @@ -285,6 +286,13 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options) } // Core things + // Create and register the user secrets manager + _userSecretsManager = UserSecretsManagerFactory.Instance.GetOrCreate(AppHostAssembly); + if (_userSecretsManager != null) + { + _innerBuilder.Services.AddSingleton(_userSecretsManager); + } + _innerBuilder.Services.AddSingleton(sp => new DistributedApplicationModel(Resources)); _innerBuilder.Services.AddSingleton(); _innerBuilder.Services.AddHostedService(sp => sp.GetRequiredService()); @@ -351,13 +359,13 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options) // If a key is generated, it's stored in the user secrets store so that it will be auto-loaded // on subsequent runs and not recreated. This is important to ensure it doesn't change the state // of persistent containers (as a new key would be a spec change). - SecretsStore.GetOrSetUserSecret(_innerBuilder.Configuration, AppHostAssembly, "AppHost:OtlpApiKey", TokenGenerator.GenerateToken); + _userSecretsManager?.GetOrSetSecret(_innerBuilder.Configuration, "AppHost:OtlpApiKey", TokenGenerator.GenerateToken); // Set a random API key for the MCP Server if one isn't already present in configuration. // If a key is generated, it's stored in the user secrets store so that it will be auto-loaded // on subsequent runs and not recreated. This is important to ensure it doesn't change the state // of MCP clients. - SecretsStore.GetOrSetUserSecret(_innerBuilder.Configuration, AppHostAssembly, "AppHost:McpApiKey", TokenGenerator.GenerateToken); + _userSecretsManager?.GetOrSetSecret(_innerBuilder.Configuration, "AppHost:McpApiKey", TokenGenerator.GenerateToken); // Determine the frontend browser token. if (_innerBuilder.Configuration.GetString(KnownConfigNames.DashboardFrontendBrowserToken, diff --git a/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs b/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs index ff602d22f09..5d856f1acb7 100644 --- a/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs +++ b/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs @@ -46,12 +46,6 @@ private sealed class SectionMetadata(long version) private JsonObject? _state; private bool _isStateLoaded; - /// - /// Gets the semaphore used for synchronizing state operations. - /// Can be overridden by derived classes to use a different synchronization mechanism. - /// - protected virtual SemaphoreSlim StateLock => _stateLock; - /// public abstract string? StateFilePath { get; } @@ -155,7 +149,7 @@ private static void FlattenJsonObjectRecursive(JsonObject source, string prefix, /// The loaded state as a JsonObject. protected async Task LoadStateAsync(CancellationToken cancellationToken = default) { - await StateLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await _stateLock.WaitAsync(cancellationToken).ConfigureAwait(false); try { if (_isStateLoaded && _state is not null) @@ -187,21 +181,21 @@ protected async Task LoadStateAsync(CancellationToken cancellationTo } finally { - StateLock.Release(); + _stateLock.Release(); } } /// public async Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default) { - await StateLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await _stateLock.WaitAsync(cancellationToken).ConfigureAwait(false); try { await SaveStateToStorageAsync(state, cancellationToken).ConfigureAwait(false); } finally { - StateLock.Release(); + _stateLock.Release(); } } @@ -225,8 +219,8 @@ public async Task AcquireSectionAsync(string sectionName var metadata = GetSectionMetadata(sectionName); - // Protect access to _state with StateLock to prevent concurrent modification during enumeration - await StateLock.WaitAsync(cancellationToken).ConfigureAwait(false); + // Protect access to _state with _stateLock to prevent concurrent modification during enumeration + await _stateLock.WaitAsync(cancellationToken).ConfigureAwait(false); try { var sectionData = _state?.TryGetPropertyValue(sectionName, out var sectionNode) == true && sectionNode is JsonObject obj @@ -237,7 +231,7 @@ public async Task AcquireSectionAsync(string sectionName } finally { - StateLock.Release(); + _stateLock.Release(); } } @@ -274,7 +268,7 @@ public async Task SaveSectionAsync(DeploymentStateSection section, CancellationT section.Version++; // Serialize state modification and file write to prevent concurrent enumeration - await StateLock.WaitAsync(cancellationToken).ConfigureAwait(false); + await _stateLock.WaitAsync(cancellationToken).ConfigureAwait(false); try { // Store a deep clone to ensure immutability @@ -283,7 +277,7 @@ public async Task SaveSectionAsync(DeploymentStateSection section, CancellationT } finally { - StateLock.Release(); + _stateLock.Release(); } } } diff --git a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs index dea3044e1c5..e15f145adfe 100644 --- a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs @@ -6,38 +6,33 @@ using System.Reflection; using System.Text.Json; using System.Text.Json.Nodes; +using Aspire.Hosting.UserSecrets; using Microsoft.Extensions.Configuration.UserSecrets; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.SecretManager.Tools.Internal; namespace Aspire.Hosting.Publishing.Internal; /// /// User secrets implementation of . /// -public sealed class UserSecretsDeploymentStateManager(ILogger logger) : DeploymentStateManagerBase(logger) +public sealed class UserSecretsDeploymentStateManager : DeploymentStateManagerBase { - private SemaphoreSlim? _sharedSemaphore; - - /// - public override string? StateFilePath => GetStatePath(); + private readonly IUserSecretsManager? _userSecretsManager; /// - /// Gets the semaphore used for synchronizing state operations. - /// Uses the shared UserSecretsFileLock semaphore to coordinate with SecretsStore. + /// Initializes a new instance of the class. /// - protected override SemaphoreSlim StateLock + /// The logger. + /// Optional service provider to resolve IUserSecretsManager. + public UserSecretsDeploymentStateManager(ILogger logger, IServiceProvider? serviceProvider = null) + : base(logger) { - get - { - if (_sharedSemaphore == null && StateFilePath != null) - { - _sharedSemaphore = UserSecretsFileLock.GetSemaphore(StateFilePath); - } - return _sharedSemaphore ?? base.StateLock; - } + _userSecretsManager = serviceProvider?.GetService(typeof(IUserSecretsManager)) as IUserSecretsManager; } + /// + public override string? StateFilePath => _userSecretsManager?.FilePath ?? GetStatePath(); + /// protected override string? GetStatePath() { @@ -53,13 +48,21 @@ protected override async Task SaveStateToStorageAsync(JsonObject state, Cancella { try { - var userSecretsPath = GetStatePath() ?? throw new InvalidOperationException("User secrets path could not be determined."); - var flattenedUserSecrets = DeploymentStateManagerBase.FlattenJsonObject(state); - Directory.CreateDirectory(Path.GetDirectoryName(userSecretsPath)!); - - var json = flattenedUserSecrets.ToJsonString(s_jsonSerializerOptions); - - await File.WriteAllTextAsync(userSecretsPath, json, System.Text.Encoding.UTF8, cancellationToken).ConfigureAwait(false); + if (_userSecretsManager != null) + { + // Use the shared manager which handles locking + await _userSecretsManager.SaveStateAsync(state, cancellationToken).ConfigureAwait(false); + } + else + { + // Fallback to direct file write if no manager is available + var userSecretsPath = GetStatePath() ?? throw new InvalidOperationException("User secrets path could not be determined."); + var flattenedUserSecrets = DeploymentStateManagerBase.FlattenJsonObject(state); + Directory.CreateDirectory(Path.GetDirectoryName(userSecretsPath)!); + + var json = flattenedUserSecrets.ToJsonString(s_jsonSerializerOptions); + await File.WriteAllTextAsync(userSecretsPath, json, System.Text.Encoding.UTF8, cancellationToken).ConfigureAwait(false); + } logger.LogInformation("Azure resource connection strings saved to user secrets."); } diff --git a/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs index 8ea2c2d3a25..ae04a882db9 100644 --- a/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs +++ b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Text.Json.Nodes; +using Microsoft.Extensions.Configuration; namespace Aspire.Hosting.UserSecrets; @@ -13,7 +14,7 @@ internal interface IUserSecretsManager /// /// Gets the path to the user secrets file. /// - string? FilePath { get; } + string FilePath { get; } /// /// Attempts to set a user secret value synchronously. @@ -33,17 +34,17 @@ internal interface IUserSecretsManager Task TrySetSecretAsync(string name, string value, CancellationToken cancellationToken = default); /// - /// Gets a secret value if it exists, or sets it using the value generator if it doesn't. + /// Gets a secret value if it exists in configuration, or sets it using the value generator if it doesn't. /// + /// The configuration manager to check and update. /// The name of the secret. /// Function to generate the value if it doesn't exist. - /// The secret value. - string GetOrSetSecret(string name, Func valueGenerator); + void GetOrSetSecret(IConfigurationManager configuration, string name, Func valueGenerator); /// /// Saves state to user secrets asynchronously (for deployment state manager). /// - /// The state to save. + /// The state to save as a JSON object. /// Cancellation token. Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default); } diff --git a/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs new file mode 100644 index 00000000000..5aaf7537ad1 --- /dev/null +++ b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs @@ -0,0 +1,257 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; +using Aspire.Hosting.Publishing.Internal; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Configuration.UserSecrets; + +namespace Aspire.Hosting.UserSecrets; + +/// +/// Factory for creating and caching IUserSecretsManager instances. +/// Uses ConditionalWeakTable to allow instances to be shared across assemblies while still being GC-friendly. +/// +internal sealed class UserSecretsManagerFactory +{ + // Singleton instance + public static readonly UserSecretsManagerFactory Instance = new(); + + // Use ConditionalWeakTable to cache instances by file path + // This allows the same instance to be reused while still being GC-friendly + private readonly ConditionalWeakTable _managerCache = new(); + + // Semaphores are stored separately and never GC'd to ensure thread safety + private readonly Dictionary _semaphores = new(); + private readonly object _semaphoresLock = new(); + + private UserSecretsManagerFactory() + { + } + + /// + /// Gets or creates a user secrets manager for the specified file path. + /// + public IUserSecretsManager GetOrCreate(string filePath) + { + ArgumentException.ThrowIfNullOrWhiteSpace(filePath); + + var normalizedPath = Path.GetFullPath(filePath); + + return _managerCache.GetValue(normalizedPath, path => + { + var semaphore = GetSemaphore(path); + return new UserSecretsManager(path, semaphore); + }); + } + + /// + /// Gets or creates a user secrets manager for the specified user secrets ID. + /// + public IUserSecretsManager? GetOrCreateFromId(string? userSecretsId) + { + if (string.IsNullOrWhiteSpace(userSecretsId)) + { + return null; + } + + var filePath = UserSecretsPathHelper.GetSecretsPathFromSecretsId(userSecretsId); + return GetOrCreate(filePath); + } + + /// + /// Gets or creates a user secrets manager for the assembly with UserSecretsIdAttribute. + /// + public IUserSecretsManager? GetOrCreate(Assembly? assembly) + { + var userSecretsId = assembly?.GetCustomAttribute()?.UserSecretsId; + return GetOrCreateFromId(userSecretsId); + } + + private SemaphoreSlim GetSemaphore(string filePath) + { + lock (_semaphoresLock) + { + if (!_semaphores.TryGetValue(filePath, out var semaphore)) + { + semaphore = new SemaphoreSlim(1, 1); + _semaphores[filePath] = semaphore; + } + return semaphore; + } + } + + private sealed class UserSecretsManager : IUserSecretsManager + { + private static readonly JsonSerializerOptions s_jsonSerializerOptions = new() { WriteIndented = true }; + private readonly SemaphoreSlim _semaphore; + + public UserSecretsManager(string filePath, SemaphoreSlim semaphore) + { + FilePath = filePath; + _semaphore = semaphore; + } + + public string FilePath { get; } + + public bool TrySetSecret(string name, string value) + { + try + { + _semaphore.Wait(); + try + { + SetSecretCore(name, value); + return true; + } + finally + { + _semaphore.Release(); + } + } + catch (Exception) + { + return false; + } + } + + public async Task TrySetSecretAsync(string name, string value, CancellationToken cancellationToken = default) + { + try + { + await _semaphore.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + await SetSecretCoreAsync(name, value, cancellationToken).ConfigureAwait(false); + return true; + } + finally + { + _semaphore.Release(); + } + } + catch (Exception) + { + return false; + } + } + + public void GetOrSetSecret(IConfigurationManager configuration, string name, Func valueGenerator) + { + var existingValue = configuration[name]; + if (existingValue is null) + { + var value = valueGenerator(); + configuration.AddInMemoryCollection( + new Dictionary + { + [name] = value + } + ); + if (!TrySetSecret(name, value)) + { + Debug.WriteLine($"Failed to save value to application user secrets."); + } + } + } + + public async Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default) + { + await _semaphore.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + var flattenedState = DeploymentStateManagerBase.FlattenJsonObject(state); + EnsureUserSecretsDirectory(); + + var json = flattenedState.ToJsonString(s_jsonSerializerOptions); + await File.WriteAllTextAsync(FilePath, json, Encoding.UTF8, cancellationToken).ConfigureAwait(false); + } + finally + { + _semaphore.Release(); + } + } + + private void SetSecretCore(string name, string value) + { + EnsureUserSecretsDirectory(); + + // Load existing secrets, merge with new value, save + var secrets = Load(); + secrets[name] = value; + Save(secrets); + } + + private async Task SetSecretCoreAsync(string name, string value, CancellationToken cancellationToken) + { + EnsureUserSecretsDirectory(); + + // Load existing secrets, merge with new value, save + var secrets = Load(); + secrets[name] = value; + await SaveAsync(secrets, cancellationToken).ConfigureAwait(false); + } + + private Dictionary Load() + { + return new ConfigurationBuilder() + .AddJsonFile(FilePath, optional: true) + .Build() + .AsEnumerable() + .Where(i => i.Value != null) + .ToDictionary(i => i.Key, i => i.Value, StringComparer.OrdinalIgnoreCase); + } + + private void Save(Dictionary secrets) + { + var contents = new JsonObject(); + foreach (var secret in secrets) + { + contents[secret.Key] = secret.Value; + } + + // Create a temp file with the correct Unix file mode before moving it to the expected path. + if (!OperatingSystem.IsWindows()) + { + var tempFilename = Path.GetTempFileName(); + File.Move(tempFilename, FilePath, overwrite: true); + } + + var json = contents.ToJsonString(s_jsonSerializerOptions); + File.WriteAllText(FilePath, json, Encoding.UTF8); + } + + private async Task SaveAsync(Dictionary secrets, CancellationToken cancellationToken) + { + var contents = new JsonObject(); + foreach (var secret in secrets) + { + contents[secret.Key] = secret.Value; + } + + // Create a temp file with the correct Unix file mode before moving it to the expected path. + if (!OperatingSystem.IsWindows()) + { + var tempFilename = Path.GetTempFileName(); + File.Move(tempFilename, FilePath, overwrite: true); + } + + var json = contents.ToJsonString(s_jsonSerializerOptions); + await File.WriteAllTextAsync(FilePath, json, Encoding.UTF8, cancellationToken).ConfigureAwait(false); + } + + private void EnsureUserSecretsDirectory() + { + var directoryName = Path.GetDirectoryName(FilePath); + if (!string.IsNullOrEmpty(directoryName) && !Directory.Exists(directoryName)) + { + Directory.CreateDirectory(directoryName); + } + } + } +} diff --git a/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs b/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs index 299111a7185..51fbfb906e6 100644 --- a/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs +++ b/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs @@ -7,11 +7,11 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using Aspire.Hosting.Resources; +using Aspire.Hosting.UserSecrets; using Aspire.Shared; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.SecretManager.Tools.Internal; using Semver; namespace Aspire.Hosting.VersionChecking; @@ -32,10 +32,12 @@ internal sealed class VersionCheckService : BackgroundService private readonly DistributedApplicationExecutionContext _executionContext; private readonly TimeProvider _timeProvider; private readonly SemVersion? _appHostVersion; + private readonly IUserSecretsManager? _userSecretsManager; public VersionCheckService(IInteractionService interactionService, ILogger logger, IConfiguration configuration, DistributedApplicationOptions options, IPackageFetcher packageFetcher, - DistributedApplicationExecutionContext executionContext, TimeProvider timeProvider, IPackageVersionProvider packageVersionProvider) + DistributedApplicationExecutionContext executionContext, TimeProvider timeProvider, IPackageVersionProvider packageVersionProvider, + IUserSecretsManager? userSecretsManager = null) { _interactionService = interactionService; _logger = logger; @@ -44,6 +46,7 @@ public VersionCheckService(IInteractionService interactionService, ILogger -/// Adapted from dotnet user-secrets at https://github.com/dotnet/aspnetcore/blob/482730a4c773ee4b3ae9525186d10999c89b556d/src/Tools/dotnet-user-secrets/src/Internal/SecretsStore.cs -/// -internal sealed class SecretsStore -{ - private readonly string _secretsFilePath; - private readonly Dictionary _secrets; - - public SecretsStore(string userSecretsId) - { - ArgumentNullException.ThrowIfNull(userSecretsId); - - _secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); - - EnsureUserSecretsDirectory(); - - _secrets = Load(_secretsFilePath); - } - - public string? this[string key] => _secrets[key]; - - public int Count => _secrets.Count; - - // For testing. - internal string SecretsFilePath => _secretsFilePath; - - public bool ContainsKey(string key) => _secrets.ContainsKey(key); - - public IEnumerable> AsEnumerable() => _secrets; - - public void Clear() => _secrets.Clear(); - - public void Set(string key, string value) => _secrets[key] = value; - - public bool Remove(string key) => _secrets.Remove(key); - - public void Save() - { - var semaphore = UserSecretsFileLock.GetSemaphore(_secretsFilePath); - semaphore.Wait(); - try - { - // Reload from disk to merge with any concurrent changes - var currentSecrets = Load(_secretsFilePath); - - // Merge our changes with what's on disk - foreach (var kvp in _secrets) - { - currentSecrets[kvp.Key] = kvp.Value; - } - - EnsureUserSecretsDirectory(); - - var contents = new JsonObject(); - foreach (var secret in currentSecrets) - { - contents[secret.Key] = secret.Value; - } - - // Create a temp file with the correct Unix file mode before moving it to the expected _filePath. - if (!OperatingSystem.IsWindows()) - { - var tempFilename = Path.GetTempFileName(); - File.Move(tempFilename, _secretsFilePath, overwrite: true); - } - - var json = contents.ToJsonString(new() - { - WriteIndented = true - }); - - File.WriteAllText(_secretsFilePath, json, Encoding.UTF8); - } - finally - { - semaphore.Release(); - } - } - - private void EnsureUserSecretsDirectory() - { - EnsureUserSecretsDirectory(_secretsFilePath); - } - - private static Dictionary Load(string secretsFilePath) - { - return new ConfigurationBuilder() - .AddJsonFile(secretsFilePath, optional: true) - .Build() - .AsEnumerable() - .Where(i => i.Value != null) - .ToDictionary(i => i.Key, i => i.Value, StringComparer.OrdinalIgnoreCase); - } - - /// - /// Sets a value in user secrets for the project associated with the given assembly if it's not already present in configuration. - /// - public static void GetOrSetUserSecret(IConfigurationManager configuration, Assembly? appHostAssembly, string name, Func valueGenerator) - { - var existingValue = configuration[name]; - if (existingValue is null) - { - var value = valueGenerator(); - configuration.AddInMemoryCollection( - new Dictionary - { - [name] = value - } - ); - if (!TrySetUserSecret(appHostAssembly, name, value)) - { - // This is a best-effort operation, so we don't throw if it fails. Common reason for failure is that the user secrets ID is not set - // in the application's assembly. Note there's no ILogger available this early in the application lifecycle. - Debug.WriteLine($"Failed to save value to application user secrets."); - } - } - } - - /// - /// Attempts to save a user secret value for the project associated with the given assembly. Returns a boolean indicating - /// success or failure. If the assembly does not have a , or if the user secrets store - /// save operation fails, this method will return false. - /// - public static bool TrySetUserSecret(Assembly? assembly, string name, string value) - { - if (assembly?.GetCustomAttribute()?.UserSecretsId is { } userSecretsId) - { - // Save the value to the secret store - try - { - var secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); - var semaphore = UserSecretsFileLock.GetSemaphore(secretsFilePath); - semaphore.Wait(); - try - { - // Load, set, and save in one atomic operation to ensure thread safety - EnsureUserSecretsDirectory(secretsFilePath); - - var secrets = Load(secretsFilePath); - secrets[name] = value; - - var contents = new JsonObject(); - foreach (var secret in secrets) - { - contents[secret.Key] = secret.Value; - } - - // Create a temp file with the correct Unix file mode before moving it to the expected _filePath. - if (!OperatingSystem.IsWindows()) - { - var tempFilename = Path.GetTempFileName(); - File.Move(tempFilename, secretsFilePath, overwrite: true); - } - - var json = contents.ToJsonString(new() - { - WriteIndented = true - }); - - File.WriteAllText(secretsFilePath, json, Encoding.UTF8); - return true; - } - finally - { - semaphore.Release(); - } - } - catch (Exception) { } // Ignore user secret store errors - } - - return false; - } - - private static void EnsureUserSecretsDirectory(string secretsFilePath) - { - var directoryName = Path.GetDirectoryName(secretsFilePath); - if (!string.IsNullOrEmpty(directoryName) && !Directory.Exists(directoryName)) - { - Directory.CreateDirectory(directoryName); - } - } -} diff --git a/src/Shared/UserSecretsFileLock.cs b/src/Shared/UserSecretsFileLock.cs deleted file mode 100644 index 9d6d85c7226..00000000000 --- a/src/Shared/UserSecretsFileLock.cs +++ /dev/null @@ -1,38 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Microsoft.Extensions.SecretManager.Tools.Internal; - -/// -/// Provides synchronized access to user secrets files within the same process. -/// Both SecretsStore and UserSecretsDeploymentStateManager use this to ensure writes are serialized. -/// -internal static class UserSecretsFileLock -{ - // Static semaphore dictionary to synchronize access per file path - private static readonly Dictionary s_semaphores = new(); - private static readonly object s_semaphoresLock = new(); - - /// - /// Gets a semaphore for the specified user secrets file path. - /// - /// The full path to the user secrets file. - /// A SemaphoreSlim that can be used for synchronization. - public static SemaphoreSlim GetSemaphore(string filePath) - { - ArgumentNullException.ThrowIfNull(filePath); - - // Normalize the path to ensure consistent locking across different path representations - var normalizedPath = Path.GetFullPath(filePath); - - lock (s_semaphoresLock) - { - if (!s_semaphores.TryGetValue(normalizedPath, out var semaphore)) - { - semaphore = new SemaphoreSlim(1, 1); - s_semaphores[normalizedPath] = semaphore; - } - return semaphore; - } - } -} diff --git a/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs b/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs index 122bc3e3e9e..e89edddbf48 100644 --- a/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs +++ b/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs @@ -3,9 +3,10 @@ using System.Reflection; using System.Reflection.Emit; +using Aspire.Hosting.Publishing.Internal; +using Aspire.Hosting.UserSecrets; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.UserSecrets; -using Microsoft.Extensions.SecretManager.Tools.Internal; namespace Aspire.Hosting.Tests; @@ -26,7 +27,8 @@ public void GetOrSetUserSecret_SavesValueToUserSecrets() var configuration = new ConfigurationManager(); var value = TokenGenerator.GenerateToken(); - SecretsStore.GetOrSetUserSecret(configuration, testAssembly, key, () => value); + var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + manager?.GetOrSetSecret(configuration, key, () => value); var userSecrets = GetUserSecrets(userSecretsId); var configValue = configuration[key]; @@ -50,7 +52,8 @@ public void GetOrSetUserSecret_ReadsValueFromConfiguration() var valueInConfig = TokenGenerator.GenerateToken(); configuration[key] = valueInConfig; - SecretsStore.GetOrSetUserSecret(configuration, testAssembly, key, TokenGenerator.GenerateToken); + var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + manager?.GetOrSetSecret(configuration, key, TokenGenerator.GenerateToken); var userSecrets = GetUserSecrets(userSecretsId); Assert.False(userSecrets.TryGetValue(key, out var savedValue)); @@ -60,20 +63,33 @@ public void GetOrSetUserSecret_ReadsValueFromConfiguration() private static Dictionary GetUserSecrets(string userSecretsId) { - var secretsStore = new SecretsStore(userSecretsId); - return secretsStore.AsEnumerable().ToDictionary(kvp => kvp.Key, kvp => kvp.Value); + var manager = UserSecretsManagerFactory.Instance.GetOrCreateFromId(userSecretsId); + if (manager == null || !File.Exists(manager.FilePath)) + { + return new Dictionary(); + } + + var config = new ConfigurationBuilder() + .AddJsonFile(manager.FilePath, optional: true) + .Build(); + + return config.AsEnumerable() + .Where(kvp => kvp.Value != null) + .ToDictionary(kvp => kvp.Key, kvp => kvp.Value); } private static void ClearUsersSecrets(string userSecretsId) { - var secretsStore = new SecretsStore(userSecretsId); - secretsStore.Clear(); - secretsStore.Save(); + var filePath = UserSecretsPathHelper.GetSecretsPathFromSecretsId(userSecretsId); + if (File.Exists(filePath)) + { + File.Delete(filePath); + } } private static void DeleteUserSecretsFile(string userSecretsId) { - var userSecretsPath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); + var userSecretsPath = UserSecretsPathHelper.GetSecretsPathFromSecretsId(userSecretsId); if (File.Exists(userSecretsPath)) { File.Delete(userSecretsPath); diff --git a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs index f17df712a80..1ec3dd12476 100644 --- a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs +++ b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs @@ -5,8 +5,10 @@ using System.Reflection.Emit; using System.Text; using Aspire.Hosting.Publishing; +using Aspire.Hosting.Publishing.Internal; +using Aspire.Hosting.UserSecrets; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.UserSecrets; -using Microsoft.Extensions.SecretManager.Tools.Internal; namespace Aspire.Hosting.Tests; @@ -94,7 +96,11 @@ public async Task TrySetUserSecret_ConcurrentWrites_PreservesAllSecrets() { var key = kvp.Key; var value = kvp.Value; - tasks.Add(Task.Run(() => SecretsStore.TrySetUserSecret(testAssembly, key, value))); + tasks.Add(Task.Run(() => + { + var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + return manager?.TrySetSecret(key, value) ?? false; + })); } var results = await Task.WhenAll(tasks); @@ -124,8 +130,16 @@ public async Task TrySetUserSecret_SqlServerAndRabbitMQ_BothSecretsPreserved() new("TestAssembly"), AssemblyBuilderAccess.RunAndCollect, [new CustomAttributeBuilder(s_userSecretsIdAttrCtor, [userSecretsId])]); // Simulate SQL Server and RabbitMQ generating passwords concurrently - var sqlTask = Task.Run(() => SecretsStore.TrySetUserSecret(testAssembly, "Parameters:sql-password", "SqlPassword123!")); - var rabbitTask = Task.Run(() => SecretsStore.TrySetUserSecret(testAssembly, "Parameters:rabbit-password", "RabbitPassword456!")); + var sqlTask = Task.Run(() => + { + var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + return manager?.TrySetSecret("Parameters:sql-password", "SqlPassword123!") ?? false; + }); + var rabbitTask = Task.Run(() => + { + var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + return manager?.TrySetSecret("Parameters:rabbit-password", "RabbitPassword456!") ?? false; + }); var results = await Task.WhenAll(sqlTask, rabbitTask); @@ -156,7 +170,11 @@ public async Task TrySetUserSecret_ConcurrentWritesSameKey_LastWriteWins() for (int i = 0; i < 10; i++) { var value = $"Value{i}"; - tasks.Add(Task.Run(() => SecretsStore.TrySetUserSecret(testAssembly, "Parameters:test-key", value))); + tasks.Add(Task.Run(() => + { + var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + return manager?.TrySetSecret("Parameters:test-key", value) ?? false; + })); } var results = await Task.WhenAll(tasks); @@ -183,15 +201,42 @@ private static void EnsureUserSecretsDirectory(string secretsFilePath) private static Dictionary GetUserSecrets(string userSecretsId) { - var secretsStore = new SecretsStore(userSecretsId); - return secretsStore.AsEnumerable().ToDictionary(kvp => kvp.Key, kvp => kvp.Value); + var manager = UserSecretsManagerFactory.Instance.GetOrCreateFromId(userSecretsId); + if (manager == null) + { + return new Dictionary(); + } + + // Read the secrets file directly + var secrets = new Dictionary(); + if (File.Exists(manager.FilePath)) + { + var json = File.ReadAllText(manager.FilePath); + if (!string.IsNullOrWhiteSpace(json)) + { + var config = new ConfigurationBuilder() + .AddJsonFile(manager.FilePath, optional: true) + .Build(); + + foreach (var kvp in config.AsEnumerable()) + { + if (kvp.Value != null) + { + secrets[kvp.Key] = kvp.Value; + } + } + } + } + return secrets; } private static void ClearUsersSecrets(string userSecretsId) { - var secretsStore = new SecretsStore(userSecretsId); - secretsStore.Clear(); - secretsStore.Save(); + var filePath = UserSecretsPathHelper.GetSecretsPathFromSecretsId(userSecretsId); + if (File.Exists(filePath)) + { + File.Delete(filePath); + } } private static void DeleteUserSecretsFile(string userSecretsId) From 6308f05f84ca260876ac657e19edab024fcd9be8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 07:03:31 +0000 Subject: [PATCH 10/16] Change UserSecretsDeploymentStateManager to take IUserSecretsManager directly instead of IServiceProvider Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../Internal/UserSecretsDeploymentStateManager.cs | 6 +++--- src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs index e15f145adfe..0bb53cffc31 100644 --- a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs @@ -23,11 +23,11 @@ public sealed class UserSecretsDeploymentStateManager : DeploymentStateManagerBa /// Initializes a new instance of the class. /// /// The logger. - /// Optional service provider to resolve IUserSecretsManager. - public UserSecretsDeploymentStateManager(ILogger logger, IServiceProvider? serviceProvider = null) + /// Optional user secrets manager for managing secrets. + public UserSecretsDeploymentStateManager(ILogger logger, IUserSecretsManager? userSecretsManager = null) : base(logger) { - _userSecretsManager = serviceProvider?.GetService(typeof(IUserSecretsManager)) as IUserSecretsManager; + _userSecretsManager = userSecretsManager; } /// diff --git a/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs index ae04a882db9..1de9549bc95 100644 --- a/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs +++ b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs @@ -9,7 +9,7 @@ namespace Aspire.Hosting.UserSecrets; /// /// Manages user secrets for an application, providing thread-safe read and write operations. /// -internal interface IUserSecretsManager +public interface IUserSecretsManager { /// /// Gets the path to the user secrets file. From f557876aa56aabba2c870bd04bed2c65f591e92c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 07:13:05 +0000 Subject: [PATCH 11/16] Make IUserSecretsManager and UserSecretsDeploymentStateManager internal Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../Publishing/Internal/UserSecretsDeploymentStateManager.cs | 2 +- src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs index 0bb53cffc31..46a34a01d3d 100644 --- a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs @@ -15,7 +15,7 @@ namespace Aspire.Hosting.Publishing.Internal; /// /// User secrets implementation of . /// -public sealed class UserSecretsDeploymentStateManager : DeploymentStateManagerBase +internal sealed class UserSecretsDeploymentStateManager : DeploymentStateManagerBase { private readonly IUserSecretsManager? _userSecretsManager; diff --git a/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs index 1de9549bc95..ae04a882db9 100644 --- a/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs +++ b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs @@ -9,7 +9,7 @@ namespace Aspire.Hosting.UserSecrets; /// /// Manages user secrets for an application, providing thread-safe read and write operations. /// -public interface IUserSecretsManager +internal interface IUserSecretsManager { /// /// Gets the path to the user secrets file. From 65060b484acfb27e1abeb230dc09c59b7eef7d15 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 07:32:59 +0000 Subject: [PATCH 12/16] Make IUserSecretsManager injectable via constructor with factory fallback and add Create methods for testing Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../UserSecretsParameterDefault.cs | 13 ++++++- .../UserSecretsDeploymentStateManager.cs | 26 +++---------- .../UserSecrets/UserSecretsManagerFactory.cs | 38 +++++++++++++++++++ .../VersionChecking/VersionCheckService.cs | 10 ++--- .../Aspire.Hosting.Tests/SecretsStoreTests.cs | 6 +-- .../UserSecretsParameterDefaultTests.cs | 8 ++-- .../VersionCheckServiceTests.cs | 23 ++++++++++- 7 files changed, 88 insertions(+), 36 deletions(-) diff --git a/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs b/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs index 9d414087dfa..fdd0eea8687 100644 --- a/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs +++ b/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs @@ -15,16 +15,25 @@ namespace Aspire.Hosting.ApplicationModel; /// The application name. /// The parameter name. /// The that will produce the default value when it isn't found in the project's user secrets store. -internal sealed class UserSecretsParameterDefault(Assembly appHostAssembly, string applicationName, string parameterName, ParameterDefault parameterDefault) +/// The factory to use for creating user secrets managers. +internal sealed class UserSecretsParameterDefault(Assembly appHostAssembly, string applicationName, string parameterName, ParameterDefault parameterDefault, UserSecretsManagerFactory factory) : ParameterDefault { + /// + /// Initializes a new instance of the class using the default factory. + /// + public UserSecretsParameterDefault(Assembly appHostAssembly, string applicationName, string parameterName, ParameterDefault parameterDefault) + : this(appHostAssembly, applicationName, parameterName, parameterDefault, UserSecretsManagerFactory.Instance) + { + } + /// public override string GetDefaultValue() { var value = parameterDefault.GetDefaultValue(); var configurationKey = $"Parameters:{parameterName}"; - var manager = UserSecretsManagerFactory.Instance.GetOrCreate(appHostAssembly); + var manager = factory.GetOrCreate(appHostAssembly); if (manager == null || !manager.TrySetSecret(configurationKey, value)) { // This is a best-effort operation, so we don't throw if it fails. Common reason for failure is that the user secrets ID is not set diff --git a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs index 46a34a01d3d..a82206ea434 100644 --- a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs @@ -17,21 +17,21 @@ namespace Aspire.Hosting.Publishing.Internal; /// internal sealed class UserSecretsDeploymentStateManager : DeploymentStateManagerBase { - private readonly IUserSecretsManager? _userSecretsManager; + private readonly IUserSecretsManager _userSecretsManager; /// /// Initializes a new instance of the class. /// /// The logger. - /// Optional user secrets manager for managing secrets. - public UserSecretsDeploymentStateManager(ILogger logger, IUserSecretsManager? userSecretsManager = null) + /// User secrets manager for managing secrets. + public UserSecretsDeploymentStateManager(ILogger logger, IUserSecretsManager userSecretsManager) : base(logger) { _userSecretsManager = userSecretsManager; } /// - public override string? StateFilePath => _userSecretsManager?.FilePath ?? GetStatePath(); + public override string? StateFilePath => _userSecretsManager.FilePath; /// protected override string? GetStatePath() @@ -48,22 +48,8 @@ protected override async Task SaveStateToStorageAsync(JsonObject state, Cancella { try { - if (_userSecretsManager != null) - { - // Use the shared manager which handles locking - await _userSecretsManager.SaveStateAsync(state, cancellationToken).ConfigureAwait(false); - } - else - { - // Fallback to direct file write if no manager is available - var userSecretsPath = GetStatePath() ?? throw new InvalidOperationException("User secrets path could not be determined."); - var flattenedUserSecrets = DeploymentStateManagerBase.FlattenJsonObject(state); - Directory.CreateDirectory(Path.GetDirectoryName(userSecretsPath)!); - - var json = flattenedUserSecrets.ToJsonString(s_jsonSerializerOptions); - await File.WriteAllTextAsync(userSecretsPath, json, System.Text.Encoding.UTF8, cancellationToken).ConfigureAwait(false); - } - + // Use the shared manager which handles locking + await _userSecretsManager.SaveStateAsync(state, cancellationToken).ConfigureAwait(false); logger.LogInformation("Azure resource connection strings saved to user secrets."); } catch (JsonException ex) diff --git a/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs index 5aaf7537ad1..d31db76270f 100644 --- a/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs +++ b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs @@ -73,6 +73,44 @@ public IUserSecretsManager GetOrCreate(string filePath) return GetOrCreateFromId(userSecretsId); } + /// + /// Creates a new user secrets manager for the specified file path without caching. + /// This method is intended for testing scenarios where isolation between tests is required. + /// + public IUserSecretsManager Create(string filePath) + { + ArgumentException.ThrowIfNullOrWhiteSpace(filePath); + + var normalizedPath = Path.GetFullPath(filePath); + var semaphore = GetSemaphore(normalizedPath); + return new UserSecretsManager(normalizedPath, semaphore); + } + + /// + /// Creates a new user secrets manager for the specified user secrets ID without caching. + /// This method is intended for testing scenarios where isolation between tests is required. + /// + public IUserSecretsManager? CreateFromId(string? userSecretsId) + { + if (string.IsNullOrWhiteSpace(userSecretsId)) + { + return null; + } + + var filePath = UserSecretsPathHelper.GetSecretsPathFromSecretsId(userSecretsId); + return Create(filePath); + } + + /// + /// Creates a new user secrets manager for the assembly with UserSecretsIdAttribute without caching. + /// This method is intended for testing scenarios where isolation between tests is required. + /// + public IUserSecretsManager? Create(Assembly? assembly) + { + var userSecretsId = assembly?.GetCustomAttribute()?.UserSecretsId; + return CreateFromId(userSecretsId); + } + private SemaphoreSlim GetSemaphore(string filePath) { lock (_semaphoresLock) diff --git a/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs b/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs index 51fbfb906e6..97acb8f83b2 100644 --- a/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs +++ b/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs @@ -32,12 +32,12 @@ internal sealed class VersionCheckService : BackgroundService private readonly DistributedApplicationExecutionContext _executionContext; private readonly TimeProvider _timeProvider; private readonly SemVersion? _appHostVersion; - private readonly IUserSecretsManager? _userSecretsManager; + private readonly IUserSecretsManager _userSecretsManager; public VersionCheckService(IInteractionService interactionService, ILogger logger, IConfiguration configuration, DistributedApplicationOptions options, IPackageFetcher packageFetcher, DistributedApplicationExecutionContext executionContext, TimeProvider timeProvider, IPackageVersionProvider packageVersionProvider, - IUserSecretsManager? userSecretsManager = null) + IUserSecretsManager userSecretsManager) { _interactionService = interactionService; _logger = logger; @@ -102,7 +102,7 @@ private async Task CheckForLatestAsync(CancellationToken cancellationToken) { var appHostDirectory = _configuration["AppHost:Directory"]!; - _userSecretsManager?.TrySetSecret(LastCheckDateKey, now.ToString("o", CultureInfo.InvariantCulture)); + _userSecretsManager.TrySetSecret(LastCheckDateKey, now.ToString("o", CultureInfo.InvariantCulture)); packages = await _packageFetcher.TryFetchPackagesAsync(appHostDirectory, cancellationToken).ConfigureAwait(false); } else @@ -133,7 +133,7 @@ private async Task CheckForLatestAsync(CancellationToken cancellationToken) if (IsVersionGreater(latestVersion, storedKnownLatestVersion) || storedKnownLatestVersion == null) { // Latest version is greater than the stored known latest version, so update it. - _userSecretsManager?.TrySetSecret(KnownLatestVersionKey, latestVersion.ToString()); + _userSecretsManager.TrySetSecret(KnownLatestVersionKey, latestVersion.ToString()); } var result = await _interactionService.PromptNotificationAsync( @@ -151,7 +151,7 @@ private async Task CheckForLatestAsync(CancellationToken cancellationToken) if (result.Data) { _logger.LogDebug("User chose to ignore version {Version}.", latestVersion); - _userSecretsManager?.TrySetSecret(IgnoreVersionKey, latestVersion.ToString()); + _userSecretsManager.TrySetSecret(IgnoreVersionKey, latestVersion.ToString()); } } diff --git a/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs b/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs index e89edddbf48..dfec357c4d0 100644 --- a/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs +++ b/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs @@ -27,7 +27,7 @@ public void GetOrSetUserSecret_SavesValueToUserSecrets() var configuration = new ConfigurationManager(); var value = TokenGenerator.GenerateToken(); - var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + var manager = UserSecretsManagerFactory.Instance.Create(testAssembly); manager?.GetOrSetSecret(configuration, key, () => value); var userSecrets = GetUserSecrets(userSecretsId); @@ -52,7 +52,7 @@ public void GetOrSetUserSecret_ReadsValueFromConfiguration() var valueInConfig = TokenGenerator.GenerateToken(); configuration[key] = valueInConfig; - var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + var manager = UserSecretsManagerFactory.Instance.Create(testAssembly); manager?.GetOrSetSecret(configuration, key, TokenGenerator.GenerateToken); var userSecrets = GetUserSecrets(userSecretsId); @@ -63,7 +63,7 @@ public void GetOrSetUserSecret_ReadsValueFromConfiguration() private static Dictionary GetUserSecrets(string userSecretsId) { - var manager = UserSecretsManagerFactory.Instance.GetOrCreateFromId(userSecretsId); + var manager = UserSecretsManagerFactory.Instance.CreateFromId(userSecretsId); if (manager == null || !File.Exists(manager.FilePath)) { return new Dictionary(); diff --git a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs index 1ec3dd12476..95a78865c58 100644 --- a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs +++ b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs @@ -98,7 +98,7 @@ public async Task TrySetUserSecret_ConcurrentWrites_PreservesAllSecrets() var value = kvp.Value; tasks.Add(Task.Run(() => { - var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + var manager = UserSecretsManagerFactory.Instance.Create(testAssembly); return manager?.TrySetSecret(key, value) ?? false; })); } @@ -132,12 +132,12 @@ public async Task TrySetUserSecret_SqlServerAndRabbitMQ_BothSecretsPreserved() // Simulate SQL Server and RabbitMQ generating passwords concurrently var sqlTask = Task.Run(() => { - var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + var manager = UserSecretsManagerFactory.Instance.Create(testAssembly); return manager?.TrySetSecret("Parameters:sql-password", "SqlPassword123!") ?? false; }); var rabbitTask = Task.Run(() => { - var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + var manager = UserSecretsManagerFactory.Instance.Create(testAssembly); return manager?.TrySetSecret("Parameters:rabbit-password", "RabbitPassword456!") ?? false; }); @@ -172,7 +172,7 @@ public async Task TrySetUserSecret_ConcurrentWritesSameKey_LastWriteWins() var value = $"Value{i}"; tasks.Add(Task.Run(() => { - var manager = UserSecretsManagerFactory.Instance.GetOrCreate(testAssembly); + var manager = UserSecretsManagerFactory.Instance.Create(testAssembly); return manager?.TrySetSecret("Parameters:test-key", value) ?? false; })); } diff --git a/tests/Aspire.Hosting.Tests/VersionChecking/VersionCheckServiceTests.cs b/tests/Aspire.Hosting.Tests/VersionChecking/VersionCheckServiceTests.cs index fbfa5abdb7e..b78dc45d8c7 100644 --- a/tests/Aspire.Hosting.Tests/VersionChecking/VersionCheckServiceTests.cs +++ b/tests/Aspire.Hosting.Tests/VersionChecking/VersionCheckServiceTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Globalization; +using Aspire.Hosting.UserSecrets; using Aspire.Hosting.VersionChecking; using Aspire.Shared; using Microsoft.AspNetCore.InternalTesting; @@ -249,7 +250,8 @@ private static VersionCheckService CreateVersionCheckService( IConfiguration? configuration = null, TimeProvider? timeProvider = null, DistributedApplicationOptions? options = null, - IPackageVersionProvider? packageVersionProvider = null) + IPackageVersionProvider? packageVersionProvider = null, + IUserSecretsManager? userSecretsManager = null) { return new VersionCheckService( interactionService ?? new TestInteractionService(), @@ -259,7 +261,8 @@ private static VersionCheckService CreateVersionCheckService( packageFetcher ?? new TestPackageFetcher(), new DistributedApplicationExecutionContext(new DistributedApplicationOperation()), timeProvider ?? new TestTimeProvider(), - packageVersionProvider ?? new TestPackageVersionProvider()); + packageVersionProvider ?? new TestPackageVersionProvider(), + userSecretsManager ?? new TestUserSecretsManager()); } private sealed class TestTimeProvider : TimeProvider @@ -306,6 +309,22 @@ public Task> TryFetchPackagesAsync(string appHostDirectory, C return _versionTask; } } + + private sealed class TestUserSecretsManager : IUserSecretsManager + { + public string FilePath => Path.GetTempFileName(); + + public bool TrySetSecret(string name, string value) => true; + + public Task TrySetSecretAsync(string name, string value, CancellationToken cancellationToken = default) => Task.FromResult(true); + + public void GetOrSetSecret(IConfigurationManager configuration, string name, Func valueGenerator) + { + // No-op for tests + } + + public Task SaveStateAsync(System.Text.Json.Nodes.JsonObject state, CancellationToken cancellationToken = default) => Task.CompletedTask; + } } #pragma warning restore ASPIREINTERACTION001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed. From 998374d12281e561944464ae541536de010824c9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 08:42:53 +0000 Subject: [PATCH 13/16] Extract FlattenJsonObject to JsonFlattener static class to fix Azure test dependency Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../Internal/DeploymentStateManagerBase.cs | 69 +------------- .../Publishing/Internal/JsonFlattener.cs | 93 +++++++++++++++++++ .../UserSecrets/UserSecretsManagerFactory.cs | 2 +- .../DefaultUserSecretsManagerTests.cs | 16 ++-- 4 files changed, 104 insertions(+), 76 deletions(-) create mode 100644 src/Aspire.Hosting/Publishing/Internal/JsonFlattener.cs diff --git a/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs b/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs index 5d856f1acb7..2bb4b125765 100644 --- a/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs +++ b/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs @@ -67,12 +67,7 @@ private sealed class SectionMetadata(long version) /// /// The source JsonObject to flatten. /// A flattened JsonObject. - public static JsonObject FlattenJsonObject(JsonObject source) - { - var result = new JsonObject(); - FlattenJsonObjectRecursive(source, string.Empty, result); - return result; - } + public static JsonObject FlattenJsonObject(JsonObject source) => JsonFlattener.FlattenJsonObject(source); /// /// Unflattens a JsonObject that uses colon-separated keys back into a nested structure. @@ -80,67 +75,7 @@ public static JsonObject FlattenJsonObject(JsonObject source) /// /// The flattened JsonObject to unflatten. /// An unflattened JsonObject with nested structure. - public static JsonObject UnflattenJsonObject(JsonObject source) - { - var result = new JsonObject(); - - foreach (var kvp in source) - { - var keys = kvp.Key.Split(':'); - var current = result; - - for (var i = 0; i < keys.Length - 1; i++) - { - var key = keys[i]; - if (!current.TryGetPropertyValue(key, out var existing) || existing is not JsonObject) - { - var newObject = new JsonObject(); - current[key] = newObject; - current = newObject; - } - else - { - current = existing.AsObject(); - } - } - - current[keys[^1]] = kvp.Value?.DeepClone(); - } - - return result; - } - - private static void FlattenJsonObjectRecursive(JsonObject source, string prefix, JsonObject result) - { - foreach (var kvp in source) - { - var key = string.IsNullOrEmpty(prefix) ? kvp.Key : $"{prefix}:{kvp.Key}"; - - if (kvp.Value is JsonObject nestedObject) - { - FlattenJsonObjectRecursive(nestedObject, key, result); - } - else if (kvp.Value is JsonArray array) - { - for (var i = 0; i < array.Count; i++) - { - var arrayKey = $"{key}:{i}"; - if (array[i] is JsonObject arrayObject) - { - FlattenJsonObjectRecursive(arrayObject, arrayKey, result); - } - else - { - result[arrayKey] = array[i]?.DeepClone(); - } - } - } - else - { - result[key] = kvp.Value?.DeepClone(); - } - } - } + public static JsonObject UnflattenJsonObject(JsonObject source) => JsonFlattener.UnflattenJsonObject(source); /// /// Loads the deployment state from storage, using caching to avoid repeated loads. diff --git a/src/Aspire.Hosting/Publishing/Internal/JsonFlattener.cs b/src/Aspire.Hosting/Publishing/Internal/JsonFlattener.cs new file mode 100644 index 00000000000..f5a7cbc23ee --- /dev/null +++ b/src/Aspire.Hosting/Publishing/Internal/JsonFlattener.cs @@ -0,0 +1,93 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text.Json.Nodes; + +namespace Aspire.Hosting.Publishing.Internal; + +/// +/// Provides utility methods for flattening and unflattening JSON objects using colon-separated keys. +/// +public static class JsonFlattener +{ + /// + /// Flattens a JsonObject using colon-separated keys for configuration compatibility. + /// Handles both nested objects and arrays with indexed keys. + /// + /// The source JsonObject to flatten. + /// A flattened JsonObject. + public static JsonObject FlattenJsonObject(JsonObject source) + { + var result = new JsonObject(); + FlattenJsonObjectRecursive(source, string.Empty, result); + return result; + } + + /// + /// Unflattens a JsonObject that uses colon-separated keys back into a nested structure. + /// Handles both nested objects and arrays with indexed keys. + /// + /// The flattened JsonObject to unflatten. + /// An unflattened JsonObject with nested structure. + public static JsonObject UnflattenJsonObject(JsonObject source) + { + var result = new JsonObject(); + + foreach (var kvp in source) + { + var keys = kvp.Key.Split(':'); + var current = result; + + for (var i = 0; i < keys.Length - 1; i++) + { + var key = keys[i]; + if (!current.TryGetPropertyValue(key, out var existing) || existing is not JsonObject) + { + var newObject = new JsonObject(); + current[key] = newObject; + current = newObject; + } + else + { + current = existing.AsObject(); + } + } + + current[keys[^1]] = kvp.Value?.DeepClone(); + } + + return result; + } + + private static void FlattenJsonObjectRecursive(JsonObject source, string prefix, JsonObject result) + { + foreach (var kvp in source) + { + var key = string.IsNullOrEmpty(prefix) ? kvp.Key : $"{prefix}:{kvp.Key}"; + + if (kvp.Value is JsonObject nestedObject) + { + FlattenJsonObjectRecursive(nestedObject, key, result); + } + else if (kvp.Value is JsonArray array) + { + for (var i = 0; i < array.Count; i++) + { + var arrayKey = $"{key}:{i}"; + if (array[i] is JsonObject arrayObject) + { + FlattenJsonObjectRecursive(arrayObject, arrayKey, result); + } + else + { + result[arrayKey] = array[i]?.DeepClone(); + } + } + } + else + { + result[key] = kvp.Value?.DeepClone(); + } + } + } +} diff --git a/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs index d31db76270f..2b3c74a5601 100644 --- a/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs +++ b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs @@ -203,7 +203,7 @@ public async Task SaveStateAsync(JsonObject state, CancellationToken cancellatio await _semaphore.WaitAsync(cancellationToken).ConfigureAwait(false); try { - var flattenedState = DeploymentStateManagerBase.FlattenJsonObject(state); + var flattenedState = JsonFlattener.FlattenJsonObject(state); EnsureUserSecretsDirectory(); var json = flattenedState.ToJsonString(s_jsonSerializerOptions); diff --git a/tests/Aspire.Hosting.Azure.Tests/DefaultUserSecretsManagerTests.cs b/tests/Aspire.Hosting.Azure.Tests/DefaultUserSecretsManagerTests.cs index 6694d72ce5e..49cd624bb59 100644 --- a/tests/Aspire.Hosting.Azure.Tests/DefaultUserSecretsManagerTests.cs +++ b/tests/Aspire.Hosting.Azure.Tests/DefaultUserSecretsManagerTests.cs @@ -30,7 +30,7 @@ public void FlattenJsonObject_HandlesNestedStructures() }; // Act - var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets); + var result = JsonFlattener.FlattenJsonObject(userSecrets); // Assert Assert.Equal("existing-flat-value", result["Azure:SubscriptionId"]!.ToString()); @@ -54,7 +54,7 @@ public void FlattenJsonObject_HandlesSimpleFlatStructure() }; // Act - var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets); + var result = JsonFlattener.FlattenJsonObject(userSecrets); // Assert Assert.Equal(3, result.Count); @@ -70,7 +70,7 @@ public void FlattenJsonObject_HandlesEmptyObject() var userSecrets = new JsonObject(); // Act - var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets); + var result = JsonFlattener.FlattenJsonObject(userSecrets); // Assert Assert.Empty(result); @@ -95,7 +95,7 @@ public void FlattenJsonObject_HandlesDeeplyNestedStructures() }; // Act - var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets); + var result = JsonFlattener.FlattenJsonObject(userSecrets); // Assert Assert.Single(result); @@ -120,7 +120,7 @@ public void FlattenJsonObject_PreservesNullAndPrimitiveValues() }; // Act - var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets); + var result = JsonFlattener.FlattenJsonObject(userSecrets); // Assert Assert.Equal("text", result["StringValue"]!.ToString()); @@ -143,7 +143,7 @@ public void FlattenJsonObject_HandlesArraysWithPrimitiveValues() }; // Act - var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets); + var result = JsonFlattener.FlattenJsonObject(userSecrets); // Assert Assert.Equal("value1", result["SimpleArray:0"]!.ToString()); @@ -183,7 +183,7 @@ public void FlattenJsonObject_HandlesArraysWithObjects() }; // Act - var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets); + var result = JsonFlattener.FlattenJsonObject(userSecrets); // Assert Assert.Equal("Item1", result["ObjectArray:0:Name"]!.ToString()); @@ -206,7 +206,7 @@ public void FlattenJsonObject_HandlesEmptyArrays() }; // Act - var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets); + var result = JsonFlattener.FlattenJsonObject(userSecrets); // Assert Assert.Single(result); From 17002e3c59ad631aab2c67c7a27c6fa323bc24f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 1 Nov 2025 22:52:36 +0000 Subject: [PATCH 14/16] Make IUserSecretsManager nullable in VersionCheckService to handle apps without UserSecretsId Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- src/Aspire.Hosting/DistributedApplicationBuilder.cs | 6 ++---- .../Internal/UserSecretsDeploymentStateManager.cs | 12 +++++++++--- .../VersionChecking/VersionCheckService.cs | 10 +++++----- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/Aspire.Hosting/DistributedApplicationBuilder.cs b/src/Aspire.Hosting/DistributedApplicationBuilder.cs index e8b99fb1e23..c064801aa5c 100644 --- a/src/Aspire.Hosting/DistributedApplicationBuilder.cs +++ b/src/Aspire.Hosting/DistributedApplicationBuilder.cs @@ -288,10 +288,8 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options) // Core things // Create and register the user secrets manager _userSecretsManager = UserSecretsManagerFactory.Instance.GetOrCreate(AppHostAssembly); - if (_userSecretsManager != null) - { - _innerBuilder.Services.AddSingleton(_userSecretsManager); - } + // Always register IUserSecretsManager so dependencies can resolve (can be null) + _innerBuilder.Services.AddSingleton(typeof(IUserSecretsManager), sp => _userSecretsManager!); _innerBuilder.Services.AddSingleton(sp => new DistributedApplicationModel(Resources)); _innerBuilder.Services.AddSingleton(); diff --git a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs index a82206ea434..c5a706e63af 100644 --- a/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs @@ -17,21 +17,21 @@ namespace Aspire.Hosting.Publishing.Internal; /// internal sealed class UserSecretsDeploymentStateManager : DeploymentStateManagerBase { - private readonly IUserSecretsManager _userSecretsManager; + private readonly IUserSecretsManager? _userSecretsManager; /// /// Initializes a new instance of the class. /// /// The logger. /// User secrets manager for managing secrets. - public UserSecretsDeploymentStateManager(ILogger logger, IUserSecretsManager userSecretsManager) + public UserSecretsDeploymentStateManager(ILogger logger, IUserSecretsManager? userSecretsManager = null) : base(logger) { _userSecretsManager = userSecretsManager; } /// - public override string? StateFilePath => _userSecretsManager.FilePath; + public override string? StateFilePath => _userSecretsManager?.FilePath; /// protected override string? GetStatePath() @@ -46,6 +46,12 @@ public UserSecretsDeploymentStateManager(ILogger protected override async Task SaveStateToStorageAsync(JsonObject state, CancellationToken cancellationToken) { + if (_userSecretsManager == null) + { + logger.LogWarning("User secrets manager is not available. Skipping saving state to user secrets."); + return; + } + try { // Use the shared manager which handles locking diff --git a/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs b/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs index 97acb8f83b2..984b8827c5b 100644 --- a/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs +++ b/src/Aspire.Hosting/VersionChecking/VersionCheckService.cs @@ -32,12 +32,12 @@ internal sealed class VersionCheckService : BackgroundService private readonly DistributedApplicationExecutionContext _executionContext; private readonly TimeProvider _timeProvider; private readonly SemVersion? _appHostVersion; - private readonly IUserSecretsManager _userSecretsManager; + private readonly IUserSecretsManager? _userSecretsManager; public VersionCheckService(IInteractionService interactionService, ILogger logger, IConfiguration configuration, DistributedApplicationOptions options, IPackageFetcher packageFetcher, DistributedApplicationExecutionContext executionContext, TimeProvider timeProvider, IPackageVersionProvider packageVersionProvider, - IUserSecretsManager userSecretsManager) + IUserSecretsManager? userSecretsManager) { _interactionService = interactionService; _logger = logger; @@ -102,7 +102,7 @@ private async Task CheckForLatestAsync(CancellationToken cancellationToken) { var appHostDirectory = _configuration["AppHost:Directory"]!; - _userSecretsManager.TrySetSecret(LastCheckDateKey, now.ToString("o", CultureInfo.InvariantCulture)); + _userSecretsManager?.TrySetSecret(LastCheckDateKey, now.ToString("o", CultureInfo.InvariantCulture)); packages = await _packageFetcher.TryFetchPackagesAsync(appHostDirectory, cancellationToken).ConfigureAwait(false); } else @@ -133,7 +133,7 @@ private async Task CheckForLatestAsync(CancellationToken cancellationToken) if (IsVersionGreater(latestVersion, storedKnownLatestVersion) || storedKnownLatestVersion == null) { // Latest version is greater than the stored known latest version, so update it. - _userSecretsManager.TrySetSecret(KnownLatestVersionKey, latestVersion.ToString()); + _userSecretsManager?.TrySetSecret(KnownLatestVersionKey, latestVersion.ToString()); } var result = await _interactionService.PromptNotificationAsync( @@ -151,7 +151,7 @@ private async Task CheckForLatestAsync(CancellationToken cancellationToken) if (result.Data) { _logger.LogDebug("User chose to ignore version {Version}.", latestVersion); - _userSecretsManager.TrySetSecret(IgnoreVersionKey, latestVersion.ToString()); + _userSecretsManager?.TrySetSecret(IgnoreVersionKey, latestVersion.ToString()); } } From 61f9af9c64c07f0d129c33acc2a2530e2655ddf4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 1 Nov 2025 23:57:22 +0000 Subject: [PATCH 15/16] Add NoopUserSecretsManager to handle apps without UserSecretsId with warning logs Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../UserSecrets/NoopUserSecretsManager.cs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs diff --git a/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs new file mode 100644 index 00000000000..5bca8b47b85 --- /dev/null +++ b/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs @@ -0,0 +1,57 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Logging; + +namespace Aspire.Hosting.UserSecrets; + +/// +/// A no-op implementation of that logs warnings when +/// operations are attempted on a project without user secrets configured. +/// +internal sealed class NoopUserSecretsManager : IUserSecretsManager +{ + private readonly ILogger _logger; + private readonly string _assemblyName; + + public NoopUserSecretsManager(ILogger logger, string assemblyName) + { + _logger = logger; + _assemblyName = assemblyName; + } + + public bool TryGetValue(string key, out string? value) + { + LogWarning(nameof(TryGetValue)); + value = null; + return false; + } + + public bool TrySetValue(string key, string value) + { + LogWarning(nameof(TrySetValue)); + return false; + } + + public Task TrySetValueAsync(string key, string value, CancellationToken cancellationToken = default) + { + LogWarning(nameof(TrySetValueAsync)); + return Task.FromResult(false); + } + + public string? GetOrSetValue(string key, Func valueFactory) + { + LogWarning(nameof(GetOrSetValue)); + return null; + } + + private void LogWarning(string operation) + { + _logger.LogWarning( + "User secrets are not enabled for assembly '{AssemblyName}'. " + + "Operation '{Operation}' will not persist data. " + + "To enable user secrets, add a UserSecretsId to your project file or use 'dotnet user-secrets init'.", + _assemblyName, + operation); + } +} From 4f29f8479628c32498a4146b1ab6fe971cea10cc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 2 Nov 2025 00:35:47 +0000 Subject: [PATCH 16/16] Fix NoopUserSecretsManager to implement all IUserSecretsManager interface members Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../UserSecrets/NoopUserSecretsManager.cs | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs index 5bca8b47b85..e53391c8774 100644 --- a/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs +++ b/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Text.Json.Nodes; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; namespace Aspire.Hosting.UserSecrets; @@ -20,29 +22,29 @@ public NoopUserSecretsManager(ILogger logger, string assemblyName) _assemblyName = assemblyName; } - public bool TryGetValue(string key, out string? value) + public string FilePath => string.Empty; + + public bool TrySetSecret(string name, string value) { - LogWarning(nameof(TryGetValue)); - value = null; + LogWarning(nameof(TrySetSecret)); return false; } - public bool TrySetValue(string key, string value) + public Task TrySetSecretAsync(string name, string value, CancellationToken cancellationToken = default) { - LogWarning(nameof(TrySetValue)); - return false; + LogWarning(nameof(TrySetSecretAsync)); + return Task.FromResult(false); } - public Task TrySetValueAsync(string key, string value, CancellationToken cancellationToken = default) + public void GetOrSetSecret(IConfigurationManager configuration, string name, Func valueGenerator) { - LogWarning(nameof(TrySetValueAsync)); - return Task.FromResult(false); + LogWarning(nameof(GetOrSetSecret)); } - public string? GetOrSetValue(string key, Func valueFactory) + public Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default) { - LogWarning(nameof(GetOrSetValue)); - return null; + LogWarning(nameof(SaveStateAsync)); + return Task.CompletedTask; } private void LogWarning(string operation)