diff --git a/api/config_handlers_test.go b/api/config_handlers_test.go index 9719d1f0..0fba6017 100644 --- a/api/config_handlers_test.go +++ b/api/config_handlers_test.go @@ -365,7 +365,7 @@ func TestGetClientConfig_TimmyReaderAbsent(t *testing.T) { assert.False(t, *cfg.Features.TimmyEnabled) } -// SEM@1aa36c06c7b700d3f00bf6f4b22125d673b1070a: verify listing system settings requires admin +// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: verify listing system settings requires admin func TestListSystemSettings_AdminRequired(t *testing.T) { // Save original admin store originalAdminStore := GlobalGroupMemberRepository @@ -405,7 +405,7 @@ func TestListSystemSettings_AdminRequired(t *testing.T) { assert.Equal(t, "forbidden", string(errResp.Error)) } -// SEM@1aa36c06c7b700d3f00bf6f4b22125d673b1070a: verify listing system settings returns 503 when the service is unavailable +// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: verify listing system settings returns 503 when the service is unavailable func TestListSystemSettings_ServiceUnavailable(t *testing.T) { // Save original admin store originalAdminStore := GlobalGroupMemberRepository @@ -812,7 +812,7 @@ func TestGetSystemSetting_ConfigSourced(t *testing.T) { assert.Equal(t, true, setting["read_only"]) } -// SEM@1aa36c06c7b700d3f00bf6f4b22125d673b1070a: verify updating a config-sourced setting returns 409 +// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: verify updating a config-sourced setting returns 409 func TestUpdateSystemSetting_409_ConfigSourced(t *testing.T) { originalAdminStore := GlobalGroupMemberRepository defer restoreConfigStores(originalAdminStore) @@ -1521,7 +1521,7 @@ func TestBuildContentProviders_PickerConfigDeepCopy(t *testing.T) { // The pass runs in one transaction (#845), so a database failure is a whole- // request failure and must be reported as 503 (transient, retry) or 500, never // as the 409 "encryption_not_enabled" precondition. -// SEM@094904c7a8ba41296e31c7fa9623c72c3f018fa2: test reencrypt maps not-enabled/unreadable to 409, transient DB failure to 503, other to 500 +// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: test reencrypt maps not-enabled/unreadable to 409, transient DB failure to 503, other to 500 func TestReencryptSystemSettings_ErrorMapping(t *testing.T) { originalAdminStore := GlobalGroupMemberRepository defer restoreConfigStores(originalAdminStore) @@ -1677,3 +1677,46 @@ func TestSystemSetting_UnclassifiedKeyGetAndDelete(t *testing.T) { assert.Equal(t, http.StatusNotFound, do("GET", "never_created_key"), "unclassified and absent is a plain 404") assert.Equal(t, http.StatusNotFound, do("DELETE", "never_created_key")) } + +// SEM@97bdf895453505ffee1c671271b82af781c61b72: verify the system-setting PUT handler returns 400 for invalid Timmy values +func TestUpdateSystemSetting_TimmyValidation(t *testing.T) { + gin.SetMode(gin.TestMode) + originalAdminStore := GlobalGroupMemberRepository + defer restoreConfigStores(originalAdminStore) + GlobalGroupMemberRepository = &mockGroupMemberStoreForAdmin{isAdminResult: true} + + put := func(t *testing.T, buildMode, key, body string) *httptest.ResponseRecorder { + t.Helper() + t.Setenv("TMI_BUILD_MODE", buildMode) + server := &Server{settingsService: NewSettingsService(setupSettingsTestDB(t), nil)} + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Set("userUUID", uuid.New().String()) + c.Set("userEmail", "test@example.com") + c.Set("userInternalUUID", uuid.New().String()) + c.Set("userProvider", "test") + c.Request = httptest.NewRequest("PUT", "/admin/settings/"+key, strings.NewReader(body)) + c.Request.Header.Set("Content-Type", "application/json") + server.UpdateSystemSetting(c, key) + return w + } + + t.Run("production dump flag true is 400", func(t *testing.T) { + w := put(t, "production", "timmy.dump_extracted_text_to_note", `{"value": "true", "type": "bool"}`) + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "invalid_input") + }) + t.Run("unknown provider is 400", func(t *testing.T) { + w := put(t, "dev", "timmy.llm_provider", `{"value": "bogus", "type": "string"}`) + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "invalid_input") + }) + t.Run("development dump flag true is 200", func(t *testing.T) { + w := put(t, "dev", "timmy.dump_extracted_text_to_note", `{"value": "true", "type": "bool"}`) + assert.Equal(t, http.StatusOK, w.Code) + }) + t.Run("known provider is 200", func(t *testing.T) { + w := put(t, "dev", "timmy.llm_provider", `{"value": "openai", "type": "string"}`) + assert.Equal(t, http.StatusOK, w.Code) + }) +} diff --git a/api/settings_service.go b/api/settings_service.go index d150d4e5..96fb4c58 100644 --- a/api/settings_service.go +++ b/api/settings_service.go @@ -494,8 +494,41 @@ func (s *SettingsService) ListByPrefix(ctx context.Context, prefix string) ([]mo return settings, nil } +// settingSemanticValidators is the single key -> validator map applied by Set, +// so semantic checks on specific settings are enforced for every writer that +// goes through Set (the admin API) rather than per handler. Writers that +// bypass Set, such as dbtool (cmd/dbtool/config.go) and raw DB writes, are not +// covered here; the startup check catches their values at the next boot. The +// validators mirror the startup checks in config.ValidateTimmyConfig (#1102). +var settingSemanticValidators = map[string]func(value string, productionMode bool) error{ + "timmy.dump_extracted_text_to_note": func(value string, productionMode bool) error { + enabled, err := strconv.ParseBool(value) + if err != nil { + return fmt.Errorf("timmy.dump_extracted_text_to_note: value is not a valid boolean") + } + buildMode := "" + if productionMode { + buildMode = "production" + } + return config.ValidateTimmyDumpFlag(enabled, buildMode) + }, + "timmy.llm_provider": func(value string, _ bool) error { + return config.TimmyConfig{LLMProvider: value}.ValidateLLMProvider() + }, +} + +// validateSettingSemantics applies the per-key validator for the setting, if any. +// SEM@57a0ccb18013f0ed5d8a8631da478fae012e3c67: validate a setting value against its key-specific semantic rules (pure) +func (s *SettingsService) validateSettingSemantics(setting *models.SystemSetting) error { + validate, ok := settingSemanticValidators[string(setting.SettingKey)] + if !ok { + return nil + } + return validate(string(setting.Value), s.isProductionMode()) +} + // Set creates or updates a setting -// SEM@3b682947: store or update a system setting, stamping explicit origin, with type validation, encryption, and cache invalidation (reads DB) +// SEM@57a0ccb18013f0ed5d8a8631da478fae012e3c67: store or update a system setting, stamping explicit origin, with type validation, encryption, and cache invalidation (reads DB) func (s *SettingsService) Set(ctx context.Context, setting *models.SystemSetting) error { logger := slogging.Get() @@ -514,6 +547,12 @@ func (s *SettingsService) Set(ctx context.Context, setting *models.SystemSetting return err } + // Per-key semantic validation (#1102): reject values the server would + // refuse to boot with. + if err := s.validateSettingSemantics(setting); err != nil { + return err + } + // Enforce the secret => encrypted invariant at write time. The Secret // classification is not masking-only: the set of keys we refuse to persist // in plaintext here is exactly the set shouldMaskSettingValue redacts from @@ -535,8 +574,8 @@ func (s *SettingsService) Set(ctx context.Context, setting *models.SystemSetting // Encrypt value before saving to database dbSetting := *setting - // Set is always an operator-initiated write (admin API, dbtool, or any - // other caller) — stamp it explicit regardless of what the caller passed + // Set is always an operator-initiated write (the admin API or any other + // caller of Set) — stamp it explicit regardless of what the caller passed // in, so an operator overwriting a seeded row promotes it (#794). dbSetting.Origin = models.NullableDBVarchar{String: models.SystemSettingOriginExplicit, Valid: true} if encryptionEnabled { diff --git a/api/settings_service_test.go b/api/settings_service_test.go index da516cce..a4e356c0 100644 --- a/api/settings_service_test.go +++ b/api/settings_service_test.go @@ -1161,3 +1161,59 @@ func TestReEncryptAll_UnreadableCap(t *testing.T) { assert.Contains(t, err.Error(), "900") assert.Len(t, errs, maxUnreadableSettings) } + +// SEM@97bdf895453505ffee1c671271b82af781c61b72: verify settings writes reject invalid Timmy values (production dump flag, unknown provider) +func TestSettingsService_SetValidatesTimmyKeys(t *testing.T) { + mk := func(key, value string, typ models.DBVarchar) *models.SystemSetting { + return &models.SystemSetting{SettingKey: models.DBVarchar(key), Value: models.DBText(value), SettingType: typ} + } + + t.Run("production rejects dump flag true", func(t *testing.T) { + t.Setenv("TMI_BUILD_MODE", "production") + // nil gormDB: a returned error proves validation ran before any write. + svc := NewSettingsService(nil, nil) + err := svc.Set(context.Background(), mk("timmy.dump_extracted_text_to_note", "true", models.SystemSettingTypeBool)) + require.Error(t, err) + assert.Contains(t, err.Error(), "timmy.dump_extracted_text_to_note") + }) + + t.Run("production accepts dump flag false", func(t *testing.T) { + t.Setenv("TMI_BUILD_MODE", "production") + svc := NewSettingsService(setupSettingsTestDB(t), nil) + require.NoError(t, svc.Set(context.Background(), mk("timmy.dump_extracted_text_to_note", "false", models.SystemSettingTypeBool))) + }) + + t.Run("development accepts dump flag true", func(t *testing.T) { + t.Setenv("TMI_BUILD_MODE", "dev") + svc := NewSettingsService(setupSettingsTestDB(t), nil) + require.NoError(t, svc.Set(context.Background(), mk("timmy.dump_extracted_text_to_note", "true", models.SystemSettingTypeBool))) + }) + + t.Run("unknown llm_provider rejected", func(t *testing.T) { + t.Setenv("TMI_BUILD_MODE", "dev") + svc := NewSettingsService(nil, nil) + err := svc.Set(context.Background(), mk("timmy.llm_provider", "bogus", models.SystemSettingTypeString)) + require.Error(t, err) + assert.Contains(t, err.Error(), "timmy.llm_provider") + }) + + for _, v := range []string{"openai", "anthropic"} { + t.Run("llm_provider accepts "+v, func(t *testing.T) { + t.Setenv("TMI_BUILD_MODE", "dev") + svc := NewSettingsService(setupSettingsTestDB(t), nil) + require.NoError(t, svc.Set(context.Background(), mk("timmy.llm_provider", v, models.SystemSettingTypeString))) + }) + } + + t.Run("llm_provider validator accepts empty", func(t *testing.T) { + // Set itself rejects empty strings for all string settings (use DELETE); + // the provider validator must not be what rejects them. + require.NoError(t, settingSemanticValidators["timmy.llm_provider"]("", true)) + }) + + t.Run("dump flag validator fails closed on unparseable value", func(t *testing.T) { + for _, prod := range []bool{true, false} { + require.Error(t, settingSemanticValidators["timmy.dump_extracted_text_to_note"]("maybe", prod)) + } + }) +} diff --git a/api/timmy_config_provider_test.go b/api/timmy_config_provider_test.go index 71adebe3..f702002a 100644 --- a/api/timmy_config_provider_test.go +++ b/api/timmy_config_provider_test.go @@ -24,6 +24,7 @@ func TestTimmyConfigProvider_Current_AssemblesFromSettings(t *testing.T) { ms.AddSetting("timmy.embedding_dimension", "3072", "int") ms.AddSetting("timmy.text_retrieval_top_k", "7", "int") ms.AddSetting("timmy.embedding_cleanup_interval_minutes", "120", "int") + ms.AddSetting("timmy.dump_extracted_text_to_note", "true", "bool") p := NewTimmyConfigProvider(ms) cfg := p.Current(context.Background()) @@ -37,6 +38,7 @@ func TestTimmyConfigProvider_Current_AssemblesFromSettings(t *testing.T) { assert.Equal(t, 3072, cfg.EmbeddingDimension) assert.Equal(t, 7, cfg.TextRetrievalTopK) assert.Equal(t, 120, cfg.EmbeddingCleanupIntervalMinutes) + assert.True(t, cfg.DumpExtractedTextToNote) assert.Equal(t, 50, cfg.MaxConversationHistory) require.True(t, cfg.IsConfigured()) } diff --git a/cmd/server/main.go b/cmd/server/main.go index 91fbaf17..c9d4463e 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -247,7 +247,7 @@ func PublicPathsMiddleware() gin.HandlerFunc { } // JWT Middleware factory function that takes config, token blacklist, auth handlers, and ticket validator -// SEM@411a53c663401d55a0f66913e00979599a208c93: build middleware that validates JWT tokens on protected paths and on bearer-carrying handler-auth paths +// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: build middleware that validates JWT tokens on protected paths and on bearer-carrying handler-auth paths func JWTMiddleware(cfg *config.Config, tokenBlacklist *auth.TokenBlacklist, authHandlers *auth.Handlers, ticketValidator *TicketValidator) gin.HandlerFunc { // Initialize authentication components publicPathChecker := &PublicPathChecker{} @@ -306,7 +306,7 @@ func JWTMiddleware(cfg *config.Config, tokenBlacklist *auth.TokenBlacklist, auth } // Dev-mode only endpoint to get current user info -// SEM@1d6e8926b4e58c0d98fff4d43bd3f6df1852d61a: build a dev-only handler that returns the authenticated user's name and role from JWT context +// SEM@db94db3c8118cffb5220bf9a44c2fde0ada20427: build a dev-only handler that returns the authenticated user's name and role from JWT context func DevUserInfoHandler() gin.HandlerFunc { return func(c *gin.Context) { logger := slogging.GetContextLogger(c) @@ -740,7 +740,7 @@ func newSystemAuditRepo(db *gorm.DB, operatorName string) api.SystemAuditReposit return repo } -// SEM@ab6a7ff21a80d96c9f7c490184f44a06dc5607b9: initialize databases and subsystems and register all API routes, returning the router +// SEM@57a0ccb18013f0ed5d8a8631da478fae012e3c67: initialize databases and subsystems and register all API routes, returning the router func setupRouter(config *config.Config) (*gin.Engine, *api.Server, *api.EmbeddingCleaner) { // Create a gin router without default middleware r := gin.New() @@ -952,6 +952,9 @@ func setupRouter(config *config.Config) (*gin.Engine, *api.Server, *api.Embeddin settingsService.SetConfigProvider(configProvider) logger.Info("Config provider set for settings migration and priority lookups") + // Refuse to start on invalid effective (DB-resolved) Timmy config (#1102). + exitOnInvalidEffectiveTimmy(context.Background(), settingsService, config.Auth.BuildMode) + // Wire optimistic-locking enforcement flag (T14 / #385). When false, missing // If-Match emits Deprecation/Warning headers; when true, returns 428. api.SetRequireIfMatch(config.Server.RequireIfMatch) @@ -1394,7 +1397,7 @@ func wireExtractionNATS(apiServer *api.Server, gormDB *gorm.DB) *api.ExtractionP // content provider), or when any required collaborator is unavailable, the // function logs and leaves the handler unset; the generated delegation // wrappers will respond with 503 for the affected endpoints. -// SEM@f69d82c5864c65f768181d82416962e3bbd63667: build and register delegated content-OAuth handlers on the API server +// SEM@d5435580d5a233d66783c8c40148f924403f1809: build and register delegated content-OAuth handlers on the API server func wireContentOAuthHandlers(apiServer *api.Server, cfg *config.Config, gormDB *gorm.DB, dbManager *db.Manager, authHandlers *auth.Handlers) (api.ContentTokenRepository, *api.ContentOAuthProviderRegistry) { logger := slogging.Get() @@ -1771,8 +1774,9 @@ func buildContentPipeline(cfg *config.Config, contentSources *api.ContentSourceR ) // Dev/test-only: dump each successful extraction's markdown as a Note on - // the parent threat model, for inspection in the existing UI. Refused at - // startup in production builds by Config.validateTimmy. + // the parent threat model, for inspection in the existing UI. Only the + // env/YAML value is consulted here (the DB value does not install the + // dumper). Production builds are refused at startup by validateTimmyAtStartup. if cfg.Timmy.DumpExtractedTextToNote { if api.GlobalNoteRepository == nil || api.GlobalDocumentRepository == nil { logger.Error("dump_extracted_text_to_note enabled but Note/Document repositories are not initialized") @@ -1913,7 +1917,7 @@ func startWebhookWorkers(ctx context.Context, cfg *config.Config) (*api.WebhookE return webhookConsumer, challengeWorker, deliveryWorker, cleanupWorker } -// SEM@d0dd04daab6a17c9492d01d80920f869be8a735a: parse flags, load config, resolve secrets, and launch the server process +// SEM@57a0ccb18013f0ed5d8a8631da478fae012e3c67: parse flags, load config, resolve secrets, and launch the server process func main() { // Parse command line flags flags, err := config.ParseFlagsExt() @@ -1947,6 +1951,13 @@ func main() { os.Exit(1) } + // Refuse to start on invalid Timmy config from env/YAML (#1102). The + // DB-resolved config is checked again once the settings service is up. + if err := validateTimmyAtStartup("env/YAML", cfg.Timmy, cfg.Auth.BuildMode); err != nil { + slogging.Get().Error("%v", err) + os.Exit(1) + } + // Initialize logger. CloudWriter/CloudLogLevel are left unset (nil): no // cloud logging provider is currently wired in (the OCI writer was // removed; NoopCloudWriter/CloudLogWriter remain as the extension point diff --git a/cmd/server/startup_checks.go b/cmd/server/startup_checks.go index 3598356e..67ac411e 100644 --- a/cmd/server/startup_checks.go +++ b/cmd/server/startup_checks.go @@ -3,8 +3,10 @@ package main import ( "context" "fmt" + "os" "strings" + "github.com/ericfitz/tmi/api" "github.com/ericfitz/tmi/api/models" "github.com/ericfitz/tmi/internal/config" "github.com/ericfitz/tmi/internal/crypto" @@ -51,6 +53,7 @@ type settingsLister interface { // anything else (request logs start with REQUEST_COMPLETE). const settingsAtRestCheckPrefix = "settings-at-rest check: " +// SEM@7430764167888f43111b8fe959e8c73449a4f9af: warn when secret-classified settings are stored as plaintext at rest (reads DB) func warnIfPlaintextSecretsAtRest( ctx context.Context, encryptor *crypto.SettingsEncryptor, @@ -243,3 +246,33 @@ func secretClassifiedKeys(cfg *config.Config) []string { } return keys } + +// validateTimmyAtStartup enforces the Timmy invariants (no dump flag in +// production, recognized llm_provider) on one Timmy config. main calls it on +// the env/YAML config after Load and again on the DB-resolved effective config +// once the settings service is up (#1102); source labels the config in the +// returned error. +// SEM@97bdf895453505ffee1c671271b82af781c61b72: validate Timmy config at startup, labeling the failing config source (pure) +func validateTimmyAtStartup(source string, timmyCfg config.TimmyConfig, buildMode string) error { + if err := config.ValidateTimmyConfig(timmyCfg, buildMode); err != nil { + return fmt.Errorf("invalid Timmy configuration (%s): %w", source, err) + } + return nil +} + +// validateEffectiveTimmyAtStartup runs validateTimmyAtStartup on the Timmy +// config resolved through the settings service (config-first, then DB). +// SEM@97bdf895453505ffee1c671271b82af781c61b72: validate the DB-resolved effective Timmy config at startup (reads DB) +func validateEffectiveTimmyAtStartup(ctx context.Context, settings api.SettingsServiceInterface, buildMode string) error { + return validateTimmyAtStartup("settings", api.NewTimmyConfigProvider(settings).Current(ctx), buildMode) +} + +// exitOnInvalidEffectiveTimmy logs the failure and exits the process when the +// effective Timmy config is invalid. +// SEM@57a0ccb18013f0ed5d8a8631da478fae012e3c67: terminate startup when the effective Timmy config is invalid (exits process) +func exitOnInvalidEffectiveTimmy(ctx context.Context, settings api.SettingsServiceInterface, buildMode string) { + if err := validateEffectiveTimmyAtStartup(ctx, settings, buildMode); err != nil { + slogging.Get().Error("%v", err) + os.Exit(1) + } +} diff --git a/cmd/server/startup_checks_test.go b/cmd/server/startup_checks_test.go index ae35d218..12dbdd9d 100644 --- a/cmd/server/startup_checks_test.go +++ b/cmd/server/startup_checks_test.go @@ -6,10 +6,12 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" "sync" "testing" + "github.com/ericfitz/tmi/api" "github.com/ericfitz/tmi/api/models" "github.com/ericfitz/tmi/internal/config" "github.com/ericfitz/tmi/internal/crypto" @@ -696,3 +698,92 @@ func TestWarnIfConfigDatabaseDiverges_RedactsProviderClientSecret(t *testing.T) "the database-side client secret must never reach a log line") } } + +// SEM@97bdf895453505ffee1c671271b82af781c61b72: verify startup Timmy validation rejects the production dump flag and unknown providers +func TestValidateTimmyAtStartup(t *testing.T) { + dump := config.TimmyConfig{DumpExtractedTextToNote: true} + badProvider := config.TimmyConfig{LLMProvider: "bogus"} + + tests := []struct { + name string + source string + cfg config.TimmyConfig + buildMode string + wantErr string + }{ + {"production dump flag from env/YAML", "env/YAML", dump, "production", "timmy.dump_extracted_text_to_note"}, + {"production dump flag from DB-resolved", "settings", dump, "production", "timmy.dump_extracted_text_to_note"}, + {"development dump flag accepted", "env/YAML", dump, "dev", ""}, + {"unknown provider rejected", "env/YAML", badProvider, "dev", "timmy.llm_provider"}, + {"unknown provider rejected in production", "settings", badProvider, "production", "timmy.llm_provider"}, + {"empty provider accepted", "env/YAML", config.TimmyConfig{}, "production", ""}, + {"known provider accepted", "env/YAML", config.TimmyConfig{LLMProvider: "anthropic"}, "production", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateTimmyAtStartup(tt.source, tt.cfg, tt.buildMode) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + assert.Contains(t, err.Error(), tt.source) + }) + } +} + +// timmyStubSettings serves canned string values for the keys read by +// api.TimmyConfigProvider.Current; every other method panics via the nil embed. +// SEM@97bdf895453505ffee1c671271b82af781c61b72: stub settings service returning canned values by key (pure) +type timmyStubSettings struct { + api.SettingsServiceInterface + values map[string]string +} + +// SEM@97bdf895453505ffee1c671271b82af781c61b72: fetch a canned string setting from the stub (pure) +func (s timmyStubSettings) GetString(_ context.Context, key string) (string, error) { + return s.values[key], nil +} + +// SEM@97bdf895453505ffee1c671271b82af781c61b72: fetch a canned int setting from the stub (pure) +func (s timmyStubSettings) GetInt(_ context.Context, key string) (int, error) { + if v, ok := s.values[key]; ok { + return strconv.Atoi(v) + } + return 0, nil +} + +// SEM@97bdf895453505ffee1c671271b82af781c61b72: fetch a canned bool setting from the stub (pure) +func (s timmyStubSettings) GetBool(_ context.Context, key string) (bool, error) { + if v, ok := s.values[key]; ok { + return strconv.ParseBool(v) + } + return false, nil +} + +// SEM@97bdf895453505ffee1c671271b82af781c61b72: verify the DB-resolved Timmy config path enforces production dump flag and provider validity +func TestValidateEffectiveTimmyAtStartup(t *testing.T) { + tests := []struct { + name string + values map[string]string + buildMode string + wantErr string + }{ + {"production dump flag from DB", map[string]string{"timmy.dump_extracted_text_to_note": "true"}, "production", "timmy.dump_extracted_text_to_note"}, + {"development dump flag from DB", map[string]string{"timmy.dump_extracted_text_to_note": "true"}, "dev", ""}, + {"unknown provider from DB", map[string]string{"timmy.llm_provider": "bogus"}, "production", "timmy.llm_provider"}, + {"valid config from DB", map[string]string{"timmy.llm_provider": "openai", "timmy.dump_extracted_text_to_note": "false"}, "production", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateEffectiveTimmyAtStartup(context.Background(), timmyStubSettings{values: tt.values}, tt.buildMode) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 32518453..390731db 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1079,19 +1079,35 @@ func (c *Config) ResolveSecretReferences(ctx context.Context, vault SecretResolv // as of #754, that a non-empty LLMProvider names a recognized backend. // // NOTE: Timmy config is CategoryOperational (DB-seeded as of #415), so this is -// NOT called from Load()-time Validate(). It is retained as a unit-testable -// validator and may be invoked by callers that hold an effective Config. -// SEM@f7cc4344884e20bc7f6fb9a5815e2e1d530c0ff6: validate dev-only Timmy flags and LLM provider settings (pure) +// NOT called from Load()-time Validate(). Instead the server calls +// ValidateTimmyConfig at startup, on both the env/YAML config and the +// DB-resolved effective config (#1102), and the settings service applies the +// per-key checks on writes. +// SEM@57a0ccb18013f0ed5d8a8631da478fae012e3c67: validate dev-only Timmy flags and LLM provider settings (pure) func (c *Config) validateTimmy() error { - if c.Timmy.DumpExtractedTextToNote && c.Auth.BuildMode == "production" { - return fmt.Errorf("timmy.dump_extracted_text_to_note is a dev/test-only flag; set TMI_BUILD_MODE != production or disable TMI_TIMMY_DUMP_EXTRACTED_TEXT_TO_NOTE") - } - if err := c.Timmy.ValidateLLMProvider(); err != nil { + return ValidateTimmyConfig(c.Timmy, c.Auth.BuildMode) +} + +// ValidateTimmyConfig enforces the Timmy invariants on any Timmy config +// (env/YAML or DB-resolved) for the given build mode. +// SEM@57a0ccb18013f0ed5d8a8631da478fae012e3c67: validate Timmy dev-only flag, LLM provider and max tokens for a build mode (pure) +func ValidateTimmyConfig(t TimmyConfig, buildMode string) error { + if err := ValidateTimmyDumpFlag(t.DumpExtractedTextToNote, buildMode); err != nil { return err } - if err := c.Timmy.ValidateLLMMaxTokens(); err != nil { + if err := t.ValidateLLMProvider(); err != nil { return err } + return t.ValidateLLMMaxTokens() +} + +// ValidateTimmyDumpFlag rejects enabling timmy.dump_extracted_text_to_note in +// production builds. +// SEM@57a0ccb18013f0ed5d8a8631da478fae012e3c67: reject enabling the dev-only extracted-text dump flag in production builds (pure) +func ValidateTimmyDumpFlag(enabled bool, buildMode string) error { + if enabled && buildMode == "production" { + return fmt.Errorf("timmy.dump_extracted_text_to_note is a dev/test-only flag; set TMI_BUILD_MODE != production or disable TMI_TIMMY_DUMP_EXTRACTED_TEXT_TO_NOTE") + } return nil }