From 89dd021a938d86c843ba1cf0cacc6c07df824cfa Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Wed, 26 Aug 2026 13:57:14 +0200 Subject: [PATCH] session: prevent duplicate sessions and agents - CreateEmpty: atomic check-and-insert under write lock prevents TOCTOU race - AddAgent: reject duplicate agent names, return error - CreateAgentWithParams: propagate AddAgent error, close orphan on conflict - persist: handle AddAgent errors during restore (log and skip) --- cmd/olliesrv/internal/session/persist.go | 12 ++++++++++-- cmd/olliesrv/internal/session/registry.go | 12 +++++++++++- cmd/olliesrv/internal/session/session.go | 9 ++++++++- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/cmd/olliesrv/internal/session/persist.go b/cmd/olliesrv/internal/session/persist.go index 5659551..4743d85 100644 --- a/cmd/olliesrv/internal/session/persist.go +++ b/cmd/olliesrv/internal/session/persist.go @@ -239,7 +239,11 @@ func restoreMultiAgentSession(ps *PersistedSession) (*RestoredSession, error) { }) ag.SetName(pa.Name) ag.RequireMemoryWake() - sess.AddAgent(ag) + if err := sess.AddAgent(ag); err != nil { + pkgLog.Error("restore agent %s: %v", pa.ID, err) + ag.Close() + continue + } restoredAgents = append(restoredAgents, RestoredAgent{ ID: pa.ID, Remote: ps.Remote, @@ -265,7 +269,11 @@ func restoreMultiAgentSession(ps *PersistedSession) (*RestoredSession, error) { continue } ag.RequireMemoryWake() - sess.AddAgent(ag) + if err := sess.AddAgent(ag); err != nil { + pkgLog.Error("restore agent %s: %v", pa.ID, err) + ag.Close() + continue + } restoredAgents = append(restoredAgents, RestoredAgent{ ID: pa.ID, Remote: ps.Remote, diff --git a/cmd/olliesrv/internal/session/registry.go b/cmd/olliesrv/internal/session/registry.go index da369c3..ea0bf23 100644 --- a/cmd/olliesrv/internal/session/registry.go +++ b/cmd/olliesrv/internal/session/registry.go @@ -293,6 +293,7 @@ func CreateEmpty(name, remote string, yolo ...bool) (*Session, error) { } } + // Early check (non-atomic, avoids expensive setup in the common case). mu.RLock() _, exists := sessions[name] mu.RUnlock() @@ -327,7 +328,13 @@ func CreateEmpty(name, remote string, yolo ...bool) (*Session, error) { sess.StartBypassLoop(pkgBypassNotify) } + // Atomic check-and-insert to prevent races. mu.Lock() + if _, exists := sessions[name]; exists { + mu.Unlock() + cancel() + return nil, fmt.Errorf("session already exists: %s", name) + } sessions[name] = sess mu.Unlock() @@ -479,7 +486,10 @@ func CreateAgentWithParams(sessName string, p AgentParams) (*agent.Agent, error) return nil, err } - sess.AddAgent(ag) + if err := sess.AddAgent(ag); err != nil { + ag.Close() + return nil, err + } pkgLog.Info("created agent %s in session %s (profile=%s)", ag.Name(), sessName, p.Profile) return ag, nil } diff --git a/cmd/olliesrv/internal/session/session.go b/cmd/olliesrv/internal/session/session.go index 7b6abc2..4dc336c 100644 --- a/cmd/olliesrv/internal/session/session.go +++ b/cmd/olliesrv/internal/session/session.go @@ -291,11 +291,18 @@ func (s *Session) FindAgent(nameOrID string) *agent.Agent { func (s *Session) Agents() []*agent.Agent { return s.agents } // AddAgent appends an agent to the session. -func (s *Session) AddAgent(ag *agent.Agent) { +// Returns false if an agent with the same name already exists. +func (s *Session) AddAgent(ag *agent.Agent) error { + for _, existing := range s.agents { + if existing.Name() == ag.Name() { + return fmt.Errorf("agent already exists: %s", ag.Name()) + } + } s.agents = append(s.agents, ag) if s.Ctx != nil { go agent.ConsumeFeed(s.Ctx, ag) } + return nil } // RemoveAgent removes an agent by ID and closes it.