Skip to content

Normalize package source names when resolving environment variable credentials #6808

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

Closed
Foitn wants to merge 1 commit into from
Closed

Normalize package source names when resolving environment variable credentials #6808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 8 additions & 1 deletion src/NuGet.Core/NuGet.Common/EnvironmentVariableWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Security;
using System.Text.RegularExpressions;

namespace NuGet.Common
{
Expand All @@ -15,13 +16,19 @@ public class EnvironmentVariableWrapper : IEnvironmentVariableReader
try
{
#pragma warning disable RS0030 // Do not use banned APIs
return Environment.GetEnvironmentVariable(variable);
return Environment.GetEnvironmentVariable(NormalizeSourceName(variable));
#pragma warning restore RS0030 // Do not use banned APIs
}
catch (SecurityException)
{
return null;
}
}

private static string NormalizeSourceName(string sourceName)
{
// Replace invalid env var chars with underscore
return Regex.Replace(sourceName, @"[^A-Za-z0-9_]", "_");
Copy link
Member

@zivkan zivkan Sep 30, 2025

Choose a reason for hiding this comment

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

This excludes a whole lot of allowed environment variable characters, basically all valid characters that are not English language letters. I tried both accented latin characters, as well as one non-latin character, and at least on Windows there's no problems using them as environment variables.

Copy link
Author

@Foitn Foitn Sep 30, 2025

Choose a reason for hiding this comment

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

Well we could also use a filter the other way around, so lets say that all characters that match "#@%!-*" etc, are replaced as '_'
That would at least solve the issue with allowed environment variable characters that are currently not in this set and it would solve the linked issue

}
}
}
18 changes: 18 additions & 0 deletions test/NuGet.Core.Tests/NuGet.Common.Test/EnvironmentVariableWrapperTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,23 @@ public void GetEnvironmentVariable_WhenVariableExists_ReturnsValue()

Assert.Equal(expectedValue, actualValue);
}

[Theory]
[InlineData("Foo", "Foo")]
[InlineData("Foo.Bar", "Foo_Bar")]
[InlineData("Foo-Bar", "Foo_Bar")]
[InlineData("Foo Bar!", "Foo_Bar_")]
[InlineData("^Foo@Bar#", "_Foo_Bar_")]
public void GetEnvironmentVariable_WhenVariableIsNotNormalized_NormalizeValue(string sourceName, string normalizedSourceName)
{
var expectedValue = Guid.NewGuid().ToString();
var instance = EnvironmentVariableWrapper.Instance;

Environment.SetEnvironmentVariable(normalizedSourceName, expectedValue);

var actualValue = instance.GetEnvironmentVariable(sourceName);

Assert.Equal(expectedValue, actualValue);
}
}
}
Loading