Skip to content

Implicitly set UserSecretsId for file-based apps #50783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 18, 2025
Merged

Implicitly set UserSecretsId for file-based apps #50783

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3f4203a
Implicitly set UserSecretsId for file-based apps
jjonescz Sep 3, 2025
16d7e59
Update the spec
jjonescz Sep 12, 2025
98a7327
Test explicit ID as well
jjonescz Sep 12, 2025
9cd2089
Test user secrets with CSC only mode
jjonescz Sep 15, 2025
5c9429b
Include UserSecretsId in the converted project
jjonescz Sep 15, 2025
348f3b8
Improve code and tests
jjonescz Sep 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions documentation/general/dotnet-run-file.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ Additionally, the implicit project file has the following customizations:

- `PublishAot` is set to `true`, see [`dotnet publish file.cs`](#other-commands) for more details.

- `UserSecretsId` is set to a hash of the entry point file path.

- [File-level directives](#directives-for-project-metadata) are applied.

- The following are virtual only, i.e., not preserved after [converting to a project](#grow-up):
Expand Down
31 changes: 20 additions & 11 deletions src/Cli/dotnet/Commands/Project/Convert/ProjectConvertCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ public override int Execute()
var sourceFile = SourceFile.Load(file);
var directives = VirtualProjectBuildingCommand.FindDirectives(sourceFile, reportAllErrors: !_force, DiagnosticBag.ThrowOnFirst());

// Create a project instance for evaluation.
string entryPointFileDirectory = PathUtility.EnsureTrailingSlash(Path.GetDirectoryName(file)!);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider making this just a parameter to FindIncludedItems, in order to make things more clear. It feels unnecessary to rely on capture, when the variable is not used in the declaring method itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most straightforward thing might be to delete the declaration here and restore the declaration in FindIncludedItems().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with Rikki, that this is only referenced via capture is somewhat confusing. I'd personally just pass this as a parameter where needed.

Copy link
Member Author

@jjonescz jjonescz Sep 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most straightforward thing might be to delete the declaration here and restore the declaration in FindIncludedItems().

Yes, thanks, I extracted this line unintentionally (because it was in the block of code that I wanted to extract).

Copy link
Member

@333fred 333fred Sep 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the latest commit really addresses the concern, am I missing something? Or were Rikki and I commenting on different things? I assumed Rikki meant the entire projectCollection/projectInstance dual locals; I'm not a fan of how they're only referenced via capture.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about pushing on the capture thing on more than just entryPointFileDirectory but decided I didn’t feel too strongly. However I would generally prefer that locals are not used as “capture only” even if multiple local functions are capturing them.

Copy link
Member Author

@jjonescz jjonescz Sep 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying we should never use non-static local functions?

I think some capturing makes for a cleaner code, although for code reviews I would recommend using VSCode with C# extension where you can actually hover over the function to see the captured variables.

I also don't like functions with a lot of "weakly" typed arguments (like bools and strings) as it's easy to pass the wrong thing to them. Capturing also helps with this, because you just reference the original thing by name.

Given this is a pre-existing issue (the existing local functions are already capturing variables) and you both signed off, I'm going to merge this, but happy to discuss further and change things in a follow up PR if needed.

var projectCollection = new ProjectCollection();
var command = new VirtualProjectBuildingCommand(
entryPointFileFullPath: file,
msbuildArgs: MSBuildArgs.FromOtherArgs([]))
{
Directives = directives,
};
var projectInstance = command.CreateProjectInstance(projectCollection);

// Find other items to copy over, e.g., default Content items like JSON files in Web apps.
var includeItems = FindIncludedItems().ToList();

Expand Down Expand Up @@ -61,7 +72,8 @@ public override int Execute()
{
using var stream = File.Open(projectFile, FileMode.Create, FileAccess.Write);
using var writer = new StreamWriter(stream, Encoding.UTF8);
VirtualProjectBuildingCommand.WriteProjectFile(writer, directives, isVirtualProject: false);
VirtualProjectBuildingCommand.WriteProjectFile(writer, directives, isVirtualProject: false,
userSecretsId: DetermineUserSecretsId());
}

// Copy or move over included items.
Expand Down Expand Up @@ -111,16 +123,6 @@ void CopyFile(string source, string target)

IEnumerable<(string FullPath, string RelativePath)> FindIncludedItems()
{
string entryPointFileDirectory = PathUtility.EnsureTrailingSlash(Path.GetDirectoryName(file)!);
var projectCollection = new ProjectCollection();
var command = new VirtualProjectBuildingCommand(
entryPointFileFullPath: file,
msbuildArgs: MSBuildArgs.FromOtherArgs([]))
{
Directives = directives,
};
var projectInstance = command.CreateProjectInstance(projectCollection);

// Include only items we know are files.
string[] itemTypes = ["Content", "None", "Compile", "EmbeddedResource"];
var items = itemTypes.SelectMany(t => projectInstance.GetItems(t));
Expand Down Expand Up @@ -151,6 +153,13 @@ void CopyFile(string source, string target)
yield return (FullPath: itemFullPath, RelativePath: itemRelativePath);
}
}

string? DetermineUserSecretsId()
{
var implicitValue = projectInstance.GetPropertyValue("_ImplicitFileBasedProgramUserSecretsId");
var actualValue = projectInstance.GetPropertyValue("UserSecretsId");
return implicitValue == actualValue ? actualValue : null;
}
}

private string DetermineOutputDirectory(string file)
Expand Down
10 changes: 9 additions & 1 deletion src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,8 @@ public static void WriteProjectFile(
bool isVirtualProject,
string? targetFilePath = null,
string? artifactsPath = null,
bool includeRuntimeConfigInformation = true)
bool includeRuntimeConfigInformation = true,
string? userSecretsId = null)
{
int processedDirectives = 0;

Expand Down Expand Up @@ -1255,6 +1256,13 @@ public static void WriteProjectFile(
}
}

if (userSecretsId != null && !customPropertyNames.Contains("UserSecretsId"))
{
writer.WriteLine($"""
<UserSecretsId>{EscapeValue(userSecretsId)}</UserSecretsId>
""");
}

// Write virtual-only properties.
if (isVirtualProject)
{
Expand Down
4 changes: 4 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ Copyright (c) .NET Foundation. All rights reserved.

<!-- Uncomment this once https://github.com/Microsoft/visualfsharp/issues/3207 gets fixed -->
<!-- <WarningsAsErrors>$(WarningsAsErrors);NU1605</WarningsAsErrors> -->

<!-- Implicitly set UserSecretsId for file-based apps. -->
<_ImplicitFileBasedProgramUserSecretsId Condition="'$(FileBasedProgram)' == 'true'">$(MSBuildProjectName)-$([MSBuild]::StableStringHash($(MSBuildProjectFullPath.ToLowerInvariant()), 'Sha256'))</_ImplicitFileBasedProgramUserSecretsId>
<UserSecretsId Condition="'$(FileBasedProgram)' == 'true' and '$(UserSecretsId)' == ''">$(_ImplicitFileBasedProgramUserSecretsId)</UserSecretsId>
</PropertyGroup>

<PropertyGroup>
Expand Down
89 changes: 87 additions & 2 deletions test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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.RegularExpressions;
using Microsoft.CodeAnalysis.Text;
using Microsoft.DotNet.Cli.Commands;
using Microsoft.DotNet.Cli.Commands.Run;
Expand Down Expand Up @@ -56,9 +57,11 @@ public void SameAsTemplate()
var dotnetProjectConvertProjectText = File.ReadAllText(dotnetProjectConvertProject);
var dotnetNewConsoleProjectText = File.ReadAllText(dotnetNewConsoleProject);

// There are some differences: we add PublishAot=true.
// There are some differences: we add PublishAot=true and UserSecretsId.
var patchedDotnetProjectConvertProjectText = dotnetProjectConvertProjectText
.Replace(" <PublishAot>true</PublishAot>" + Environment.NewLine, string.Empty);
patchedDotnetProjectConvertProjectText = Regex.Replace(patchedDotnetProjectConvertProjectText,
""" <UserSecretsId>[^<]*<\/UserSecretsId>""" + Environment.NewLine, string.Empty);

patchedDotnetProjectConvertProjectText.Should().Be(dotnetNewConsoleProjectText)
.And.StartWith("""<Project Sdk="Microsoft.NET.Sdk">""");
Expand Down Expand Up @@ -648,7 +651,7 @@ public void ProcessingSucceeds()
.Should().Be("Console.WriteLine();");

File.ReadAllText(Path.Join(testInstance.Path, "Program", "Program.csproj"))
.Should().Be($"""
.Should().Match($"""
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
Expand All @@ -657,6 +660,7 @@ public void ProcessingSucceeds()
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<PublishAot>true</PublishAot>
<UserSecretsId>Program-*</UserSecretsId>
</PropertyGroup>

<ItemGroup>
Expand All @@ -668,6 +672,87 @@ public void ProcessingSucceeds()
""");
}

[Theory, CombinatorialData]
public void UserSecretsId_Overridden_ViaDirective(bool hasDirectiveBuildProps)
{
var testInstance = _testAssetsManager.CreateTestDirectory();
File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), """
#:property UserSecretsId=MyIdFromDirective
Console.WriteLine();
""");

if (hasDirectiveBuildProps)
{
File.WriteAllText(Path.Join(testInstance.Path, "Directory.Build.props"), """
<Project>
<PropertyGroup>
<UserSecretsId>MyIdFromDirBuildProps</UserSecretsId>
</PropertyGroup>
</Project>
""");
}

new DotnetCommand(Log, "project", "convert", "Program.cs")
.WithWorkingDirectory(testInstance.Path)
.Execute()
.Should().Pass();

File.ReadAllText(Path.Join(testInstance.Path, "Program", "Program.csproj"))
.Should().Be($"""
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>{ToolsetInfo.CurrentTargetFramework}</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<PublishAot>true</PublishAot>
<UserSecretsId>MyIdFromDirective</UserSecretsId>
</PropertyGroup>

</Project>

""");
}

[Fact]
public void UserSecretsId_Overridden_ViaDirectoryBuildProps()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following are corner cases, but it may be worthwhile to test them, just so we know the behavior.

  • UserSecretsId is specified via #:property and the value is exactly the same as the one we determined implicitly.
  • UserSecretsId is specified via Directory.Build.props and the value is exactly the same as the one we determined implicitly.

{
var testInstance = _testAssetsManager.CreateTestDirectory();

File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), """
Console.WriteLine();
""");
File.WriteAllText(Path.Join(testInstance.Path, "Directory.Build.props"), """
<Project>
<PropertyGroup>
<UserSecretsId>MyIdFromDirBuildProps</UserSecretsId>
</PropertyGroup>
</Project>
""");

new DotnetCommand(Log, "project", "convert", "Program.cs")
.WithWorkingDirectory(testInstance.Path)
.Execute()
.Should().Pass();

File.ReadAllText(Path.Join(testInstance.Path, "Program", "Program.csproj"))
.Should().Be($"""
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>{ToolsetInfo.CurrentTargetFramework}</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<PublishAot>true</PublishAot>
</PropertyGroup>

</Project>

""");
}

[Fact]
public void Directives()
{
Expand Down
64 changes: 64 additions & 0 deletions test/dotnet.Tests/CommandTests/Run/RunFileTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2177,6 +2177,70 @@ public void ProjectReference_Errors()
string.Format(CliStrings.MoreThanOneProjectInDirectory, Path.Join(testInstance.Path, "dir/"))));
}

[Theory] // https://github.com/dotnet/aspnetcore/issues/63440
[InlineData(true, null)]
[InlineData(false, null, Skip = "Needs https://github.com/dotnet/aspnetcore/pull/63496")]
[InlineData(true, "test-id")]
[InlineData(false, "test-id", Skip = "Needs https://github.com/dotnet/aspnetcore/pull/63496")]
public void UserSecrets(bool useIdArg, string? userSecretsId)
{
var testInstance = _testAssetsManager.CreateTestDirectory();

string code = $"""
#:package Microsoft.Extensions.Configuration.UserSecrets@*-*
Copy link
Member

@RikkiGibson RikkiGibson Sep 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure if it is best to use a wildcard version here. For example, it could be disruptive if someone were PRing into a servicing branch, and some interaction with a newer version of the package, was causing the old branch version of this test to fail.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, thanks.

{(userSecretsId is null ? "" : $"#:property UserSecretsId={userSecretsId}")}

using Microsoft.Extensions.Configuration;

IConfigurationRoot config = new ConfigurationBuilder()
.AddUserSecrets<Program>()
.Build();

Console.WriteLine("v1");
Console.WriteLine(config.GetDebugView());
""";

var programPath = Path.Join(testInstance.Path, "Program.cs");
File.WriteAllText(programPath, code);

if (useIdArg)
{
if (userSecretsId == null)
{
var result = new DotnetCommand(Log, "build", "-getProperty:UserSecretsId", "Program.cs")
.WithWorkingDirectory(testInstance.Path)
.Execute();
result.Should().Pass();
userSecretsId = result.StdOut!.Trim();
}

new DotnetCommand(Log, "user-secrets", "set", "MySecret", "MyValue", "--id", userSecretsId)
.WithWorkingDirectory(testInstance.Path)
.Execute()
.Should().Pass();
}
else
{
new DotnetCommand(Log, "user-secrets", "set", "MySecret", "MyValue", "--file", "Program.cs")
.WithWorkingDirectory(testInstance.Path)
.Execute()
.Should().Pass();
}

Build(testInstance, BuildLevel.All, expectedOutput: """
v1
MySecret=MyValue (JsonConfigurationProvider for 'secrets.json' (Optional))
""");

code = code.Replace("v1", "v2");
File.WriteAllText(programPath, code);

Build(testInstance, BuildLevel.Csc, expectedOutput: """
v2
MySecret=MyValue (JsonConfigurationProvider for 'secrets.json' (Optional))
""");
}

/// <summary>
/// Verifies that msbuild-based runs use CSC args equivalent to csc-only runs.
/// Can regenerate CSC arguments template in <see cref="CSharpCompilerCommand"/>.
Expand Down
Loading