-
Notifications
You must be signed in to change notification settings - Fork 13
Add .env integration #106
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.
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
base: master
Are you sure you want to change the base?
Add .env integration #106
Conversation
WalkthroughIntroduces dotenv loading support. Adds a DotEnv utility to parse .env files, updates Config.FromCLI to resolve environment (DOTNET_ENVIRONMENT → ASPNETCORE_ENVIRONMENT → ENVIRONMENT) and call DotEnv.LoadAll before building providers, and updates .gitignore to ignore .env files while keeping templates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as ThunderstoreCLI
participant Config as Config.FromCLI
participant DotEnv as DotEnv
participant OS as Environment Vars
Note over User,CLI: Start CLI with potential .env files present
User->>CLI: Invoke command
CLI->>Config: Build configuration
Config->>OS: Read DOTNET_ENVIRONMENT / ASPNETCORE_ENVIRONMENT / ENVIRONMENT
Config->>DotEnv: LoadAll(resolvedEnvironment)
DotEnv->>DotEnv: Determine candidate .env files (base, env-specific, local)
DotEnv->>OS: Set environment variables for each parsed KEY=VALUE
Config->>Config: Build providers (CLI, Environment, Project, Default)
Config-->>CLI: Resolved configuration (e.g., TCLI_AUTH_TOKEN)
CLI-->>User: Execute with loaded settings
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (3)
.gitignore (1)
370-374
: Unignore common “example” variants.Projects often commit .env.example files. Consider adding these exceptions.
# Environment files *.env *.env.* !*.env.template !*.env.*.template +!*.env.example +!*.env.*.exampleThunderstoreCLI/Configuration/DotEnv.cs (2)
37-54
: Optional: de-duplicate candidate list.Lower/upper variants can collide; HashSet avoids redundant IO.
- foreach (var file in filesToLoad) + foreach (var file in new HashSet<string>(filesToLoad)) { Load(file); }
5-33
: Add tests for parser edge cases.Cover comments, quotes, export prefix, empty keys, and no-override behavior.
I can add unit tests under ThunderstoreCLI/Configuration.Tests/DotEnvTests.cs validating these cases—want me to open a follow-up PR?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.gitignore
(1 hunks)ThunderstoreCLI/Configuration/Config.cs
(1 hunks)ThunderstoreCLI/Configuration/DotEnv.cs
(1 hunks)
🔇 Additional comments (3)
.gitignore (1)
370-374
: Good ignore rules for dotenv files.This prevents accidental commits of secrets while preserving templates.
ThunderstoreCLI/Configuration/DotEnv.cs (1)
1-1
: No action needed—ImplicitUsings is enabled
The main ThunderstoreCLI.csproj has<ImplicitUsings>enable</ImplicitUsings>
(line 25), so the common namespaces (including System, System.IO, System.Collections.Generic, etc.) are already imported.ThunderstoreCLI/Configuration/Config.cs (1)
35-39
: Env provider mapping verified. EnvironmentConfig.GetAuthConfig readsTCLI_AUTH_TOKEN
intoAuthConfig.AuthToken
, and that value is subsequently consumed by ApiHelper and validated in PublishCommand.
var environment = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT") | ||
?? Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") | ||
?? Environment.GetEnvironmentVariable("ENVIRONMENT"); | ||
DotEnv.LoadAll(environment); | ||
|
There was a problem hiding this comment.
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.
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.
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(); | ||
|
There was a problem hiding this comment.
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.
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); |
There was a problem hiding this comment.
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.
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.
if (!string.IsNullOrEmpty(environment)) | ||
{ | ||
filesToLoad.Add($".env.{environment}"); | ||
filesToLoad.Add($".env.{environment.ToLowerInvariant()}"); | ||
filesToLoad.Add($".{environment}.env"); | ||
filesToLoad.Add($".{environment.ToLowerInvariant()}.env"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
closes #105
Summary by CodeRabbit
New Features
Chores