diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 4dd0bcd22..f20edfc93 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -650,7 +650,10 @@ That same shared `internal/api/` dependency also assumes those auth stores tear down synchronously when lifecycle-adjacent routers or hosted runtimes are reconfigured: session and CSRF workers may not rely on best-effort background signals that can wedge teardown, block temp-path cleanup, or leave first- -session and hosted handoff validation hanging behind a stale auth worker. +session and hosted handoff validation hanging behind a stale auth worker, and +each router must retain the exact session, CSRF, and recovery-token workers it +initialized so later global rebinds cannot orphan a live test or hosted-runtime +data path. That same path-ownership rule also applies to bootstrap-token recovery and adjacent hosted billing side effects that share the `internal/api/` boundary: CLI/bootstrap retrieval, webhook dedupe state, and customer-index persistence diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index d34573ce7..0bb24c513 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -160,7 +160,7 @@ Own canonical runtime payload shapes between backend and frontend. 19. Keep mobile onboarding payload reads aligned with the server-owned relay-mobile credential: `internal/api/router_routes_ai_relay.go`, `internal/api/onboarding_handlers.go`, and `internal/api/contract_test.go` must allow the dedicated `relay:mobile:access` scope to reach the governed QR, deep-link, and connection-validation payloads without reintroducing a broader `settings:read` requirement for token-authenticated pairing clients. 20. Keep hosted billing-state quickstart payload fields on the shared API contract: `internal/api/hosted_entitlement_refresh.go`, `internal/api/subscription_state_handlers.go`, and `internal/api/contract_test.go` must preserve `quickstart_credits_granted`, `quickstart_credits_used`, and `quickstart_credits_granted_at` through hosted signup, hosted lease refresh, and billing-state reads instead of letting lease rewrites silently erase seeded quickstart inventory. 21. Keep hosted AI settings bootstrap on the shared API contract: `internal/api/ai_hosted_runtime.go`, `internal/api/ai_handlers.go`, `internal/api/ai_handler.go`, and `internal/api/contract_test.go` must treat a missing `ai.enc` in hosted mode as a canonical bootstrap condition, persist one machine-owned quickstart-backed AI config when hosted entitlements grant AI capability, and preserve that configured settings payload as the same public contract that Chat, Patrol, and AI Settings consume. -22. Keep shared auth-store and universal rate-limit behavior on canonical router ownership: `internal/api/router.go`, `internal/api/rate_limit_config.go`, `internal/api/session_store.go`, `internal/api/csrf_store.go`, `internal/api/cloud_handoff_handlers.go`, and `internal/api/contract_test.go` must keep session/CSRF persistence scoped to the configured runtime data path, shut those workers down synchronously during router teardown or reinitialization, preserve hosted browser-session precedence during handoff-adjacent auth checks, and ensure hosted rate-limit presentation plus in-memory limiter counters stay owned by the current router instance instead of leaking across fresh routers or test harnesses. +22. Keep shared auth-store and universal rate-limit behavior on canonical router ownership: `internal/api/router.go`, `internal/api/rate_limit_config.go`, `internal/api/session_store.go`, `internal/api/csrf_store.go`, `internal/api/recovery_tokens.go`, `internal/api/cloud_handoff_handlers.go`, and `internal/api/contract_test.go` must keep session/CSRF/recovery-token persistence scoped to the configured runtime data path, make each router retain and synchronously shut down the exact auth-store workers it initialized during router teardown or reinitialization, preserve hosted browser-session precedence during handoff-adjacent auth checks, and ensure hosted rate-limit presentation plus in-memory limiter counters stay owned by the current router instance instead of leaking across fresh routers or test harnesses. ## Forbidden Paths diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 90d8e41fc..03db47dc9 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -126,7 +126,10 @@ teardown is synchronous when recovery-adjacent runtimes reinitialize. Session, CSRF, and recovery-token workers may not leave stale background goroutines or half-shutdown path ownership behind, because hosted handoff, recovery inspection, and adjacent temp-path tests all depend on the same canonical -runtime data-dir authority being replaceable without hangs or leaked state. +runtime data-dir authority being replaceable without hangs or leaked state, +and router teardown must close the exact session, CSRF, and recovery-token +workers that router initialized instead of assuming a later global auth-store +binding will clean them up. That shared `internal/api/` dependency now also assumes hosted tenant AI bootstrap and chat-runtime reads resolve through one effective hosted billing lease before storage- or recovery-adjacent runtime consumers inspect diff --git a/internal/api/ai_handlers_knowledge_test.go b/internal/api/ai_handlers_knowledge_test.go index 219a582e2..f06e5460e 100644 --- a/internal/api/ai_handlers_knowledge_test.go +++ b/internal/api/ai_handlers_knowledge_test.go @@ -1292,6 +1292,7 @@ func TestKnowledgeEndpoints_RequireAuth(t *testing.T) { AuthPass: "$2a$10$abcdefghijklmnopqrstuuABCDEFGHIJKLMNOPQRSTUVWXYZ012", // bcrypt hash placeholder } router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + t.Cleanup(router.shutdownBackgroundWorkers) endpoints := []struct { method string @@ -1332,6 +1333,7 @@ func TestKnowledgeEndpoints_RequireAIChatScope(t *testing.T) { record := newTokenRecord(t, rawToken, []string{config.ScopeMonitoringRead}, nil) cfg := newTestConfigWithTokens(t, record) router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + t.Cleanup(router.shutdownBackgroundWorkers) endpoints := []struct { method string diff --git a/internal/api/auth.go b/internal/api/auth.go index b75a24487..4866a6fea 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -186,16 +186,20 @@ const ( // InitSessionStore initializes the persistent session store func InitSessionStore(dataPath string) { + _ = ensureSessionStore(dataPath) +} + +func ensureSessionStore(dataPath string) *SessionStore { newDataPath := strings.TrimSpace(dataPath) if newDataPath == "" { - return + return nil } sessionStoreMu.Lock() defer sessionStoreMu.Unlock() if sessionStore != nil && sessionStoreDataPath == newDataPath { - return + return sessionStore } oldStore := sessionStore @@ -204,6 +208,7 @@ func InitSessionStore(dataPath string) { if oldStore != nil { oldStore.Shutdown() } + return sessionStore } func InitPersistentAuthStores(dataPath string) { diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index f0beb4cce..e33db0679 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -1478,6 +1478,75 @@ func TestContract_UniversalRateLimitStateIsScopedPerRouterConfig(t *testing.T) { } } +func TestContract_RouterShutdownClosesOwnedPersistentAuthStoresAfterGlobalRebind(t *testing.T) { + resetPersistentAuthStoresForTests() + t.Cleanup(resetPersistentAuthStoresForTests) + + routerOne := NewRouter(&config.Config{DataPath: t.TempDir()}, nil, nil, nil, nil, "1.0.0") + routerTwo := NewRouter(&config.Config{DataPath: t.TempDir()}, nil, nil, nil, nil, "1.0.0") + t.Cleanup(routerTwo.shutdownBackgroundWorkers) + + if routerOne.sessionStore == nil || routerOne.csrfStore == nil { + t.Fatal("routerOne should capture initialized persistent auth stores") + } + if routerOne.recoveryTokenStore == nil { + t.Fatal("routerOne should capture initialized recovery token store") + } + if routerTwo.sessionStore == nil || routerTwo.csrfStore == nil { + t.Fatal("routerTwo should capture initialized persistent auth stores") + } + if routerTwo.recoveryTokenStore == nil { + t.Fatal("routerTwo should capture initialized recovery token store") + } + if routerOne.sessionStore == routerTwo.sessionStore { + t.Fatal("router instances should not share the same session store after rebind") + } + if routerOne.csrfStore == routerTwo.csrfStore { + t.Fatal("router instances should not share the same csrf store after rebind") + } + if routerOne.recoveryTokenStore == routerTwo.recoveryTokenStore { + t.Fatal("router instances should not share the same recovery token store after rebind") + } + + routerOne.shutdownBackgroundWorkers() + + select { + case <-routerOne.sessionStore.workerDone: + default: + t.Fatal("routerOne session store worker should be closed after router shutdown") + } + + select { + case <-routerOne.csrfStore.workerDone: + default: + t.Fatal("routerOne csrf store worker should be closed after router shutdown") + } + + select { + case <-routerTwo.sessionStore.workerDone: + t.Fatal("routerTwo session store should remain active when routerOne shuts down") + default: + } + + select { + case <-routerTwo.csrfStore.workerDone: + t.Fatal("routerTwo csrf store should remain active when routerOne shuts down") + default: + } + + select { + case <-routerOne.recoveryTokenStore.stopCleanup: + default: + t.Fatal("routerOne recovery token store should be closed after router shutdown") + } + + select { + case <-routerTwo.recoveryTokenStore.stopCleanup: + t.Fatal("routerTwo recovery token store should remain active when routerOne shuts down") + default: + } +} + func TestContract_HostedOrgManagerSessionCanMintRelayMobileToken(t *testing.T) { defer SetMultiTenantEnabled(false) SetMultiTenantEnabled(true) diff --git a/internal/api/csrf_store.go b/internal/api/csrf_store.go index af3c18c07..6faa8042d 100644 --- a/internal/api/csrf_store.go +++ b/internal/api/csrf_store.go @@ -99,16 +99,20 @@ var ( // InitCSRFStore initializes the persistent CSRF token store func InitCSRFStore(dataPath string) { + _ = ensureCSRFStore(dataPath) +} + +func ensureCSRFStore(dataPath string) *CSRFTokenStore { newDataPath := strings.TrimSpace(dataPath) if newDataPath == "" { - return + return nil } csrfStoreMu.Lock() defer csrfStoreMu.Unlock() if csrfStore != nil && csrfStoreDataPath == newDataPath { - return + return csrfStore } oldStore := csrfStore @@ -130,6 +134,7 @@ func InitCSRFStore(dataPath string) { if oldStore != nil { oldStore.Shutdown() } + return csrfStore } // GetCSRFStore returns the global CSRF token store diff --git a/internal/api/csrf_store_test.go b/internal/api/csrf_store_test.go index f03ff97bf..3cfe75faf 100644 --- a/internal/api/csrf_store_test.go +++ b/internal/api/csrf_store_test.go @@ -674,6 +674,39 @@ func TestCSRFTokenStore_InitReconfiguresDataPath(t *testing.T) { } } +func TestEnsureCSRFStoreReturnsOwnedStoreForPath(t *testing.T) { + resetCSRFStoreForTests() + t.Cleanup(resetCSRFStoreForTests) + + dirOne := t.TempDir() + dirTwo := t.TempDir() + + storeOne := ensureCSRFStore(dirOne) + if storeOne == nil { + t.Fatal("ensureCSRFStore should initialize a store for a valid data path") + } + if storeOne != GetCSRFStore() { + t.Fatal("ensureCSRFStore should return the active store for the configured data path") + } + + storeTwo := ensureCSRFStore(dirTwo) + if storeTwo == nil { + t.Fatal("ensureCSRFStore should initialize a reconfigured store") + } + if storeTwo != GetCSRFStore() { + t.Fatal("ensureCSRFStore should return the reconfigured active store") + } + if storeOne == storeTwo { + t.Fatal("ensureCSRFStore should return a distinct store after data-path reconfiguration") + } + + select { + case <-storeOne.workerDone: + default: + t.Fatal("reconfigured csrf store should shut down the previous worker") + } +} + func TestCSRFTokenStore_ShutdownReturnsWithoutWaitingForWorkerReceive(t *testing.T) { storeDir := t.TempDir() InitCSRFStore(storeDir) diff --git a/internal/api/recovery_tokens.go b/internal/api/recovery_tokens.go index a0c73336a..4b9bd060b 100644 --- a/internal/api/recovery_tokens.go +++ b/internal/api/recovery_tokens.go @@ -55,16 +55,20 @@ func recoveryTokenHash(token string) string { // InitRecoveryTokenStore initializes the recovery token store func InitRecoveryTokenStore(dataPath string) { + _ = ensureRecoveryTokenStore(dataPath) +} + +func ensureRecoveryTokenStore(dataPath string) *RecoveryTokenStore { newDataPath := strings.TrimSpace(dataPath) if newDataPath == "" { - return + return nil } recoveryStoreMu.Lock() defer recoveryStoreMu.Unlock() if recoveryStore != nil && recoveryStoreDataPath == newDataPath { - return + return recoveryStore } oldStore := recoveryStore @@ -82,6 +86,7 @@ func InitRecoveryTokenStore(dataPath string) { if oldStore != nil { oldStore.Shutdown() } + return recoveryStore } // GetRecoveryTokenStore returns the global recovery token store diff --git a/internal/api/router.go b/internal/api/router.go index 3c4bc24d9..55d60efa7 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -110,6 +110,9 @@ type Router struct { oidcManager *OIDCServiceManager samlManager *SAMLServiceManager ssoConfig *config.SSOConfig + sessionStore *SessionStore + csrfStore *CSRFTokenStore + recoveryTokenStore *RecoveryTokenStore authorizer auth.Authorizer wrapped http.Handler serverVersion string @@ -173,9 +176,9 @@ func NewRouter(cfg *config.Config, monitor *monitoring.Monitor, mtMonitor *monit store = conversionStores[0] } - // Initialize persistent session and CSRF stores - InitSessionStore(cfg.DataPath) - InitCSRFStore(cfg.DataPath) + // Initialize persistent auth stores and capture the exact workers this router owns. + sessionStore := ensureSessionStore(cfg.DataPath) + csrfStore := ensureCSRFStore(cfg.DataPath) updateHistory, err := updates.NewUpdateHistory(cfg.DataPath) if err != nil { @@ -206,6 +209,8 @@ func NewRouter(cfg *config.Config, monitor *monitoring.Monitor, mtMonitor *monit handoffExchangeRateLimiter: NewRateLimiter(20, 1*time.Minute), // cloud handoff token exchange per minute per IP persistence: config.NewConfigPersistence(cfg.DataPath), multiTenant: config.NewMultiTenantPersistence(cfg.DataPath), + sessionStore: sessionStore, + csrfStore: csrfStore, authorizer: auth.GetAuthorizer(), serverVersion: strings.TrimSpace(serverVersion), projectRoot: projectRoot, @@ -577,8 +582,8 @@ func (r *Router) setupRoutes() { } } - // Initialize recovery token store - InitRecoveryTokenStore(r.config.DataPath) + // Initialize recovery token store and capture the exact worker this router owns. + r.recoveryTokenStore = ensureRecoveryTokenStore(r.config.DataPath) r.registerPublicAndAuthRoutes() r.registerMonitoringRoutes(guestMetadataHandler, dockerMetadataHandler, hostMetadataHandler, infraUpdateHandlers) @@ -2059,6 +2064,15 @@ func (r *Router) shutdownBackgroundWorkers() { if r.lifecycleCancel != nil { r.lifecycleCancel() } + if r.sessionStore != nil { + r.sessionStore.Shutdown() + } + if r.csrfStore != nil { + r.csrfStore.Shutdown() + } + if r.recoveryTokenStore != nil { + r.recoveryTokenStore.Shutdown() + } if r.trueNASPoller != nil { r.trueNASPoller.Stop() } diff --git a/internal/api/router_routes_ai_execute_stream_test.go b/internal/api/router_routes_ai_execute_stream_test.go index c008f4f2e..6fcbc639c 100644 --- a/internal/api/router_routes_ai_execute_stream_test.go +++ b/internal/api/router_routes_ai_execute_stream_test.go @@ -7,7 +7,6 @@ import ( "strings" "testing" - "github.com/rcourtman/pulse-go-rewrite/internal/ai" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -84,27 +83,7 @@ func TestRouteExecuteStream_NoAuth(t *testing.T) { func TestRouteExecuteStream_WrongScope(t *testing.T) { t.Parallel() - rawToken := "ai-stream-wrong-scope-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsRead}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = "http://192.0.2.1:11434" - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc + router, rawToken := setupExecuteRouterForScopes(t, "http://192.0.2.1:11434", []string{config.ScopeSettingsRead}, true) body := `{"prompt":"hi"}` req := httptest.NewRequest(http.MethodPost, "/api/ai/execute/stream", strings.NewReader(body)) @@ -183,25 +162,7 @@ func TestRouteExecuteStream_InvalidJSON(t *testing.T) { func TestRouteExecuteStream_AIDisabled(t *testing.T) { t.Parallel() - rawToken := "ai-stream-disabled-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeAIExecute}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = false - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc + router, rawToken := setupExecuteRouterForScopes(t, "http://192.0.2.1:11434", []string{config.ScopeAIExecute}, false) body := `{"prompt":"hi"}` req := httptest.NewRequest(http.MethodPost, "/api/ai/execute/stream", strings.NewReader(body)) diff --git a/internal/api/router_routes_ai_execute_test.go b/internal/api/router_routes_ai_execute_test.go index cd5dc97c8..0d1ec1a92 100644 --- a/internal/api/router_routes_ai_execute_test.go +++ b/internal/api/router_routes_ai_execute_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/rcourtman/pulse-go-rewrite/internal/ai" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -17,34 +16,13 @@ import ( // setupExecuteRouter creates a Router with an Ollama mock server and a valid // ai:execute API token for route-level /api/ai/execute tests. func setupExecuteRouter(t *testing.T, ollamaURL string) (*Router, string) { - t.Helper() - resetPersistentAuthStoresForTests() - t.Cleanup(resetPersistentAuthStoresForTests) + return setupExecuteRouterForScopes(t, ollamaURL, []string{config.ScopeAIExecute}, true) +} - rawToken := "ai-execute-route-token-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeAIExecute}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = ollamaURL - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - t.Cleanup(router.shutdownBackgroundWorkers) - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc - - return router, rawToken +func setupExecuteRouterForScopes(t *testing.T, ollamaURL string, scopes []string, aiEnabled bool) (*Router, string) { + opts := newAIRouteTestOptions(scopes, ollamaURL) + opts.aiEnabled = aiEnabled + return setupAIRouteRouter(t, opts) } // mockOllamaForExecute returns an HTTP handler that mocks the Ollama API @@ -143,27 +121,7 @@ func TestRouteExecute_NoAuth(t *testing.T) { func TestRouteExecute_WrongScope(t *testing.T) { t.Parallel() - rawToken := "ai-execute-wrong-scope-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsRead}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = "http://192.0.2.1:11434" - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc + router, rawToken := setupExecuteRouterForScopes(t, "http://192.0.2.1:11434", []string{config.ScopeSettingsRead}, true) body := `{"prompt":"hi"}` req := httptest.NewRequest(http.MethodPost, "/api/ai/execute", strings.NewReader(body)) @@ -234,26 +192,7 @@ func TestRouteExecute_InvalidJSON(t *testing.T) { func TestRouteExecute_AIDisabled(t *testing.T) { t.Parallel() - rawToken := "ai-execute-disabled-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeAIExecute}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - // Save default AI config with Enabled = false (default) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = false - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc + router, rawToken := setupExecuteRouterForScopes(t, "http://192.0.2.1:11434", []string{config.ScopeAIExecute}, false) body := `{"prompt":"hi"}` req := httptest.NewRequest(http.MethodPost, "/api/ai/execute", strings.NewReader(body)) diff --git a/internal/api/router_routes_ai_helpers_test.go b/internal/api/router_routes_ai_helpers_test.go new file mode 100644 index 000000000..b8ae15d5a --- /dev/null +++ b/internal/api/router_routes_ai_helpers_test.go @@ -0,0 +1,70 @@ +package api + +import ( + "sync" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/ai" + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +var aiRouteAuthStoreTestMu sync.Mutex + +func lockAIRouteAuthStoreTests(t *testing.T) { + t.Helper() + aiRouteAuthStoreTestMu.Lock() + t.Cleanup(aiRouteAuthStoreTestMu.Unlock) +} + +type aiRouteTestOptions struct { + scopes []string + aiEnabled bool + configureAI bool + model string + ollamaURL string +} + +func newAIRouteTestOptions(scopes []string, ollamaURL string) aiRouteTestOptions { + return aiRouteTestOptions{ + scopes: scopes, + aiEnabled: true, + configureAI: true, + model: "ollama:llama3", + ollamaURL: ollamaURL, + } +} + +func setupAIRouteRouter(t *testing.T, opts aiRouteTestOptions) (*Router, string) { + t.Helper() + + lockAIRouteAuthStoreTests(t) + resetPersistentAuthStoresForTests() + t.Cleanup(resetPersistentAuthStoresForTests) + + rawToken := "ai-route-token-" + t.Name() + ".12345678" + record := newTokenRecord(t, rawToken, opts.scopes, nil) + cfg := newTestConfigWithTokens(t, record) + + persistence := config.NewConfigPersistence(cfg.DataPath) + if opts.configureAI { + aiCfg := config.NewDefaultAIConfig() + aiCfg.Enabled = opts.aiEnabled + aiCfg.Model = opts.model + aiCfg.OllamaBaseURL = opts.ollamaURL + if err := persistence.SaveAIConfig(*aiCfg); err != nil { + t.Fatalf("SaveAIConfig: %v", err) + } + } + + router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + t.Cleanup(router.shutdownBackgroundWorkers) + router.aiSettingsHandler.defaultConfig = cfg + router.aiSettingsHandler.defaultPersistence = persistence + svc := ai.NewService(persistence, nil) + if err := svc.LoadConfig(); err != nil { + t.Fatalf("LoadConfig: %v", err) + } + router.aiSettingsHandler.defaultAIService = svc + + return router, rawToken +} diff --git a/internal/api/router_routes_ai_models_test.go b/internal/api/router_routes_ai_models_test.go index 163c775b3..7c6995add 100644 --- a/internal/api/router_routes_ai_models_test.go +++ b/internal/api/router_routes_ai_models_test.go @@ -6,7 +6,6 @@ import ( "net/http/httptest" "testing" - "github.com/rcourtman/pulse-go-rewrite/internal/ai" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -15,31 +14,7 @@ import ( // setupModelsRouter creates a Router with an Ollama mock server and a valid // ai:chat API token for route-level /api/ai/models tests. func setupModelsRouter(t *testing.T, ollamaURL string) (*Router, string) { - t.Helper() - - rawToken := "ai-models-route-token-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeAIChat}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = ollamaURL - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc - - return router, rawToken + return setupAIRouteRouter(t, newAIRouteTestOptions([]string{config.ScopeAIChat}, ollamaURL)) } // TestRouteListModels_Success verifies that GET /api/ai/models dispatches @@ -125,27 +100,7 @@ func TestRouteListModels_NoAuth(t *testing.T) { func TestRouteListModels_WrongScope(t *testing.T) { t.Parallel() - rawToken := "ai-models-wrong-scope-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsRead}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = "http://192.0.2.1:11434" - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc + router, rawToken := setupAIRouteRouter(t, newAIRouteTestOptions([]string{config.ScopeSettingsRead}, "http://192.0.2.1:11434")) req := httptest.NewRequest(http.MethodGet, "/api/ai/models", nil) req.Header.Set("X-API-Token", rawToken) diff --git a/internal/api/router_routes_ai_test_connection_test.go b/internal/api/router_routes_ai_test_connection_test.go index df00218bf..ab4aba28e 100644 --- a/internal/api/router_routes_ai_test_connection_test.go +++ b/internal/api/router_routes_ai_test_connection_test.go @@ -6,7 +6,6 @@ import ( "net/http/httptest" "testing" - "github.com/rcourtman/pulse-go-rewrite/internal/ai" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -15,31 +14,7 @@ import ( // setupTestConnectionRouter creates a Router with an Ollama mock server and a // valid settings:write API token for route-level /api/ai/test tests. func setupTestConnectionRouter(t *testing.T, ollamaURL string) (*Router, string) { - t.Helper() - - rawToken := "ai-test-conn-route-token-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsWrite}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = ollamaURL - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc - - return router, rawToken + return setupAIRouteRouter(t, newAIRouteTestOptions([]string{config.ScopeSettingsWrite}, ollamaURL)) } // TestRouteTestConnection_Success verifies that POST /api/ai/test dispatches @@ -145,27 +120,7 @@ func TestRouteTestConnection_NoAuth(t *testing.T) { func TestRouteTestConnection_WrongScope(t *testing.T) { t.Parallel() - rawToken := "ai-test-conn-wrong-scope-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsRead}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = "http://192.0.2.1:11434" - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc + router, rawToken := setupAIRouteRouter(t, newAIRouteTestOptions([]string{config.ScopeSettingsRead}, "http://192.0.2.1:11434")) req := httptest.NewRequest(http.MethodPost, "/api/ai/test", nil) req.Header.Set("X-API-Token", rawToken) @@ -181,21 +136,9 @@ func TestRouteTestConnection_WrongScope(t *testing.T) { func TestRouteTestConnection_NoConfig(t *testing.T) { t.Parallel() - rawToken := "ai-test-conn-no-config-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsWrite}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - // Don't save any AI config — service will have no configured provider - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc + opts := newAIRouteTestOptions([]string{config.ScopeSettingsWrite}, "") + opts.configureAI = false + router, rawToken := setupAIRouteRouter(t, opts) req := httptest.NewRequest(http.MethodPost, "/api/ai/test", nil) req.Header.Set("X-API-Token", rawToken) diff --git a/internal/api/router_routes_ai_test_provider_test.go b/internal/api/router_routes_ai_test_provider_test.go index fd4186442..b61a0173d 100644 --- a/internal/api/router_routes_ai_test_provider_test.go +++ b/internal/api/router_routes_ai_test_provider_test.go @@ -7,7 +7,6 @@ import ( "strings" "testing" - "github.com/rcourtman/pulse-go-rewrite/internal/ai" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -16,34 +15,11 @@ import ( // setupTestProviderRouter creates a Router with an Ollama mock server and a valid // settings:write API token for route-level /api/ai/test/{provider} tests. func setupTestProviderRouter(t *testing.T, ollamaURL string) (*Router, string) { - t.Helper() - resetPersistentAuthStoresForTests() - t.Cleanup(resetPersistentAuthStoresForTests) + return setupTestProviderRouterForScopes(t, ollamaURL, []string{config.ScopeSettingsWrite}) +} - rawToken := "ai-test-provider-route-token-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsWrite}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = ollamaURL - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - t.Cleanup(router.shutdownBackgroundWorkers) - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - svc := ai.NewService(persistence, nil) - if err := svc.LoadConfig(); err != nil { - t.Fatalf("LoadConfig: %v", err) - } - router.aiSettingsHandler.defaultAIService = svc - - return router, rawToken +func setupTestProviderRouterForScopes(t *testing.T, ollamaURL string, scopes []string) (*Router, string) { + return setupAIRouteRouter(t, newAIRouteTestOptions(scopes, ollamaURL)) } // TestRouteTestProvider_Success verifies that POST /api/ai/test/{provider} @@ -250,11 +226,7 @@ func TestRouteTestProvider_UnknownProvider(t *testing.T) { func TestRouteTestProvider_WrongScope(t *testing.T) { t.Parallel() - rawToken := "ai-test-provider-wrong-scope-" + t.Name() + ".12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeAIExecute}, nil) - cfg := newTestConfigWithTokens(t, record) - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + router, rawToken := setupTestProviderRouterForScopes(t, "http://192.0.2.1:11434", []string{config.ScopeAIExecute}) req := httptest.NewRequest(http.MethodPost, "/api/ai/test/ollama", nil) req.Header.Set("X-API-Token", rawToken) diff --git a/internal/api/security_regression_test.go b/internal/api/security_regression_test.go index 8b3141603..66f7ed0a2 100644 --- a/internal/api/security_regression_test.go +++ b/internal/api/security_regression_test.go @@ -2695,23 +2695,7 @@ func TestAITestConnectionRouteWithValidScope(t *testing.T) { })) defer ollama.Close() - rawToken := "ai-test-conn-valid-token-123.12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsWrite}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = ollama.URL - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - router.aiSettingsHandler.defaultAIService = newLegacyAIServiceForTest(persistence) + router, rawToken := setupTestConnectionRouter(t, ollama.URL) req := httptest.NewRequest(http.MethodPost, "/api/ai/test", nil) req.Header.Set("X-API-Token", rawToken) @@ -2747,23 +2731,7 @@ func TestAITestProviderRouteWithValidScope(t *testing.T) { })) defer ollama.Close() - rawToken := "ai-test-prov-valid-token-123.12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsWrite}, nil) - cfg := newTestConfigWithTokens(t, record) - - persistence := config.NewConfigPersistence(cfg.DataPath) - aiCfg := config.NewDefaultAIConfig() - aiCfg.Enabled = true - aiCfg.Model = "ollama:llama3" - aiCfg.OllamaBaseURL = ollama.URL - if err := persistence.SaveAIConfig(*aiCfg); err != nil { - t.Fatalf("SaveAIConfig: %v", err) - } - - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") - router.aiSettingsHandler.defaultConfig = cfg - router.aiSettingsHandler.defaultPersistence = persistence - router.aiSettingsHandler.defaultAIService = newLegacyAIServiceForTest(persistence) + router, rawToken := setupTestProviderRouter(t, ollama.URL) req := httptest.NewRequest(http.MethodPost, "/api/ai/test/ollama", nil) req.Header.Set("X-API-Token", rawToken) @@ -2789,11 +2757,7 @@ func TestAITestProviderRouteWithValidScope(t *testing.T) { func TestAITestConnectionRouteRejectsWrongScope(t *testing.T) { t.Parallel() - // Token with monitoring:read scope — NOT settings:write - rawToken := "ai-test-wrong-scope-token-123.12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeMonitoringRead}, nil) - cfg := newTestConfigWithTokens(t, record) - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + router, rawToken := setupAIRouteRouter(t, newAIRouteTestOptions([]string{config.ScopeMonitoringRead}, "http://192.0.2.1:11434")) req := httptest.NewRequest(http.MethodPost, "/api/ai/test", strings.NewReader(`{}`)) req.Header.Set("X-API-Token", rawToken) @@ -2811,11 +2775,7 @@ func TestAITestConnectionRouteRejectsWrongScope(t *testing.T) { func TestAITestProviderRouteRejectsWrongScope(t *testing.T) { t.Parallel() - // Token with monitoring:read scope — NOT settings:write - rawToken := "ai-test-prov-wrong-scope-token-123.12345678" - record := newTokenRecord(t, rawToken, []string{config.ScopeMonitoringRead}, nil) - cfg := newTestConfigWithTokens(t, record) - router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + router, rawToken := setupTestProviderRouterForScopes(t, "http://192.0.2.1:11434", []string{config.ScopeMonitoringRead}) req := httptest.NewRequest(http.MethodPost, "/api/ai/test/openai", strings.NewReader(`{}`)) req.Header.Set("X-API-Token", rawToken) diff --git a/internal/api/session_store_test.go b/internal/api/session_store_test.go index 9128e835e..e13c05ba8 100644 --- a/internal/api/session_store_test.go +++ b/internal/api/session_store_test.go @@ -768,6 +768,39 @@ func TestSessionStore_InitReconfiguresDataPath(t *testing.T) { } } +func TestEnsureSessionStoreReturnsOwnedStoreForPath(t *testing.T) { + resetSessionStoreForTests() + t.Cleanup(resetSessionStoreForTests) + + dirOne := t.TempDir() + dirTwo := t.TempDir() + + storeOne := ensureSessionStore(dirOne) + if storeOne == nil { + t.Fatal("ensureSessionStore should initialize a store for a valid data path") + } + if storeOne != GetSessionStore() { + t.Fatal("ensureSessionStore should return the active store for the configured data path") + } + + storeTwo := ensureSessionStore(dirTwo) + if storeTwo == nil { + t.Fatal("ensureSessionStore should initialize a reconfigured store") + } + if storeTwo != GetSessionStore() { + t.Fatal("ensureSessionStore should return the reconfigured active store") + } + if storeOne == storeTwo { + t.Fatal("ensureSessionStore should return a distinct store after data-path reconfiguration") + } + + select { + case <-storeOne.workerDone: + default: + t.Fatal("reconfigured session store should shut down the previous worker") + } +} + func TestSessionStore_SaveUnsafe_DropsRefreshTokenWithoutCrypto(t *testing.T) { tmpDir := t.TempDir() store := &SessionStore{