From f803656239270e0fe861d8d60cd97756c51d3b98 Mon Sep 17 00:00:00 2001 From: Jeremiah Ojonuba Date: Sat, 25 Jul 2026 20:23:17 +0100 Subject: [PATCH] fix: warn on malformed .env lines instead of silently skipping them --- internal/config/dotenv.go | 57 ++++++++++++- internal/config/dotenv_test.go | 146 +++++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 internal/config/dotenv_test.go diff --git a/internal/config/dotenv.go b/internal/config/dotenv.go index e5bfdda1..5ab8f3b1 100644 --- a/internal/config/dotenv.go +++ b/internal/config/dotenv.go @@ -1,6 +1,8 @@ package config import ( + "bufio" + "log" "os" "path/filepath" "strings" @@ -18,13 +20,13 @@ func LoadDotenv() { parts := strings.Split(v, ",") for i := range parts { parts[i] = strings.TrimSpace(parts[i]) + loadEnvFile(parts[i]) } - _ = godotenv.Load(parts...) return } // Default: try a few common locations, so starting the process from repo root or backend/ works. - // We keep it silent and best-effort. + // We keep it silent and best-effort for missing files, but warn on malformed content. candidates := []string{".env"} if wd, err := os.Getwd(); err == nil { @@ -40,12 +42,61 @@ func LoadDotenv() { continue } if _, err := os.Stat(p); err == nil { - _ = godotenv.Load(p) + loadEnvFile(p) return } } } +func loadEnvFile(path string) { + file, err := os.Open(path) + if err != nil { + return + } + defer file.Close() + + scanner := bufio.NewScanner(file) + var buf strings.Builder + envMap := make(map[string]string) + + for scanner.Scan() { + line := scanner.Text() + + if buf.Len() > 0 { + buf.WriteString("\n") + } + buf.WriteString(line) + + current := buf.String() + + // Skip empty lines and pure comments + if strings.TrimSpace(current) == "" || strings.HasPrefix(strings.TrimSpace(current), "#") { + buf.Reset() + continue + } + parsedMap, err := godotenv.Unmarshal(current) + if err == nil { + for k, v := range parsedMap { + if k == "" { + log.Printf("WARNING: malformed line in %s (missing '='): %q\n", path, current) + } else { + envMap[k] = v + } + } + buf.Reset() + } + } + if err := scanner.Err(); err != nil { + log.Printf("WARNING: error reading %s: %v\n", path, err) + } else if buf.Len() > 0 { + log.Printf("WARNING: malformed or unterminated line in %s: %q\n", path, buf.String()) + } + for k, v := range envMap { + if _, exists := os.LookupEnv(k); !exists { + os.Setenv(k, v) + } + } +} diff --git a/internal/config/dotenv_test.go b/internal/config/dotenv_test.go new file mode 100644 index 00000000..aa8f42d0 --- /dev/null +++ b/internal/config/dotenv_test.go @@ -0,0 +1,146 @@ +package config + +import ( + "bytes" + "log" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestLoadDotenv(t *testing.T) { + // Create a temporary directory for our .env files + tempDir := t.TempDir() + + tests := []struct { + name string + envContent string + expectedEnv map[string]string + notExpectedEnv []string + expectLogMsg string + }{ + { + name: "Valid file", + envContent: `A=1 +B="multi +line" +C=3`, + expectedEnv: map[string]string{ + "A": "1", + "B": "multi\nline", + "C": "3", + }, + }, + { + name: "Missing equals sign", + envContent: `A=1 +BAD_LINE +B=2`, + expectedEnv: map[string]string{ + "A": "1", + "B": "2", + }, + expectLogMsg: `WARNING: malformed line in`, + }, + { + name: "Unterminated quote", + envContent: `A=1 +B="unterminated +C=3`, + expectedEnv: map[string]string{ + "A": "1", + }, + notExpectedEnv: []string{"B", "C"}, + expectLogMsg: `WARNING: malformed or unterminated line in`, + }, + { + name: "Duplicate key (last-wins)", + envContent: `A=1 +A=2`, + expectedEnv: map[string]string{ + "A": "2", + }, + }, + { + name: "Empty file", + envContent: ``, + expectedEnv: map[string]string{}, + }, + { + name: "Comment-only line", + envContent: `# This is a comment +# Another comment`, + expectedEnv: map[string]string{}, + }, + { + name: "Mixed with comments", + envContent: `A=1 +# A comment +B=2`, + expectedEnv: map[string]string{ + "A": "1", + "B": "2", + }, + }, + { + name: "Does not overwrite existing OS env", + envContent: `A=1`, + // "A" is set to "EXISTING" in test setup + expectedEnv: map[string]string{ + "A": "EXISTING", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Clear all environment variables for a clean state + os.Clearenv() + + if tc.name == "Does not overwrite existing OS env" { + os.Setenv("A", "EXISTING") + } + + // Write the env content to a file + envPath := filepath.Join(tempDir, ".env") + if err := os.WriteFile(envPath, []byte(tc.envContent), 0644); err != nil { + t.Fatalf("failed to write .env file: %v", err) + } + + // Capture log output + var logBuf bytes.Buffer + log.SetOutput(&logBuf) + defer log.SetOutput(os.Stderr) + + // Tell LoadDotenv to use our specific file + os.Setenv("ENV_FILE", envPath) + + LoadDotenv() + + logOutput := logBuf.String() + + // Verify expected env variables + for k, v := range tc.expectedEnv { + actual := os.Getenv(k) + if actual != v { + t.Errorf("expected env %s=%q, got %q", k, v, actual) + } + } + + // Verify not expected env variables + for _, k := range tc.notExpectedEnv { + if val, exists := os.LookupEnv(k); exists { + t.Errorf("expected env %s to not be set, but got %q", k, val) + } + } + + // Verify log messages + if tc.expectLogMsg != "" && !strings.Contains(logOutput, tc.expectLogMsg) { + t.Errorf("expected log output to contain %q, got: %q", tc.expectLogMsg, logOutput) + } else if tc.expectLogMsg == "" && logOutput != "" { + t.Errorf("expected no log output, got: %q", logOutput) + } + }) + } +}