mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-17 18:14:13 +00:00
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.
This commit is contained in:
1 parent
5a86c18402
commit
a4df5a08e3
17 files changed
+714
-6
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -33,6 +33,7 @@ type RunContext struct {
|
||||
SelfEvolve bool
|
||||
SharedMemory bool
|
||||
SharedKG bool
|
||||
SharedSessions bool
|
||||
RestrictToWorkspace bool
|
||||
|
||||
// Tool configuration
|
||||
|
||||
@@ -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]
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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")
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -762,7 +762,10 @@
|
||||
"shareMemoryNote": "切换不会迁移数据——切换到共享模式后,现有的用户独立记忆将无法访问。",
|
||||
"shareKG": "共享知识图谱",
|
||||
"shareKGTip": "所有用户访问相同的知识图谱。关闭时,每个用户拥有独立的知识图谱,读取操作会回退到全局规范数据。",
|
||||
"shareKGNote": "切换不会迁移数据——切换到共享模式后,现有的用户独立知识图谱实体将无法访问。"
|
||||
"shareKGNote": "切换不会迁移数据——切换到共享模式后,现有的用户独立知识图谱实体将无法访问。",
|
||||
"shareSessions": "共享会话",
|
||||
"shareSessionsTip": "允许 agent 访问所有群组的会话记录。关闭时,每个用户的会话保持隔离。",
|
||||
"shareSessionsNote": "切换不会迁移数据——切换到共享模式后,现有的用户独立会话将无法访问。"
|
||||
},
|
||||
"compaction": {
|
||||
"title": "压缩",
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -86,6 +86,21 @@ export function WorkspaceSharingSection({ value, onChange }: WorkspaceSharingSec
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<div className={`rounded-lg border p-3 sm:p-4 ${value.share_sessions ? "border-orange-400/60 bg-orange-50/30 dark:border-orange-500/30 dark:bg-orange-950/10" : ""}`}>
|
||||
<div className="flex items-center justify-between">
|
||||
<InfoLabel tip={t(`${s}.shareSessionsTip`)}>{t(`${s}.shareSessions`)}</InfoLabel>
|
||||
<Switch
|
||||
checked={value.share_sessions ?? false}
|
||||
onCheckedChange={(v) => onChange({ ...value, share_sessions: v })}
|
||||
/>
|
||||
</div>
|
||||
{value.share_sessions && (
|
||||
<p className="mt-2 text-xs text-orange-600 dark:text-orange-400">
|
||||
{t(`${s}.shareSessionsNote`)}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</section>
|
||||
|
||||
|
||||
@@ -89,6 +89,7 @@ export interface WorkspaceSharingConfig {
|
||||
shared_users?: string[];
|
||||
share_memory?: boolean;
|
||||
share_knowledge_graph?: boolean;
|
||||
share_sessions?: boolean;
|
||||
}
|
||||
|
||||
export type ChatGPTOAuthRoutingStrategy =
|
||||
|
||||
Reference in new issue
Block a user