Merge branch 'next' into fix-require-guests-login

service/internal/acl/acl.go

@@ -199,61 +199,64 @@ func getHeaderKeyOrEmpty(headers http.Header, key string) string {
 
 // UserFromContext tries to find a user from a Connect RPC context
 func UserFromContext[T any](ctx context.Context, req *connect.Request[T], cfg *config.Config) *AuthenticatedUser {
-	var ret *AuthenticatedUser
+	user := userFromHeaders(req, cfg)
-
+	if user.Username == "" {
-	if req != nil {
+		user = userFromLocalSession(req, cfg, user)
-		ret = &AuthenticatedUser{}
-		// Only trust headers if explicitly configured
-		if cfg.AuthHttpHeaderUsername != "" {
-			ret.Username = getHeaderKeyOrEmpty(req.Header(), cfg.AuthHttpHeaderUsername)
-		}
-
-		if cfg.AuthHttpHeaderUserGroup != "" {
-			ret.UsergroupLine = getHeaderKeyOrEmpty(req.Header(), cfg.AuthHttpHeaderUserGroup)
-		}
-		// Optional provider header; otherwise infer below
-		prov := getHeaderKeyOrEmpty(req.Header(), "provider")
-		if prov != "" {
-			ret.Provider = prov
-		}
-
-		// If no username from headers, fall back to local session cookie
-		if ret.Username == "" {
-			// Build a minimal http.Request to parse cookies from headers
-			dummy := &http.Request{Header: req.Header()}
-			if c, err := dummy.Cookie("olivetin-sid-local"); err == nil && c != nil && c.Value != "" {
-				if sess := auth.GetUserSession("local", c.Value); sess != nil {
-					if u := cfg.FindUserByUsername(sess.Username); u != nil {
-						ret.Username = u.Username
-						ret.UsergroupLine = u.Usergroup
-						ret.Provider = "local"
-						ret.SID = c.Value
-					} else {
-						log.WithFields(log.Fields{"username": sess.Username}).Warn("UserFromContext: local session user not in config")
-					}
-				} else {
-					log.WithFields(log.Fields{"sid": c.Value, "provider": "local"}).Warn("UserFromContext: stale local session")
-				}
-			}
-		}
-
-		if ret.Username != "" {
-			buildUserAcls(cfg, ret)
-		}
 	}
-
+	if user.Username == "" {
-	if ret == nil || ret.Username == "" {
+		user = *UserGuest(cfg)
-		ret = UserGuest(cfg)
+	} else {
+		buildUserAcls(cfg, &user)
 	}
-
 	log.WithFields(log.Fields{
-		"username":      ret.Username,
+		"username":      user.Username,
-		"usergroupLine": ret.UsergroupLine,
+		"usergroupLine": user.UsergroupLine,
-		"provider":      ret.Provider,
+		"provider":      user.Provider,
-		"acls":          ret.Acls,
+		"acls":          user.Acls,
 	}).Debugf("UserFromContext")
+	return &user
+}
 
-	return ret
+func userFromHeaders[T any](req *connect.Request[T], cfg *config.Config) AuthenticatedUser {
+	var u AuthenticatedUser
+	if req == nil {
+		return u
+	}
+	if cfg.AuthHttpHeaderUsername != "" {
+		u.Username = getHeaderKeyOrEmpty(req.Header(), cfg.AuthHttpHeaderUsername)
+	}
+	if cfg.AuthHttpHeaderUserGroup != "" {
+		u.UsergroupLine = getHeaderKeyOrEmpty(req.Header(), cfg.AuthHttpHeaderUserGroup)
+	}
+	if prov := getHeaderKeyOrEmpty(req.Header(), "provider"); prov != "" {
+		u.Provider = prov
+	}
+	return u
+}
+
+func userFromLocalSession[T any](req *connect.Request[T], cfg *config.Config, u AuthenticatedUser) AuthenticatedUser {
+	if req == nil || u.Username != "" {
+		return u
+	}
+	dummy := &http.Request{Header: req.Header()}
+	c, err := dummy.Cookie("olivetin-sid-local")
+	if err != nil || c == nil || c.Value == "" {
+		return u
+	}
+	sess := auth.GetUserSession("local", c.Value)
+	if sess == nil {
+		log.WithFields(log.Fields{"sid": c.Value, "provider": "local"}).Warn("UserFromContext: stale local session")
+		return u
+	}
+	if cfgUser := cfg.FindUserByUsername(sess.Username); cfgUser != nil {
+		u.Username = cfgUser.Username
+		u.UsergroupLine = cfgUser.Usergroup
+		u.Provider = "local"
+		u.SID = c.Value
+		return u
+	}
+	log.WithFields(log.Fields{"username": sess.Username}).Warn("UserFromContext: local session user not in config")
+	return u
 }
 
 func UserGuest(cfg *config.Config) *AuthenticatedUser {

service/internal/api/api.go

@@ -452,31 +452,14 @@ func (api *oliveTinAPI) GetLogs(ctx ctx.Context, req *connect.Request[apiv1.GetL
 		return nil, err
 	}
 
-	ret := &apiv1.GetLogsResponse{}
+    ret := &apiv1.GetLogsResponse{}
-
+    logEntries, paging := api.executor.GetLogTrackingIds(req.Msg.StartOffset, api.cfg.LogHistoryPageSize)
-	logEntries, pagingResult := api.executor.GetLogTrackingIds(req.Msg.StartOffset, api.cfg.LogHistoryPageSize)
+    ret.Logs = api.pbLogsFiltered(logEntries, user)
-
+    ret.CountRemaining = paging.CountRemaining
-	for _, logEntry := range logEntries {
+    ret.PageSize = paging.PageSize
-		// Skip if binding is nil or action is nil
+    ret.TotalCount = paging.TotalCount
-		if logEntry.Binding == nil || logEntry.Binding.Action == nil {
+    ret.StartOffset = paging.StartOffset
-			continue
+    return connect.NewResponse(ret), nil
-		}
-
-		action := logEntry.Binding.Action
-
-		if acl.IsAllowedLogs(api.cfg, user, action) {
-			pbLogEntry := api.internalLogEntryToPb(logEntry, user)
-
-			ret.Logs = append(ret.Logs, pbLogEntry)
-		}
-	}
-
-	ret.CountRemaining = pagingResult.CountRemaining
-	ret.PageSize = pagingResult.PageSize
-	ret.TotalCount = pagingResult.TotalCount
-	ret.StartOffset = pagingResult.StartOffset
-
-	return connect.NewResponse(ret), nil
 }
 
 func (api *oliveTinAPI) GetActionLogs(ctx ctx.Context, req *connect.Request[apiv1.GetActionLogsRequest]) (*connect.Response[apiv1.GetActionLogsResponse], error) {
@@ -486,66 +469,72 @@ func (api *oliveTinAPI) GetActionLogs(ctx ctx.Context, req *connect.Request[apiv
 		return nil, err
 	}
 
-	ret := &apiv1.GetActionLogsResponse{}
+    ret := &apiv1.GetActionLogsResponse{}
+    filtered := api.filterLogsByACL(api.executor.GetLogsByActionId(req.Msg.ActionId), user)
+    page := paginate(int64(len(filtered)), api.cfg.LogHistoryPageSize, req.Msg.StartOffset)
+    if page.empty {
+        ret.CountRemaining = 0
+        ret.PageSize = page.size
+        ret.TotalCount = page.total
+        ret.StartOffset = page.start
+        return connect.NewResponse(ret), nil
+    }
+    for _, le := range filtered[page.start:page.end] {
+        ret.Logs = append(ret.Logs, api.internalLogEntryToPb(le, user))
+    }
+    ret.CountRemaining = page.total - page.end
+    ret.PageSize = page.size
+    ret.TotalCount = page.total
+    ret.StartOffset = page.start
+    return connect.NewResponse(ret), nil
+}
 
-	logs := api.executor.GetLogsByActionId(req.Msg.ActionId)
+func (api *oliveTinAPI) pbLogsFiltered(entries []*executor.InternalLogEntry, user *acl.AuthenticatedUser) []*apiv1.LogEntry {
+    out := make([]*apiv1.LogEntry, 0, len(entries))
+    for _, e := range entries {
+        if e == nil || e.Binding == nil || e.Binding.Action == nil {
+            continue
+        }
+        if acl.IsAllowedLogs(api.cfg, user, e.Binding.Action) {
+            out = append(out, api.internalLogEntryToPb(e, user))
+        }
+    }
+    return out
+}
 
-	// Apply ACL filtering
+func (api *oliveTinAPI) filterLogsByACL(entries []*executor.InternalLogEntry, user *acl.AuthenticatedUser) []*executor.InternalLogEntry {
-	filteredLogs := make([]*executor.InternalLogEntry, 0)
+    filtered := make([]*executor.InternalLogEntry, 0, len(entries))
-	for _, logEntry := range logs {
+    for _, e := range entries {
-		// Skip if binding is nil or action is nil
+        if e == nil || e.Binding == nil || e.Binding.Action == nil {
-		if logEntry.Binding == nil || logEntry.Binding.Action == nil {
+            continue
-			continue
+        }
-		}
+        if acl.IsAllowedLogs(api.cfg, user, e.Binding.Action) {
+            filtered = append(filtered, e)
+        }
+    }
+    return filtered
+}
 
-		action := logEntry.Binding.Action
+type pageInfo struct {
-		if acl.IsAllowedLogs(api.cfg, user, action) {
+    total int64
-			filteredLogs = append(filteredLogs, logEntry)
+    size  int64
-		}
+    start int64
-	}
+    end   int64
+    empty bool
+}
 
-	// Pagination
+func paginate(total int64, size int64, start int64) pageInfo {
-	totalCount := int64(len(filteredLogs))
+    if start < 0 {
-	pageSize := api.cfg.LogHistoryPageSize
+        start = 0
-	startOffset := req.Msg.StartOffset
+    }
-
+    if start >= total {
-	// Validate and clamp offset to prevent out-of-bounds access
+        return pageInfo{total: total, size: size, start: start, end: start, empty: true}
-	if startOffset < 0 {
+    }
-		startOffset = 0
+    end := start + size
-	}
+    if end > total {
-
+        end = total
-	// If offset is beyond available data, return empty result with correct metadata
+    }
-	if startOffset >= totalCount {
+    return pageInfo{total: total, size: size, start: start, end: end, empty: false}
-		ret.CountRemaining = 0
-		ret.PageSize = pageSize
-		ret.TotalCount = totalCount
-		ret.StartOffset = startOffset
-		return connect.NewResponse(ret), nil
-	}
-
-	startIdx := startOffset
-	endIdx := startOffset + pageSize
-	if endIdx > totalCount {
-		endIdx = totalCount
-	}
-
-	logEntries := filteredLogs[startIdx:endIdx]
-	countRemaining := totalCount - endIdx
-	if countRemaining < 0 {
-		countRemaining = 0
-	}
-
-	for _, logEntry := range logEntries {
-		pbLogEntry := api.internalLogEntryToPb(logEntry, user)
-		ret.Logs = append(ret.Logs, pbLogEntry)
-	}
-
-	ret.CountRemaining = countRemaining
-	ret.PageSize = pageSize
-	ret.TotalCount = totalCount
-	ret.StartOffset = startOffset
-
-	return connect.NewResponse(ret), nil
 }
 
 /*

service/internal/auth/sessions.go

@@ -82,35 +82,41 @@ func GetUserSession(provider string, sid string) *UserSession {
 
 // LoadUserSessions loads sessions from disk
 func LoadUserSessions(cfg *config.Config) {
-	sessionStorageMutex.Lock()
+    sessionStorageMutex.Lock()
-	defer sessionStorageMutex.Unlock()
+    defer sessionStorageMutex.Unlock()
 
-	data, err := os.ReadFile(cfg.GetDir() + "/sessions.yaml")
+    data, err := os.ReadFile(cfg.GetDir() + "/sessions.yaml")
-	if err != nil {
+    if err != nil {
-		logrus.WithError(err).Warn("Failed to read sessions.yaml file")
+        logrus.WithError(err).Warn("Failed to read sessions.yaml file")
-		// Always reset in-memory sessions on load error
+        ensureEmptySessionStorage()
-		sessionStorage = &SessionStorage{Providers: make(map[string]*SessionProvider)}
+        return
-		return
+    }
-	}
 
-	err = yaml.Unmarshal(data, &sessionStorage)
+    if err := yaml.Unmarshal(data, &sessionStorage); err != nil {
-	if err != nil {
+        logrus.WithError(err).Error("Failed to unmarshal sessions.yaml")
-		logrus.WithError(err).Error("Failed to unmarshal sessions.yaml")
+        ensureEmptySessionStorage()
-		// Always reset in-memory sessions on parse error
+        return
-		sessionStorage = &SessionStorage{Providers: make(map[string]*SessionProvider)}
+    }
-		return
-	}
 
-	// Ensure sessionStorage and Providers are properly initialized
+    ensureSessionStorageInitialized()
-	if sessionStorage == nil {
+}
-		sessionStorage = &SessionStorage{
-			Providers: make(map[string]*SessionProvider),
-		}
-	}
 
-	if sessionStorage.Providers == nil {
+func ensureEmptySessionStorage() {
-		sessionStorage.Providers = make(map[string]*SessionProvider)
+    if sessionStorage == nil {
-	}
+        sessionStorage = &SessionStorage{Providers: make(map[string]*SessionProvider)}
+    }
+    if sessionStorage.Providers == nil {
+        sessionStorage.Providers = make(map[string]*SessionProvider)
+    }
+}
+
+func ensureSessionStorageInitialized() {
+    if sessionStorage == nil {
+        sessionStorage = &SessionStorage{Providers: make(map[string]*SessionProvider)}
+    }
+    if sessionStorage.Providers == nil {
+        sessionStorage.Providers = make(map[string]*SessionProvider)
+    }
 }
 
 func saveUserSessions(cfg *config.Config) {

service/internal/config/config_reloader.go

@@ -46,70 +46,88 @@ func AppendSourceWithIncludes(cfg *Config, k *koanf.Koanf, configPath string) {
 }
 
 func AppendSource(cfg *Config, k *koanf.Koanf, configPath string) {
-	log.Infof("Appending cfg source: %s", configPath)
+    log.Infof("Appending cfg source: %s", configPath)
 
-	// Unmarshal config - koanf will handle mapstructure tags automatically
+    if !unmarshalRoot(k, cfg) {
-	err := k.Unmarshal(".", cfg)
+        return
-	if err != nil {
+    }
-		log.Errorf("Error unmarshalling config: %v", err)
-		return
-	}
 
-	// Fallback for complex nested structures that might not unmarshal correctly
+    loadCollectionsFallbacks(k, cfg)
-	// Only attempt manual unmarshaling if the automatic approach didn't populate the fields
-	if len(cfg.Actions) == 0 && k.Exists("actions") {
-		var actions []*Action
-		if err := k.Unmarshal("actions", &actions); err == nil {
-			cfg.Actions = actions
-			log.Debugf("Manually loaded %d actions", len(actions))
-		}
-	}
 
-	if len(cfg.Dashboards) == 0 && k.Exists("dashboards") {
+    applyConfigOverrides(k, cfg)
-		var dashboards []*DashboardComponent
-		if err := k.Unmarshal("dashboards", &dashboards); err == nil {
-			cfg.Dashboards = dashboards
-			log.Debugf("Manually loaded %d dashboards", len(dashboards))
-		}
-	}
 
-	if len(cfg.Entities) == 0 && k.Exists("entities") {
+    afterLoadFinalize(cfg, configPath)
-		var entities []*EntityFile
+}
-		if err := k.Unmarshal("entities", &entities); err == nil {
-			cfg.Entities = entities
-			log.Debugf("Manually loaded %d entities", len(entities))
-		}
-	}
 
-	if len(cfg.AuthLocalUsers.Users) == 0 && k.Exists("authLocalUsers") {
+func unmarshalRoot(k *koanf.Koanf, cfg *Config) bool {
-		var authLocalUsers AuthLocalUsersConfig
+    if err := k.Unmarshal(".", cfg); err != nil {
-		if err := k.Unmarshal("authLocalUsers", &authLocalUsers); err == nil {
+        log.Errorf("Error unmarshalling config: %v", err)
-			cfg.AuthLocalUsers = authLocalUsers
+        return false
-			log.Debugf("Manually loaded local auth config")
+    }
-		}
+    return true
-	}
+}
 
-	if len(cfg.AccessControlLists) == 0 && k.Exists("accessControlLists") {
+func loadCollectionsFallbacks(k *koanf.Koanf, cfg *Config) {
-		var acls []*AccessControlList
+    maybeUnmarshalActions(k, cfg)
-		if err := k.Unmarshal("accessControlLists", &acls); err == nil {
+    maybeUnmarshalDashboards(k, cfg)
-			cfg.AccessControlLists = acls
+    maybeUnmarshalEntities(k, cfg)
-			log.Debugf("Manually loaded %d access control lists", len(acls))
+    maybeUnmarshalAuthLocalUsers(k, cfg)
-		}
+}
-	}
 
-	// Map structure tags should handle these automatically, but we keep fallbacks
+func maybeUnmarshalActions(k *koanf.Koanf, cfg *Config) {
-	// for fields that might not unmarshal correctly
+    if len(cfg.Actions) != 0 || !k.Exists("actions") {
-	applyConfigOverrides(k, cfg)
+        return
+    }
+    var actions []*Action
+    if err := k.Unmarshal("actions", &actions); err == nil {
+        cfg.Actions = actions
+        log.Debugf("Manually loaded %d actions", len(actions))
+    }
+}
 
-	metricConfigReloadedCount.Inc()
+func maybeUnmarshalDashboards(k *koanf.Koanf, cfg *Config) {
-	metricConfigActionCount.Set(float64(len(cfg.Actions)))
+    if len(cfg.Dashboards) != 0 || !k.Exists("dashboards") {
+        return
+    }
+    var dashboards []*DashboardComponent
+    if err := k.Unmarshal("dashboards", &dashboards); err == nil {
+        cfg.Dashboards = dashboards
+        log.Debugf("Manually loaded %d dashboards", len(dashboards))
+    }
+}
 
-	cfg.SetDir(filepath.Dir(configPath))
+func maybeUnmarshalEntities(k *koanf.Koanf, cfg *Config) {
-	cfg.Sanitize()
+    if len(cfg.Entities) != 0 || !k.Exists("entities") {
+        return
+    }
+    var entities []*EntityFile
+    if err := k.Unmarshal("entities", &entities); err == nil {
+        cfg.Entities = entities
+        log.Debugf("Manually loaded %d entities", len(entities))
+    }
+}
 
-	for _, l := range listeners {
+func maybeUnmarshalAuthLocalUsers(k *koanf.Koanf, cfg *Config) {
-		l()
+    if len(cfg.AuthLocalUsers.Users) != 0 || !k.Exists("authLocalUsers") {
-	}
+        return
+    }
+    var authLocalUsers AuthLocalUsersConfig
+    if err := k.Unmarshal("authLocalUsers", &authLocalUsers); err == nil {
+        cfg.AuthLocalUsers = authLocalUsers
+        log.Debugf("Manually loaded local auth config")
+    }
+}
+
+func afterLoadFinalize(cfg *Config, configPath string) {
+    metricConfigReloadedCount.Inc()
+    metricConfigActionCount.Set(float64(len(cfg.Actions)))
+
+    cfg.SetDir(filepath.Dir(configPath))
+    cfg.Sanitize()
+
+    for _, l := range listeners {
+        l()
+    }
 }
 
 func applyConfigOverrides(k *koanf.Koanf, cfg *Config) {
@@ -139,181 +157,170 @@ func applyConfigOverrides(k *koanf.Koanf, cfg *Config) {
 
 // LoadIncludedConfigs loads configuration files from an include directory and merges them
 func LoadIncludedConfigs(cfg *Config, k *koanf.Koanf, baseConfigPath string) {
-	if cfg.Include == "" {
+    if cfg.Include == "" {
-		return
+        return
-	}
+    }
 
-	configDir := filepath.Dir(baseConfigPath)
+    includePath := filepath.Join(filepath.Dir(baseConfigPath), cfg.Include)
-	includePath := filepath.Join(configDir, cfg.Include)
+    log.Infof("Loading included configs from: %s", includePath)
 
-	log.Infof("Loading included configs from: %s", includePath)
+    yamlFiles, ok := listYamlFiles(includePath)
+    if !ok || len(yamlFiles) == 0 {
+        return
+    }
 
-	// Check if the include directory exists
+    sort.Strings(yamlFiles)
-	dirInfo, err := os.Stat(includePath)
+    for _, filename := range yamlFiles {
-	if err != nil {
+        loadAndMergeIncludedFile(cfg, includePath, filename)
-		log.Warnf("Include directory not found: %s", includePath)
+    }
-		return
-	}
 
-	if !dirInfo.IsDir() {
+    log.Infof("Finished loading %d included config file(s)", len(yamlFiles))
-		log.Warnf("Include path is not a directory: %s", includePath)
+    cfg.Sanitize()
-		return
+}
-	}
 
-	// Read all .yml files from the directory
+func listYamlFiles(includePath string) ([]string, bool) {
-	entries, err := os.ReadDir(includePath)
+    dirInfo, err := os.Stat(includePath)
-	if err != nil {
+    if err != nil {
-		log.Errorf("Error reading include directory: %v", err)
+        log.Warnf("Include directory not found: %s", includePath)
-		return
+        return nil, false
-	}
+    }
+    if !dirInfo.IsDir() {
+        log.Warnf("Include path is not a directory: %s", includePath)
+        return nil, false
+    }
+    entries, err := os.ReadDir(includePath)
+    if err != nil {
+        log.Errorf("Error reading include directory: %v", err)
+        return nil, false
+    }
+    var yamlFiles []string
+    for _, entry := range entries {
+        if entry.IsDir() {
+            continue
+        }
+        name := entry.Name()
+        if strings.HasSuffix(name, ".yml") || strings.HasSuffix(name, ".yaml") {
+            yamlFiles = append(yamlFiles, name)
+        }
+    }
+    if len(yamlFiles) == 0 {
+        log.Infof("No YAML files found in include directory: %s", includePath)
+    }
+    return yamlFiles, true
+}
 
-	// Filter and sort .yml files
+func loadAndMergeIncludedFile(cfg *Config, includePath, filename string) {
-	var yamlFiles []string
+    filePath := filepath.Join(includePath, filename)
-	for _, entry := range entries {
+    log.Infof("Loading included config file: %s", filePath)
-		if !entry.IsDir() && (strings.HasSuffix(entry.Name(), ".yml") || strings.HasSuffix(entry.Name(), ".yaml")) {
-			yamlFiles = append(yamlFiles, entry.Name())
-		}
-	}
 
-	if len(yamlFiles) == 0 {
+    includeK := koanf.New(".")
-		log.Infof("No YAML files found in include directory: %s", includePath)
+    if err := includeK.Load(file.Provider(filePath), yaml.Parser()); err != nil {
-		return
+        log.Errorf("Error loading included config file %s: %v", filePath, err)
-	}
+        return
+    }
 
-	// Sort files to ensure deterministic load order
+    tempCfg := &Config{}
-	sort.Strings(yamlFiles)
+    if err := includeK.Unmarshal(".", tempCfg); err != nil {
+        log.Errorf("Error unmarshalling included config file %s: %v", filePath, err)
+        return
+    }
+    // Fallbacks similar to AppendSource
+    if len(tempCfg.Actions) == 0 && includeK.Exists("actions") {
+        var actions []*Action
+        if err := includeK.Unmarshal("actions", &actions); err == nil {
+            tempCfg.Actions = actions
+            log.Debugf("Manually loaded %d actions from %s", len(actions), filename)
+        }
+    }
 
-	// Load each file and merge into config
+    mergeConfig(cfg, tempCfg)
-	for _, filename := range yamlFiles {
+    log.Infof("Successfully loaded and merged %s", filename)
-		filePath := filepath.Join(includePath, filename)
-		log.Infof("Loading included config file: %s", filePath)
-
-		includeK := koanf.New(".")
-		f := file.Provider(filePath)
-
-		if err := includeK.Load(f, yaml.Parser()); err != nil {
-			log.Errorf("Error loading included config file %s: %v", filePath, err)
-			continue
-		}
-
-		// Unmarshal into a temporary config to process properly
-		tempCfg := &Config{}
-		if err := includeK.Unmarshal(".", tempCfg); err != nil {
-			log.Errorf("Error unmarshalling included config file %s: %v", filePath, err)
-			continue
-		}
-
-		// Apply the same manual loading workarounds as in AppendSource
-		if len(tempCfg.Actions) == 0 && includeK.Exists("actions") {
-			var actions []*Action
-			if err := includeK.Unmarshal("actions", &actions); err == nil {
-				tempCfg.Actions = actions
-				log.Debugf("Manually loaded %d actions from %s", len(actions), filename)
-			}
-		}
-
-		// Merge the temp config into the main config
-		// Later files override earlier ones
-		mergeConfig(cfg, tempCfg)
-
-		log.Infof("Successfully loaded and merged %s", filename)
-	}
-
-	log.Infof("Finished loading %d included config file(s)", len(yamlFiles))
-
-	// Sanitize the merged config
-	cfg.Sanitize()
 }
 
 func mergeConfig(base *Config, overlay *Config) {
-	// Merge Actions - overlay appends to base
+    mergeSlices(base, overlay)
-	if len(overlay.Actions) > 0 {
+    overrideSimple(base, overlay)
-		base.Actions = append(base.Actions, overlay.Actions...)
+    overrideNested(base, overlay)
-	}
+    overrideStrings(base, overlay)
+}
 
-	// Merge Dashboards - overlay appends to base
+func mergeSlices(base *Config, overlay *Config) {
-	if len(overlay.Dashboards) > 0 {
+    if len(overlay.Actions) > 0 {
-		base.Dashboards = append(base.Dashboards, overlay.Dashboards...)
+        base.Actions = append(base.Actions, overlay.Actions...)
-		log.Debugf("Merged %d dashboards from include", len(overlay.Dashboards))
+    }
-	}
+    if len(overlay.Dashboards) > 0 {
+        base.Dashboards = append(base.Dashboards, overlay.Dashboards...)
+        log.Debugf("Merged %d dashboards from include", len(overlay.Dashboards))
+    }
+    if len(overlay.Entities) > 0 {
+        base.Entities = append(base.Entities, overlay.Entities...)
+        log.Debugf("Merged %d entities from include", len(overlay.Entities))
+    }
+    if len(overlay.AccessControlLists) > 0 {
+        base.AccessControlLists = append(base.AccessControlLists, overlay.AccessControlLists...)
+        log.Debugf("Merged %d access control lists from include", len(overlay.AccessControlLists))
+    }
+    if len(overlay.AuthLocalUsers.Users) > 0 {
+        base.AuthLocalUsers.Users = append(base.AuthLocalUsers.Users, overlay.AuthLocalUsers.Users...)
+        log.Debugf("Merged %d local users from include", len(overlay.AuthLocalUsers.Users))
+    }
+    if len(overlay.StyleMods) > 0 {
+        base.StyleMods = append(base.StyleMods, overlay.StyleMods...)
+    }
+    if len(overlay.AdditionalNavigationLinks) > 0 {
+        base.AdditionalNavigationLinks = append(base.AdditionalNavigationLinks, overlay.AdditionalNavigationLinks...)
+    }
+}
 
-	// Merge Entities - overlay appends to base
+func overrideSimple(base *Config, overlay *Config) {
-	if len(overlay.Entities) > 0 {
+    if overlay.LogLevel != "" {
-		base.Entities = append(base.Entities, overlay.Entities...)
+        base.LogLevel = overlay.LogLevel
-		log.Debugf("Merged %d entities from include", len(overlay.Entities))
+    }
-	}
+    if overlay.PageTitle != "" {
+        base.PageTitle = overlay.PageTitle
+    }
+    if overlay.ShowFooter != base.ShowFooter {
+        base.ShowFooter = overlay.ShowFooter
+    }
+    if overlay.ShowNavigation != base.ShowNavigation {
+        base.ShowNavigation = overlay.ShowNavigation
+    }
+    if overlay.CheckForUpdates != base.CheckForUpdates {
+        base.CheckForUpdates = overlay.CheckForUpdates
+    }
+    if overlay.UseSingleHTTPFrontend != base.UseSingleHTTPFrontend {
+        base.UseSingleHTTPFrontend = overlay.UseSingleHTTPFrontend
+    }
+    if overlay.AuthRequireGuestsToLogin != base.AuthRequireGuestsToLogin {
+        base.AuthRequireGuestsToLogin = overlay.AuthRequireGuestsToLogin
+    }
+    if overlay.AuthLocalUsers.Enabled {
+        base.AuthLocalUsers.Enabled = overlay.AuthLocalUsers.Enabled
+    }
+}
 
-	// Merge AccessControlLists - overlay appends to base
+func overrideNested(base *Config, overlay *Config) {
-	if len(overlay.AccessControlLists) > 0 {
+    if overlay.DefaultPolicy.ShowDiagnostics != base.DefaultPolicy.ShowDiagnostics {
-		base.AccessControlLists = append(base.AccessControlLists, overlay.AccessControlLists...)
+        base.DefaultPolicy.ShowDiagnostics = overlay.DefaultPolicy.ShowDiagnostics
-		log.Debugf("Merged %d access control lists from include", len(overlay.AccessControlLists))
+    }
-	}
+    if overlay.DefaultPolicy.ShowLogList != base.DefaultPolicy.ShowLogList {
+        base.DefaultPolicy.ShowLogList = overlay.DefaultPolicy.ShowLogList
+    }
+    if overlay.Prometheus.Enabled != base.Prometheus.Enabled {
+        base.Prometheus.Enabled = overlay.Prometheus.Enabled
+    }
+    if overlay.Prometheus.DefaultGoMetrics != base.Prometheus.DefaultGoMetrics {
+        base.Prometheus.DefaultGoMetrics = overlay.Prometheus.DefaultGoMetrics
+    }
+}
 
-	// Merge AuthLocalUsers.Users - overlay appends to base
+func overrideStrings(base *Config, overlay *Config) {
-	if len(overlay.AuthLocalUsers.Users) > 0 {
+    overrideString(&base.BannerMessage, overlay.BannerMessage)
-		base.AuthLocalUsers.Users = append(base.AuthLocalUsers.Users, overlay.AuthLocalUsers.Users...)
+    overrideString(&base.BannerCSS, overlay.BannerCSS)
-		log.Debugf("Merged %d local users from include", len(overlay.AuthLocalUsers.Users))
+    overrideString(&base.LogLevel, overlay.LogLevel)
-	}
+    overrideString(&base.PageTitle, overlay.PageTitle)
-
+    overrideString(&base.SectionNavigationStyle, overlay.SectionNavigationStyle)
-	// Merge slices by appending
+    overrideString(&base.DefaultPopupOnStart, overlay.DefaultPopupOnStart)
-	if len(overlay.StyleMods) > 0 {
-		base.StyleMods = append(base.StyleMods, overlay.StyleMods...)
-	}
-
-	if len(overlay.AdditionalNavigationLinks) > 0 {
-		base.AdditionalNavigationLinks = append(base.AdditionalNavigationLinks, overlay.AdditionalNavigationLinks...)
-	}
-
-	// Override simple fields (later files win)
-	if overlay.LogLevel != "" {
-		base.LogLevel = overlay.LogLevel
-	}
-	if overlay.PageTitle != "" {
-		base.PageTitle = overlay.PageTitle
-	}
-	if overlay.ShowFooter != base.ShowFooter {
-		base.ShowFooter = overlay.ShowFooter
-	}
-	if overlay.ShowNavigation != base.ShowNavigation {
-		base.ShowNavigation = overlay.ShowNavigation
-	}
-	if overlay.CheckForUpdates != base.CheckForUpdates {
-		base.CheckForUpdates = overlay.CheckForUpdates
-	}
-	if overlay.UseSingleHTTPFrontend != base.UseSingleHTTPFrontend {
-		base.UseSingleHTTPFrontend = overlay.UseSingleHTTPFrontend
-	}
-	if overlay.AuthRequireGuestsToLogin != base.AuthRequireGuestsToLogin {
-		base.AuthRequireGuestsToLogin = overlay.AuthRequireGuestsToLogin
-	}
-
-	// Override nested structs
-	if overlay.DefaultPolicy.ShowDiagnostics != base.DefaultPolicy.ShowDiagnostics {
-		base.DefaultPolicy.ShowDiagnostics = overlay.DefaultPolicy.ShowDiagnostics
-	}
-	if overlay.DefaultPolicy.ShowLogList != base.DefaultPolicy.ShowLogList {
-		base.DefaultPolicy.ShowLogList = overlay.DefaultPolicy.ShowLogList
-	}
-
-	if overlay.Prometheus.Enabled != base.Prometheus.Enabled {
-		base.Prometheus.Enabled = overlay.Prometheus.Enabled
-	}
-	if overlay.Prometheus.DefaultGoMetrics != base.Prometheus.DefaultGoMetrics {
-		base.Prometheus.DefaultGoMetrics = overlay.Prometheus.DefaultGoMetrics
-	}
-
-	// Override AuthLocalUsers.Enabled if set
-	if overlay.AuthLocalUsers.Enabled {
-		base.AuthLocalUsers.Enabled = overlay.AuthLocalUsers.Enabled
-	}
-
-	// Override string fields if non-empty
-	overrideString(&base.BannerMessage, overlay.BannerMessage)
-	overrideString(&base.BannerCSS, overlay.BannerCSS)
-	overrideString(&base.LogLevel, overlay.LogLevel)
-	overrideString(&base.PageTitle, overlay.PageTitle)
-	overrideString(&base.SectionNavigationStyle, overlay.SectionNavigationStyle)
-	overrideString(&base.DefaultPopupOnStart, overlay.DefaultPopupOnStart)
 }
 
 func overrideString(base *string, overlay string) {

service/internal/config/config_reloader_test.go

@@ -90,55 +90,63 @@ var envConfigTests = []struct {
 }
 
 func TestEnvInConfig(t *testing.T) {
-	for _, tt := range envConfigTests {
+    for _, tt := range envConfigTests {
-		cfg := DefaultConfig()
+        cfg := DefaultConfig()
-
+        setIfNotEmpty("INPUT", tt.input)
-		if tt.input != "" {
+        processed := processYamlWithEnv(tt.yaml)
-			os.Setenv("INPUT", tt.input)
+        k, err := loadKoanf(processed)
-		}
+        if err != nil {
-
+            t.Errorf("Error loading YAML: %v", err)
-		// Process the YAML content to replace environment variables
+            continue
-		processedYaml := envRegex.ReplaceAllStringFunc(tt.yaml, func(match string) string {
+        }
-			submatches := envRegex.FindStringSubmatch(match)
+        if err := k.Unmarshal(".", cfg); err != nil {
-			key := submatches[1]
+            t.Errorf("Error unmarshalling config: %v", err)
-			val, _ := os.LookupEnv(key)
+            continue
-			return val
+        }
-		})
+        manualAssigns(k, cfg)
-
+        field := tt.selector(cfg)
-		k := koanf.New(".")
+        assert.Equal(t, tt.output, field, "Unmarshaled config field doesn't match expected value: env=\"%s\"", tt.input)
-		err := k.Load(rawbytes.Provider([]byte(processedYaml)), yaml.Parser())
+        os.Unsetenv("INPUT")
-		if err != nil {
+    }
-			t.Errorf("Error loading YAML: %v", err)
+}
-			continue
+
-		}
+func setIfNotEmpty(key, val string) {
-
+    if val != "" {
-		// Try default unmarshaling
+        os.Setenv(key, val)
-		err = k.Unmarshal(".", cfg)
+    }
-		if err != nil {
+}
-			t.Errorf("Error unmarshalling config: %v", err)
+
-			continue
+func processYamlWithEnv(content string) string {
-		}
+    return envRegex.ReplaceAllStringFunc(content, func(match string) string {
-
+        submatches := envRegex.FindStringSubmatch(match)
-		// Manual field assignment for testing (since default unmarshaling has issues with field mapping)
+        key := submatches[1]
-		if k.Exists("PageTitle") {
+        val, _ := os.LookupEnv(key)
-			cfg.PageTitle = k.String("PageTitle")
+        return val
-		}
+    })
-		if k.Exists("CheckForUpdates") {
+}
-			cfg.CheckForUpdates = k.Bool("CheckForUpdates")
+
-		}
+func loadKoanf(processed string) (*koanf.Koanf, error) {
-		if k.Exists("LogHistoryPageSize") {
+    k := koanf.New(".")
-			cfg.LogHistoryPageSize = k.Int64("LogHistoryPageSize")
+    if err := k.Load(rawbytes.Provider([]byte(processed)), yaml.Parser()); err != nil {
-		}
+        return nil, err
-		if k.Exists("actions") {
+    }
-			var actions []*Action
+    return k, nil
-			if err := k.Unmarshal("actions", &actions); err == nil {
+}
-				cfg.Actions = actions
+
-			}
+func manualAssigns(k *koanf.Koanf, cfg *Config) {
-		}
+    if k.Exists("PageTitle") {
-
+        cfg.PageTitle = k.String("PageTitle")
-		field := tt.selector(cfg)
+    }
-		assert.Equal(t, tt.output, field, "Unmarshaled config field doesn't match expected value: env=\"%s\"", tt.input)
+    if k.Exists("CheckForUpdates") {
-
+        cfg.CheckForUpdates = k.Bool("CheckForUpdates")
-		os.Unsetenv("INPUT")
+    }
-	}
+    if k.Exists("LogHistoryPageSize") {
+        cfg.LogHistoryPageSize = k.Int64("LogHistoryPageSize")
+    }
+    if k.Exists("actions") {
+        var actions []*Action
+        if err := k.Unmarshal("actions", &actions); err == nil {
+            cfg.Actions = actions
+        }
+    }
 }

service/internal/entities/entities.go

@@ -30,36 +30,33 @@ func AddListener(l func()) {
 }
 
 func SetupEntityFileWatchers(cfg *config.Config) {
-	configDir := cfg.GetDir()
+    baseDir := resolveEntitiesBaseDir(cfg.GetDir())
+    for i := range cfg.Entities { // #337 - iterate by key, not by value
+        ef := cfg.Entities[i]
+        watchAndLoadEntity(baseDir, ef)
+    }
+}
 
-	// Only use var directory if not in integration test mode
+func resolveEntitiesBaseDir(configDir string) string {
-	absConfigDir, _ := filepath.Abs(configDir)
+    absConfigDir, _ := filepath.Abs(configDir)
-	if !strings.Contains(absConfigDir, "integration-tests") {
+    if strings.Contains(absConfigDir, "integration-tests") {
-		configDirVar := filepath.Join(configDir, "var") // for development purposes
+        return configDir
+    }
+    devVar := filepath.Join(configDir, "var")
+    if _, err := os.Stat(devVar); err == nil {
+        return devVar
+    }
+    return configDir
+}
 
-		if _, err := os.Stat(configDirVar); err == nil {
+func watchAndLoadEntity(baseDir string, ef *config.EntityFile) {
-			configDir = configDirVar
+    p := ef.File
-		}
+    if !filepath.IsAbs(p) {
-	}
+        p = filepath.Join(baseDir, p)
-
+        log.WithFields(log.Fields{"entityFile": p}).Debugf("Adding config dir to entity file path")
-	for entityIndex := range cfg.Entities { // #337 - iterate by key, not by value
+    }
-		ef := cfg.Entities[entityIndex]
+    go filehelper.WatchFileWrite(p, func(filename string) { loadEntityFile(p, ef.Name) })
-		p := ef.File
+    loadEntityFile(p, ef.Name)
-
-		if !filepath.IsAbs(p) {
-			p = filepath.Join(configDir, p)
-
-			log.WithFields(log.Fields{
-				"entityFile": p,
-			}).Debugf("Adding config dir to entity file path")
-		}
-
-		go filehelper.WatchFileWrite(p, func(filename string) {
-			loadEntityFile(p, ef.Name)
-		})
-
-		loadEntityFile(p, ef.Name)
-	}
 }
 
 func loadEntityFile(filename string, entityname string) {

service/internal/executor/arguments.go

@@ -43,45 +43,37 @@ func parseCommandForReplacements(shellCommand string, values map[string]string,
 }
 
 func parseActionExec(values map[string]string, action *config.Action, entity *entities.Entity) ([]string, error) {
-	if action == nil {
+    if action == nil {
-		return nil, fmt.Errorf("action is nil")
+        return nil, fmt.Errorf("action is nil")
-	}
+    }
+    if err := validateArguments(values, action); err != nil {
+        return nil, err
+    }
+    parsed := make([]string, len(action.Exec))
+    for i, a := range action.Exec {
+        arg, err := parseCommandForReplacements(a, values, entity)
+        if err != nil {
+            return nil, err
+        }
+        parsed[i] = entities.ParseTemplateWithArgs(arg, entity, values)
+    }
+    logParsedExec(action, parsed, values)
+    return parsed, nil
+}
 
-	for _, arg := range action.Arguments {
+func validateArguments(values map[string]string, action *config.Action) error {
-		argName := arg.Name
+    for _, arg := range action.Arguments {
-		argValue := values[argName]
+        if err := typecheckActionArgument(&arg, values[arg.Name], action); err != nil {
+            return err
+        }
+        log.WithFields(log.Fields{"name": arg.Name, "value": values[arg.Name]}).Debugf("Arg assigned")
+    }
+    return nil
+}
 
-		err := typecheckActionArgument(&arg, argValue, action)
+func logParsedExec(action *config.Action, parsed []string, values map[string]string) {
-
+    redacted := redactExecArgs(parsed, action.Arguments, values)
-		if err != nil {
+    log.WithFields(log.Fields{"actionTitle": action.Title, "cmd": redacted}).Infof("Action parse args - After (Exec)")
-			return nil, err
-		}
-
-		log.WithFields(log.Fields{
-			"name":  argName,
-			"value": argValue,
-		}).Debugf("Arg assigned")
-	}
-
-	parsedArgs := make([]string, len(action.Exec))
-	for i, arg := range action.Exec {
-		parsedArg, err := parseCommandForReplacements(arg, values, entity)
-		if err != nil {
-			return nil, err
-		}
-
-		parsedArg = entities.ParseTemplateWithArgs(parsedArg, entity, values)
-		parsedArgs[i] = parsedArg
-	}
-
-	redactedArgs := redactExecArgs(parsedArgs, action.Arguments, values)
-
-	log.WithFields(log.Fields{
-		"actionTitle": action.Title,
-		"cmd":         redactedArgs,
-	}).Infof("Action parse args - After (Exec)")
-
-	return parsedArgs, nil
 }
 
 func parseActionArguments(values map[string]string, action *config.Action, entity *entities.Entity) (string, error) {
@@ -295,21 +287,16 @@ func typeSafetyCheckUrl(value string) error {
 }
 
 func checkShellArgumentSafety(action *config.Action) error {
-	if action.Shell == "" {
+    if action.Shell == "" {
-		return nil
+        return nil
-	}
+    }
-
+    unsafe := map[string]struct{}{"url": {}, "email": {}, "raw_string_multiline": {}, "very_dangerous_raw_string": {}}
-	unsafeTypes := []string{"url", "email", "raw_string_multiline", "very_dangerous_raw_string"}
+    for _, arg := range action.Arguments {
-
+        if _, bad := unsafe[arg.Type]; bad {
-	for _, arg := range action.Arguments {
+            return fmt.Errorf("unsafe argument type '%s' cannot be used with Shell execution. Use 'exec' instead. See https://docs.olivetin.app/action_execution/shellvsexec.html", arg.Type)
-		for _, unsafeType := range unsafeTypes {
+        }
-			if arg.Type == unsafeType {
+    }
-				return fmt.Errorf("unsafe argument type '%s' cannot be used with Shell execution. Use 'exec' instead. See https://docs.olivetin.app/action_execution/shellvsexec.html", arg.Type)
+    return nil
-			}
-		}
-	}
-
-	return nil
 }
 
 func mangleInvalidArgumentValues(req *ExecutionRequest) {

service/internal/executor/executor.go

@@ -427,52 +427,65 @@ func stepACLCheck(req *ExecutionRequest) bool {
 }
 
 func stepParseArgs(req *ExecutionRequest) bool {
-	var err error
+	ensureArgumentMap(req)
+	injectSystemArgs(req)
 
+	if !hasBindingAndAction(req) {
+		return fail(req, fmt.Errorf("cannot parse arguments: Binding or Action is nil"))
+	}
+  
+  mangleInvalidArgumentValues(req)
+  
+	if hasExec(req) {
+		return parseExec(req)
+	}
+	if err := checkShellArgumentSafety(req.Binding.Action); err != nil {
+		return fail(req, err)
+	}
+	cmd, err := parseActionArguments(req.Arguments, req.Binding.Action, req.Binding.Entity)
+	if err != nil {
+		return fail(req, err)
+	}
+	req.useDirectExec = false
+	req.finalParsedCommand = cmd
+	return true
+}
+
+func ensureArgumentMap(req *ExecutionRequest) {
 	if req.Arguments == nil {
 		req.Arguments = make(map[string]string)
 	}
+}
 
+func injectSystemArgs(req *ExecutionRequest) {
 	req.Arguments["ot_executionTrackingId"] = req.TrackingID
 	req.Arguments["ot_username"] = req.AuthenticatedUser.Username
+}
 
-	if req.Binding == nil || req.Binding.Action == nil {
+func hasBindingAndAction(req *ExecutionRequest) bool {
-		err = fmt.Errorf("cannot parse arguments: Binding or Action is nil")
+	return !(req.Binding == nil || req.Binding.Action == nil)
-		req.logEntry.Output = err.Error()
+}
-		log.Warn(err.Error())
-		return false
-	}
 
-	// Only mangle arguments when we have a valid binding/action
+func hasExec(req *ExecutionRequest) bool {
-	mangleInvalidArgumentValues(req)
+	return len(req.Binding.Action.Exec) > 0
-
+}
-	if len(req.Binding.Action.Exec) > 0 {
-		req.useDirectExec = true
-		req.execArgs, err = parseActionExec(req.Arguments, req.Binding.Action, req.Binding.Entity)
-	} else {
-		req.useDirectExec = false
-
-		err = checkShellArgumentSafety(req.Binding.Action)
-		if err != nil {
-			req.logEntry.Output = err.Error()
-			log.Warn(err.Error())
-			return false
-		}
-
-		req.finalParsedCommand, err = parseActionArguments(req.Arguments, req.Binding.Action, req.Binding.Entity)
-	}
 
+func parseExec(req *ExecutionRequest) bool {
+	req.useDirectExec = true
+	args, err := parseActionExec(req.Arguments, req.Binding.Action, req.Binding.Entity)
 	if err != nil {
-		req.logEntry.Output = err.Error()
+		return fail(req, err)
-
-		log.Warn(err.Error())
-
-		return false
 	}
-
+	req.execArgs = args
 	return true
 }
 
+func fail(req *ExecutionRequest, err error) bool {
+	req.logEntry.Output = err.Error()
+	log.Warn(err.Error())
+	return false
+}
+
 func stepRequestAction(req *ExecutionRequest) bool {
 	metricActionsRequested.Inc()
 
@@ -587,34 +600,17 @@ func buildEnv(args map[string]string) []string {
 func stepExec(req *ExecutionRequest) bool {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(req.Binding.Action.Timeout)*time.Second)
 	defer cancel()
-
 	streamer := &OutputStreamer{Req: req}
-
+	cmd := buildCommand(ctx, req)
-	var cmd *exec.Cmd
-	if req.useDirectExec {
-		cmd = wrapCommandDirect(ctx, req.execArgs)
-	} else {
-		cmd = wrapCommandInShell(ctx, req.finalParsedCommand)
-	}
-
 	if cmd == nil {
 		req.logEntry.Output = "Cannot execute: no command arguments provided"
 		log.Warn("Cannot execute: no command arguments provided")
 		return false
 	}
-
+	prepareCommand(cmd, streamer, req)
-	cmd.Stdout = streamer
-	cmd.Stderr = streamer
-	cmd.Env = buildEnv(req.Arguments)
-
-	req.logEntry.ExecutionStarted = true
-
 	runerr := cmd.Start()
-
 	req.logEntry.Process = cmd.Process
-
 	waiterr := cmd.Wait()
-
 	req.logEntry.ExitCode = int32(cmd.ProcessState.ExitCode())
 	req.logEntry.Output = streamer.String()
 
@@ -644,6 +640,20 @@ func stepExec(req *ExecutionRequest) bool {
 	return true
 }
 
+func buildCommand(ctx context.Context, req *ExecutionRequest) *exec.Cmd {
+	if req.useDirectExec {
+		return wrapCommandDirect(ctx, req.execArgs)
+	}
+	return wrapCommandInShell(ctx, req.finalParsedCommand)
+}
+
+func prepareCommand(cmd *exec.Cmd, streamer *OutputStreamer, req *ExecutionRequest) {
+	cmd.Stdout = streamer
+	cmd.Stderr = streamer
+	cmd.Env = buildEnv(req.Arguments)
+	req.logEntry.ExecutionStarted = true
+}
+
 func stepExecAfter(req *ExecutionRequest) bool {
 	if req.Binding.Action.ShellAfterCompleted == "" {
 		return true