fix(config): let TMI_OAUTH_CALLBACK_URL outrank the database row

internal/config/migratable_settings.go

@@ -29,6 +29,11 @@ type MigratableSetting struct {
 }
 
 // settingSource returns "environment" if the given env var is set, otherwise "config".
+//
+// The env var named here must be one this package's struct tags actually bind.
+// Reporting "environment" for a name the struct does not read would mark the
+// struct DEFAULT as Explicit, and an Explicit default outranks the database —
+// turning a fallback into an override.
 // SEM@ef979cc7527137e0448782341a4ffe8e944571f5: return 'environment' when the given env var is set, otherwise 'config' (pure)
 func settingSource(envVar string) string {
 	if os.Getenv(envVar) != "" {
@@ -185,14 +190,30 @@ func (c *Config) getMigratableFeatureFlags() []MigratableSetting {
 func (c *Config) getMigratableOAuthSettings() []MigratableSetting {
 	settings := []MigratableSetting{}
 
-	// OAuth callback URL (non-sensitive, useful for diagnostics)
+	// OAuth callback URL (non-sensitive, useful for diagnostics).
+	//
+	// Source MUST go through settingSource, not a hardcoded "config". This is an
+	// operational (database-backed) key, and SettingsService.getConfigSetting
+	// discards any operational setting that is not Explicit — so a hardcoded
+	// "config" made GetString("auth.oauth_callback_url") always fall through to
+	// the database, no matter what the operator set. On AWS that let a stale
+	// http://localhost:8080/oauth2/callback row outrank a correctly-set
+	// TMI_OAUTH_CALLBACK_URL, and every provider (Google, GitHub, Microsoft)
+	// advertised a localhost redirect_uri, so OAuth sign-in could not complete.
+	//
+	// Only TMI_OAUTH_CALLBACK_URL is named, matching the OAuthConfig.CallbackURL
+	// struct tag. auth/config.go additionally honors a legacy OAUTH_CALLBACK_URL,
+	// but this package does not bind it, so counting it here would report
+	// "environment" while Value was still the localhost default — making that
+	// default Explicit and letting it outrank a correct database row.
 	if c.Auth.OAuth.CallbackURL != "" {
 		settings = append(settings, MigratableSetting{
 			Key:         "auth.oauth_callback_url",
 			Value:       c.Auth.OAuth.CallbackURL,
 			Type:        "string",
 			Description: "OAuth callback URL",
-			Source:      "config",
+			Source:      settingSource("TMI_OAUTH_CALLBACK_URL"),
+			EnvVar:      "TMI_OAUTH_CALLBACK_URL",
 		})
 	}
 

internal/config/migratable_settings_test.go

@@ -184,3 +184,60 @@ func TestDefaultOperationalSettings_OnlyOperational(t *testing.T) {
 		t.Error("expected websocket.inactivity_timeout_seconds among operational settings")
 	}
 }
+
+// A callback URL supplied via TMI_OAUTH_CALLBACK_URL must be reported as
+// Source "environment" so GetMigratableSettings marks it Explicit.
+//
+// This is load-bearing, not cosmetic. SettingsService.getConfigSetting drops
+// any non-Explicit operational setting, so GetString("auth.oauth_callback_url")
+// falls through to the database. On AWS that meant a stale
+// http://localhost:8080/oauth2/callback row won over a correctly-set
+// TMI_OAUTH_CALLBACK_URL, and every provider's redirect_uri (Google, GitHub,
+// Microsoft) was advertised as localhost — OAuth sign-in could not complete.
+// The setting used to hardcode Source: "config", unlike the 130 others that
+// route through settingSource().
+func TestGetMigratableSettings_OAuthCallbackURLHonorsEnvironment(t *testing.T) {
+	t.Setenv("TMI_OAUTH_CALLBACK_URL", "https://api.example.test/oauth2/callback")
+	cfg := &Config{Auth: AuthConfig{OAuth: OAuthConfig{
+		CallbackURL: "https://api.example.test/oauth2/callback",
+	}}}
+	found := findSetting(cfg.GetMigratableSettings(), "auth.oauth_callback_url")
+	require.NotNil(t, found)
+	assert.Equal(t, "environment", found.Source)
+	assert.True(t, found.Explicit, "env-supplied callback URL must outrank the database row")
+	assert.Equal(t, "TMI_OAUTH_CALLBACK_URL", found.EnvVar)
+}
+
+// With no env var set, the struct default must stay non-Explicit so the
+// database remains reachable for this admin-only, hot-reloadable setting.
+func TestGetMigratableSettings_OAuthCallbackURLDefaultDefersToDatabase(t *testing.T) {
+	t.Setenv("TMI_OAUTH_CALLBACK_URL", "")
+	t.Setenv("OAUTH_CALLBACK_URL", "")
+	cfg := &Config{Auth: AuthConfig{OAuth: OAuthConfig{
+		CallbackURL: "http://localhost:8080/oauth2/callback",
+	}}}
+	found := findSetting(cfg.GetMigratableSettings(), "auth.oauth_callback_url")
+	require.NotNil(t, found)
+	assert.Equal(t, "config", found.Source)
+	assert.False(t, found.Explicit, "an unset callback URL must not shadow the database")
+}
+
+// The legacy OAUTH_CALLBACK_URL is honored by auth/config.go but is NOT bound by
+// this package's OAuthConfig.CallbackURL struct tag. It must therefore NOT count
+// as "environment" here: doing so would report Explicit while Value was still the
+// localhost struct default, and an Explicit default outranks the database — the
+// fallback would become an override, which is a worse failure than the bug this
+// fix addresses.
+func TestGetMigratableSettings_OAuthCallbackURLIgnoresUnboundLegacyEnvName(t *testing.T) {
+	t.Setenv("TMI_OAUTH_CALLBACK_URL", "")
+	t.Setenv("OAUTH_CALLBACK_URL", "https://legacy.example.test/oauth2/callback")
+	// Value is the struct default, because this package never reads the legacy name.
+	cfg := &Config{Auth: AuthConfig{OAuth: OAuthConfig{
+		CallbackURL: "http://localhost:8080/oauth2/callback",
+	}}}
+	found := findSetting(cfg.GetMigratableSettings(), "auth.oauth_callback_url")
+	require.NotNil(t, found)
+	assert.Equal(t, "config", found.Source)
+	assert.False(t, found.Explicit,
+		"an env name this package does not bind must not make the default Explicit")
+}