Refactor user secrets to DI-based factory with thread-safe synchronization by Copilot · Pull Request #12732 · microsoft/aspire

Copilot · 2025-11-06T00:59:51Z

Description

Ports PR #12692 to release/13.0. Replaces static SecretsStore methods with a dependency injection-based factory pattern that provides thread-safe synchronization for user secrets file access.

Architecture Changes

New Components:

IUserSecretsManager - interface for user secrets operations
UserSecretsManagerFactory - singleton factory with lock-based instance caching
NoopUserSecretsManager - null object pattern when user secrets not configured
JsonFlattener - utility for JSON flattening/unflattening (extracted from DeploymentStateManagerBase)

Thread Safety:

Each UserSecretsManager instance has its own SemaphoreSlim
Factory ensures same instance per normalized file path
Prevents race conditions when multiple resources generate passwords concurrently

Core Changes

DistributedApplicationBuilder - registers IUserSecretsManager singleton in DI container
UserSecretsParameterDefault - factory-based constructor with optional custom factory for test isolation
UserSecretsDeploymentStateManager - injects IUserSecretsManager via constructor
VersionCheckService - injects IUserSecretsManager instead of using static methods
Removed src/Shared/SecretsStore.cs and static method calls

Test Updates

Added 6 concurrency tests validating thread-safe behavior
Moved JsonFlattenerTests from Azure.Tests to Aspire.Hosting.Tests
Updated all test helpers to use new factory pattern

Before:

// Static method with race conditions
SecretsStore.TrySetUserSecret(assembly, "Parameters:password", value);

After:

// DI-injected manager with per-instance locking
_userSecretsManager.TrySetSecret("Parameters:password", value);

Checklist

Is this feature complete?
- Yes. Ready to ship.
- No. Follow-up changes expected.
Are you including unit tests for the changes and scenario tests if relevant?
- Yes
- No
Did you add public API?
- Yes
- No
Does the change make any security assumptions or guarantees?
- Yes
- No
Does the change require an update in our Aspire docs?
- Yes
- No

Original prompt

Make the changes from #12695

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

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

joperezr · 2025-11-06T01:47:54Z

Discussed offline. Approved. @davidfowl please let me know when this is ready to go.

github-actions · 2025-11-06T02:01:34Z

🚀 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 -- 12732

Or

Run remotely in PowerShell:

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

Copilot

Pull Request Overview

This PR refactors the user secrets management infrastructure to fix concurrency issues and improve modularity. The main issue addressed is that concurrent writes to user secrets (e.g., when multiple resources generate passwords simultaneously) can result in data loss due to race conditions.

Key changes:

Replaced the static SecretsStore class with a new factory-based IUserSecretsManager interface and implementation that uses a SemaphoreSlim for thread-safe file operations
Moved JSON flattening logic from DeploymentStateManagerBase to a dedicated JsonFlattener utility class for better code organization
Updated all consumers of SecretsStore to use the new IUserSecretsManager abstraction

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/Shared/SecretsStore.cs`	Deleted the old `SecretsStore` class that had concurrency issues
`src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs`	New factory class that creates and caches thread-safe `IUserSecretsManager` instances using `SemaphoreSlim` for synchronization
`src/Aspire.Hosting/UserSecrets/IUserSecretsManager.cs`	New interface defining the contract for user secrets operations
`src/Aspire.Hosting/UserSecrets/NoopUserSecretsManager.cs`	New no-op implementation used when user secrets are not configured
`src/Aspire.Hosting/Pipelines/Internal/JsonFlattener.cs`	Extracted JSON flattening logic into a dedicated utility class
`src/Aspire.Hosting/Pipelines/Internal/UserSecretsDeploymentStateManager.cs`	Updated to use `IUserSecretsManager` instead of direct file operations
`src/Aspire.Hosting/Pipelines/Internal/FileDeploymentStateManager.cs`	Updated to use `JsonFlattener` instead of the base class method
`src/Aspire.Hosting/Pipelines/Internal/DeploymentStateManagerBase.cs`	Removed JSON flattening methods (moved to `JsonFlattener`)
`src/Aspire.Hosting/DistributedApplicationBuilder.cs`	Updated to create and register `IUserSecretsManager` using the factory
`src/Aspire.Hosting/ApplicationModel/UserSecretsParameterDefault.cs`	Updated to use `UserSecretsManagerFactory` with optional factory parameter for testability
`src/Aspire.Hosting/VersionChecking/VersionCheckService.cs`	Updated to use `IUserSecretsManager` instead of static `SecretsStore` methods
`src/Aspire.Hosting/Aspire.Hosting.csproj`	Removed the link to the deleted `SecretsStore.cs` file
`tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs`	Added comprehensive concurrency tests and updated existing tests to use the new API
`tests/Aspire.Hosting.Tests/SecretsStoreTests.cs`	Updated tests to use `IUserSecretsManager`
`tests/Aspire.Hosting.Tests/JsonFlattenerTests.cs`	Updated tests to use `JsonFlattener` static class
`tests/Aspire.Hosting.Tests/VersionChecking/VersionCheckServiceTests.cs`	Updated test helper to accept and pass `IUserSecretsManager` parameter

Copilot · 2025-11-06T02:11:42Z

+    // Dictionary to cache instances by file path
+    private readonly Dictionary<string, IUserSecretsManager> _managerCache = new();
+    private readonly object _lock = new();
+


The constructor lacks XML documentation. Add a summary explaining this is internal and used for creating isolated factory instances for testing.

Suggested change

/// <summary>

/// Initializes a new instance of the <see cref="UserSecretsManagerFactory"/> class.

/// This constructor is internal and used for creating isolated factory instances for testing.

/// </summary>

Copilot · 2025-11-06T02:11:43Z

 /// <param name="parameterDefault">The <see cref="ParameterDefault"/> that will produce the default value when it isn't found in the project's user secrets store.</param>
-internal sealed class UserSecretsParameterDefault(Assembly appHostAssembly, string applicationName, string parameterName, ParameterDefault parameterDefault)
+/// <param name="factory">The factory to use for creating user secrets managers.</param>
+internal sealed class UserSecretsParameterDefault(Assembly appHostAssembly, string applicationName, string parameterName, ParameterDefault parameterDefault, UserSecretsManagerFactory factory)


The new constructor parameter factory is documented in the param tag, but the class-level summary on line 12-17 should be updated to mention that multiple constructors exist and explain when to use the factory parameter overload.

Copilot · 2025-11-06T02:11:43Z

+        var userSecretsId = assembly?.GetCustomAttribute<UserSecretsIdAttribute>()?.UserSecretsId;
+        return GetOrCreateFromId(userSecretsId);
+    }
+


The nested UserSecretsManager class lacks XML documentation. Add a summary explaining it's the concrete implementation that provides thread-safe file operations via SemaphoreSlim.

Suggested change

/// <summary>

/// Concrete <see cref="IUserSecretsManager"/> implementation that provides thread-safe file operations using <see cref="SemaphoreSlim"/>.

/// </summary>

Copilot · 2025-11-06T02:11:43Z

Extra blank line at the end of the file should be removed according to standard formatting conventions.

Suggested change

Initial plan

4786f6d

Copilot AI assigned Copilot and davidfowl Nov 6, 2025

Copilot started work on behalf of davidfowl November 6, 2025 01:00 View session

Copilot AI and others added 3 commits November 6, 2025 01:11

Create new UserSecrets infrastructure and update core classes

fa4d9c8

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

Update tests to use new UserSecrets infrastructure

5836564

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

Fix compilation errors in tests and UserSecretsManagerFactory

901db8d

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

Copilot AI changed the title ~~[WIP] Update Aspire to latest version as per PR 12695~~ Refactor user secrets to DI-based factory with thread-safe synchronization Nov 6, 2025

Copilot AI requested a review from davidfowl November 6, 2025 01:30

Copilot finished work on behalf of davidfowl November 6, 2025 01:30

joperezr added the Servicing-approved Approved for servicing release label Nov 6, 2025

davidfowl marked this pull request as ready for review November 6, 2025 02:01

davidfowl requested a review from mitchdenny as a code owner November 6, 2025 02:01

Copilot AI review requested due to automatic review settings November 6, 2025 02:01

Copilot AI reviewed Nov 6, 2025

View reviewed changes

davidfowl approved these changes Nov 6, 2025

View reviewed changes

mitchdenny approved these changes Nov 6, 2025

View reviewed changes

joperezr merged commit 00f258c into release/13.0 Nov 6, 2025
306 of 308 checks passed

joperezr deleted the copilot/update-aspire-to-latest-version branch November 6, 2025 18:28

github-actions Bot locked and limited conversation to collaborators Dec 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

Refactor user secrets to DI-based factory with thread-safe synchronization#12732
joperezr merged 4 commits into
release/13.0from
copilot/update-aspire-to-latest-version

Copilot AI commented Nov 6, 2025 •

edited

Loading

Uh oh!

joperezr commented Nov 6, 2025

Uh oh!

github-actions Bot commented Nov 6, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 6, 2025

Uh oh!

Copilot AI Nov 6, 2025

Uh oh!

Copilot AI Nov 6, 2025

Uh oh!

Copilot AI Nov 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

+    /// <summary>
+    /// Initializes a new instance of the <see cref="UserSecretsManagerFactory"/> class.
+    /// This constructor is internal and used for creating isolated factory instances for testing.
+    /// </summary>

Uh oh!

Conversation

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

Uh oh!

Description

Architecture Changes

Core Changes

Test Updates

Checklist

Uh oh!

joperezr commented Nov 6, 2025

Uh oh!

github-actions Bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Copilot AI commented Nov 6, 2025 •

edited

Loading

github-actions Bot commented Nov 6, 2025 •

edited

Loading