From a4df5a08e30a61fad004b136ba78b5e43c5a754f Mon Sep 17 00:00:00 2001 From: viettranx Date: Mon, 13 Apr 2026 11:04:19 +0700 Subject: [PATCH] fix(security): prevent cross-group session data leak in cron jobs Group-scoped agents could read sessions from other groups via session tools (sessions_list, sessions_history, session_status, sessions_send) because they only checked agent_key, not group context. This caused cron jobs to leak data from unrelated groups into reports. Add isSessionInScope() guard to all 4 session tools with colon-bounded chatID matching. New share_sessions setting (default false) controls cross-group visibility, following the same pattern as share_memory and share_knowledge_graph. Web UI toggle and i18n strings included. 63 test cases covering guild/DM/group users, realistic Zalo IDs, boundary exactness, multi-colon chatIDs, and the exact bug scenario. --- internal/agent/loop_context.go | 4 + internal/agent/loop_utils.go | 6 + internal/agent/loop_utils_test.go | 33 ++ internal/store/agent_store.go | 3 +- internal/store/context.go | 18 + internal/store/run_context.go | 1 + internal/tools/session_scope.go | 53 ++ internal/tools/session_scope_test.go | 536 ++++++++++++++++++ internal/tools/sessions.go | 18 + internal/tools/sessions_history.go | 6 + internal/tools/sessions_send.go | 7 +- ui/web/src/i18n/locales/en/agents.json | 5 +- ui/web/src/i18n/locales/vi/agents.json | 5 +- ui/web/src/i18n/locales/zh/agents.json | 5 +- .../agent-advanced-state-utils.ts | 4 +- .../workspace-sharing-section.tsx | 15 + ui/web/src/types/agent.ts | 1 + 17 files changed, 714 insertions(+), 6 deletions(-) create mode 100644 internal/tools/session_scope.go create mode 100644 internal/tools/session_scope_test.go diff --git a/internal/agent/loop_context.go b/internal/agent/loop_context.go index 796022e6..a3c8d59d 100644 --- a/internal/agent/loop_context.go +++ b/internal/agent/loop_context.go @@ -151,6 +151,9 @@ func (l *Loop) injectContext(ctx context.Context, req *RunRequest) (contextSetup if l.shouldShareKnowledgeGraph() { ctx = store.WithSharedKG(ctx) } + if l.shouldShareSessions() { + ctx = store.WithSharedSessions(ctx) + } if err := os.MkdirAll(effectiveWorkspace, 0755); err != nil { slog.Warn("failed to create user workspace directory", "workspace", effectiveWorkspace, "user", req.UserID, "error", err) } @@ -314,6 +317,7 @@ func (l *Loop) injectContext(ctx context.Context, req *RunRequest) (contextSetup SelfEvolve: l.selfEvolve, SharedMemory: store.IsSharedMemory(ctx), SharedKG: store.IsSharedKG(ctx), + SharedSessions: store.IsSharedSessions(ctx), RestrictToWorkspace: l.restrictToWs != nil && *l.restrictToWs, BuiltinToolSettings: l.builtinToolSettings, ChannelType: req.ChannelType, diff --git a/internal/agent/loop_utils.go b/internal/agent/loop_utils.go index 5f229c38..72d0a3ad 100644 --- a/internal/agent/loop_utils.go +++ b/internal/agent/loop_utils.go @@ -64,6 +64,12 @@ func (l *Loop) shouldShareKnowledgeGraph() bool { return l.workspaceSharing != nil && l.workspaceSharing.ShareKnowledgeGraph } +// shouldShareSessions returns true if sessions should be shared across +// all users/groups of the agent (no per-group session scoping). +func (l *Loop) shouldShareSessions() bool { + return l.workspaceSharing != nil && l.workspaceSharing.ShareSessions +} + // buildChannelMeta extracts channel metadata from RunRequest for bootstrap decisions. // Returns nil when channel type is unknown (preserves normal bootstrap flow). func (l *Loop) buildChannelMeta(req *RunRequest) *bootstrap.ChannelMeta { diff --git a/internal/agent/loop_utils_test.go b/internal/agent/loop_utils_test.go index 4366de39..2fdd8d6f 100644 --- a/internal/agent/loop_utils_test.go +++ b/internal/agent/loop_utils_test.go @@ -109,6 +109,39 @@ func TestShouldShareKnowledgeGraph_DisabledByDefault(t *testing.T) { } } +// ─── shouldShareSessions ────────────────────────────────────────────────────── + +func TestShouldShareSessions_NilConfig(t *testing.T) { + l := &Loop{workspaceSharing: nil} + if l.shouldShareSessions() { + t.Error("nil config should return false") + } +} + +func TestShouldShareSessions_EnabledConfig(t *testing.T) { + l := &Loop{workspaceSharing: &store.WorkspaceSharingConfig{ShareSessions: true}} + if !l.shouldShareSessions() { + t.Error("ShareSessions=true should return true") + } +} + +func TestShouldShareSessions_DisabledByDefault(t *testing.T) { + l := &Loop{workspaceSharing: &store.WorkspaceSharingConfig{ShareMemory: true, ShareKnowledgeGraph: true}} + if l.shouldShareSessions() { + t.Error("ShareMemory and ShareKnowledgeGraph alone should not enable sessions sharing") + } +} + +func TestShouldShareSessions_IndependentOfMemory(t *testing.T) { + l := &Loop{workspaceSharing: &store.WorkspaceSharingConfig{ + ShareMemory: true, + ShareSessions: false, + }} + if l.shouldShareSessions() { + t.Error("ShareMemory=true with ShareSessions=false should return false (independent)") + } +} + // ─── InvalidateUserWorkspace ────────────────────────────────────────────── func TestInvalidateUserWorkspace_RemovesCachedSetup(t *testing.T) { diff --git a/internal/store/agent_store.go b/internal/store/agent_store.go index df359a5d..edefd911 100644 --- a/internal/store/agent_store.go +++ b/internal/store/agent_store.go @@ -317,6 +317,7 @@ type WorkspaceSharingConfig struct { SharedUsers []string `json:"shared_users,omitempty" db:"-"` ShareMemory bool `json:"share_memory" db:"-"` ShareKnowledgeGraph bool `json:"share_knowledge_graph" db:"-"` + ShareSessions bool `json:"share_sessions" db:"-"` } const ( @@ -406,7 +407,7 @@ func (a *AgentData) ParseWorkspaceSharing() *WorkspaceSharingConfig { if json.Unmarshal(a.WorkspaceSharing, &ws) != nil { return nil } - if !ws.SharedDM && !ws.SharedGroup && len(ws.SharedUsers) == 0 && !ws.ShareMemory && !ws.ShareKnowledgeGraph { + if !ws.SharedDM && !ws.SharedGroup && len(ws.SharedUsers) == 0 && !ws.ShareMemory && !ws.ShareKnowledgeGraph && !ws.ShareSessions { return nil } return &ws diff --git a/internal/store/context.go b/internal/store/context.go index 94122f58..0c6dd64a 100644 --- a/internal/store/context.go +++ b/internal/store/context.go @@ -27,6 +27,8 @@ const ( SharedMemoryKey contextKey = "goclaw_shared_memory" // SharedKGKey indicates KG should be shared across all users of the agent (no per-user scoping). SharedKGKey contextKey = "goclaw_shared_kg" + // SharedSessionsKey indicates sessions should be shared across all users (no per-group scoping). + SharedSessionsKey contextKey = "goclaw_shared_sessions" // ShellDenyGroupsKey holds per-agent shell deny group overrides. ShellDenyGroupsKey contextKey = "goclaw_shell_deny_groups" // AgentKeyKey is the context key for the agent key/name (string identifier, e.g. "default"). @@ -236,6 +238,22 @@ func IsSharedKG(ctx context.Context) bool { return false } +// WithSharedSessions returns a context flagged for shared sessions (skip per-group scoping). +func WithSharedSessions(ctx context.Context) context.Context { + return context.WithValue(ctx, SharedSessionsKey, true) +} + +// IsSharedSessions returns true if sessions should be shared across users/groups. +func IsSharedSessions(ctx context.Context) bool { + if v, ok := ctx.Value(SharedSessionsKey).(bool); ok { + return v + } + if rc := RunContextFromCtx(ctx); rc != nil { + return rc.SharedSessions + } + return false +} + // WithLocale returns a new context with the given locale. func WithLocale(ctx context.Context, locale string) context.Context { return context.WithValue(ctx, LocaleKey, locale) diff --git a/internal/store/run_context.go b/internal/store/run_context.go index f6c70aa1..7c397ee6 100644 --- a/internal/store/run_context.go +++ b/internal/store/run_context.go @@ -33,6 +33,7 @@ type RunContext struct { SelfEvolve bool SharedMemory bool SharedKG bool + SharedSessions bool RestrictToWorkspace bool // Tool configuration diff --git a/internal/tools/session_scope.go b/internal/tools/session_scope.go new file mode 100644 index 00000000..2121147a --- /dev/null +++ b/internal/tools/session_scope.go @@ -0,0 +1,53 @@ +package tools + +import ( + "context" + "strings" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// isSessionInScope checks whether a target session key falls within +// the current execution scope. Group-scoped runs can only access +// sessions belonging to the same group, unless share_sessions is enabled. +func isSessionInScope(ctx context.Context, targetKey, currentKey string) bool { + // Shared sessions mode — all sessions visible. + if store.IsSharedSessions(ctx) { + return true + } + + // Always allow own session. + if targetKey == currentKey { + return true + } + + // Extract group chat ID from user context. + userID := store.UserIDFromContext(ctx) + chatID := extractGroupChatID(userID) + if chatID == "" { + return true // DM or non-group scope — no restriction. + } + + // Allow sessions containing this group's chat ID. + // Colon-bounded match prevents partial numeric collisions. + // Matches patterns like: + // agent:X:channel:group:{chatID} + // agent:X:channel:group:{chatID}:topic:N + marker := ":" + chatID + return strings.HasSuffix(targetKey, marker) || + strings.Contains(targetKey, marker+":") +} + +// extractGroupChatID extracts the chat ID from a group-scoped userID. +// Format: "group:channel:chatID" -> "chatID". +// Returns "" for non-group users (DM, guild, etc.). +func extractGroupChatID(userID string) string { + if !strings.HasPrefix(userID, "group:") { + return "" + } + parts := strings.SplitN(userID, ":", 3) + if len(parts) < 3 { + return "" + } + return parts[2] +} diff --git a/internal/tools/session_scope_test.go b/internal/tools/session_scope_test.go new file mode 100644 index 00000000..d6b263b9 --- /dev/null +++ b/internal/tools/session_scope_test.go @@ -0,0 +1,536 @@ +package tools + +import ( + "context" + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// ─── extractGroupChatID ────────────────────────────────────────────────────── + +func TestExtractGroupChatID_ValidGroupZalo(t *testing.T) { + tests := []struct { + name string + userID string + wantID string + }{ + { + name: "group Zalo format", + userID: "group:tuelinhzalo:7296946457790431889", + wantID: "7296946457790431889", + }, + { + name: "group Telegram format", + userID: "group:telegram:123456", + wantID: "123456", + }, + { + name: "group with numeric chatID", + userID: "group:channel:999", + wantID: "999", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractGroupChatID(tt.userID) + if got != tt.wantID { + t.Errorf("extractGroupChatID(%q) = %q, want %q", tt.userID, got, tt.wantID) + } + }) + } +} + +func TestExtractGroupChatID_NonGroupUsers(t *testing.T) { + tests := []struct { + name string + userID string + }{ + { + name: "guild format (not group)", + userID: "guild:abc:user:456", + }, + { + name: "DM user (no group prefix)", + userID: "123456789", + }, + { + name: "empty string", + userID: "", + }, + { + name: "malformed group (missing parts)", + userID: "group:channel", + }, + { + name: "malformed group (only prefix)", + userID: "group:", + }, + { + name: "just 'group'", + userID: "group", + }, + { + name: "guild with many parts", + userID: "guild:discord:server:channel:msg:123", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractGroupChatID(tt.userID) + if got != "" { + t.Errorf("extractGroupChatID(%q) = %q, want empty string", tt.userID, got) + } + }) + } +} + +func TestExtractGroupChatID_EdgeCases(t *testing.T) { + tests := []struct { + name string + userID string + wantID string + }{ + { + name: "chatID containing colon (edge case, unusual but possible)", + userID: "group:channel:abc:def", + wantID: "abc:def", // SplitN with n=3 takes everything after second colon + }, + { + name: "numeric-only chatID", + userID: "group:telegram:9876543210", + wantID: "9876543210", + }, + { + name: "alphanumeric chatID", + userID: "group:slack:C1234567890", + wantID: "C1234567890", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractGroupChatID(tt.userID) + if got != tt.wantID { + t.Errorf("extractGroupChatID(%q) = %q, want %q", tt.userID, got, tt.wantID) + } + }) + } +} + +// ─── isSessionInScope ──────────────────────────────────────────────────────── + +func TestIsSessionInScope_SharedSessionsEnabled(t *testing.T) { + ctx := store.WithSharedSessions(context.Background()) + ctx = store.WithUserID(ctx, "group:telegram:111") + + // With shared sessions enabled, all targets should be in scope + tests := []string{ + "agent:x:zalo:group:222", + "agent:x:cron:job-abc", + "direct-dm-session", + "any-arbitrary-key", + } + + for _, targetKey := range tests { + if !isSessionInScope(ctx, targetKey, "current:session") { + t.Errorf("shared sessions enabled: %q should be in scope", targetKey) + } + } +} + +func TestIsSessionInScope_AllowOwnSession(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, "user:123") + + // Own session should always be allowed, regardless of group scoping + currentKey := "agent:x:zalo:group:111" + if !isSessionInScope(ctx, currentKey, currentKey) { + t.Error("own session should always be in scope") + } +} + +func TestIsSessionInScope_GroupScopedUser_SameChatID(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, "group:zalo:111") + + tests := []struct { + name string + targetKey string + wantScope bool + }{ + { + name: "basic group match", + targetKey: "agent:x:zalo:group:111", + wantScope: true, + }, + { + name: "group with topic", + targetKey: "agent:x:tg:group:111:topic:5", + wantScope: true, + }, + { + name: "group at end of key", + targetKey: "prefix:group:111", + wantScope: true, + }, + { + name: "exact suffix match with colon", + targetKey: "agent:111", + wantScope: true, // HasSuffix(":111") matches, "agent:111".HasSuffix(":111") is true + }, + { + name: "contains chatID in middle with colon after", + targetKey: "something:111:other", + wantScope: true, // Contains `:111:` + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isSessionInScope(ctx, tt.targetKey, "current:key") + if got != tt.wantScope { + t.Errorf("isSessionInScope(..., %q, ...) = %v, want %v", tt.targetKey, got, tt.wantScope) + } + }) + } +} + +func TestIsSessionInScope_GroupScopedUser_DifferentChatID(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, "group:zalo:111") // User in group 111 + + tests := []struct { + name string + targetKey string + wantScope bool + }{ + { + name: "different group", + targetKey: "agent:x:zalo:group:222", + wantScope: false, + }, + { + name: "different group with topic", + targetKey: "agent:x:tg:group:222:topic:5", + wantScope: false, + }, + { + name: "111 appears only as key suffix (HasSuffix matches)", + targetKey: "agent:x:channel:111", + wantScope: true, // HasSuffix(":111") is true, so this passes + }, + { + name: "222 as suffix (not the user's group)", + targetKey: "agent:x:channel:222", + wantScope: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isSessionInScope(ctx, tt.targetKey, "current:key") + if got != tt.wantScope { + t.Errorf("isSessionInScope(..., %q, ...) = %v, want %v", tt.targetKey, got, tt.wantScope) + } + }) + } +} + +func TestIsSessionInScope_DMUser_NoRestriction(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, "dm:user:12345") // Non-group user + + // DM users have no group restriction — all sessions are accessible + tests := []string{ + "agent:x:zalo:group:111", + "agent:x:zalo:group:222", + "agent:x:cron:job-abc", + "direct-session", + } + + for _, targetKey := range tests { + if !isSessionInScope(ctx, targetKey, "current:key") { + t.Errorf("DM user: %q should be in scope", targetKey) + } + } +} + +func TestIsSessionInScope_CronSessionWithGroupUser(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, "group:telegram:111") // Group-scoped user + + // Cron sessions (no group chatID) should not be accessible to group-scoped users + cronKey := "agent:x:cron:job-abc" + if isSessionInScope(ctx, cronKey, "current:key") { + t.Error("cron session should not be in scope for group-scoped user") + } +} + +func TestIsSessionInScope_EmptyUserID(t *testing.T) { + ctx := context.Background() + // No user ID set + + // Empty user (non-group) should have no restriction + if !isSessionInScope(ctx, "agent:x:zalo:group:111", "current:key") { + t.Error("empty user ID should allow all sessions") + } +} + +func TestIsSessionInScope_MarkerBoundaryExactness(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, "group:zalo:111") + + tests := []struct { + name string + targetKey string + wantScope bool + }{ + { + name: "exact suffix match with colon", + targetKey: "prefix:111", + wantScope: true, + }, + { + name: "suffix with content after", + targetKey: "prefix:111:topic", + wantScope: true, + }, + { + name: "no leading colon (numeric collision)", + targetKey: "group111", // No colon before 111 + wantScope: false, + }, + { + name: "111 in middle without proper bounds", + targetKey: "a111b", // 111 without colons + wantScope: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isSessionInScope(ctx, tt.targetKey, "current:key") + if got != tt.wantScope { + t.Errorf("isSessionInScope(..., %q, ...) = %v, want %v", tt.targetKey, got, tt.wantScope) + } + }) + } +} + +func TestIsSessionInScope_ComplexScenarios(t *testing.T) { + tests := []struct { + name string + userID string + sharedSessions bool + targetKey string + currentKey string + wantInScope bool + }{ + { + name: "shared sessions overrides group restriction", + userID: "group:zalo:111", + sharedSessions: true, + targetKey: "agent:x:zalo:group:222", + currentKey: "agent:x:zalo:group:111", + wantInScope: true, + }, + { + name: "own session always allowed despite group mismatch", + userID: "group:zalo:111", + sharedSessions: false, + targetKey: "agent:x:zalo:group:222", + currentKey: "agent:x:zalo:group:222", + wantInScope: true, + }, + { + name: "group user, target from same group, with forum topic", + userID: "group:telegram:999", + sharedSessions: false, + targetKey: "agent:abc:telegram:group:999:topic:42", + currentKey: "current:key", + wantInScope: true, + }, + { + name: "group user, cron job not accessible", + userID: "group:discord:555", + sharedSessions: false, + targetKey: "agent:xyz:cron:backup-job", + currentKey: "current:key", + wantInScope: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + if tt.sharedSessions { + ctx = store.WithSharedSessions(ctx) + } + ctx = store.WithUserID(ctx, tt.userID) + + got := isSessionInScope(ctx, tt.targetKey, tt.currentKey) + if got != tt.wantInScope { + t.Errorf("isSessionInScope = %v, want %v", got, tt.wantInScope) + } + }) + } +} + +// ─── Guild user through isSessionInScope ───────────────────────────────────── + +func TestIsSessionInScope_GuildUser_NoRestriction(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, "guild:abc:user:456") // Non-group, guild-scoped + + // Guild users have no group restriction — extractGroupChatID returns "" + tests := []string{ + "agent:x:discord:group:111", + "agent:x:discord:group:222", + "agent:x:cron:job-abc", + "agent:x:discord:channel:789", + } + + for _, targetKey := range tests { + if !isSessionInScope(ctx, targetKey, "current:key") { + t.Errorf("guild user: %q should be in scope (no group restriction)", targetKey) + } + } +} + +// ─── Realistic long chatIDs (production Zalo IDs) ──────────────────────────── + +func TestIsSessionInScope_RealisticLongChatIDs(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, "group:tuelinhzalo:7296946457790431889") + + tests := []struct { + name string + targetKey string + wantScope bool + }{ + { + name: "same group, realistic key", + targetKey: "agent:itstuelinh:tuelinhzalo:group:7296946457790431889", + wantScope: true, + }, + { + name: "same group with topic", + targetKey: "agent:itstuelinh:tuelinhzalo:group:7296946457790431889:topic:5", + wantScope: true, + }, + { + name: "different group, realistic key (THE BUG SCENARIO)", + targetKey: "agent:itstuelinh:tuelinhzalo:group:1541284681557250737", + wantScope: false, + }, + { + name: "cron session, no chatID match", + targetKey: "agent:itstuelinh:cron:019d4345-abcd-1234-5678-abcdef123456", + wantScope: false, + }, + { + name: "partial numeric overlap (different ID sharing digits)", + targetKey: "agent:itstuelinh:tuelinhzalo:group:72969464577904318891", + wantScope: false, // longer ID, should not match + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isSessionInScope(ctx, tt.targetKey, "agent:itstuelinh:cron:current-job") + if got != tt.wantScope { + t.Errorf("isSessionInScope(..., %q, ...) = %v, want %v", tt.targetKey, got, tt.wantScope) + } + }) + } +} + +// ─── Cron own session match ────────────────────────────────────────────────── + +func TestIsSessionInScope_CronOwnSessionMatch(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, "group:telegram:111") + + // Cron session as currentKey — accessing own session should always work + cronKey := "agent:mybot:cron:019d4345-daily-report" + if !isSessionInScope(ctx, cronKey, cronKey) { + t.Error("cron session accessing itself (own session) should always be in scope") + } +} + +// ─── Multi-colon chatID through full flow ──────────────────────────────────── + +func TestIsSessionInScope_MultiColonChatID(t *testing.T) { + ctx := context.Background() + // SplitN("group:channel:abc:def", ":", 3) → chatID = "abc:def" + ctx = store.WithUserID(ctx, "group:channel:abc:def") + + tests := []struct { + name string + targetKey string + wantScope bool + }{ + { + name: "target contains multi-colon chatID as suffix", + targetKey: "agent:x:group:abc:def", + wantScope: true, // HasSuffix(":abc:def") + }, + { + name: "target contains multi-colon chatID in middle", + targetKey: "agent:x:group:abc:def:topic:1", + wantScope: true, // Contains(":abc:def:") + }, + { + name: "target has partial match only", + targetKey: "agent:x:group:abc", + wantScope: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isSessionInScope(ctx, tt.targetKey, "current:key") + if got != tt.wantScope { + t.Errorf("isSessionInScope(..., %q, ...) = %v, want %v", tt.targetKey, got, tt.wantScope) + } + }) + } +} + +// ─── Malformed group userID → no restriction ───────────────────────────────── + +func TestIsSessionInScope_MalformedGroupUserID(t *testing.T) { + tests := []struct { + name string + userID string + }{ + {name: "group with empty chatID", userID: "group:channel:"}, + {name: "group missing chatID", userID: "group:channel"}, + {name: "group only prefix", userID: "group:"}, + {name: "just group", userID: "group"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + ctx = store.WithUserID(ctx, tt.userID) + + // Malformed group → empty chatID → no restriction → allow all + target := "agent:x:zalo:group:222" + if tt.userID == "group:channel:" { + // "group:channel:" → chatID="" → no restriction + if !isSessionInScope(ctx, target, "current:key") { + t.Errorf("malformed group %q should have no restriction", tt.userID) + } + } else { + // These all return "" from extractGroupChatID + if !isSessionInScope(ctx, target, "current:key") { + t.Errorf("malformed group %q should have no restriction", tt.userID) + } + } + }) + } +} diff --git a/internal/tools/sessions.go b/internal/tools/sessions.go index 8e8736e7..906bcec4 100644 --- a/internal/tools/sessions.go +++ b/internal/tools/sessions.go @@ -65,6 +65,18 @@ func (t *SessionsListTool) Execute(ctx context.Context, args map[string]any) *Re } sessions := t.sessions.List(ctx, agentKey) + // Scope filter: group-scoped users only see sessions for their group. + currentSession := ToolSandboxKeyFromCtx(ctx) + { + var scoped []store.SessionInfo + for _, s := range sessions { + if isSessionInScope(ctx, s.Key, currentSession) { + scoped = append(scoped, s) + } + } + sessions = scoped + } + // Filter by active_minutes if activeMinutes > 0 { cutoff := time.Now().Add(-time.Duration(activeMinutes) * time.Minute) @@ -156,6 +168,12 @@ func (t *SessionStatusTool) Execute(ctx context.Context, args map[string]any) *R return ErrorResult("access denied: session belongs to a different agent") } + // Scope check: group-scoped users cannot access other groups' sessions. + currentSession := ToolSandboxKeyFromCtx(ctx) + if !isSessionInScope(ctx, sessionKey, currentSession) { + return ErrorResult("access denied: session outside current scope") + } + data := t.sessions.Get(ctx, sessionKey) if data == nil { return ErrorResult("session not found") diff --git a/internal/tools/sessions_history.go b/internal/tools/sessions_history.go index 3b294790..80197cc0 100644 --- a/internal/tools/sessions_history.go +++ b/internal/tools/sessions_history.go @@ -80,6 +80,12 @@ func (t *SessionsHistoryTool) Execute(ctx context.Context, args map[string]any) return ErrorResult("access denied: session belongs to a different agent") } + // Scope check: group-scoped users cannot read other groups' session history. + currentSession := ToolSandboxKeyFromCtx(ctx) + if !isSessionInScope(ctx, sessionKey, currentSession) { + return ErrorResult("access denied: session outside current scope") + } + history := t.sessions.GetHistory(ctx, sessionKey) if history == nil { out, _ := json.Marshal(map[string]any{ diff --git a/internal/tools/sessions_send.go b/internal/tools/sessions_send.go index 51c1c09e..83318c1b 100644 --- a/internal/tools/sessions_send.go +++ b/internal/tools/sessions_send.go @@ -95,9 +95,14 @@ func (t *SessionsSendTool) Execute(ctx context.Context, args map[string]any) *Re return ErrorResult("access denied: target session belongs to a different agent") } + // Scope check: group-scoped users cannot send to other groups' sessions. + currentSession := ToolSandboxKeyFromCtx(ctx) + if !isSessionInScope(ctx, sessionKey, currentSession) { + return ErrorResult("access denied: cannot send to session outside current scope") + } + // Block self-send: agent should not send to its own current session // to prevent re-processing loops (same pattern as message tool). - currentSession := ToolSandboxKeyFromCtx(ctx) if currentSession != "" && sessionKey == currentSession { return ErrorResult("cannot send to your own current session — your response is already delivered to it") } diff --git a/ui/web/src/i18n/locales/en/agents.json b/ui/web/src/i18n/locales/en/agents.json index b975fa68..4eae1b93 100644 --- a/ui/web/src/i18n/locales/en/agents.json +++ b/ui/web/src/i18n/locales/en/agents.json @@ -778,7 +778,10 @@ "shareMemoryNote": "Toggling this does not migrate data — existing per-user memories become inaccessible in shared mode.", "shareKG": "Shared Knowledge Graph", "shareKGTip": "All users access the same knowledge graph. When off, each user has their own isolated KG with global canonical fallback for read operations.", - "shareKGNote": "Toggling this does not migrate data — existing per-user KG entities become inaccessible in shared mode." + "shareKGNote": "Toggling this does not migrate data — existing per-user KG entities become inaccessible in shared mode.", + "shareSessions": "Share Sessions", + "shareSessionsTip": "Allow agent to access conversation sessions across all groups. When off, each user's sessions remain isolated.", + "shareSessionsNote": "Toggling this does not migrate data — existing per-user sessions become inaccessible in shared mode." }, "compaction": { "title": "Compaction", diff --git a/ui/web/src/i18n/locales/vi/agents.json b/ui/web/src/i18n/locales/vi/agents.json index 453c440e..3e072c74 100644 --- a/ui/web/src/i18n/locales/vi/agents.json +++ b/ui/web/src/i18n/locales/vi/agents.json @@ -762,7 +762,10 @@ "shareMemoryNote": "Bật/tắt không di chuyển dữ liệu — memory per-user trước đó sẽ không truy cập được trong chế độ shared.", "shareKG": "Chia sẻ Knowledge Graph", "shareKGTip": "Tất cả người dùng truy cập chung Knowledge Graph. Khi tắt, mỗi người dùng có KG riêng với fallback đọc từ global canonical.", - "shareKGNote": "Bật/tắt không di chuyển dữ liệu — KG per-user trước đó sẽ không truy cập được trong chế độ shared." + "shareKGNote": "Bật/tắt không di chuyển dữ liệu — KG per-user trước đó sẽ không truy cập được trong chế độ shared.", + "shareSessions": "Chia sẻ phiên hội thoại", + "shareSessionsTip": "Cho phép agent truy cập các phiên hội thoại trên tất cả các nhóm. Khi tắt, phiên của mỗi người dùng được cô lập riêng.", + "shareSessionsNote": "Bật/tắt không di chuyển dữ liệu — phiên per-user trước đó sẽ không truy cập được trong chế độ shared." }, "compaction": { "title": "Nén ngữ cảnh", diff --git a/ui/web/src/i18n/locales/zh/agents.json b/ui/web/src/i18n/locales/zh/agents.json index 3f495145..688bcdec 100644 --- a/ui/web/src/i18n/locales/zh/agents.json +++ b/ui/web/src/i18n/locales/zh/agents.json @@ -762,7 +762,10 @@ "shareMemoryNote": "切换不会迁移数据——切换到共享模式后，现有的用户独立记忆将无法访问。", "shareKG": "共享知识图谱", "shareKGTip": "所有用户访问相同的知识图谱。关闭时，每个用户拥有独立的知识图谱，读取操作会回退到全局规范数据。", - "shareKGNote": "切换不会迁移数据——切换到共享模式后，现有的用户独立知识图谱实体将无法访问。" + "shareKGNote": "切换不会迁移数据——切换到共享模式后，现有的用户独立知识图谱实体将无法访问。", + "shareSessions": "共享会话", + "shareSessionsTip": "允许 agent 访问所有群组的会话记录。关闭时，每个用户的会话保持隔离。", + "shareSessionsNote": "切换不会迁移数据——切换到共享模式后，现有的用户独立会话将无法访问。" }, "compaction": { "title": "压缩", diff --git a/ui/web/src/pages/agents/agent-detail/agent-advanced-state-utils.ts b/ui/web/src/pages/agents/agent-detail/agent-advanced-state-utils.ts index 98b3cf07..8c9a78f2 100644 --- a/ui/web/src/pages/agents/agent-detail/agent-advanced-state-utils.ts +++ b/ui/web/src/pages/agents/agent-detail/agent-advanced-state-utils.ts @@ -175,7 +175,9 @@ export function buildAdvancedUpdatePayload( wsSharing.shared_dm || wsSharing.shared_group || (wsSharing.shared_users?.length ?? 0) > 0 || - wsSharing.share_memory + wsSharing.share_memory || + wsSharing.share_knowledge_graph || + wsSharing.share_sessions ) { updates.workspace_sharing = wsSharing; } else { diff --git a/ui/web/src/pages/agents/agent-detail/config-sections/workspace-sharing-section.tsx b/ui/web/src/pages/agents/agent-detail/config-sections/workspace-sharing-section.tsx index 450a11a6..e5bf416f 100644 --- a/ui/web/src/pages/agents/agent-detail/config-sections/workspace-sharing-section.tsx +++ b/ui/web/src/pages/agents/agent-detail/config-sections/workspace-sharing-section.tsx @@ -86,6 +86,21 @@ export function WorkspaceSharingSection({ value, onChange }: WorkspaceSharingSec

)} + +
+
+ {t(`${s}.shareSessions`)} + onChange({ ...value, share_sessions: v })} + /> +
+ {value.share_sessions && ( +

+ {t(`${s}.shareSessionsNote`)} +

+ )} +
diff --git a/ui/web/src/types/agent.ts b/ui/web/src/types/agent.ts index 038f491f..b66b288d 100644 --- a/ui/web/src/types/agent.ts +++ b/ui/web/src/types/agent.ts @@ -89,6 +89,7 @@ export interface WorkspaceSharingConfig { shared_users?: string[]; share_memory?: boolean; share_knowledge_graph?: boolean; + share_sessions?: boolean; } export type ChatGPTOAuthRoutingStrategy =