Refactor user secrets management to use DI-based factory pattern with thread-safe synchronization by Copilot · Pull Request #12482 · dotnet/aspire

Copilot · 2025-10-29T07:50:08Z

Fix SecretsStore Concurrent Access Issue

This PR fixes the issue where multiple auto-generated secrets (e.g., SQL Server and RabbitMQ passwords) are not properly saved to user secrets because of concurrent access issues. The fix implements a complete architectural refactoring from static methods to a dependency injection-based factory pattern with proper synchronization.

Solution

Complete refactoring to DI-based architecture with:

IUserSecretsManager interface - Internal interface with IsAvailable property to check if user secrets are configured
UserSecretsManagerFactory - Singleton factory with ConditionalWeakTable caching and semaphore locking; returns NoopUserSecretsManager for apps without UserSecretsId (never cached)
NoopUserSecretsManager - Null object pattern that logs one warning in constructor, debug messages for operations
Always non-null DI registration - All consumers receive non-nullable IUserSecretsManager
Injectable consumers - All services take non-nullable IUserSecretsManager via constructor
JSON flattening extracted - JsonFlattener static class for reuse
Thread-safe synchronization - Per-file SemaphoreSlim locking

Technical Details

Null Object Pattern: NoopUserSecretsManager with IsAvailable = false, minimal logging (one warning in constructor, debug for operations)
Never Null: All consumers receive non-nullable IUserSecretsManager via DI or factory
Not Cached: NoopUserSecretsManager is never cached by factory
IsAvailable Property: Allows services to check if user secrets are actually configured
Cross-Assembly Sharing: ConditionalWeakTable ensures real instances are reused
Thread-Safe: Per-file SemaphoreSlim locking

Test Updates

Tests assert manager is non-null
All UserSecrets tests pass (including 3 new concurrent write tests)
All functional tests resolve IUserSecretsManager properly
Fixes Auto generated secrets do not get written to the user secrets #12481

Original prompt

This section details on the original issue you should resolve

<issue_title>Auto generated secrets do not get written to the user secrets</issue_title>
<issue_description>### Is there an existing issue for this?

I have searched the existing issues

Describe the bug

When multiple components in an Aspire app generate secrets (for example, a SQL Server container and a RabbitMQ container), only one of those secrets is actually preserved in the user secrets store.

The issue appears to be that SecretsStore.Save() is invoked multiple times — once per secret — and each call overwrites the previously saved data rather than merging or appending new keys.

As a result, only one of the auto-generated secrets (randomly the SQL Server or the RabbitMQ password) remains in the secrets.json file after startup.

Expected Behavior

All auto-generated secrets (e.g., SQL Server password and RabbitMQ password) should be written and preserved in the user secrets store.

Steps To Reproduce

Create an Aspire AppHost project that includes both a SQL Server container and a RabbitMQ container.
builder.AddRabbitMQ("rabbit")
    .WithLifetime(ContainerLifetime.Persistent)
    .WithDataVolume("rabbitmq");

builder.AddSqlServer("sql")
    .WithLifetime(ContainerLifetime.Persistent)
    .WithDataVolume("sqlserver")
    .AddDatabase("sqlserver-db");
Run the app to allow Aspire to auto-generate passwords for both containers.

Inspect the user secrets (secrets.json) after startup.

You can work around this issue by manually adding the auto-generated passwords:
var rabbitMqPassword =
    builder.AddParameter("rabbitmq-password", new GenerateParameterDefault { MinLength = 22, Special = false }, true, true);

builder
    .AddRabbitMQ("rabbitmq", password:rabbitMqPassword)
    .WithLifetime(ContainerLifetime.Persistent)
    .WithDataVolume("rabbitmq");

var sqlServerPassword =
    builder.AddParameter("sqlserver-password", new GenerateParameterDefault { MinLength = 22, MinLower = 1, MinUpper = 1, MinNumeric = 1 }, true, true);

var sqlServer = builder
    .AddSqlServer("sqlserver", password:sqlServerPassword)
    .WithLifetime(ContainerLifetime.Persistent)
    .WithDataVolume("sqlserver")
    .AddDatabase("sqlserver-db");
Exceptions (if any)

No response

.NET Version info

No response

Anything else?

Aspire version: 9.5.2
.NET SDK: 10.0.100-rc.2.25502.107
OS: Windows 11
IDE: Jetbrains Rider</issue_description>

<agent_instructions>https://github.com/dotnet/aspire/blob/f792aac439b0bca02404cfaacd8550e8530cc2e1/src/Shared/SecretsStore.cs

This implementation is naive for our needs. We have a manage concurrent access in the same process and a model where conflicts on the same key throw. Look at how SecretStore is used and tweak the API to make it safe for concurrent readers and writers.

As an example, DeploymentStateManagerBase has a design that prevents this and the API shape tries to prevent misuse by throwing.

https://github.com/dotnet/aspire/blob/f792aac439b0bca02404cfaacd8550e8530cc2e1/src/Aspire.Hosting/Publishing/Internal/DeploymentStateManagerBase.cs</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes Auto generated secrets do not get written to the user secrets #12481

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

…ites Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

davidfowl · 2025-10-29T18:26:15Z

@copilot Do we need a semaphore slim ? We can just use a dictionary of locks right

src/Shared/SecretsStore.cs

Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot · 2025-10-29T18:54:38Z

@copilot Do we need a semaphore slim ? We can just use a dictionary of locks right

Changed to use Dictionary<string, object> with simple lock statements instead of SemaphoreSlim. Since we're not using any async operations, regular locks are simpler and more appropriate. (b852826)

github-actions · 2025-10-30T00:17:46Z

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12482

Or

Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12482"

davidfowl · 2025-10-31T03:46:41Z

@copilot We have one more problem, user secrets are being written to by another system in aspire the UserSecretsDeploymentStateManager @dotnet/aspire/files/src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs . We need to make sure that there's a single source of truth for synchronization for these files within the same process.

The SecretStore and UserSecretsDeploymentStateManager need to synchronize access to the file and make sure that writes are serialized.

Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot · 2025-10-31T03:56:56Z

@copilot We have one more problem, user secrets are being written to by another system in aspire the UserSecretsDeploymentStateManager @dotnet/aspire/files/src/Aspire.Hosting/Publishing/Internal/User...

Created UserSecretsFileLock to provide a single source of truth for synchronization. Both SecretsStore and UserSecretsDeploymentStateManager now use UserSecretsFileLock.GetLock(filePath) to serialize writes to the same user secrets file. The lock is keyed by the normalized file path to ensure both systems use the same lock object. (ed54150)

davidfowl · 2025-10-31T04:14:00Z

@copilot The implementation does not look great. Base UserSecretsFileLock on a SemaphoreSlim instead of a lock object. Make it possible for DeploymentStateManagerBase to overrride the synchronization of the file and have UserSecretsDeploymentStateManager use the same SemaphoreSlim based on the StateFilePath. SecretsStore should expose the same SemaphoreSlim instance to synchronize access.

…erride Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot · 2025-10-31T04:21:51Z

@copilot The implementation does not look great. Base UserSecretsFileLock on a SemaphoreSlim instead of a lock object. Make it possible for DeploymentStateManagerBase to overrride the synchronization ...

Refactored the implementation based on feedback:

UserSecretsFileLock now uses SemaphoreSlim instead of object locks, with GetSemaphore() method
DeploymentStateManagerBase now has a virtual StateLock property that derived classes can override
UserSecretsDeploymentStateManager overrides StateLock to return the shared SemaphoreSlim from UserSecretsFileLock.GetSemaphore(StateFilePath)
SecretsStore uses the same shared SemaphoreSlim instances via UserSecretsFileLock.GetSemaphore()

Both systems now use the same SemaphoreSlim instance for synchronization, keyed by the normalized file path. (2c9ad74)

davidfowl · 2025-10-31T04:26:26Z

src/Aspire.Hosting/Publishing/Internal/UserSecretsDeploymentStateManager.cs

+            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(() =>


@copilot We don't need to Task.Run

Removed Task.Run and replaced with direct File.WriteAllTextAsync call. Since the base class manages synchronization via the StateLock semaphore, we can use async file operations directly. (482e759)

Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot · 2025-10-31T07:33:16Z

@copilot In the places where we use UserSecretsManagerFactory.Instance, always pass the instance in via a new constructor , or method parameter so that we can pass UserSecretsManagerFactory.Instance f...

Implemented as requested. UserSecretsParameterDefault now has a constructor that takes UserSecretsManagerFactory with an overload defaulting to Instance. Also added Create, CreateFromId, and Create(Assembly) methods to the factory for testing that create non-cached instances. Tests updated to use these new methods for test isolation. (65060b4)

davidfowl · 2025-10-31T08:03:43Z

@copilot var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets);

The azure tests are failing with this, but they don't need to us that type.

Copilot · 2025-10-31T08:07:40Z

@copilot var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets);

...

Extracted FlattenJsonObject and UnflattenJsonObject to a new JsonFlattener static class. Updated Azure tests to use JsonFlattener.FlattenJsonObject instead of DeploymentStateManagerBase<UserSecretsDeploymentStateManager>.FlattenJsonObject, eliminating the dependency on UserSecretsDeploymentStateManager. (f0ea2a2)

@copilot var result = DeploymentStateManagerBase.FlattenJsonObject(userSecrets);

...

Extracted FlattenJsonObject and UnflattenJsonObject to a new public JsonFlattener static class. Updated Azure tests to use JsonFlattener.FlattenJsonObject instead of DeploymentStateManagerBase<UserSecretsDeploymentStateManager>.FlattenJsonObject, eliminating the dependency on UserSecretsDeploymentStateManager. (998374d)

…test dependency Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

davidfowl · 2025-11-01T22:36:31Z

@copilot A set of tests are breaking that look like this:

failed Aspire.Hosting.MongoDB.Tests.MongoDbFunctionalTests.VerifyMongoDBResource (5ms)
Xunit.Runner.InProc.SystemConsole.TestingPlatform.XunitException: System.InvalidOperationException : Unable to resolve service for type 'Aspire.Hosting.UserSecrets.IUserSecretsManager' while attempting to activate 'Aspire.Hosting.VersionChecking.VersionCheckService'.

Look at the failing tests in the action https://github.com/dotnet/aspire/actions/runs/18967342489/job/54247686436?pr=12482 and fix them

…ps without UserSecretsId Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot · 2025-11-01T22:52:54Z

@copilot A set of tests are breaking that look like this:

failed Aspire.Hosting.MongoDB.Tests.MongoDbFunctionalTests.VerifyMongoDBResource (5ms)...

Fixed the DI resolution issue by making IUserSecretsManager nullable in VersionCheckService constructor and always registering it in DI (even when null for apps without UserSecretsId). The service now handles null gracefully using null-conditional operators. (17002e3)

davidfowl · 2025-11-01T23:06:38Z

@copilot If there's no user secrets id for the assembly then we should return a NoopIUserSecretsManager that logs a warning. That way callers don't need to handle null vs not null, we can tell the user via a log that user secrets is not enabled for this project.

…warning logs Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot · 2025-11-01T23:57:40Z

@copilot If there's no user secrets id for the assembly then we should return a NoopIUserSecretsManager that logs a warning. That way callers don't need to handle null vs not null, we can tell the use...

Implemented NoopUserSecretsManager using the null object pattern. It logs warnings when operations are attempted on apps without UserSecretsId configured. Updated DistributedApplicationBuilder to always return a non-null IUserSecretsManager (either real or noop), and removed all nullable checks from VersionCheckService and other consumers. (61f9af9)

@copilot If there's no user secrets id for the assembly then we should return a NoopIUserSecretsManager that logs a warning. That way callers don't need to handle null vs not null, we can tell the use...

Implemented NoopUserSecretsManager that logs warnings when user secrets are not enabled. Factory now returns this instead of null for apps without UserSecretsId. All consumers now receive non-nullable IUserSecretsManager, eliminating null checks throughout the codebase. (61f9af9)

@copilot If there's no user secrets id for the assembly then we should return a NoopIUserSecretsManager that logs a warning. That way callers don't need to handle null vs not null, we can tell the use...

Implemented NoopUserSecretsManager that logs warnings when user secrets are not enabled for a project. Factory now returns this noop implementation instead of null when there's no UserSecretsId, eliminating null checks throughout. All consumers now accept non-nullable IUserSecretsManager. (61f9af9)

davidfowl · 2025-11-02T00:32:20Z

@copilot src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs(12,48): error CS0535: (NETCORE_ENGINEERING_TELEMETRY=Build) 'NoopUserSecretsManager' does not implement interface member 'IUserSecretsManager.FilePath'

…face members Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

davidfowl · 2025-11-02T05:22:59Z

src/Aspire.Hosting/DistributedApplicationBuilder.cs


    private readonly DistributedApplicationOptions _options;
    private readonly HostApplicationBuilder _innerBuilder;
+    private readonly IUserSecretsManager? _userSecretsManager;


@copilot This should no longer be nullable.

Initial plan

bdfddf4

Copilot AI assigned Copilot and davidfowl Oct 29, 2025

Copilot started work on behalf of davidfowl October 29, 2025 07:50 View session

Copilot AI and others added 2 commits October 29, 2025 08:02

Fix SecretsStore concurrent access issue with locking mechanism

9222e94

Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Add specific test case for SQL Server + RabbitMQ concurrent secret wr…

02de777

…ites Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix user secrets not preserving all auto-generated secrets~~ Fix concurrent writes to user secrets overwriting each other Oct 29, 2025

Copilot finished work on behalf of davidfowl October 29, 2025 08:14

Copilot AI requested a review from davidfowl October 29, 2025 08:14

Copilot started work on behalf of davidfowl October 29, 2025 18:33 View session

captainsafia reviewed Oct 29, 2025

View reviewed changes

src/Shared/SecretsStore.cs Outdated Show resolved Hide resolved

Replace SemaphoreSlim with simple object locks

b852826

Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot finished work on behalf of davidfowl October 29, 2025 18:55

Copilot started work on behalf of davidfowl October 31, 2025 03:47 View session

Share lock between SecretsStore and UserSecretsDeploymentStateManager

ed54150

Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot finished work on behalf of davidfowl October 31, 2025 03:58

Copilot started work on behalf of davidfowl October 31, 2025 04:14 View session

Refactor to use SemaphoreSlim and allow DeploymentStateManagerBase ov…

2c9ad74

…erride Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot finished work on behalf of davidfowl October 31, 2025 04:22

davidfowl reviewed Oct 31, 2025

View reviewed changes

Copilot started work on behalf of davidfowl October 31, 2025 04:26 View session

Remove unnecessary Task.Run in UserSecretsDeploymentStateManager

482e759

Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot AI requested a review from davidfowl October 31, 2025 04:32

Copilot finished work on behalf of davidfowl October 31, 2025 07:34

Copilot started work on behalf of davidfowl October 31, 2025 08:04 View session

Extract FlattenJsonObject to JsonFlattener static class to fix Azure …

998374d

…test dependency Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot finished work on behalf of davidfowl October 31, 2025 08:44

Copilot started work on behalf of davidfowl November 1, 2025 22:37 View session

Make IUserSecretsManager nullable in VersionCheckService to handle ap…

17002e3

…ps without UserSecretsId Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot finished work on behalf of davidfowl November 1, 2025 22:53

Copilot started work on behalf of davidfowl November 1, 2025 23:07 View session

Add NoopUserSecretsManager to handle apps without UserSecretsId with …

61f9af9

…warning logs Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot finished work on behalf of davidfowl November 2, 2025 00:01

Copilot started work on behalf of davidfowl November 2, 2025 00:32 View session

Fix NoopUserSecretsManager to implement all IUserSecretsManager inter…

4f29f84

…face members Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>

Copilot finished work on behalf of davidfowl November 2, 2025 00:36

davidfowl reviewed Nov 2, 2025

View reviewed changes

Copilot started work on behalf of davidfowl November 2, 2025 05:23 View session

Copilot AI requested a review from davidfowl November 2, 2025 05:35

Copilot finished work on behalf of davidfowl November 2, 2025 05:35

Copilot AI mentioned this pull request Nov 2, 2025

Refactor user secrets to DI-based factory with thread-safe synchronization #12607

Closed

24 tasks

davidfowl closed this Nov 2, 2025

dotnet-policy-service bot added this to the 13.1 milestone Nov 2, 2025

github-actions bot locked and limited conversation to collaborators Dec 2, 2025

sebastienros deleted the copilot/fix-user-secrets-overwrite branch January 15, 2026 16:32

Conversation

Copilot AI commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix SecretsStore Concurrent Access Issue

Solution

Technical Details

Test Updates

Describe the bug

Expected Behavior

Steps To Reproduce

Exceptions (if any)

.NET Version info

Anything else?

Comments on the Issue (you are @copilot in this section)

Uh oh!

davidfowl commented Oct 29, 2025

Uh oh!

Uh oh!

Copilot AI commented Oct 29, 2025

Uh oh!

github-actions bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidfowl commented Oct 31, 2025

Uh oh!

Copilot AI commented Oct 31, 2025

Uh oh!

davidfowl commented Oct 31, 2025

Uh oh!

Copilot AI commented Oct 31, 2025

Uh oh!

davidfowl Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Oct 31, 2025

Uh oh!

davidfowl commented Oct 31, 2025

Uh oh!

Copilot AI commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidfowl commented Nov 1, 2025

Uh oh!

Copilot AI commented Nov 1, 2025

Uh oh!

davidfowl commented Nov 1, 2025

Uh oh!

Copilot AI commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidfowl commented Nov 2, 2025

Uh oh!

davidfowl Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Copilot AI commented Oct 29, 2025 •

edited

Loading

github-actions bot commented Oct 30, 2025 •

edited

Loading

Copilot AI commented Oct 31, 2025 •

edited

Loading

Copilot AI commented Nov 1, 2025 •

edited

Loading