Env: add support for detecting env template variables #310
Conversation
WalkthroughThe changes enhance environment variable management during cloud deployment and environment file handling. They add support for parsing template files with comments, improve detection of secret variables via comments, and refine user prompts for missing variables. Related function signatures and struct fields are updated to support these features, and a dependency is bumped. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant EnvFile
participant TemplateFile
User->>CLI: Initiate cloud deployment
CLI->>EnvFile: Parse .env with comments
CLI->>TemplateFile: Parse .env.example/.env.template with comments
CLI->>CLI: Compare variables between files
alt Missing variables found
CLI->>User: Show styled warning, prompt to add variable
User->>CLI: Provide value (if accepted)
CLI->>EnvFile: Append variable with comment
end
CLI->>User: Proceed with deployment
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/cloud.go (1)
310-332: Remove redundant assignment_ = filenameThe blank-identifier assignment on line 317 serves no purpose (the variable is used below, so it will never trigger the “unused” compiler warning).
- _ = filenameEliminating the line tightens the code and avoids confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
cmd/cloud.go(7 hunks)cmd/dev.go(1 hunks)cmd/env.go(4 hunks)go.mod(1 hunks)internal/deployer/deployer.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
cmd/cloud.go (5)
internal/util/io.go (1)
Exists(14-19)internal/errsystem/errsystem.go (2)
New(33-63)WithContextMessage(100-104)internal/errsystem/errorcodes.go (1)
ErrParseEnvironmentFile(41-44)internal/deployer/deployer.go (1)
EnvFile(14-17)internal/util/strings.go (1)
Pluralize(32-40)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build and Test (windows-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (11)
cmd/dev.go (1)
46-46: Signal handling simplificationThe signal handling has been simplified by directly passing all signals (os.Interrupt, syscall.SIGINT, syscall.SIGTERM) to signal.NotifyContext rather than using conditional OS-specific logic.
go.mod (1)
7-7:Details
✅ Verification successful
Dependency update to support environment template variables
The go-common dependency has been updated from v1.0.59 to v1.0.60 to support the new environment template variables functionality.
🏁 Script executed:
#!/bin/bash # Check what changes were introduced in this dependency version git log --oneline -1 v1.0.60 || echo "Cannot find version tag in local repo" # Try to fetch the repository to check the changes cd /tmp && git clone --depth=2 https://github.com/agentuity/go-common.git 2>/dev/null && cd go-common && git log --name-status -1Length of output: 0
🏁 Script executed:
#!/bin/bash # Clone the specific v1.0.60 tag to inspect its commit git clone --single-branch --branch v1.0.60 --depth=1 https://github.com/agentuity/go-common.git /tmp/go-common cd /tmp/go-common git log --name-status -1Length of output: 807
🏁 Script executed:
#!/bin/bash # Clone a shallow copy with tags to compare v1.0.59 and v1.0.60 rm -rf /tmp/go-common git clone --depth 5 https://github.com/agentuity/go-common.git /tmp/go-common cd /tmp/go-common git fetch --tags --depth=5 echo "Changes in v1.0.60:" git show v1.0.60 --name-status echo echo "Changes in v1.0.59:" git show v1.0.59 --name-status echo echo "Diff between v1.0.59 and v1.0.60:" git diff --name-status v1.0.59 v1.0.60Length of output: 4138
Approve dependency update: go-common v1.0.60
- Bumps github.com/agentuity/go-common from v1.0.59 to v1.0.60.
- Adds a new utility for parsing environment files and returning EnvLineComment (PR #70).
- Changes are limited to env/env.go and its tests; no breaking API modifications detected.
internal/deployer/deployer.go (2)
16-16: Enhanced environment variable support with commentsThe
EnvFile.Envfield type changed from[]env.EnvLineto[]env.EnvLineCommentto support preservation and utilization of comments in environment files.
44-44: Extended PromptForEnv with default value and placeholder supportThe
PromptForEnvfunction signature now includes two additional parameters (defaultValueandplaceholder) to provide more user-friendly and contextual prompts when requesting environment variable values.cmd/env.go (5)
44-59: Added helper to detect secrets based on descriptionNew helper function to identify secrets by analyzing comments for keywords like "secret", "password", or "key" - enabling better automatic secret detection.
61-61: Updated loadEnvFile to support comment preservationThe function now accepts
[]env.EnvLineCommentinstead of[]env.EnvLineto leverage the comments associated with environment variables.
68-68: Enhanced secret detection logicSecret detection now considers comment content in addition to key names and the forceSecret flag, improving automatic identification of sensitive values.
168-169: Parse environment files with commentsUpdated to use
env.ParseEnvFileWithCommentsinstead ofenv.ParseEnvFileto support the comment-aware loading of environment variables.
254-254: Updated promptForEnv call with new parametersThe call to
promptForEnvnow uses the updated signature, passing empty strings for the newdefaultValueandplaceholderparameters.cmd/cloud.go (2)
179-181: Nice touch adding styled diff boxesThe
lipgloss-basedborderandredDiffstyles provide clear visual feedback for missing variables.
Well-implemented!
154-177:Details
✅ Verification successful
Preserve original file permissions when rewriting
.env
appendToEnvFilerewrites the entire file with a hard-coded0644mode.
If the user deliberately made the file non-world-readable (e.g.0600for secrets), we unintentionally widen the permissions.- if err := os.WriteFile(envfile, []byte(buf.String()), 0644); err != nil { + // Keep current permissions if possible + mode := os.FileMode(0644) + if fi, statErr := os.Stat(envfile); statErr == nil { + mode = fi.Mode() + } + if err := os.WriteFile(envfile, []byte(buf.String()), mode); err != nil { return nil, err }Minor, but important for security-conscious users.
No change required: os.WriteFile already preserves existing permissions
Go’s os.WriteFile only applies the provided permission when creating a new file; if the file exists, it’s truncated and rewritten without altering its mode. Since
appendToEnvFileparses an existing.env(failing early if it doesn’t exist), its call toos.WriteFile(..., 0644)will not widen permissions on an already-present file.
| func promptForEnv(logger logger.Logger, key string, isSecret bool, localenv map[string]string, osenv map[string]string, defaultValue string, placeholder string) string { | ||
| prompt := "Enter the value for " + key | ||
| var help string | ||
| var defaultValue string | ||
| var value string | ||
| if isSecret { | ||
| prompt = "Enter the secret value for " + key | ||
| if val, ok := localenv[key]; ok { | ||
| help = "Press enter to set as " + maxString(cstr.Mask(val), 30) + " from your .env file" | ||
| defaultValue = val | ||
| if defaultValue == "" { | ||
| defaultValue = val | ||
| } | ||
| } else if val, ok := osenv[key]; ok { | ||
| help = "Press enter to set as " + maxString(cstr.Mask(val), 30) + " from your environment" | ||
| defaultValue = val | ||
| if defaultValue == "" { | ||
| defaultValue = val | ||
| } | ||
| } else { | ||
| help = "Your input will be masked" | ||
| } | ||
| value = tui.Password(logger, prompt, help) | ||
| } else { | ||
| value = tui.Input(logger, prompt, help) | ||
| if placeholder == "" { | ||
| value = tui.InputWithPlaceholder(logger, prompt, placeholder, defaultValue) | ||
| } else { | ||
| value = tui.InputWithPlaceholder(logger, prompt, help, defaultValue) | ||
| } |
There was a problem hiding this comment.
Enhanced promptForEnv with better default handling and placeholder support
The function has been extended to:
- Accept defaultValue and placeholder parameters
- Only use values from local/OS environment if no explicit default is provided
- Support placeholders for non-secret inputs, improving the UX
There appears to be a small logic error with placeholder handling.
Lines 98-102 contain a logical error in the conditional block. If placeholder is provided (not empty), it uses help as the placeholder parameter instead of the actual placeholder. This should be fixed:
- if placeholder == "" {
- value = tui.InputWithPlaceholder(logger, prompt, placeholder, defaultValue)
- } else {
- value = tui.InputWithPlaceholder(logger, prompt, help, defaultValue)
- }
+ if placeholder == "" {
+ value = tui.InputWithPlaceholder(logger, prompt, help, defaultValue)
+ } else {
+ value = tui.InputWithPlaceholder(logger, prompt, placeholder, defaultValue)
+ }📝 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.
| func promptForEnv(logger logger.Logger, key string, isSecret bool, localenv map[string]string, osenv map[string]string, defaultValue string, placeholder string) string { | |
| prompt := "Enter the value for " + key | |
| var help string | |
| var defaultValue string | |
| var value string | |
| if isSecret { | |
| prompt = "Enter the secret value for " + key | |
| if val, ok := localenv[key]; ok { | |
| help = "Press enter to set as " + maxString(cstr.Mask(val), 30) + " from your .env file" | |
| defaultValue = val | |
| if defaultValue == "" { | |
| defaultValue = val | |
| } | |
| } else if val, ok := osenv[key]; ok { | |
| help = "Press enter to set as " + maxString(cstr.Mask(val), 30) + " from your environment" | |
| defaultValue = val | |
| if defaultValue == "" { | |
| defaultValue = val | |
| } | |
| } else { | |
| help = "Your input will be masked" | |
| } | |
| value = tui.Password(logger, prompt, help) | |
| } else { | |
| value = tui.Input(logger, prompt, help) | |
| if placeholder == "" { | |
| value = tui.InputWithPlaceholder(logger, prompt, placeholder, defaultValue) | |
| } else { | |
| value = tui.InputWithPlaceholder(logger, prompt, help, defaultValue) | |
| } | |
| func promptForEnv(logger logger.Logger, key string, isSecret bool, localenv map[string]string, osenv map[string]string, defaultValue string, placeholder string) string { | |
| prompt := "Enter the value for " + key | |
| var help string | |
| var value string | |
| if isSecret { | |
| // … secret‐input logic … | |
| value = tui.Password(logger, prompt, help) | |
| } else { | |
| if placeholder == "" { | |
| value = tui.InputWithPlaceholder(logger, prompt, help, defaultValue) | |
| } else { | |
| value = tui.InputWithPlaceholder(logger, prompt, placeholder, defaultValue) | |
| } | |
| } | |
| return value | |
| } |
🤖 Prompt for AI Agents
In cmd/env.go around lines 98 to 102, the conditional incorrectly uses the
variable 'help' as the placeholder argument when 'placeholder' is non-empty. To
fix this, update the else branch to pass the actual 'placeholder' variable to
tui.InputWithPlaceholder instead of 'help', ensuring the correct placeholder
text is shown for non-secret inputs.
There was a problem hiding this comment.
Yep, good catch. This needs fixing
There was a problem hiding this comment.
No description provided.
There was a problem hiding this comment.
A summary of the edits CodeRabbit can apply:
Fix the placeholder handling logic in promptForEnv in cmd/env.go to correctly use help when placeholder is empty and placeholder when provided.
A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):
✏️ cmd/env.go
Open cmd/env.go and find the promptForEnv function around line 77.
Locate the non-secret input placeholder block (currently lines 100–103):
if placeholder == "" {
value = tui.InputWithPlaceholder(logger, prompt, placeholder, defaultValue)
} else {
value = tui.InputWithPlaceholder(logger, prompt, help, defaultValue)
}Replace lines 100–103 with the corrected logic so that help is used when placeholder is empty and placeholder when provided:
if placeholder == "" {
value = tui.InputWithPlaceholder(logger, prompt, help, defaultValue)
} else {
value = tui.InputWithPlaceholder(logger, prompt, placeholder, defaultValue)
}Verify the indentation uses tabs to align with surrounding code, save cmd/env.go, then run the CLI in both placeholder-present and placeholder-empty scenarios to ensure the fix works.
A summary of the context that CodeRabbit is considering across the codebase:
Display the relevant lines around the InputWithPlaceholder calls to confirm the exact block to update.
- ✅ Create PR with these edits
- 📌 Create commit in current branch
- 📋 Get copyable edits
| func readPossibleEnvTemplateFiles(baseDir string) map[string][]env.EnvLineComment { | ||
| var results map[string][]env.EnvLineComment | ||
| keys := make(map[string]bool) | ||
| for _, file := range envTemplateFileNames { | ||
| filename := filepath.Join(baseDir, file) | ||
| if !util.Exists(filename) { | ||
| continue | ||
| } | ||
| efc, err := env.ParseEnvFileWithComments(filename) | ||
| if err == nil { | ||
| if results == nil { | ||
| results = make(map[string][]env.EnvLineComment) | ||
| } | ||
| for _, ev := range efc { | ||
| if _, ok := keys[ev.Key]; !ok { | ||
| if isAgentuityEnv.MatchString(ev.Key) { | ||
| continue | ||
| } | ||
| keys[ev.Key] = true | ||
| results[file] = append(results[file], ev) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return results | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Surface parse errors from template files instead of silently ignoring them
readPossibleEnvTemplateFiles swallows any parsing error coming from env.ParseEnvFileWithComments.
If a user mistypes a line in .env.example, the CLI will behave as if the variable is simply missing, giving no clue about the real problem.
Consider propagating or at least logging the error so the user can fix the malformed template.
-func readPossibleEnvTemplateFiles(baseDir string) map[string][]env.EnvLineComment {
- var results map[string][]env.EnvLineComment
+// returns map[template-filename]vars and an optional error so callers can decide how to react
+func readPossibleEnvTemplateFiles(baseDir string) (map[string][]env.EnvLineComment, error) {
+ var results map[string][]env.EnvLineComment
keys := make(map[string]bool)
@@
- efc, err := env.ParseEnvFileWithComments(filename)
- if err == nil {
+ efc, err := env.ParseEnvFileWithComments(filename)
+ if err != nil {
+ // bubble up – caller can choose to warn the user
+ return nil, fmt.Errorf("parsing %s: %w", filename, err)
+ }
+
if results == nil {
results = make(map[string][]env.EnvLineComment)
}
@@
- return results
+ return results, nil
}You will need to adjust the single call-site accordingly.
Fail-fast behaviour avoids silent mis-configuration.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In cmd/cloud.go around lines 127 to 152, the function
readPossibleEnvTemplateFiles currently ignores errors from
env.ParseEnvFileWithComments, which hides parsing issues in env template files.
Modify the function to return an error along with the results map, propagate any
parsing errors instead of silently continuing, and update the call-site to
handle this error. This change will enable fail-fast behavior and prevent silent
misconfiguration by surfacing malformed template problems to the user.
There was a problem hiding this comment.
He's got a valid point... happy path is covered
There was a problem hiding this comment.
No description provided.
There was a problem hiding this comment.
A summary of the edits CodeRabbit can apply:
Update readPossibleEnvTemplateFiles in cmd/cloud.go to return an error and propagate parsing failures.
Update the call site in cmd/cloud.go to handle the error returned by readPossibleEnvTemplateFiles.
A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):
✏️ cmd/cloud.go
Replace the function signature and add documentation comments. Overwrite lines 127–127 with:
// readPossibleEnvTemplateFiles reads and parses environment template files
// from the specified base directory.
func readPossibleEnvTemplateFiles(baseDir string) (map[string][]env.EnvLineComment, error) {
Inside the loop, replace the error check at lines 135–136 from:
efc, err := env.ParseEnvFileWithComments(filename)
if err == nil {
to:
efc, err := env.ParseEnvFileWithComments(filename)
if err != nil {
return nil, fmt.Errorf("parsing %s: %w", filename, err)
}
Replace the final return at line 151 from:
return results
to:
return results, nil
Change the call at line 290 from:
templateEnvs := readPossibleEnvTemplateFiles(dir)
to:
templateEnvs, err := readPossibleEnvTemplateFiles(dir)
Immediately after, insert an error check:
if err != nil {
errsystem.New(errsystem.ErrParseEnvironmentFile, err,
errsystem.WithContextMessage("Error parsing environment template file")).ShowErrorAndExit()
}
A summary of the context that CodeRabbit is considering across the codebase:
We need to inspect the existing readPossibleEnvTemplateFiles definition—including its signature, body, and return—to determine the precise lines to update.
We need to locate the call site(s) of readPossibleEnvTemplateFiles in order to update them to handle the new error return.
- ✅ Create PR with these edits
- 📌 Create commit in current branch
- 📋 Get copyable edits
| func readPossibleEnvTemplateFiles(baseDir string) map[string][]env.EnvLineComment { | ||
| var results map[string][]env.EnvLineComment | ||
| keys := make(map[string]bool) | ||
| for _, file := range envTemplateFileNames { | ||
| filename := filepath.Join(baseDir, file) | ||
| if !util.Exists(filename) { | ||
| continue | ||
| } | ||
| efc, err := env.ParseEnvFileWithComments(filename) | ||
| if err == nil { | ||
| if results == nil { | ||
| results = make(map[string][]env.EnvLineComment) | ||
| } | ||
| for _, ev := range efc { | ||
| if _, ok := keys[ev.Key]; !ok { | ||
| if isAgentuityEnv.MatchString(ev.Key) { | ||
| continue | ||
| } | ||
| keys[ev.Key] = true | ||
| results[file] = append(results[file], ev) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return results | ||
| } |
There was a problem hiding this comment.
He's got a valid point... happy path is covered
| func promptForEnv(logger logger.Logger, key string, isSecret bool, localenv map[string]string, osenv map[string]string, defaultValue string, placeholder string) string { | ||
| prompt := "Enter the value for " + key | ||
| var help string | ||
| var defaultValue string | ||
| var value string | ||
| if isSecret { | ||
| prompt = "Enter the secret value for " + key | ||
| if val, ok := localenv[key]; ok { | ||
| help = "Press enter to set as " + maxString(cstr.Mask(val), 30) + " from your .env file" | ||
| defaultValue = val | ||
| if defaultValue == "" { | ||
| defaultValue = val | ||
| } | ||
| } else if val, ok := osenv[key]; ok { | ||
| help = "Press enter to set as " + maxString(cstr.Mask(val), 30) + " from your environment" | ||
| defaultValue = val | ||
| if defaultValue == "" { | ||
| defaultValue = val | ||
| } | ||
| } else { | ||
| help = "Your input will be masked" | ||
| } | ||
| value = tui.Password(logger, prompt, help) | ||
| } else { | ||
| value = tui.Input(logger, prompt, help) | ||
| if placeholder == "" { | ||
| value = tui.InputWithPlaceholder(logger, prompt, placeholder, defaultValue) | ||
| } else { | ||
| value = tui.InputWithPlaceholder(logger, prompt, help, defaultValue) | ||
| } |
There was a problem hiding this comment.
Yep, good catch. This needs fixing
|
CodeRabbit made a good couple of points. The second one is a bug, the first is about error handling. Also, how do I test it? |
|
@pec1985 if you can take this one over and i have a couple of follows after this PR. to test: create a file in your project named Then run that project deploy (make sure to delete from .env and dashboard if testing more than once). Once this PR merges, it would be good to refactor this into a set of utilities we can reuse for the following:
Also, I would like to add both TTY and non-TTY versions. for the TTY version, it can have user interaction like it does now. but we should support non-TTY version (like the CI build) and have it show a nice table of the issues and exit(1) so the build will fail with instructions on why. |
Summary by CodeRabbit