Guard legacyMonitor typed-nil and add OIDC refresh panic recovery

Normalize SystemSettingsMonitor interface assignments via reflect to
prevent typed-nil-in-interface (same class as #1324 fix). Also add
defer/recover to the background OIDC token refresh goroutine so a
panic there cannot take down the process.
This commit is contained in:
rcourtman 2026-03-07 10:09:15 +00:00
parent 23a9fa70da
commit ddecf6d00c
3 changed files with 37 additions and 6 deletions

View file

@ -9,6 +9,7 @@ import (
"fmt"
"net/http"
"os"
"runtime/debug"
"strings"
"sync"
"time"
@ -1040,6 +1041,15 @@ var oidcRefreshMutex sync.Map
// refreshOIDCSessionTokens refreshes OIDC tokens for a session in the background
// If refresh fails, the session is invalidated and the user will need to re-login
func refreshOIDCSessionTokens(cfg *config.Config, sessionToken string, session *SessionData) {
defer func() {
if r := recover(); r != nil {
log.Error().
Interface("panic", r).
Bytes("stack", debug.Stack()).
Msg("Recovered panic in OIDC token refresh")
}
}()
// Prevent concurrent refresh attempts for the same session
if _, loaded := oidcRefreshMutex.LoadOrStore(sessionToken, true); loaded {
return // Another goroutine is already refreshing this session

View file

@ -2136,7 +2136,11 @@ func (r *Router) SetMonitor(m *monitoring.Monitor) {
r.hostAgentHandlers.SetMonitor(m)
}
if r.systemSettingsHandler != nil {
r.systemSettingsHandler.SetMonitor(m)
if m == nil {
r.systemSettingsHandler.SetMonitor(nil)
} else {
r.systemSettingsHandler.SetMonitor(m)
}
}
if m != nil {
if url := strings.TrimSpace(r.config.PublicURL); url != "" {

View file

@ -11,6 +11,7 @@ import (
"net/http"
"os"
"path/filepath"
"reflect"
"strings"
"time"
@ -44,11 +45,29 @@ type SystemSettingsHandler struct {
legacyMonitor SystemSettingsMonitor
}
func normalizeSystemSettingsMonitor(m SystemSettingsMonitor) SystemSettingsMonitor {
if m == nil {
return nil
}
v := reflect.ValueOf(m)
switch v.Kind() {
case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Pointer, reflect.Slice:
if v.IsNil() {
return nil
}
}
return m
}
// NewSystemSettingsHandler creates a new system settings handler
func NewSystemSettingsHandler(cfg *config.Config, persistence *config.ConfigPersistence, wsHub *websocket.Hub, mtm *monitoring.MultiTenantMonitor, monitor SystemSettingsMonitor, reloadSystemSettingsFunc func(), reloadMonitorFunc func() error) *SystemSettingsHandler {
monitor = normalizeSystemSettingsMonitor(monitor)
// If mtm is provided, try to populate legacyMonitor from "default" org if not provided
if monitor == nil && mtm != nil {
if m, err := mtm.GetMonitor("default"); err == nil {
if m, err := mtm.GetMonitor("default"); err == nil && m != nil {
monitor = m
}
}
@ -64,17 +83,15 @@ func NewSystemSettingsHandler(cfg *config.Config, persistence *config.ConfigPers
}
// SetMonitor updates the monitor reference used by the handler at runtime.
// Callers must not pass a typed nil (e.g. (*Monitor)(nil)) — use an untyped nil instead,
// or guard at the call site, since Go interfaces hide typed nils.
func (h *SystemSettingsHandler) SetMonitor(m SystemSettingsMonitor) {
h.legacyMonitor = m
h.legacyMonitor = normalizeSystemSettingsMonitor(m)
}
// SetMultiTenantMonitor updates the multi-tenant monitor reference
func (h *SystemSettingsHandler) SetMultiTenantMonitor(mtm *monitoring.MultiTenantMonitor) {
h.mtMonitor = mtm
if mtm != nil {
if m, err := mtm.GetMonitor("default"); err == nil {
if m, err := mtm.GetMonitor("default"); err == nil && m != nil {
h.legacyMonitor = m
}
}