mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-18 16:13:53 +00:00
fix(gateway): add session ownership checks to chat.* WS methods (#676)
chat.history, chat.inject, chat.abort, and chat.session.status accepted any sessionKey without verifying the caller owns the session. A non-admin user could read, write, or disrupt another user's conversations by supplying their sessionKey. Apply the same requireSessionOwner() guard already used by sessions.* methods: canSeeAll() bypass for admin/owner, sess.UserID match for regular users. Extracted shared helper to access.go to reduce duplication. Also fixes: handleSessionStatus i18n compliance (was hardcoded English), and closes runId-only abort gap (non-admin must provide sessionKey).
This commit is contained in:
1 parent
3d9c71a8a2
commit
e85545dc1b
3 files changed
+65
-10
No files matched your search
@@ -17,7 +17,7 @@ func registerAllMethods(server *gateway.Server, agents *agent.Router, sessStore
|
||||
router := server.Router()
|
||||
|
||||
// Phase 1: Core methods
|
||||
chatMethods := methods.NewChatMethods(agents, sessStore, server.RateLimiter(), msgBus)
|
||||
chatMethods := methods.NewChatMethods(agents, sessStore, cfg, server.RateLimiter(), msgBus)
|
||||
chatMethods.Register(router)
|
||||
methods.NewAgentsMethods(agents, cfg, cfgPath, workspace, agentStore, contextFileInterceptor, msgBus).Register(router)
|
||||
methods.NewSessionsMethods(sessStore, msgBus, cfg).Register(router)
|
||||
|
||||
@@ -1,9 +1,15 @@
|
||||
package methods
|
||||
|
||||
import (
|
||||
"context"
|
||||
"slices"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/config"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/gateway"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/i18n"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/permissions"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
|
||||
)
|
||||
|
||||
// canSeeAll checks if user has full data visibility (admin role OR owner user).
|
||||
@@ -16,3 +22,23 @@ func canSeeAll(role permissions.Role, ownerIDs []string, userID string) bool {
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// requireSessionOwner verifies the caller owns the session identified by key.
|
||||
// Returns true if the caller is authorized (owner, admin, or matching user).
|
||||
// On failure, sends an error response and returns false.
|
||||
func requireSessionOwner(ctx context.Context, sessions store.SessionStore, cfg *config.Config, client *gateway.Client, reqID string, key string) bool {
|
||||
if canSeeAll(client.Role(), cfg.Gateway.OwnerIDs, client.UserID()) {
|
||||
return true
|
||||
}
|
||||
locale := store.LocaleFromContext(ctx)
|
||||
sess := sessions.Get(ctx, key)
|
||||
if sess == nil {
|
||||
client.SendResponse(protocol.NewErrorResponse(reqID, protocol.ErrNotFound, i18n.T(locale, i18n.MsgNotFound, "session", key)))
|
||||
return false
|
||||
}
|
||||
if sess.UserID != client.UserID() {
|
||||
client.SendResponse(protocol.NewErrorResponse(reqID, protocol.ErrUnauthorized, i18n.T(locale, i18n.MsgPermissionDenied, "session")))
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/agent"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/bus"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/config"
|
||||
httpapi "github.com/nextlevelbuilder/goclaw/internal/http"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/channels/media"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/gateway"
|
||||
@@ -23,15 +24,16 @@ import (
|
||||
|
||||
// ChatMethods handles chat.send, chat.history, chat.abort, chat.inject.
|
||||
type ChatMethods struct {
|
||||
agents *agent.Router
|
||||
sessions store.SessionStore
|
||||
rateLimiter *gateway.RateLimiter
|
||||
eventBus bus.EventPublisher
|
||||
postTurn tools.PostTurnProcessor
|
||||
agents *agent.Router
|
||||
sessions store.SessionStore
|
||||
cfg *config.Config
|
||||
rateLimiter *gateway.RateLimiter
|
||||
eventBus bus.EventPublisher
|
||||
postTurn tools.PostTurnProcessor
|
||||
}
|
||||
|
||||
func NewChatMethods(agents *agent.Router, sess store.SessionStore, rl *gateway.RateLimiter, eventBus bus.EventPublisher) *ChatMethods {
|
||||
return &ChatMethods{agents: agents, sessions: sess, rateLimiter: rl, eventBus: eventBus}
|
||||
func NewChatMethods(agents *agent.Router, sess store.SessionStore, cfg *config.Config, rl *gateway.RateLimiter, eventBus bus.EventPublisher) *ChatMethods {
|
||||
return &ChatMethods{agents: agents, sessions: sess, cfg: cfg, rateLimiter: rl, eventBus: eventBus}
|
||||
}
|
||||
|
||||
// SetPostTurnProcessor sets the post-turn processor for team task dispatch.
|
||||
@@ -50,12 +52,18 @@ func (m *ChatMethods) Register(router *gateway.MethodRouter) {
|
||||
|
||||
// handleSessionStatus returns the running state and activity for a session.
|
||||
// Used by the frontend to restore UI state after switching between sessions.
|
||||
func (m *ChatMethods) handleSessionStatus(_ context.Context, client *gateway.Client, req *protocol.RequestFrame) {
|
||||
func (m *ChatMethods) handleSessionStatus(ctx context.Context, client *gateway.Client, req *protocol.RequestFrame) {
|
||||
locale := store.LocaleFromContext(ctx)
|
||||
var params struct {
|
||||
SessionKey string `json:"sessionKey"`
|
||||
}
|
||||
if err := json.Unmarshal(req.Params, ¶ms); err != nil || params.SessionKey == "" {
|
||||
client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, "sessionKey required"))
|
||||
client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgRequired, "sessionKey")))
|
||||
return
|
||||
}
|
||||
|
||||
// Ownership check: non-admin users can only query their own sessions.
|
||||
if !requireSessionOwner(ctx, m.sessions, m.cfg, client, req.ID, params.SessionKey) {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -316,6 +324,11 @@ func (m *ChatMethods) handleHistory(ctx context.Context, client *gateway.Client,
|
||||
sessionKey = sessions.BuildWSSessionKey(params.AgentID, uuid.NewString())
|
||||
}
|
||||
|
||||
// Ownership check: non-admin users can only read their own session history.
|
||||
if params.SessionKey != "" && !requireSessionOwner(ctx, m.sessions, m.cfg, client, req.ID, sessionKey) {
|
||||
return
|
||||
}
|
||||
|
||||
history := m.sessions.GetHistory(ctx, sessionKey)
|
||||
|
||||
// Sign file URLs before delivery — sessions store clean paths.
|
||||
@@ -355,6 +368,11 @@ func (m *ChatMethods) handleInject(ctx context.Context, client *gateway.Client,
|
||||
return
|
||||
}
|
||||
|
||||
// Ownership check: non-admin users can only inject into their own sessions.
|
||||
if !requireSessionOwner(ctx, m.sessions, m.cfg, client, req.ID, params.SessionKey) {
|
||||
return
|
||||
}
|
||||
|
||||
// Truncate label
|
||||
if len(params.Label) > 100 {
|
||||
params.Label = params.Label[:100]
|
||||
@@ -405,6 +423,17 @@ func (m *ChatMethods) handleAbort(ctx context.Context, client *gateway.Client, r
|
||||
return
|
||||
}
|
||||
|
||||
// Non-admin users must provide sessionKey for ownership verification.
|
||||
if params.SessionKey == "" && params.RunID != "" && !canSeeAll(client.Role(), m.cfg.Gateway.OwnerIDs, client.UserID()) {
|
||||
client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgRequired, "sessionKey")))
|
||||
return
|
||||
}
|
||||
|
||||
// Ownership check: non-admin users can only abort their own sessions.
|
||||
if params.SessionKey != "" && !requireSessionOwner(ctx, m.sessions, m.cfg, client, req.ID, params.SessionKey) {
|
||||
return
|
||||
}
|
||||
|
||||
var abortedIDs []string
|
||||
|
||||
if params.RunID != "" {
|
||||
|
||||
Reference in new issue
Block a user