From e8712f70469d872fc0a7bf5adf390942cf318605 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 01:47:19 +0000 Subject: [PATCH 1/5] Initial plan From 4d2cfac8157aba68289180c482be373b5803b093 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 02:02:36 +0000 Subject: [PATCH 2/5] Implement user secrets refactoring with DI-based factory pattern Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../UserSecretsParameterDefault.cs | 16 +- src/Aspire.Hosting/Aspire.Hosting.csproj | 1 - .../DistributedApplicationBuilder.cs | 12 +- .../Internal/DeploymentStateManagerBase.cs | 83 +------- .../Internal/FileDeploymentStateManager.cs | 2 +- .../Pipelines/Internal/JsonFlattener.cs | 93 ++++++++ .../UserSecretsDeploymentStateManager.cs | 31 +-- .../UserSecrets/IUserSecretsManager.cs | 42 ++++ .../UserSecrets/NoopUserSecretsManager.cs | 42 ++++ .../UserSecrets/UserSecretsManagerFactory.cs | 201 ++++++++++++++++++ .../VersionChecking/VersionCheckService.cs | 13 +- ...sManagerTests.cs => JsonFlattenerTests.cs} | 20 +- .../Aspire.Hosting.Tests/SecretsStoreTests.cs | 36 +++- .../UserSecretsParameterDefaultTests.cs | 168 ++++++++++++++- .../VersionCheckServiceTests.cs | 7 +- 15 files changed, 630 insertions(+), 137 deletions(-) create mode 100644 src/Aspire.Hosting/Pipelines/Internal/JsonFlattener.cs create mode 100644 src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs create mode 100644 src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs create mode 100644 src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs rename tests/Aspire.Hosting.Tests/{Publishing/DefaultUserSecretsManagerTests.cs => JsonFlattenerTests.cs} (86%) diff --git a/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs b/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs index 0294b82b23a..4e38a1f70b1 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; @@ -15,15 +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}"; - if (!SecretsStore.TrySetUserSecret(appHostAssembly, configurationKey, value)) + + var manager = factory.GetOrCreate(appHostAssembly); + if (!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 079083aadfe..65535adf8aa 100644 --- a/src/Aspire.Hosting/Aspire.Hosting.csproj +++ b/src/Aspire.Hosting/Aspire.Hosting.csproj @@ -31,7 +31,6 @@ - diff --git a/src/Aspire.Hosting/DistributedApplicationBuilder.cs b/src/Aspire.Hosting/DistributedApplicationBuilder.cs index 9fc825b03c8..b79aa7f6621 100644 --- a/src/Aspire.Hosting/DistributedApplicationBuilder.cs +++ b/src/Aspire.Hosting/DistributedApplicationBuilder.cs @@ -26,6 +26,7 @@ using Aspire.Hosting.Pipelines; using Aspire.Hosting.Pipelines.Internal; using Aspire.Hosting.Publishing; +using Aspire.Hosting.UserSecrets; using Aspire.Hosting.VersionChecking; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -34,7 +35,6 @@ using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Microsoft.Extensions.SecretManager.Tools.Internal; namespace Aspire.Hosting; @@ -64,6 +64,7 @@ public class DistributedApplicationBuilder : IDistributedApplicationBuilder private readonly DistributedApplicationOptions _options; private readonly HostApplicationBuilder _innerBuilder; + private readonly IUserSecretsManager _userSecretsManager; /// public IHostEnvironment Environment => _innerBuilder.Environment; @@ -288,6 +289,11 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options) } // Core things + // Create and register the user secrets manager + _userSecretsManager = UserSecretsManagerFactory.Instance.GetOrCreate(AppHostAssembly); + // Always register IUserSecretsManager so dependencies can resolve + _innerBuilder.Services.AddSingleton(_userSecretsManager); + _innerBuilder.Services.AddSingleton(sp => new DistributedApplicationModel(Resources)); _innerBuilder.Services.AddSingleton(); _innerBuilder.Services.AddHostedService(sp => sp.GetRequiredService()); @@ -356,13 +362,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/Pipelines/Internal/DeploymentStateManagerBase.cs b/src/Aspire.Hosting/Pipelines/Internal/DeploymentStateManagerBase.cs index 3164970f8f4..1c7f4ff1d39 100644 --- a/src/Aspire.Hosting/Pipelines/Internal/DeploymentStateManagerBase.cs +++ b/src/Aspire.Hosting/Pipelines/Internal/DeploymentStateManagerBase.cs @@ -61,87 +61,6 @@ private sealed class SectionMetadata(long version) /// Cancellation token. protected abstract Task SaveStateToStorageAsync(JsonObject state, CancellationToken cancellationToken); - /// - /// 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(); - } - } - } - /// /// Loads the deployment state from storage, using caching to avoid repeated loads. /// @@ -169,7 +88,7 @@ protected async Task LoadStateAsync(CancellationToken cancellationTo { var fileContent = await File.ReadAllTextAsync(statePath, cancellationToken).ConfigureAwait(false); var flattenedState = JsonNode.Parse(fileContent, documentOptions: jsonDocumentOptions)!.AsObject(); - _state = UnflattenJsonObject(flattenedState); + _state = JsonFlattener.UnflattenJsonObject(flattenedState); } else { diff --git a/src/Aspire.Hosting/Pipelines/Internal/FileDeploymentStateManager.cs b/src/Aspire.Hosting/Pipelines/Internal/FileDeploymentStateManager.cs index ca78266dcc6..5354eced2eb 100644 --- a/src/Aspire.Hosting/Pipelines/Internal/FileDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Pipelines/Internal/FileDeploymentStateManager.cs @@ -63,7 +63,7 @@ protected override async Task SaveStateToStorageAsync(JsonObject state, Cancella return; } - var flattenedSecrets = FlattenJsonObject(state); + var flattenedSecrets = JsonFlattener.FlattenJsonObject(state); Directory.CreateDirectory(Path.GetDirectoryName(deploymentStatePath)!); await File.WriteAllTextAsync( deploymentStatePath, diff --git a/src/Aspire.Hosting/Pipelines/Internal/JsonFlattener.cs b/src/Aspire.Hosting/Pipelines/Internal/JsonFlattener.cs new file mode 100644 index 00000000000..d7dd9be0c07 --- /dev/null +++ b/src/Aspire.Hosting/Pipelines/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.Pipelines.Internal; + +/// +/// Provides utility methods for flattening and unflattening JSON objects using colon-separated keys. +/// +internal 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/Pipelines/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Pipelines/Internal/UserSecretsDeploymentStateManager.cs index c063913575a..08c262d1907 100644 --- a/src/Aspire.Hosting/Pipelines/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Pipelines/Internal/UserSecretsDeploymentStateManager.cs @@ -3,10 +3,9 @@ #pragma warning disable ASPIREPIPELINES002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed. -using System.Reflection; using System.Text.Json; using System.Text.Json.Nodes; -using Microsoft.Extensions.Configuration.UserSecrets; +using Aspire.Hosting.UserSecrets; using Microsoft.Extensions.Logging; namespace Aspire.Hosting.Pipelines.Internal; @@ -14,19 +13,28 @@ namespace Aspire.Hosting.Pipelines.Internal; /// /// User secrets implementation of . /// -internal sealed class UserSecretsDeploymentStateManager(ILogger logger) : DeploymentStateManagerBase(logger) +internal sealed class UserSecretsDeploymentStateManager : DeploymentStateManagerBase { + 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) + : base(logger) + { + _userSecretsManager = userSecretsManager; + } + /// public override string? StateFilePath => GetStatePath(); /// protected override string? GetStatePath() { - return Assembly.GetEntryAssembly()?.GetCustomAttribute()?.UserSecretsId switch - { - null => Environment.GetEnvironmentVariable("DOTNET_USER_SECRETS_ID"), - string id => UserSecretsPathHelper.GetSecretsPathFromSecretsId(id) - }; + return _userSecretsManager.FilePath; } /// @@ -34,11 +42,8 @@ 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)!); - await File.WriteAllTextAsync(userSecretsPath, flattenedUserSecrets.ToJsonString(s_jsonSerializerOptions), 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/IUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs new file mode 100644 index 00000000000..94d83544871 --- /dev/null +++ b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs @@ -0,0 +1,42 @@ +// 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; + +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); + + /// + /// 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. + void GetOrSetSecret(IConfigurationManager configuration, string name, Func valueGenerator); + + /// + /// Saves state to user secrets asynchronously (for deployment state manager). + /// If multiple callers save state concurrently, the last write wins. + /// + /// The state to save as a JSON object. + /// Cancellation token. + Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default); +} diff --git a/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs new file mode 100644 index 00000000000..6b7c8f4b69a --- /dev/null +++ b/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs @@ -0,0 +1,42 @@ +// 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.Text.Json.Nodes; +using Microsoft.Extensions.Configuration; + +namespace Aspire.Hosting.UserSecrets; + +/// +/// A no-op implementation of used when +/// user secrets are not configured for a project. +/// +internal sealed class NoopUserSecretsManager : IUserSecretsManager +{ + public static readonly NoopUserSecretsManager Instance = new(); + + private NoopUserSecretsManager() + { + } + + public string FilePath => string.Empty; + + public bool TrySetSecret(string name, string value) + { + Debug.WriteLine($"User secrets are not enabled. Cannot set secret '{name}'."); + return false; + } + + public void GetOrSetSecret(IConfigurationManager configuration, string name, Func valueGenerator) + { + Debug.WriteLine($"User secrets are not enabled. Generating and adding secret '{name}' to configuration in-memory."); + var value = valueGenerator(); + configuration.AddInMemoryCollection(new Dictionary { [name] = value }); + } + + public Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default) + { + Debug.WriteLine("User secrets are not enabled. Cannot save state."); + return Task.CompletedTask; + } +} diff --git a/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs new file mode 100644 index 00000000000..989d3615133 --- /dev/null +++ b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs @@ -0,0 +1,201 @@ +// 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.Text; +using System.Text.Json; +using System.Text.Json.Nodes; +using Aspire.Hosting.Pipelines.Internal; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Configuration.UserSecrets; + +namespace Aspire.Hosting.UserSecrets; + +/// +/// Factory for creating and caching IUserSecretsManager instances. +/// Uses a lock to ensure thread-safe creation and a dictionary to cache instances by normalized file path. +/// +internal sealed class UserSecretsManagerFactory +{ + // Singleton instance + public static readonly UserSecretsManagerFactory Instance = new(); + + // Dictionary to cache instances by file path + private readonly Dictionary _managerCache = new(); + private readonly object _lock = new(); + + internal 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); + + lock (_lock) + { + if (!_managerCache.TryGetValue(normalizedPath, out var manager)) + { + manager = new UserSecretsManager(normalizedPath); + _managerCache[normalizedPath] = manager; + } + return manager; + } + } + + /// + /// Gets or creates a user secrets manager for the specified user secrets ID. + /// + public IUserSecretsManager GetOrCreateFromId(string? userSecretsId) + { + if (string.IsNullOrWhiteSpace(userSecretsId)) + { + return NoopUserSecretsManager.Instance; + } + + 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 sealed class UserSecretsManager : IUserSecretsManager + { + private static readonly JsonSerializerOptions s_jsonSerializerOptions = new() { WriteIndented = true }; + + private readonly SemaphoreSlim _semaphore = new(1, 1); + + public UserSecretsManager(string filePath) + { + FilePath = filePath; + } + + 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 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."); + } + } + } + + /// + /// Saves state to user secrets asynchronously (for deployment state manager). + /// If multiple callers save state concurrently, the last write wins. + /// + public async Task SaveStateAsync(JsonObject state, CancellationToken cancellationToken = default) + { + await _semaphore.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + var flattenedState = JsonFlattener.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 Dictionary Load() + { + return new ConfigurationBuilder() + .AddJsonFile(FilePath, optional: true) + .Build() + .AsEnumerable() + .Where(i => i.Value != null) + .ToDictionary(i => i.Key, i => i.Value); + } + + private void Save(Dictionary secrets) + { + var contents = new JsonObject(); + foreach (var secret in secrets) + { + contents[secret.Key] = secret.Value; + } + + var json = contents.ToJsonString(s_jsonSerializerOptions); + + // 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.WriteAllText(tempFilename, json, Encoding.UTF8); + File.Move(tempFilename, FilePath, overwrite: true); + } + else + { + File.WriteAllText(FilePath, json, Encoding.UTF8); + } + } + + 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..97acb8f83b2 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) { _interactionService = interactionService; _logger = logger; @@ -44,6 +46,7 @@ public VersionCheckService(IInteractionService interactionService, ILogger.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); diff --git a/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs b/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs index 122bc3e3e9e..46af674af11 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.Pipelines.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,9 @@ public void GetOrSetUserSecret_SavesValueToUserSecrets() var configuration = new ConfigurationManager(); var value = TokenGenerator.GenerateToken(); - SecretsStore.GetOrSetUserSecret(configuration, testAssembly, key, () => value); + var factory = new UserSecretsManagerFactory(); + var manager = factory.GetOrCreate(testAssembly); + manager?.GetOrSetSecret(configuration, key, () => value); var userSecrets = GetUserSecrets(userSecretsId); var configValue = configuration[key]; @@ -50,7 +53,9 @@ public void GetOrSetUserSecret_ReadsValueFromConfiguration() var valueInConfig = TokenGenerator.GenerateToken(); configuration[key] = valueInConfig; - SecretsStore.GetOrSetUserSecret(configuration, testAssembly, key, TokenGenerator.GenerateToken); + var factory = new UserSecretsManagerFactory(); + var manager = factory.GetOrCreate(testAssembly); + manager?.GetOrSetSecret(configuration, key, TokenGenerator.GenerateToken); var userSecrets = GetUserSecrets(userSecretsId); Assert.False(userSecrets.TryGetValue(key, out var savedValue)); @@ -60,20 +65,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 (!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 d9d3f5d9723..df85e10c207 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; @@ -49,7 +51,7 @@ public void UserSecretsParameterDefault_GetDefaultValue_DoesntThrowIfSecretsFile { var userSecretsId = Guid.NewGuid().ToString("N"); DeleteUserSecretsFile(userSecretsId); - var userSecretsPath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); + var userSecretsPath = UserSecretsPathHelper.GetSecretsPathFromSecretsId(userSecretsId); if (File.Exists(userSecretsPath)) { File.Delete(userSecretsPath); @@ -71,6 +73,132 @@ 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])]); + + // Create an isolated factory instance for this test to avoid cross-contamination + var factory = new UserSecretsManagerFactory(); + + // 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(() => + { + var manager = factory.GetOrCreate(testAssembly); + return manager?.TrySetSecret(key, value) ?? false; + })); + } + + 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_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])]); + + // Create an isolated factory instance for this test to avoid cross-contamination + var factory = new UserSecretsManagerFactory(); + + // Simulate SQL Server and RabbitMQ generating passwords concurrently + var sqlTask = Task.Run(() => + { + var manager = factory.GetOrCreate(testAssembly); + return manager?.TrySetSecret("Parameters:sql-password", "SqlPassword123!") ?? false; + }); + var rabbitTask = Task.Run(() => + { + var manager = factory.GetOrCreate(testAssembly); + return manager?.TrySetSecret("Parameters:rabbit-password", "RabbitPassword456!") ?? false; + }); + + 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() + { + var userSecretsId = Guid.NewGuid().ToString("N"); + ClearUsersSecrets(userSecretsId); + + var testAssembly = AssemblyBuilder.DefineDynamicAssembly( + new("TestAssembly"), AssemblyBuilderAccess.RunAndCollect, [new CustomAttributeBuilder(s_userSecretsIdAttrCtor, [userSecretsId])]); + + // Create an isolated factory instance for this test to avoid cross-contamination + var factory = new UserSecretsManagerFactory(); + + // 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(() => + { + var manager = factory.GetOrCreate(testAssembly); + return manager?.TrySetSecret("Parameters:test-key", value) ?? false; + })); + } + + 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); @@ -82,20 +210,43 @@ 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); + + // 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) { - var userSecretsPath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId); + var userSecretsPath = UserSecretsPathHelper.GetSecretsPathFromSecretsId(userSecretsId); if (File.Exists(userSecretsPath)) { File.Delete(userSecretsPath); @@ -115,3 +266,4 @@ public override void WriteToManifest(ManifestPublishingContext context) } } } + diff --git a/tests/Aspire.Hosting.Tests/VersionChecking/VersionCheckServiceTests.cs b/tests/Aspire.Hosting.Tests/VersionChecking/VersionCheckServiceTests.cs index fbfa5abdb7e..1159b67a507 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 ?? NoopUserSecretsManager.Instance); } private sealed class TestTimeProvider : TimeProvider From b5cd119e98c31c1f07a9509dfd20db9107b048fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 02:13:08 +0000 Subject: [PATCH 3/5] Fix missing namespace in UserSecretsParameterDefaultTests Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs index df85e10c207..49fce77b15a 100644 --- a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs +++ b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs @@ -4,8 +4,8 @@ using System.Reflection; using System.Reflection.Emit; using System.Text; +using Aspire.Hosting.Pipelines.Internal; using Aspire.Hosting.Publishing; -using Aspire.Hosting.Publishing.Internal; using Aspire.Hosting.UserSecrets; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.UserSecrets; From 81d3417dc99a3626c328dcfec4c0cbd0198f8a8a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 03:05:47 +0000 Subject: [PATCH 4/5] Address PR feedback: delete SecretsStore.cs and simplify documentation Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../UserSecretsParameterDefault.cs | 4 +- .../UserSecretsDeploymentStateManager.cs | 5 - .../UserSecrets/IUserSecretsManager.cs | 2 +- .../UserSecrets/NoopUserSecretsManager.cs | 3 +- .../UserSecrets/UserSecretsManagerFactory.cs | 6 +- src/Shared/SecretsStore.cs | 143 ------------------ 6 files changed, 7 insertions(+), 156 deletions(-) delete mode 100644 src/Shared/SecretsStore.cs diff --git a/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs b/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs index 4e38a1f70b1..faa4c199115 100644 --- a/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs +++ b/src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs @@ -19,13 +19,11 @@ namespace Aspire.Hosting.ApplicationModel; 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() { diff --git a/src/Aspire.Hosting/Pipelines/Internal/UserSecretsDeploymentStateManager.cs b/src/Aspire.Hosting/Pipelines/Internal/UserSecretsDeploymentStateManager.cs index 08c262d1907..a4e66b705fd 100644 --- a/src/Aspire.Hosting/Pipelines/Internal/UserSecretsDeploymentStateManager.cs +++ b/src/Aspire.Hosting/Pipelines/Internal/UserSecretsDeploymentStateManager.cs @@ -17,11 +17,6 @@ internal sealed class UserSecretsDeploymentStateManager : DeploymentStateManager { 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) : base(logger) { diff --git a/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs index 94d83544871..a821782d989 100644 --- a/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs +++ b/src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs @@ -7,7 +7,7 @@ namespace Aspire.Hosting.UserSecrets; /// -/// Manages user secrets for an application, providing thread-safe read and write operations. +/// Defines an interface for managing user secrets with support for read and write operations. /// internal interface IUserSecretsManager { diff --git a/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs b/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs index 6b7c8f4b69a..1a2e133039e 100644 --- a/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs +++ b/src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs @@ -8,8 +8,7 @@ namespace Aspire.Hosting.UserSecrets; /// -/// A no-op implementation of used when -/// user secrets are not configured for a project. +/// A no-op implementation of used when user secrets are not configured for a project. /// internal sealed class NoopUserSecretsManager : IUserSecretsManager { diff --git a/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs index 989d3615133..281c8d20d8f 100644 --- a/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs +++ b/src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs @@ -13,9 +13,11 @@ namespace Aspire.Hosting.UserSecrets; /// -/// Factory for creating and caching IUserSecretsManager instances. -/// Uses a lock to ensure thread-safe creation and a dictionary to cache instances by normalized file path. +/// Factory for creating and caching instances. /// +/// +/// Uses a lock to ensure thread-safe creation and a dictionary to cache instances by normalized file path. +/// internal sealed class UserSecretsManagerFactory { // Singleton instance diff --git a/src/Shared/SecretsStore.cs b/src/Shared/SecretsStore.cs deleted file mode 100644 index 5c8f550002a..00000000000 --- a/src/Shared/SecretsStore.cs +++ /dev/null @@ -1,143 +0,0 @@ -// 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.Text; -using System.Text.Json.Nodes; -using Aspire.Hosting; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.Configuration.UserSecrets; - -namespace Microsoft.Extensions.SecretManager.Tools.Internal; - -/// -/// 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() - { - EnsureUserSecretsDirectory(); - - var contents = new JsonObject(); - if (_secrets is not null) - { - foreach (var secret in _secrets.AsEnumerable()) - { - 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); - } - - private void EnsureUserSecretsDirectory() - { - var directoryName = Path.GetDirectoryName(_secretsFilePath); - if (!string.IsNullOrEmpty(directoryName) && !Directory.Exists(directoryName)) - { - Directory.CreateDirectory(directoryName); - } - } - - 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 secretsStore = new SecretsStore(userSecretsId); - secretsStore.Set(name, value); - secretsStore.Save(); - return true; - } - catch (Exception) { } // Ignore user secret store errors - } - - return false; - } -} From 50647b475d8b54f72009d949a7ac6772c3c019d4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 03:14:47 +0000 Subject: [PATCH 5/5] Add tests for UserSecretsParameterDefault constructor with factory parameter Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> --- .../UserSecretsParameterDefaultTests.cs | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs index 49fce77b15a..a1142fa79e9 100644 --- a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs +++ b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs @@ -199,6 +199,107 @@ public async Task TrySetUserSecret_ConcurrentWritesSameKey_LastWriteWins() DeleteUserSecretsFile(userSecretsId); } + [Fact] + public void UserSecretsParameterDefault_WithCustomFactory_UsesProvidedFactory() + { + // This test verifies that the constructor overload taking a factory parameter + // uses the provided factory instead of the singleton instance + var userSecretsId = Guid.NewGuid().ToString("N"); + ClearUsersSecrets(userSecretsId); + + var testAssembly = AssemblyBuilder.DefineDynamicAssembly( + new("TestAssembly"), AssemblyBuilderAccess.RunAndCollect, [new CustomAttributeBuilder(s_userSecretsIdAttrCtor, [userSecretsId])]); + + // Create a custom factory instance for test isolation + var customFactory = new UserSecretsManagerFactory(); + var paramDefault = new TestParameterDefault(); + var userSecretDefault = new UserSecretsParameterDefault(testAssembly, "TestApplication.AppHost", "param1", paramDefault, customFactory); + + var initialValue = userSecretDefault.GetDefaultValue(); + + var userSecrets = GetUserSecrets(userSecretsId); + Assert.Equal(initialValue, userSecrets["Parameters:param1"]); + + DeleteUserSecretsFile(userSecretsId); + } + + [Fact] + public void UserSecretsParameterDefault_WithCustomFactory_IsolatesFromGlobalInstance() + { + // This test verifies that using a custom factory provides isolation + // between test runs and doesn't interfere with the singleton instance + var userSecretsId1 = Guid.NewGuid().ToString("N"); + var userSecretsId2 = Guid.NewGuid().ToString("N"); + ClearUsersSecrets(userSecretsId1); + ClearUsersSecrets(userSecretsId2); + + var testAssembly1 = AssemblyBuilder.DefineDynamicAssembly( + new("TestAssembly1"), AssemblyBuilderAccess.RunAndCollect, [new CustomAttributeBuilder(s_userSecretsIdAttrCtor, [userSecretsId1])]); + var testAssembly2 = AssemblyBuilder.DefineDynamicAssembly( + new("TestAssembly2"), AssemblyBuilderAccess.RunAndCollect, [new CustomAttributeBuilder(s_userSecretsIdAttrCtor, [userSecretsId2])]); + + // Use custom factory for first parameter default + var customFactory = new UserSecretsManagerFactory(); + var paramDefault1 = new TestParameterDefault(); + var userSecretDefault1 = new UserSecretsParameterDefault(testAssembly1, "TestApp1.AppHost", "param1", paramDefault1, customFactory); + + // Use default singleton factory for second parameter default + var paramDefault2 = new TestParameterDefault(); + var userSecretDefault2 = new UserSecretsParameterDefault(testAssembly2, "TestApp2.AppHost", "param2", paramDefault2); + + var value1 = userSecretDefault1.GetDefaultValue(); + var value2 = userSecretDefault2.GetDefaultValue(); + + // Both should save successfully to their respective user secrets files + var userSecrets1 = GetUserSecrets(userSecretsId1); + var userSecrets2 = GetUserSecrets(userSecretsId2); + + Assert.Equal(value1, userSecrets1["Parameters:param1"]); + Assert.Equal(value2, userSecrets2["Parameters:param2"]); + + DeleteUserSecretsFile(userSecretsId1); + DeleteUserSecretsFile(userSecretsId2); + } + + [Fact] + public async Task UserSecretsParameterDefault_WithCustomFactory_ConcurrentAccess() + { + // This test verifies that the custom factory properly handles concurrent access + var userSecretsId = Guid.NewGuid().ToString("N"); + ClearUsersSecrets(userSecretsId); + + var testAssembly = AssemblyBuilder.DefineDynamicAssembly( + new("TestAssembly"), AssemblyBuilderAccess.RunAndCollect, [new CustomAttributeBuilder(s_userSecretsIdAttrCtor, [userSecretsId])]); + + var customFactory = new UserSecretsManagerFactory(); + + // Create multiple UserSecretsParameterDefault instances with different parameter names + var tasks = new List>(); + for (int i = 0; i < 5; i++) + { + var paramName = $"param{i}"; + tasks.Add(Task.Run(() => + { + var paramDefault = new TestParameterDefault(); + var userSecretDefault = new UserSecretsParameterDefault(testAssembly, "TestApp.AppHost", paramName, paramDefault, customFactory); + return userSecretDefault.GetDefaultValue(); + })); + } + + var values = await Task.WhenAll(tasks); + + // All parameters should be saved + var userSecrets = GetUserSecrets(userSecretsId); + for (int i = 0; i < 5; i++) + { + var paramKey = $"Parameters:param{i}"; + Assert.True(userSecrets.ContainsKey(paramKey), $"Parameter '{paramKey}' was not found in user secrets"); + Assert.Equal(values[i], userSecrets[paramKey]); + } + + DeleteUserSecretsFile(userSecretsId); + } + private static void EnsureUserSecretsDirectory(string secretsFilePath) { var directoryName = Path.GetDirectoryName(secretsFilePath);