Add new commands, fix bugs, and improve .env parsing by bacarndiaye · Pull Request #6 · BinSquare/envmap

bacarndiaye · 2026-01-21T12:08:48Z

New features:

Add 'list' command to display configured environments (--json option)
Add 'doctor' command for configuration diagnostics
Add 'completion' command for shell autocompletion (bash/zsh/fish/powershell)

Bug fixes:

Fix spawn.go: secrets now properly override existing environment variables
Fix init_interactive.go: only prompt for .env import when file exists

Improvements:

Improve .env parser to handle escaped quotes within double-quoted strings
Add support for escape sequences (\n, \t, \) in quoted values
Add findClosingQuote() helper for escape-aware quote detection

Tests:

Add commands_test.go with tests for new commands
Extend envfile_test.go with advanced parsing edge cases

Copilot

Pull request overview

This pull request adds new diagnostic and utility commands, fixes bugs in environment variable handling and .env file import prompting, and improves .env file parsing to support escape sequences.

Changes:

Added three new commands: list (display configured environments), doctor (configuration diagnostics), and completion (shell autocompletion)
Fixed secrets in spawn.go to properly override existing environment variables
Fixed init_interactive.go to only prompt for .env import when file exists
Enhanced .env parser to handle escape sequences (\n, \t, \, ") in quoted strings

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
spawn.go	Refactored environment merging to use a map, ensuring secrets override existing variables
init_interactive.go	Added guard to only prompt for .env import when file exists
envfile.go	Enhanced parser with escape sequence support and improved quote handling
envfile_test.go	Added comprehensive tests for advanced .env parsing scenarios
main.go	Added three new commands: list, doctor, and completion with full implementations
commands_test.go	Added test coverage for the new commands

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-21T12:13:47Z

commands_test.go

+	oldDir, _ := os.Getwd()
+	defer os.Chdir(oldDir)
+	os.Chdir(dir)


The error returned from os.Getwd() is being silently ignored. If getting the current directory fails, the deferred os.Chdir(oldDir) will use an empty string, which may cause unexpected behavior. Consider handling this error or failing the test if it occurs.

Copilot · 2026-01-21T12:13:47Z

commands_test.go

+	oldDir, _ := os.Getwd()
+	defer os.Chdir(oldDir)
+	os.Chdir(dir)


The error returned from os.Getwd() is being silently ignored. If getting the current directory fails, the deferred os.Chdir(oldDir) will use an empty string, which may cause unexpected behavior. Consider handling this error or failing the test if it occurs.

Copilot · 2026-01-21T12:13:47Z

commands_test.go

+	oldDir, _ := os.Getwd()
+	defer os.Chdir(oldDir)
+	os.Chdir(dir)


The error returned from os.Getwd() is being silently ignored. If getting the current directory fails, the deferred os.Chdir(oldDir) will use an empty string, which may cause unexpected behavior. Consider handling this error or failing the test if it occurs.

Copilot · 2026-01-21T12:13:48Z

envfile.go

+			val = strings.ReplaceAll(val, `\\`, `\`)
+			val = strings.ReplaceAll(val, `\n`, "\n")
+			val = strings.ReplaceAll(val, `\t`, "\t")


The order of escape sequence replacements is incorrect and will produce wrong results. When you replace \\ with \ before processing other escape sequences like \n and \t, the literal \\n will first become \n (a backslash followed by 'n'), which will then incorrectly be replaced with a newline character.

For example, if the input is "\\n" (representing a literal backslash followed by 'n'), the current code will:

Replace \\ with \ → "\n"

Replace \n with newline → produces a newline instead of the intended \n

The correct order should be to process specific escape sequences first (\n, \t, \") and then handle \\ last.

Suggested change

val = strings.ReplaceAll(val, `\\`, `\`)

val = strings.ReplaceAll(val, `\n`, "\n")

val = strings.ReplaceAll(val, `\t`, "\t")

val = strings.ReplaceAll(val, `\n`, "\n")

val = strings.ReplaceAll(val, `\t`, "\t")

val = strings.ReplaceAll(val, `\\`, `\`)

This ai comment is correct - the order was on purpose here.

Copilot · 2026-01-21T12:13:48Z

spawn.go

+	// Build environment with secrets taking precedence over existing vars
+	envMap := make(map[string]string)
+
+	// First, load existing environment
+	for _, e := range os.Environ() {
+		parts := strings.SplitN(e, "=", 2)
+		if len(parts) == 2 {
+			envMap[parts[0]] = parts[1]
+		}
+	}
+
+	// Then override with secrets (secrets take precedence)
 	for k, v := range secretEnv {
+		envMap[k] = v
+	}
+
+	// Convert back to slice
+	merged := make([]string, 0, len(envMap))
+	for k, v := range envMap {
 		merged = append(merged, fmt.Sprintf("%s=%s", k, v))
 	}
 	cmd.Env = merged


The new behavior in SpawnWithEnv where secrets override existing environment variables lacks test coverage. The spawn_test.go file exists but doesn't test the SpawnWithEnv function. Consider adding a test that verifies secrets properly override existing environment variables, including edge cases like values containing equals signs (e.g., VAR=foo=bar).

Copilot · 2026-01-21T12:13:49Z

envfile_test.go

+func TestParseDotEnvAdvanced(t *testing.T) {
+	content := `# Advanced parsing test
+export EXPORTED_VAR=exported_value
+INLINE_COMMENT=value # this is a comment
+DOUBLE_QUOTED="hello world"
+SINGLE_QUOTED='hello world'
+ESCAPED_QUOTE="say \"hello\""
+NEWLINE_ESCAPE="line1\nline2"
+TAB_ESCAPE="col1\tcol2"
+SPACES_AROUND = value_with_spaces
+NO_QUOTES=simple
+EMPTY_QUOTED=""
+URL=https://example.com/path?query=1
+`
+	dir := t.TempDir()
+	path := filepath.Join(dir, ".env")
+	if err := os.WriteFile(path, []byte(content), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	got, err := parseDotEnv(path)
+	if err != nil {
+		t.Fatalf("parseDotEnv: %v", err)
+	}
+
+	tests := []struct {
+		key      string
+		expected string
+	}{
+		{"EXPORTED_VAR", "exported_value"},
+		{"INLINE_COMMENT", "value"},
+		{"DOUBLE_QUOTED", "hello world"},
+		{"SINGLE_QUOTED", "hello world"},
+		{"ESCAPED_QUOTE", `say "hello"`},
+		{"NEWLINE_ESCAPE", "line1\nline2"},
+		{"TAB_ESCAPE", "col1\tcol2"},
+		{"SPACES_AROUND", "value_with_spaces"},
+		{"NO_QUOTES", "simple"},
+		{"EMPTY_QUOTED", ""},
+		{"URL", "https://example.com/path?query=1"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.key, func(t *testing.T) {
+			val, ok := got[tt.key]
+			if !ok {
+				t.Errorf("key %q not found", tt.key)
+				return
+			}
+			if val != tt.expected {
+				t.Errorf("got[%q] = %q, want %q", tt.key, val, tt.expected)
+			}
+		})
+	}
+}


Missing test coverage for escaped backslashes in double-quoted strings. The test should include a case like ESCAPED_BACKSLASH="\\n" which should produce the literal string \n (backslash followed by n), not a newline character. This would catch the incorrect ordering of escape sequence replacements.

Copilot · 2026-01-21T12:13:49Z

main.go

+				for envName, envCfg := range projectCfg.Envs {
+					providerName := envCfg.GetProvider()
+					fmt.Printf("  %s -> %s: ", envName, providerName)
+					if _, ok := providers[providerName]; ok {
+						fmt.Println("OK")
+					} else {
+						fmt.Println("MISSING")
+						issues++
+					}
+				}


The iteration over projectCfg.Envs map has non-deterministic order, which can make the output inconsistent between runs. Consider sorting the environment names before iterating to ensure consistent output in diagnostic messages.

Copilot · 2026-01-21T12:13:49Z

main.go

+			for name, pCfg := range globalCfg.GetProviders() {
+				if pCfg.Type == "local-file" || pCfg.Type == "local-store" {
+					fmt.Printf("  %s key: ", name)
+					if pCfg.Encryption == nil {
+						fmt.Println("NOT CONFIGURED")
+						issues++
+						continue
+					}
+					if pCfg.Encryption.KeyFile != "" {
+						info, err := os.Stat(pCfg.Encryption.KeyFile)
+						if err != nil {
+							fmt.Printf("MISSING (%s)\n", pCfg.Encryption.KeyFile)
+							issues++
+						} else if info.Mode().Perm()&0o077 != 0 {
+							fmt.Printf("INSECURE PERMISSIONS (%#o, should be 0600)\n", info.Mode().Perm())
+							issues++
+						} else {
+							fmt.Println("OK")
+						}
+					} else if pCfg.Encryption.KeyEnv != "" {
+						if os.Getenv(pCfg.Encryption.KeyEnv) == "" {
+							fmt.Printf("ENV NOT SET (%s)\n", pCfg.Encryption.KeyEnv)
+							issues++
+						} else {
+							fmt.Println("OK (from env)")
+						}
+					}
+				}
+			}


The iteration over globalCfg.GetProviders() map has non-deterministic order, which can make the output inconsistent between runs. Consider sorting the provider names before iterating to ensure consistent output in diagnostic messages.

Copilot · 2026-01-21T12:13:49Z

commands_test.go

+	oldDir, _ := os.Getwd()
+	defer os.Chdir(oldDir)
+	os.Chdir(dir)


The error returned from os.Getwd() is being silently ignored. If getting the current directory fails, the deferred os.Chdir(oldDir) will use an empty string, which may cause unexpected behavior. Consider handling this error or failing the test if it occurs.

BinSquare · 2026-01-21T20:36:11Z

I'm not actually sure where the --list command code is added, but I already have below:
envmap get --env <name> --all [--raw] [--global] – list all secrets (masked by default); --global iterates all envs.

Is there a particular usecase you're running into? Otherwise, I'd prefer to keep code surface minimal.

BinSquare · 2026-01-21T20:38:02Z

Overall while I think the intent of this PR is good - it combines multiple discrete changes that we should isolate into separate PR's for a better review.

Add new commands, fix bugs, and improve .env parsing

1a721af

Copilot AI review requested due to automatic review settings January 21, 2026 12:08

Copilot started reviewing on behalf of bacarndiaye January 21, 2026 12:09 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

Conversation

bacarndiaye commented Jan 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

BinSquare Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

BinSquare commented Jan 21, 2026

Uh oh!

BinSquare commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants