From accecdb50b95881fc0b7086d795a3640d58049f3 Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Tue, 11 Nov 2025 00:17:40 +0000
Subject: [PATCH] Make api_tokens.json authoritative source for API tokens
 (fixes #685)

This is the proper architectural fix for #685. The previous commit was a
bandaid that prevented unnecessary .env writes. This commit addresses the
root cause: dual-source-of-truth for API tokens (.env vs api_tokens.json).

Changes:

1. Startup Migration (config.go:896-951):
   - When loading config, if API_TOKEN/API_TOKENS exist in .env but not in
     api_tokens.json, automatically migrate them
   - Migrated tokens are named "Migrated from .env (prefix)" for clarity
   - Logs a deprecation warning: API_TOKEN/API_TOKENS in .env are deprecated
   - Leaves .env untouched (safe for existing deployments)

2. Config Watcher Changes (watcher.go:338-424):
   - Only load tokens from .env if api_tokens.json is EMPTY
   - Once api_tokens.json has records, it becomes the authoritative source
   - .env changes no longer trigger token overwrites when api_tokens.json exists
   - Logs debug message when ignoring env tokens

Result:
- Existing deployments: env tokens automatically migrated to api_tokens.json
- UI-created tokens: never overwritten by .env changes
- Dark mode toggle: no longer triggers token reload from .env
- Backward compatible: fresh installs with API_TOKEN in .env still work
- Migration path: users can safely keep API_TOKEN in .env, it will be ignored

Future improvement: Add UI warning when API_TOKEN/API_TOKENS still present
in .env, prompting users to rotate tokens via the UI.
---
 internal/config/config.go  | 27 +++++++++++++++++++++++----
 internal/config/watcher.go | 17 +++++++++++------
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/internal/config/config.go b/internal/config/config.go
index ea8c0b7..73eb7a0 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -896,6 +896,11 @@ func Load() (*Config, error) {
 	if len(envTokens) > 0 {
 		cfg.EnvOverrides["API_TOKEN"] = true
 		cfg.EnvOverrides["API_TOKENS"] = true
+
+		// Track if we migrated any new tokens from env to persistence
+		migratedCount := 0
+		needsPersist := false
+
 		for _, tokenValue := range envTokens {
 			if tokenValue == "" {
 				continue
@@ -909,18 +914,18 @@ func Load() (*Config, error) {
 				hashed = auth.HashAPIToken(tokenValue)
 				prefix = tokenPrefix(tokenValue)
 				suffix = tokenSuffix(tokenValue)
-				log.Info().Msg("Auto-hashed plain text API token from environment variable")
-			} else {
-				log.Debug().Msg("Loaded pre-hashed API token from env var")
+				log.Debug().Msg("Auto-hashed plain text API token from environment variable")
 			}
 
+			// Check if this token already exists in api_tokens.json
 			if cfg.HasAPITokenHash(hashed) {
 				continue
 			}
 
+			// Migrate env token to api_tokens.json
 			record := APITokenRecord{
 				ID:        uuid.NewString(),
-				Name:      "Environment token",
+				Name:      "Migrated from .env (" + prefix + ")",
 				Hash:      hashed,
 				Prefix:    prefix,
 				Suffix:    suffix,
@@ -928,8 +933,22 @@ func Load() (*Config, error) {
 				Scopes:    []string{ScopeWildcard},
 			}
 			cfg.APITokens = append(cfg.APITokens, record)
+			migratedCount++
+			needsPersist = true
 		}
+
 		cfg.SortAPITokens()
+
+		// Persist migrated tokens to api_tokens.json
+		if needsPersist && persistence != nil {
+			if err := persistence.SaveAPITokens(cfg.APITokens); err != nil {
+				log.Error().Err(err).Msg("Failed to persist migrated API tokens from environment")
+			} else {
+				log.Warn().
+					Int("count", migratedCount).
+					Msg("Migrated API tokens from .env to api_tokens.json - API_TOKEN/API_TOKENS in .env are deprecated and will be ignored in future releases. Manage tokens via the UI instead.")
+			}
+		}
 	}
 
 	// Check if API token is enabled
diff --git a/internal/config/watcher.go b/internal/config/watcher.go
index 6bf992f..57ee6f1 100644
--- a/internal/config/watcher.go
+++ b/internal/config/watcher.go
@@ -335,7 +335,8 @@ func (cw *ConfigWatcher) reloadConfig() {
 		}
 	}
 
-	// Apply API tokens if present in .env (legacy support)
+	// Legacy env token support: only process if api_tokens.json is empty
+	// This prevents .env changes from overwriting UI-managed tokens (fixes #685)
 	rawTokens := make([]string, 0, 4)
 	if raw, ok := envMap["API_TOKENS"]; ok {
 		raw = strings.Trim(raw, "'\"")
@@ -347,17 +348,19 @@ func (cw *ConfigWatcher) reloadConfig() {
 					rawTokens = append(rawTokens, token)
 				}
 			}
-		} else {
-			// Explicit empty list clears tokens
-			rawTokens = []string{}
 		}
 	}
 	if raw, ok := envMap["API_TOKEN"]; ok {
 		raw = strings.Trim(raw, "'\"")
-		rawTokens = append(rawTokens, raw)
+		if raw != "" {
+			rawTokens = append(rawTokens, raw)
+		}
 	}
 
-	if len(rawTokens) > 0 {
+	// Only reload tokens from .env if NO tokens exist in api_tokens.json
+	// This makes api_tokens.json the authoritative source once it has records
+	if len(rawTokens) > 0 && len(cw.config.APITokens) == 0 {
+		log.Debug().Msg("No existing API tokens found - loading from .env (legacy)")
 		seen := make(map[string]struct{}, len(rawTokens))
 		newRecords := make([]APITokenRecord, 0, len(rawTokens))
 		for _, tokenValue := range rawTokens {
@@ -416,6 +419,8 @@ func (cw *ConfigWatcher) reloadConfig() {
 				}
 			}
 		}
+	} else if len(rawTokens) > 0 && len(cw.config.APITokens) > 0 {
+		log.Debug().Msg("Ignoring API_TOKEN/API_TOKENS from .env - api_tokens.json is authoritative")
 	}
 
 	// REMOVED: POLLING_INTERVAL from .env - now ONLY in system.json