Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -366,3 +366,9 @@ FodyWeavers.xsd

# Rider config directory
.idea/

# Environment files
*.env
*.env.*
!*.env.template
!*.env.*.template
5 changes: 5 additions & 0 deletions ThunderstoreCLI/Configuration/Config.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ private Config()
}
public static Config FromCLI(IConfigProvider cliConfig)
{
var environment = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT")
?? Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT")
?? Environment.GetEnvironmentVariable("ENVIRONMENT");
DotEnv.LoadAll(environment);

Comment on lines +35 to +39
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Load base .env before resolving environment; then load env-specific files.

Allows ENVIRONMENT/DOTNET_ENVIRONMENT values set in .env to influence which files are loaded next. Safe with the “don’t override existing vars” change.

-        var environment = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT")
-                          ?? Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT")
-                          ?? Environment.GetEnvironmentVariable("ENVIRONMENT");
-        DotEnv.LoadAll(environment);
+        // Load base first so it can define ENVIRONMENT variables for subsequent resolution
+        DotEnv.Load(".env");
+        var environment = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT")
+                          ?? Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT")
+                          ?? Environment.GetEnvironmentVariable("ENVIRONMENT");
+        DotEnv.LoadAll(environment);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var environment = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT")
?? Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT")
?? Environment.GetEnvironmentVariable("ENVIRONMENT");
DotEnv.LoadAll(environment);
// Load base first so it can define ENVIRONMENT variables for subsequent resolution
DotEnv.Load(".env");
var environment = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT")
?? Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT")
?? Environment.GetEnvironmentVariable("ENVIRONMENT");
DotEnv.LoadAll(environment);
🤖 Prompt for AI Agents
In ThunderstoreCLI/Configuration/Config.cs around lines 35 to 39, the code
currently reads the environment variables to determine which .env files to load
before loading any .env files; change the order so you first load the base .env
(call DotEnv.LoadAll() with no environment) so values defined in .env can set
DOTNET_ENVIRONMENT/ASPNETCORE_ENVIRONMENT/ENVIRONMENT, then re-read those
environment variables and call DotEnv.LoadAll(environment) to load
environment-specific files; ensure the loader is called with the non-overriding
option so existing vars from the base .env are not overwritten.

List<IConfigProvider> providers = new();
providers.Add(cliConfig);
providers.Add(new EnvironmentConfig());
Expand Down
55 changes: 55 additions & 0 deletions ThunderstoreCLI/Configuration/DotEnv.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
namespace ThunderstoreCLI.Configuration;

public static class DotEnv
{
public static void Load(string filePath)
{
if (!File.Exists(filePath))
return;

foreach (var line in File.ReadAllLines(filePath))
{
if (string.IsNullOrWhiteSpace(line) || line.TrimStart().StartsWith('#'))
continue;

var parts = line.Split('=', 2);
if (parts.Length != 2)
continue;

var key = parts[0].Trim();
var value = parts[1].Trim();

Comment on lines +12 to +21
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Harden parsing: support “export”, skip empty keys, handle inline comments.

Prevents malformed entries from polluting the env and aligns with common dotenv behavior.

-            var parts = line.Split('=', 2);
+            var parts = line.Split('=', 2);
             if (parts.Length != 2)
                 continue;
 
-            var key = parts[0].Trim();
-            var value = parts[1].Trim();
+            var key = parts[0].Trim();
+            if (key.StartsWith("export ", StringComparison.Ordinal))
+                key = key.Substring(7).Trim();
+            if (string.IsNullOrWhiteSpace(key))
+                continue;
+
+            var value = parts[1].Trim();
+            // Remove inline comments for unquoted values: KEY=val # comment
+            if (!(value.StartsWith("\"") || value.StartsWith("'")))
+            {
+                var hashIdx = value.IndexOf('#');
+                if (hashIdx >= 0)
+                    value = value.Substring(0, hashIdx).TrimEnd();
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (string.IsNullOrWhiteSpace(line) || line.TrimStart().StartsWith('#'))
continue;
var parts = line.Split('=', 2);
if (parts.Length != 2)
continue;
var key = parts[0].Trim();
var value = parts[1].Trim();
if (string.IsNullOrWhiteSpace(line) || line.TrimStart().StartsWith('#'))
continue;
var parts = line.Split('=', 2);
if (parts.Length != 2)
continue;
var key = parts[0].Trim();
if (key.StartsWith("export ", StringComparison.Ordinal))
key = key.Substring(7).Trim();
if (string.IsNullOrWhiteSpace(key))
continue;
var value = parts[1].Trim();
// Remove inline comments for unquoted values: KEY=val # comment
if (!(value.StartsWith("\"") || value.StartsWith("'")))
{
var hashIdx = value.IndexOf('#');
if (hashIdx >= 0)
value = value.Substring(0, hashIdx).TrimEnd();
}
🤖 Prompt for AI Agents
In ThunderstoreCLI/Configuration/DotEnv.cs around lines 12–21, the current
parser ignores lines starting with '#' but doesn't handle variable lines
prefixed with "export", allows empty keys, or strip inline comments in values;
update parsing to first TrimStart the line, remove a leading "export " if
present, locate the first '=' and split into key and value from that index, trim
both, skip the entry if key is empty, and for the value strip an inline comment
beginning with an unescaped '#' (but preserve '#' inside quoted values), then
remove surrounding single/double quotes from the value; ensure you continue on
malformed lines rather than throwing.

if (value.Length >= 2)
{
if ((value.StartsWith('"') && value.EndsWith('"')) ||
(value.StartsWith('\'') && value.EndsWith('\'')))
{
value = value.Substring(1, value.Length - 2);
}
}

Environment.SetEnvironmentVariable(key, value);
Comment on lines +22 to +31
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Don’t override existing OS environment variables by default.

Standard dotenv practice: keep OS-provided vars; only fill missing.

-            if (value.Length >= 2)
+            if (value.Length >= 2)
             {
                 if ((value.StartsWith('"') && value.EndsWith('"')) ||
                     (value.StartsWith('\'') && value.EndsWith('\'')))
                 {
                     value = value.Substring(1, value.Length - 2);
                 }
             }
 
-            Environment.SetEnvironmentVariable(key, value);
+            if (Environment.GetEnvironmentVariable(key) is null)
+            {
+                Environment.SetEnvironmentVariable(key, value);
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (value.Length >= 2)
{
if ((value.StartsWith('"') && value.EndsWith('"')) ||
(value.StartsWith('\'') && value.EndsWith('\'')))
{
value = value.Substring(1, value.Length - 2);
}
}
Environment.SetEnvironmentVariable(key, value);
if (value.Length >= 2)
{
if ((value.StartsWith('"') && value.EndsWith('"')) ||
(value.StartsWith('\'') && value.EndsWith('\'')))
{
value = value.Substring(1, value.Length - 2);
}
}
if (Environment.GetEnvironmentVariable(key) is null)
{
Environment.SetEnvironmentVariable(key, value);
}
🤖 Prompt for AI Agents
In ThunderstoreCLI/Configuration/DotEnv.cs around lines 22 to 31, the current
code unconditionally calls Environment.SetEnvironmentVariable(key, value) which
overrides existing OS environment variables; change it to only set the variable
if it is not already present by checking Environment.GetEnvironmentVariable(key)
(treat null/empty as missing) and call SetEnvironmentVariable only when the
existing value is null or empty, preserving OS-provided vars.

}
}

public static void LoadAll(string? environment = null)
{
var filesToLoad = new List<string> { ".env" };

if (!string.IsNullOrEmpty(environment))
{
filesToLoad.Add($".env.{environment}");
filesToLoad.Add($".env.{environment.ToLowerInvariant()}");
filesToLoad.Add($".{environment}.env");
filesToLoad.Add($".{environment.ToLowerInvariant()}.env");
}
Comment on lines +39 to +45
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Sanitize environment name to avoid path traversal and weird filenames.

An attacker-controlled ENVIRONMENT could inject path separators. Allow only [A-Za-z0-9_-].

-        if (!string.IsNullOrEmpty(environment))
+        if (!string.IsNullOrEmpty(environment))
         {
-            filesToLoad.Add($".env.{environment}");
-            filesToLoad.Add($".env.{environment.ToLowerInvariant()}");
-            filesToLoad.Add($".{environment}.env");
-            filesToLoad.Add($".{environment.ToLowerInvariant()}.env");
+            var envSafe = new string(environment.Where(ch => char.IsLetterOrDigit(ch) || ch is '-' or '_').ToArray());
+            if (!string.IsNullOrEmpty(envSafe))
+            {
+                filesToLoad.Add($".env.{envSafe}");
+                filesToLoad.Add($".env.{envSafe.ToLowerInvariant()}");
+                filesToLoad.Add($".{envSafe}.env");
+                filesToLoad.Add($".{envSafe.ToLowerInvariant()}.env");
+            }
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!string.IsNullOrEmpty(environment))
{
filesToLoad.Add($".env.{environment}");
filesToLoad.Add($".env.{environment.ToLowerInvariant()}");
filesToLoad.Add($".{environment}.env");
filesToLoad.Add($".{environment.ToLowerInvariant()}.env");
}
if (!string.IsNullOrEmpty(environment))
{
var envSafe = new string(
environment
.Where(ch => char.IsLetterOrDigit(ch) || ch is '-' or '_')
.ToArray()
);
if (!string.IsNullOrEmpty(envSafe))
{
filesToLoad.Add($".env.{envSafe}");
filesToLoad.Add($".env.{envSafe.ToLowerInvariant()}");
filesToLoad.Add($".{envSafe}.env");
filesToLoad.Add($".{envSafe.ToLowerInvariant()}.env");
}
}
🤖 Prompt for AI Agents
In ThunderstoreCLI/Configuration/DotEnv.cs around lines 39 to 45, the code
appends environment-derived filenames without sanitizing the environment string,
allowing path traversal or odd filenames; validate the environment value to only
allow characters A-Za-z0-9_- (e.g., use a Regex.IsMatch or sanitize by removing
invalid characters) and if it fails validation either skip adding those files or
throw a clear ArgumentException; then build filenames only from the
validated/sanitized value (and its ToLowerInvariant()) before adding them to
filesToLoad.


filesToLoad.Add(".env.local");
filesToLoad.Add(".local.env");

foreach (var file in filesToLoad)
{
Load(file);
}
}
}