diff --git a/cmd/proxsave/config_helpers.go b/cmd/proxsave/config_helpers.go index 9644de0..0d3d0ca 100644 --- a/cmd/proxsave/config_helpers.go +++ b/cmd/proxsave/config_helpers.go @@ -5,6 +5,8 @@ import ( "os" "path/filepath" "strings" + + "github.com/tis24dev/proxsave/pkg/utils" ) type configStatusLogger interface { @@ -47,43 +49,7 @@ func ensureConfigExists(path string, logger configStatusLogger) error { } func setEnvValue(template, key, value string) string { - target := key + "=" - lines := strings.Split(template, "\n") - replaced := false - for i, line := range lines { - trimmed := strings.TrimSpace(line) - if strings.HasPrefix(trimmed, target) { - leadingLen := len(line) - len(strings.TrimLeft(line, " \t")) - leading := "" - if leadingLen > 0 { - leading = line[:leadingLen] - } - rest := line[leadingLen:] - commentSpacing := "" - comment := "" - if idx := strings.Index(rest, "#"); idx >= 0 { - before := rest[:idx] - comment = rest[idx:] - trimmedBefore := strings.TrimRight(before, " \t") - commentSpacing = before[len(trimmedBefore):] - rest = trimmedBefore - } - newLine := leading + key + "=" + value - if comment != "" { - spacing := commentSpacing - if spacing == "" { - spacing = " " - } - newLine += spacing + comment - } - lines[i] = newLine - replaced = true - } - } - if !replaced { - lines = append(lines, key+"="+value) - } - return strings.Join(lines, "\n") + return utils.SetEnvValue(template, key, value) } func sanitizeEnvValue(value string) string { diff --git a/cmd/proxsave/helpers_test.go b/cmd/proxsave/helpers_test.go index abcbb40..e27d735 100644 --- a/cmd/proxsave/helpers_test.go +++ b/cmd/proxsave/helpers_test.go @@ -36,6 +36,24 @@ func TestSetEnvValue_PreserveComment(t *testing.T) { assertContains(t, result, "# this is a comment") } +func TestSetEnvValue_PreserveCommentAfterQuotedValue(t *testing.T) { + template := `FOO="old # keep" # trailing comment` + + result := setEnvValue(template, "FOO", "new") + + assertContains(t, result, "FOO=new") + assertContains(t, result, "# trailing comment") +} + +func TestSetEnvValue_PreserveCommentWithEscapedHash(t *testing.T) { + template := `FOO=old\#keep # trailing comment` + + result := setEnvValue(template, "FOO", "new") + + assertContains(t, result, "FOO=new") + assertContains(t, result, "# trailing comment") +} + func TestSetEnvValue_AddNew(t *testing.T) { template := "FOO=bar" diff --git a/cmd/proxsave/main.go b/cmd/proxsave/main.go index d7e9c34..0daa06d 100644 --- a/cmd/proxsave/main.go +++ b/cmd/proxsave/main.go @@ -334,6 +334,12 @@ func run() int { bootstrap.Error("ERROR: Failed to plan configuration upgrade: %v", err) return types.ExitConfigError.Int() } + if len(result.Warnings) > 0 { + bootstrap.Warning("Config upgrade warnings (%d):", len(result.Warnings)) + for _, warning := range result.Warnings { + bootstrap.Warning(" - %s", warning) + } + } if !result.Changed { bootstrap.Println("Configuration is already up to date with the embedded template; no changes are required.") return types.ExitSuccess.Int() @@ -437,6 +443,12 @@ func run() int { bootstrap.Error("ERROR: Failed to upgrade configuration: %v", err) return types.ExitConfigError.Int() } + if len(result.Warnings) > 0 { + bootstrap.Warning("Config upgrade warnings (%d):", len(result.Warnings)) + for _, warning := range result.Warnings { + bootstrap.Warning(" - %s", warning) + } + } if !result.Changed { bootstrap.Println("Configuration is already up to date with the embedded template; no changes were made.") return types.ExitSuccess.Int() diff --git a/cmd/proxsave/upgrade.go b/cmd/proxsave/upgrade.go index 9a09107..6a35f7b 100644 --- a/cmd/proxsave/upgrade.go +++ b/cmd/proxsave/upgrade.go @@ -589,6 +589,14 @@ func printUpgradeFooter(upgradeErr error, version, configPath, baseDir, telegram } } + if cfgUpgradeResult != nil && len(cfgUpgradeResult.Warnings) > 0 { + fmt.Printf("Configuration warnings (%d):\n", len(cfgUpgradeResult.Warnings)) + for _, warning := range cfgUpgradeResult.Warnings { + fmt.Printf(" - %s\n", warning) + } + fmt.Println() + } + fmt.Println("Next steps:") if strings.TrimSpace(configPath) != "" { fmt.Printf("1. Verify configuration: %s\n", configPath) diff --git a/internal/config/config.go b/internal/config/config.go index a566c72..5f02609 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1218,8 +1218,12 @@ func parseEnvFile(path string) (map[string]string, error) { if !ok { continue } + if fields := strings.Fields(key); len(fields) >= 2 && fields[0] == "export" { + key = fields[1] + } + upperKey := strings.ToUpper(key) - if blockValueKeys[key] && trimmed == fmt.Sprintf("%s=\"", key) { + if blockValueKeys[upperKey] && trimmed == fmt.Sprintf("%s=\"", key) { var blockLines []string terminated := false for scanner.Scan() { @@ -1228,23 +1232,25 @@ func parseEnvFile(path string) (map[string]string, error) { terminated = true break } - blockLines = append(blockLines, next) + if !utils.IsComment(strings.TrimSpace(next)) { + blockLines = append(blockLines, next) + } } if !terminated { return nil, fmt.Errorf("unterminated multi-line value for %s", key) } - raw[key] = strings.Join(blockLines, "\n") + raw[upperKey] = strings.Join(blockLines, "\n") continue } - if multiValueKeys[key] { - if existing, ok := raw[key]; ok && existing != "" { - raw[key] = existing + "\n" + value + if multiValueKeys[upperKey] { + if existing, ok := raw[upperKey]; ok && existing != "" { + raw[upperKey] = existing + "\n" + value } else { - raw[key] = value + raw[upperKey] = value } } else { - raw[key] = value + raw[upperKey] = value } } diff --git a/internal/config/migration_test.go b/internal/config/migration_test.go index 26917fa..450d770 100644 --- a/internal/config/migration_test.go +++ b/internal/config/migration_test.go @@ -50,6 +50,60 @@ BACKUP_BLACKLIST=" } } +func TestParseEnvFileHandlesLowercaseKeys(t *testing.T) { + tmpDir := t.TempDir() + envFile := filepath.Join(tmpDir, "legacy.env") + content := ` +custom_backup_paths=" +/etc/custom +" +backup_blacklist=" +/tmp +" +backup_exclude_patterns=*.log +backup_exclude_patterns=*.tmp +` + if err := os.WriteFile(envFile, []byte(content), 0600); err != nil { + t.Fatalf("failed to write temp legacy env: %v", err) + } + + values, err := parseEnvFile(envFile) + if err != nil { + t.Fatalf("parseEnvFile returned error: %v", err) + } + + if got := values["CUSTOM_BACKUP_PATHS"]; got != "/etc/custom" { + t.Fatalf("CUSTOM_BACKUP_PATHS = %q; want block content", got) + } + if got := values["BACKUP_BLACKLIST"]; got != "/tmp" { + t.Fatalf("BACKUP_BLACKLIST = %q; want block content", got) + } + if got := values["BACKUP_EXCLUDE_PATTERNS"]; got != "*.log\n*.tmp" { + t.Fatalf("BACKUP_EXCLUDE_PATTERNS = %q; want \"*.log\\n*.tmp\"", got) + } +} + +func TestParseEnvFileHandlesExportLines(t *testing.T) { + tmpDir := t.TempDir() + envFile := filepath.Join(tmpDir, "legacy.env") + content := "export BACKUP_PATH=/data\nexport\t\tLOG_PATH=/logs\n" + if err := os.WriteFile(envFile, []byte(content), 0600); err != nil { + t.Fatalf("failed to write temp legacy env: %v", err) + } + + values, err := parseEnvFile(envFile) + if err != nil { + t.Fatalf("parseEnvFile returned error: %v", err) + } + + if got := values["BACKUP_PATH"]; got != "/data" { + t.Fatalf("BACKUP_PATH = %q; want %q", got, "/data") + } + if got := values["LOG_PATH"]; got != "/logs" { + t.Fatalf("LOG_PATH = %q; want %q", got, "/logs") + } +} + const testTemplate = `# test template BACKUP_PATH=/default/backup LOG_PATH=/default/log diff --git a/internal/config/upgrade.go b/internal/config/upgrade.go index 2b59107..c2779c8 100644 --- a/internal/config/upgrade.go +++ b/internal/config/upgrade.go @@ -20,6 +20,7 @@ type envValue struct { kind envValueKind rawValue string blockLines []string + comment string } // UpgradeResult describes the outcome of a configuration upgrade. @@ -32,6 +33,8 @@ type UpgradeResult struct { // ExtraKeys are keys that were present in the user's config but not in the // template. They are preserved in a dedicated "Custom keys" section. ExtraKeys []string + // Warnings includes non-fatal parsing or merge issues detected while upgrading. + Warnings []string // PreservedValues is the number of existing key=value pairs from the user's // configuration that were kept during the merge for keys present in the // template. @@ -155,7 +158,7 @@ func computeConfigUpgrade(configPath string) (*UpgradeResult, string, []byte, er originalLines := strings.Split(normalizedOriginal, "\n") // 1. Collect user values: for each KEY we store all VALUE entries in order. - userValues, userKeyOrder, err := parseEnvValues(originalLines) + userValues, userKeyOrder, caseMap, caseConflicts, warnings, err := parseEnvValues(originalLines) if err != nil { return result, "", originalContent, fmt.Errorf("failed to parse config %s: %w", configPath, err) } @@ -166,8 +169,10 @@ func computeConfigUpgrade(configPath string) (*UpgradeResult, string, []byte, er templateLines := strings.Split(normalizedTemplate, "\n") templateKeys := make(map[string]bool) + templateKeyByUpper := make(map[string]string) missingKeys := make([]string, 0) newLines := make([]string, 0, len(templateLines)+len(userValues)) + processedUserKeys := make(map[string]bool) // Track which user keys (original case) have been used for i := 0; i < len(templateLines); i++ { line := templateLines[i] @@ -177,22 +182,43 @@ func computeConfigUpgrade(configPath string) (*UpgradeResult, string, []byte, er continue } - key, _, ok := splitKeyValueRaw(line) + key, _, _, ok := splitKeyValueRaw(line) if !ok || key == "" { newLines = append(newLines, line) continue } templateKeys[key] = true + upperKey := strings.ToUpper(key) + if existing, ok := templateKeyByUpper[upperKey]; ok { + if existing != key { + warnings = append(warnings, fmt.Sprintf("Template contains duplicate keys differing only by case: %q and %q", existing, key)) + } + } else { + templateKeyByUpper[upperKey] = key + } + + // Logic to find the user's values for this key. + // 1. Try exact match + targetUserKey := key + if _, ok := userValues[key]; !ok { + // 2. Try case-insensitive match + if mappedKey, ok := caseMap[strings.ToUpper(key)]; ok { + targetUserKey = mappedKey + } + } - if blockValueKeys[key] && trimmed == fmt.Sprintf("%s=\"", key) { + // Handle block values + if blockValueKeys[upperKey] && trimmed == fmt.Sprintf("%s=\"", key) { blockEnd, err := findClosingQuoteLine(templateLines, i+1) if err != nil { return result, "", originalContent, fmt.Errorf("template %s block invalid: %w", key, err) } - if values, ok := userValues[key]; ok && len(values) > 0 { + if values, ok := userValues[targetUserKey]; ok && len(values) > 0 { + processedUserKeys[targetUserKey] = true for _, v := range values { + // Use TEMPLATE Key casing to enforce consistency newLines = append(newLines, renderEnvValue(key, v)...) } } else { @@ -204,8 +230,10 @@ func computeConfigUpgrade(configPath string) (*UpgradeResult, string, []byte, er continue } - if values, ok := userValues[key]; ok && len(values) > 0 { + if values, ok := userValues[targetUserKey]; ok && len(values) > 0 { + processedUserKeys[targetUserKey] = true for _, v := range values { + // Use TEMPLATE Key casing to enforce consistency newLines = append(newLines, renderEnvValue(key, v)...) } } else { @@ -220,15 +248,35 @@ func computeConfigUpgrade(configPath string) (*UpgradeResult, string, []byte, er extraLines := make([]string, 0) for _, key := range userKeyOrder { + if processedUserKeys[key] { + continue + } + // If exact match was in template keys (should have been processed above), skip if templateKeys[key] { continue } + // If case-insensitive match was in template keys (should have been processed), skip + // Check by upper casing the user key and seeing if it exists in templateKeys? + // But wait, templateKeys stores exact keys. + // If user has "Backup_Enabled", and template has "BACKUP_ENABLED". + // We processed "BACKUP_ENABLED", found "Backup_Enabled" via caseMap, and marked "Backup_Enabled" as processed. + // So `processedUserKeys["Backup_Enabled"]` is true. We skip. + // Correct. + + upperKey := strings.ToUpper(key) + if templateKey, ok := templateKeyByUpper[upperKey]; ok && templateKey != key { + if !caseConflicts[upperKey] { + warnings = append(warnings, fmt.Sprintf("Key %q differs only by case from template key %q; preserved as custom entry", key, templateKey)) + } + } + values := userValues[key] if len(values) == 0 { continue } extraKeys = append(extraKeys, key) for _, v := range values { + // Preserve USER's original key casing for extras extraLines = append(extraLines, renderEnvValue(key, v)...) } } @@ -243,20 +291,17 @@ func computeConfigUpgrade(configPath string) (*UpgradeResult, string, []byte, er newLines = append(newLines, extraLines...) } - // Count preserved values: key=value pairs coming from user config for - // keys that exist in the template. + // Count preserved values preserved := 0 - for key, values := range userValues { - if templateKeys[key] { - preserved += len(values) - } + for key := range processedUserKeys { + preserved += len(userValues[key]) } // If nothing changed (no missing keys and no extras), we can return early. - if len(missingKeys) == 0 && len(extraKeys) == 0 { - result.PreservedValues = preserved - return result, "", originalContent, nil - } + // BUT checking "nothing changed" is harder now because we might have renamed keys. + // If we renamed a key, the content CHANGED. + // So we should compare normalized content? + // Or just assume if we parsed everything and re-rendered, and it matches original string... newContent := strings.Join(newLines, lineEnding) // Preserve trailing newline if template had one. @@ -264,16 +309,27 @@ func computeConfigUpgrade(configPath string) (*UpgradeResult, string, []byte, er newContent += lineEnding } + if newContent == string(originalContent) { + result.Changed = false + result.Warnings = warnings + result.PreservedValues = preserved + return result, "", originalContent, nil + } + result.MissingKeys = missingKeys result.ExtraKeys = extraKeys result.PreservedValues = preserved + result.Warnings = warnings result.Changed = true return result, newContent, originalContent, nil } -func parseEnvValues(lines []string) (map[string][]envValue, []string, error) { +func parseEnvValues(lines []string) (map[string][]envValue, []string, map[string]string, map[string]bool, []string, error) { userValues := make(map[string][]envValue) userKeyOrder := make([]string, 0) + caseMap := make(map[string]string) // UPPER -> original + caseConflicts := make(map[string]bool) + warnings := make([]string, 0) for i := 0; i < len(lines); i++ { line := lines[i] @@ -282,16 +338,27 @@ func parseEnvValues(lines []string) (map[string][]envValue, []string, error) { continue } - key, rawValue, ok := splitKeyValueRaw(line) + key, rawValue, comment, ok := splitKeyValueRaw(line) if !ok || key == "" { + if trimmed != "" { + warnings = append(warnings, fmt.Sprintf("Ignored line %d: not a KEY=VALUE entry", i+1)) + } continue } - if blockValueKeys[key] && trimmed == fmt.Sprintf("%s=\"", key) { + upperKey := strings.ToUpper(key) + if existing, ok := caseMap[upperKey]; ok && existing != key { + caseConflicts[upperKey] = true + warnings = append(warnings, fmt.Sprintf("Duplicate keys differ only by case: %q and %q (using last occurrence %q)", existing, key, key)) + } + + caseMap[upperKey] = key + + if blockValueKeys[upperKey] && trimmed == fmt.Sprintf("%s=\"", key) { blockLines := make([]string, 0) blockEnd, err := findClosingQuoteLine(lines, i+1) if err != nil { - return nil, nil, fmt.Errorf("unterminated multi-line value for %s starting at line %d", key, i+1) + return nil, nil, nil, nil, nil, fmt.Errorf("unterminated multi-line value for %s starting at line %d", key, i+1) } blockLines = append(blockLines, lines[i+1:blockEnd]...) @@ -307,38 +374,42 @@ func parseEnvValues(lines []string) (map[string][]envValue, []string, error) { if _, seen := userValues[key]; !seen { userKeyOrder = append(userKeyOrder, key) } - userValues[key] = append(userValues[key], envValue{kind: envValueKindLine, rawValue: rawValue}) + userValues[key] = append(userValues[key], envValue{kind: envValueKindLine, rawValue: rawValue, comment: comment}) } - return userValues, userKeyOrder, nil + return userValues, userKeyOrder, caseMap, caseConflicts, warnings, nil } -func splitKeyValueRaw(line string) (string, string, bool) { - parts := strings.SplitN(line, "=", 2) +func splitKeyValueRaw(line string) (string, string, string, bool) { + comment := "" + body := line + if commentIdx := utils.FindInlineCommentIndex(line); commentIdx >= 0 { + comment = strings.TrimSpace(line[commentIdx:]) + body = line[:commentIdx] + } + + parts := strings.SplitN(body, "=", 2) if len(parts) != 2 { - return "", "", false + return "", "", "", false } key := strings.TrimSpace(parts[0]) + // Handle legacy "export KEY=VALUE" lines with arbitrary whitespace + if fields := strings.Fields(key); len(fields) >= 2 && fields[0] == "export" { + key = fields[1] + } + valuePart := strings.TrimSpace(parts[1]) - // Remove inline comments (but respect quotes) value := valuePart if strings.HasPrefix(valuePart, "\"") || strings.HasPrefix(valuePart, "'") { quote := valuePart[0] - endIdx := strings.IndexByte(valuePart[1:], quote) - if endIdx >= 0 { - value = valuePart[:endIdx+2] + if endIdx := utils.FindClosingQuoteIndex(valuePart, quote); endIdx >= 0 { + value = valuePart[:endIdx+1] } - return key, value, true } - // Not quoted, remove everything after # - if idx := strings.Index(valuePart, "#"); idx >= 0 { - value = strings.TrimSpace(valuePart[:idx]) - } - - return key, value, true + return key, value, comment, true } func findClosingQuoteLine(lines []string, start int) (int, error) { @@ -357,5 +428,9 @@ func renderEnvValue(key string, value envValue) []string { lines = append(lines, "\"") return lines } - return []string{fmt.Sprintf("%s=%s", key, value.rawValue)} + line := fmt.Sprintf("%s=%s", key, value.rawValue) + if value.comment != "" { + line += " " + value.comment + } + return []string{line} } diff --git a/internal/config/upgrade_test.go b/internal/config/upgrade_test.go index 822e4da..196d89d 100644 --- a/internal/config/upgrade_test.go +++ b/internal/config/upgrade_test.go @@ -232,3 +232,193 @@ CUSTOM_BACKUP_PATHS=" } }) } + +func TestPlanUpgradeConfigHandlesExportWhitespace(t *testing.T) { + template := "KEY1=default\n" + withTemplate(t, template, func() { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "backup.env") + content := "export KEY1=custom\nexport\t\tKEY1=custom-tabbed\n" + if err := os.WriteFile(configPath, []byte(content), 0600); err != nil { + t.Fatalf("failed to write config: %v", err) + } + + result, err := PlanUpgradeConfigFile(configPath) + if err != nil { + t.Fatalf("PlanUpgradeConfigFile returned error: %v", err) + } + if len(result.MissingKeys) != 0 { + t.Fatalf("MissingKeys = %v; want []", result.MissingKeys) + } + }) +} + +func TestPlanUpgradeConfigDoesNotStripExporterKey(t *testing.T) { + template := "exporter=default\n" + withTemplate(t, template, func() { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "backup.env") + content := "exporter=custom\n" + if err := os.WriteFile(configPath, []byte(content), 0600); err != nil { + t.Fatalf("failed to write config: %v", err) + } + + result, err := PlanUpgradeConfigFile(configPath) + if err != nil { + t.Fatalf("PlanUpgradeConfigFile returned error: %v", err) + } + if len(result.MissingKeys) != 0 { + t.Fatalf("MissingKeys = %v; want []", result.MissingKeys) + } + if len(result.ExtraKeys) != 0 { + t.Fatalf("ExtraKeys = %v; want []", result.ExtraKeys) + } + }) +} + +func TestUpgradeConfigPreservesInlineComments(t *testing.T) { + template := "KEY1=default1\nKEY2=default2\nKEY3=default3\n" + withTemplate(t, template, func() { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "backup.env") + userConfig := "KEY1=value # keep\nKEY2=\"quoted # keep\" # trailing\n" + if err := os.WriteFile(configPath, []byte(userConfig), 0600); err != nil { + t.Fatalf("failed to seed config: %v", err) + } + + result, err := UpgradeConfigFile(configPath) + if err != nil { + t.Fatalf("UpgradeConfigFile returned error: %v", err) + } + if !result.Changed { + t.Fatal("expected result.Changed=true when keys missing") + } + + data, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("failed to read upgraded config: %v", err) + } + content := string(data) + if !strings.Contains(content, "KEY1=value # keep") { + t.Fatalf("expected inline comment preserved for KEY1, got:\n%s", content) + } + if !strings.Contains(content, "KEY2=\"quoted # keep\" # trailing") { + t.Fatalf("expected inline comment preserved for KEY2, got:\n%s", content) + } + }) +} + +func TestUpgradeConfigPreservesBlockComments(t *testing.T) { + template := "CUSTOM_BACKUP_PATHS=\"\n# template comment\n\"\nKEY1=default\n" + withTemplate(t, template, func() { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "backup.env") + userConfig := "CUSTOM_BACKUP_PATHS=\"\n# keep this\n/path/one\n# keep too\n\"\n" + if err := os.WriteFile(configPath, []byte(userConfig), 0600); err != nil { + t.Fatalf("failed to seed config: %v", err) + } + + result, err := UpgradeConfigFile(configPath) + if err != nil { + t.Fatalf("UpgradeConfigFile returned error: %v", err) + } + if !result.Changed { + t.Fatal("expected result.Changed=true when keys missing") + } + + data, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("failed to read upgraded config: %v", err) + } + content := strings.ReplaceAll(string(data), "\r\n", "\n") + if !strings.Contains(content, "# keep this\n/path/one\n# keep too\n") { + t.Fatalf("expected block comments preserved, got:\n%s", content) + } + }) +} + +func TestPlanUpgradeConfigWarnsOnCaseCollision(t *testing.T) { + template := "BACKUP_PATH=/default/backup\n" + withTemplate(t, template, func() { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "backup.env") + content := "backup_path=/lower\nBACKUP_PATH=/upper\n" + if err := os.WriteFile(configPath, []byte(content), 0600); err != nil { + t.Fatalf("failed to write config: %v", err) + } + + result, err := PlanUpgradeConfigFile(configPath) + if err != nil { + t.Fatalf("PlanUpgradeConfigFile returned error: %v", err) + } + if len(result.ExtraKeys) != 1 || result.ExtraKeys[0] != "backup_path" { + t.Fatalf("ExtraKeys = %v; want [backup_path]", result.ExtraKeys) + } + warnings := strings.Join(result.Warnings, "\n") + if !strings.Contains(warnings, "Duplicate keys differ only by case") { + t.Fatalf("expected case-collision warning, got: %s", warnings) + } + }) +} + +func TestPlanUpgradeConfigWarnsOnIgnoredLine(t *testing.T) { + template := "BACKUP_PATH=/default/backup\n" + withTemplate(t, template, func() { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "backup.env") + content := "BACKUP_PATH=/legacy\nNOT_A_KEY\n" + if err := os.WriteFile(configPath, []byte(content), 0600); err != nil { + t.Fatalf("failed to write config: %v", err) + } + + result, err := PlanUpgradeConfigFile(configPath) + if err != nil { + t.Fatalf("PlanUpgradeConfigFile returned error: %v", err) + } + warnings := strings.Join(result.Warnings, "\n") + if !strings.Contains(warnings, "Ignored line 2") { + t.Fatalf("expected ignored-line warning, got: %s", warnings) + } + }) +} + +func TestUpgradeConfigPreservesBlockValuesCaseInsensitive(t *testing.T) { + template := `BACKUP_PATH=/default/backup +LOG_PATH=/default/log +CUSTOM_BACKUP_PATHS=" +# /template/example +" +` + withTemplate(t, template, func() { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "backup.env") + userConfig := `BACKUP_PATH=/legacy/backup +Custom_Backup_Paths=" +/etc/custom.conf +" +` + if err := os.WriteFile(configPath, []byte(userConfig), 0600); err != nil { + t.Fatalf("failed to seed config: %v", err) + } + + result, err := UpgradeConfigFile(configPath) + if err != nil { + t.Fatalf("UpgradeConfigFile returned error: %v", err) + } + if !result.Changed { + t.Fatal("expected result.Changed=true when template has missing keys") + } + + data, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("failed to read upgraded config: %v", err) + } + // strings.ReplaceAll might not be available if "strings" isn't imported, but it is imported in upgrade_test.go + content := strings.ReplaceAll(string(data), "\r\n", "\n") + + expectedBlock := "CUSTOM_BACKUP_PATHS=\"\n/etc/custom.conf\n\"\n" + if !strings.Contains(content, expectedBlock) { + t.Fatalf("upgraded config missing preserved block with fixed casing:\nGot:\n%s\nWant contains:\n%s", content, expectedBlock) + } + }) +} diff --git a/internal/orchestrator/.backup.lock b/internal/orchestrator/.backup.lock index 2893e45..fffdac2 100644 --- a/internal/orchestrator/.backup.lock +++ b/internal/orchestrator/.backup.lock @@ -1,3 +1,3 @@ -pid=17956 +pid=99930 host=pve -time=2026-02-03T00:07:31+01:00 +time=2026-02-03T19:42:03+01:00 diff --git a/internal/tui/wizard/install.go b/internal/tui/wizard/install.go index f446cb3..e82c220 100644 --- a/internal/tui/wizard/install.go +++ b/internal/tui/wizard/install.go @@ -15,6 +15,7 @@ import ( "github.com/tis24dev/proxsave/internal/config" "github.com/tis24dev/proxsave/internal/tui" "github.com/tis24dev/proxsave/internal/tui/components" + "github.com/tis24dev/proxsave/pkg/utils" ) // InstallWizardData holds the collected installation data @@ -54,10 +55,10 @@ var ( func RunInstallWizard(ctx context.Context, configPath string, baseDir string, buildSig string) (*InstallWizardData, error) { defaultFirewallRules := false data := &InstallWizardData{ - BaseDir: baseDir, - ConfigPath: configPath, - CronTime: "02:00", - EnableEncryption: false, // Default to disabled + BaseDir: baseDir, + ConfigPath: configPath, + CronTime: "02:00", + EnableEncryption: false, // Default to disabled BackupFirewallRules: &defaultFirewallRules, } @@ -190,34 +191,34 @@ func RunInstallWizard(ctx context.Context, configPath string, baseDir string, bu SetLabel(" └─ Rclone Log Remote"). SetText("myremote:pbs-logs"). SetFieldWidth(40) - rcloneLogField.SetDisabled(true) - form.Form.AddFormItem(rcloneLogField) - - // Firewall rules backup (system collection) - firewallEnabled := false - firewallDropdown := tview.NewDropDown(). - SetLabel("Backup Firewall Rules (iptables/nftables)"). - SetOptions([]string{"No", "Yes"}, func(option string, index int) { - firewallEnabled = (option == "Yes") - dropdownOpen = false - }). - SetCurrentOption(0) - - firewallDropdown.SetInputCapture(func(event *tcell.EventKey) *tcell.EventKey { - if event.Key() == tcell.KeyEnter { - dropdownOpen = !dropdownOpen - } else if event.Key() == tcell.KeyEscape { - dropdownOpen = false - } - return event - }) + rcloneLogField.SetDisabled(true) + form.Form.AddFormItem(rcloneLogField) + + // Firewall rules backup (system collection) + firewallEnabled := false + firewallDropdown := tview.NewDropDown(). + SetLabel("Backup Firewall Rules (iptables/nftables)"). + SetOptions([]string{"No", "Yes"}, func(option string, index int) { + firewallEnabled = (option == "Yes") + dropdownOpen = false + }). + SetCurrentOption(0) + + firewallDropdown.SetInputCapture(func(event *tcell.EventKey) *tcell.EventKey { + if event.Key() == tcell.KeyEnter { + dropdownOpen = !dropdownOpen + } else if event.Key() == tcell.KeyEscape { + dropdownOpen = false + } + return event + }) - form.Form.AddFormItem(firewallDropdown) + form.Form.AddFormItem(firewallDropdown) - // Notifications (header + two toggles) - var telegramEnabled, emailEnabled bool - notificationHeader := tview.NewInputField(). - SetLabel("Notifications"). + // Notifications (header + two toggles) + var telegramEnabled, emailEnabled bool + notificationHeader := tview.NewInputField(). + SetLabel("Notifications"). SetFieldWidth(0). SetText(""). SetDisabled(true) @@ -293,10 +294,10 @@ func RunInstallWizard(ctx context.Context, configPath string, baseDir string, bu form.Form.AddFormItem(cronField) // Set up form submission - form.SetOnSubmit(func(values map[string]string) error { - // Collect data - data.EnableSecondaryStorage = secondaryEnabled - if secondaryEnabled { + form.SetOnSubmit(func(values map[string]string) error { + // Collect data + data.EnableSecondaryStorage = secondaryEnabled + if secondaryEnabled { data.SecondaryPath = secondaryPathField.GetText() data.SecondaryLogPath = secondaryLogField.GetText() @@ -309,8 +310,8 @@ func RunInstallWizard(ctx context.Context, configPath string, baseDir string, bu } } - data.EnableCloudStorage = cloudEnabled - if cloudEnabled { + data.EnableCloudStorage = cloudEnabled + if cloudEnabled { data.RcloneBackupRemote = rcloneBackupField.GetText() data.RcloneLogRemote = rcloneLogField.GetText() @@ -321,14 +322,14 @@ func RunInstallWizard(ctx context.Context, configPath string, baseDir string, bu if !strings.Contains(data.RcloneLogRemote, ":") { return fmt.Errorf("rclone log remote must be in format 'remote:path'") } - } + } - data.BackupFirewallRules = &firewallEnabled + data.BackupFirewallRules = &firewallEnabled - // Get notification mode from two toggles - switch { - case telegramEnabled && emailEnabled: - data.NotificationMode = "both" + // Get notification mode from two toggles + switch { + case telegramEnabled && emailEnabled: + data.NotificationMode = "both" case telegramEnabled: data.NotificationMode = "telegram" case emailEnabled: @@ -530,24 +531,7 @@ func ApplyInstallData(baseTemplate string, data *InstallWizardData) (string, err // setEnvValue sets or updates an environment variable in the template func setEnvValue(template, key, value string) string { - lines := strings.Split(template, "\n") - found := false - - for i, line := range lines { - trimmed := strings.TrimSpace(line) - if strings.HasPrefix(trimmed, key+"=") { - lines[i] = key + "=" + value - found = true - break - } - } - - if !found { - // Add at the end - lines = append(lines, key+"="+value) - } - - return strings.Join(lines, "\n") + return utils.SetEnvValue(template, key, value) } // CheckExistingConfig checks if config file exists and asks how to proceed diff --git a/internal/tui/wizard/install_test.go b/internal/tui/wizard/install_test.go index 8a21e8f..0b94b8d 100644 --- a/internal/tui/wizard/install_test.go +++ b/internal/tui/wizard/install_test.go @@ -23,6 +23,28 @@ func TestSetEnvValueUpdateAndAppend(t *testing.T) { } } +func TestSetEnvValuePreservesComments(t *testing.T) { + template := "FOO=old # comment" + updated := setEnvValue(template, "FOO", "new") + if !strings.Contains(updated, "FOO=new") { + t.Fatalf("expected FOO updated, got %q", updated) + } + if !strings.Contains(updated, "# comment") { + t.Fatalf("expected comment preserved, got %q", updated) + } +} + +func TestSetEnvValuePreservesCommentAfterQuotedValue(t *testing.T) { + template := `FOO="old # keep" # trailing comment` + updated := setEnvValue(template, "FOO", "new") + if !strings.Contains(updated, "FOO=new") { + t.Fatalf("expected FOO updated, got %q", updated) + } + if !strings.Contains(updated, "# trailing comment") { + t.Fatalf("expected trailing comment preserved, got %q", updated) + } +} + func TestApplyInstallDataRespectsBaseTemplate(t *testing.T) { baseTemplate := "BASE_DIR=\nMARKER=1\nTELEGRAM_ENABLED=false\nEMAIL_ENABLED=false\nENCRYPT_ARCHIVE=false\n" backupFirewallRules := false diff --git a/pkg/utils/strings.go b/pkg/utils/strings.go index 93753c7..9da308d 100644 --- a/pkg/utils/strings.go +++ b/pkg/utils/strings.go @@ -38,6 +38,62 @@ func TrimQuotes(s string) string { return s } +// FindInlineCommentIndex returns the index of a # that starts an inline comment. +// A # inside quotes or escaped with a backslash is ignored. +func FindInlineCommentIndex(line string) int { + inQuote := false + var quoteChar byte + escaped := false + + for i := 0; i < len(line); i++ { + ch := line[i] + if escaped { + escaped = false + continue + } + if ch == '\\' { + escaped = true + continue + } + if inQuote { + if ch == quoteChar { + inQuote = false + } + continue + } + if ch == '"' || ch == '\'' { + inQuote = true + quoteChar = ch + continue + } + if ch == '#' { + return i + } + } + return -1 +} + +// FindClosingQuoteIndex returns the index of the closing quote in s, +// honoring backslash escapes. Assumes s[0] is the opening quote. +func FindClosingQuoteIndex(s string, quote byte) int { + escaped := false + for i := 1; i < len(s); i++ { + ch := s[i] + if escaped { + escaped = false + continue + } + if ch == '\\' { + escaped = true + continue + } + if ch == quote { + return i + } + } + return -1 +} + // SplitKeyValue splits a "key=value" string into key and value. // Supports inline comments too: KEY="value" # comment func SplitKeyValue(line string) (string, string, bool) { @@ -48,28 +104,70 @@ func SplitKeyValue(line string) (string, string, bool) { key := strings.TrimSpace(parts[0]) valuePart := strings.TrimSpace(parts[1]) - // Remove inline comments (but respect quotes) - // If the value is quoted, take everything inside quotes - // Otherwise, stop at the first # - value := valuePart if strings.HasPrefix(valuePart, "\"") || strings.HasPrefix(valuePart, "'") { - // Find the closing quote quote := valuePart[0] - endIdx := strings.IndexByte(valuePart[1:], quote) - if endIdx >= 0 { - value = valuePart[:endIdx+2] // Include both quotes - } - } else { - // Not quoted, remove everything after # - if idx := strings.Index(valuePart, "#"); idx >= 0 { - value = strings.TrimSpace(valuePart[:idx]) + if endIdx := FindClosingQuoteIndex(valuePart, quote); endIdx >= 0 { + valuePart = valuePart[:endIdx+1] } + } else if idx := FindInlineCommentIndex(valuePart); idx >= 0 { + valuePart = strings.TrimSpace(valuePart[:idx]) } - value = TrimQuotes(strings.TrimSpace(value)) + value := TrimQuotes(strings.TrimSpace(valuePart)) return key, value, true } +// SetEnvValue sets or updates a KEY=VALUE line in a template, preserving indentation and comments. +func SetEnvValue(template, key, value string) string { + lines := strings.Split(template, "\n") + replaced := false + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if IsComment(trimmed) { + continue + } + + parts := strings.SplitN(trimmed, "=", 2) + if len(parts) >= 1 && strings.TrimSpace(parts[0]) == key { + // Found match. + leadingLen := len(line) - len(strings.TrimLeft(line, " \t")) + leading := "" + if leadingLen > 0 { + leading = line[:leadingLen] + } + + comment := "" + commentSpacing := "" + + commentIdx := FindInlineCommentIndex(line) + if commentIdx >= 0 { + comment = line[commentIdx:] + + beforeComment := line[:commentIdx] + trimmedBefore := strings.TrimRight(beforeComment, " \t") + if len(trimmedBefore) < len(beforeComment) { + commentSpacing = beforeComment[len(trimmedBefore):] + } + } + + newLine := leading + key + "=" + value + if comment != "" { + spacing := commentSpacing + if spacing == "" { + spacing = " " + } + newLine += spacing + comment + } + lines[i] = newLine + replaced = true + } + } + if !replaced { + lines = append(lines, key+"="+value) + } + return strings.Join(lines, "\n") +} + // IsComment checks whether a line is a comment (starts with #). func IsComment(line string) bool { trimmed := strings.TrimSpace(line) diff --git a/pkg/utils/strings_test.go b/pkg/utils/strings_test.go index c3208ad..f960cba 100644 --- a/pkg/utils/strings_test.go +++ b/pkg/utils/strings_test.go @@ -92,6 +92,8 @@ func TestSplitKeyValue(t *testing.T) { {"empty value", "KEY=", "KEY", "", true}, {"inline comment", "KEY=value # comment", "KEY", "value", true}, {"quoted hash", `KEY="value # keep"`, "KEY", "value # keep", true}, + {"escaped hash", `KEY=value\#keep`, "KEY", `value\#keep`, true}, + {"escaped quote", `KEY="value\"quoted" # comment`, "KEY", `value\"quoted`, true}, } for _, tt := range tests { @@ -112,6 +114,84 @@ func TestSplitKeyValue(t *testing.T) { } } +func TestFindInlineCommentIndex(t *testing.T) { + tests := []struct { + name string + line string + expected int + }{ + {"no comment", "KEY=value", -1}, + {"simple comment", "KEY=value # comment", 10}, + {"quoted hash", `KEY="value # keep"`, -1}, + {"escaped hash", `KEY=value\#keep # comment`, 16}, + {"escaped quote", `KEY="value\"#still" # comment`, 20}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := FindInlineCommentIndex(tt.line) + if got != tt.expected { + t.Errorf("FindInlineCommentIndex(%q) = %d; want %d", tt.line, got, tt.expected) + } + }) + } +} + +func TestSetEnvValue(t *testing.T) { + tests := []struct { + name string + input string + key string + value string + expected string + }{ + { + name: "update existing", + input: "FOO=old\nBAR=keep", + key: "FOO", + value: "new", + expected: "FOO=new\nBAR=keep", + }, + { + name: "append new", + input: "FOO=old", + key: "BAR", + value: "val", + expected: "FOO=old\nBAR=val", + }, + { + name: "preserve indentation and comment spacing", + input: " FOO=old # comment", + key: "FOO", + value: "new", + expected: " FOO=new # comment", + }, + { + name: "preserve comment after quoted hash", + input: `FOO="old # keep" # trailing`, + key: "FOO", + value: "new", + expected: `FOO=new # trailing`, + }, + { + name: "preserve comment after escaped hash", + input: `FOO=old\#keep # trailing`, + key: "FOO", + value: "new", + expected: `FOO=new # trailing`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := SetEnvValue(tt.input, tt.key, tt.value) + if got != tt.expected { + t.Fatalf("SetEnvValue() = %q; want %q", got, tt.expected) + } + }) + } +} + func TestIsComment(t *testing.T) { tests := []struct { name string