From 9625a20e8bdd913adc9fac66d4bcf7fa6ce15cf9 Mon Sep 17 00:00:00 2001 From: viettranx Date: Thu, 23 Apr 2026 18:56:57 +0700 Subject: [PATCH] fix(vault): complete chat_id isolation coverage - Extend migration 000056 with legacy-docs backfill (telegram/, ws/, delegate/, and agent/bot/ path shapes) so scope=shared/personal docs created before team-scope refactor also get stamped with chat_id and stop leaking across chats - rescan: inferOwnerFromPath now returns chat_id extracted from team path third segment; stamp onto VaultDocument so admin /vault/rescan preserves chat isolation - HTTP vault search: accept optional chat_id body param, set TeamIsolated=true when TeamID + ChatID both present (admin UI can view vault from a specific chat perspective) Applied to staging DB via MCP: 91 legacy rows backfilled. --- internal/http/vault_handler_links.go | 7 ++++ internal/vault/rescan.go | 44 +++++++++++++---------- internal/vault/rescan_test.go | 2 +- migrations/000056_vault_chat_id.up.sql | 49 +++++++++++++++++++++++++- 4 files changed, 82 insertions(+), 20 deletions(-) diff --git a/internal/http/vault_handler_links.go b/internal/http/vault_handler_links.go index 1ddf0bf3..505ca37b 100644 --- a/internal/http/vault_handler_links.go +++ b/internal/http/vault_handler_links.go @@ -30,6 +30,7 @@ func (h *VaultHandler) doSearch(w http.ResponseWriter, r *http.Request, agentID DocTypes []string `json:"doc_types"` MaxResults int `json:"max_results"` TeamID string `json:"team_id"` + ChatID string `json:"chat_id"` // optional: when set with TeamID, restrict to same-chat + team-wide docs (isolated semantics) } if !bindJSON(w, r, locale, &body) { return @@ -59,6 +60,12 @@ func (h *VaultHandler) doSearch(w http.ResponseWriter, r *http.Request, agentID return } searchOpts.TeamID = &body.TeamID + // Caller-supplied chat scope: apply isolation filter when searching a specific chat. + if body.ChatID != "" { + cid := body.ChatID + searchOpts.ChatID = &cid + searchOpts.TeamIsolated = true + } } else if !store.IsOwnerRole(r.Context()) { if ids := h.userAccessibleTeamIDs(r.Context()); len(ids) > 0 { searchOpts.TeamIDs = ids diff --git a/internal/vault/rescan.go b/internal/vault/rescan.go index dd396783..5a1eec20 100644 --- a/internal/vault/rescan.go +++ b/internal/vault/rescan.go @@ -55,7 +55,7 @@ func RescanWorkspace(ctx context.Context, params RescanParams, vs store.VaultSto } for _, entry := range entries { - agentID, teamID, scope, strippedPath := inferOwnerFromPath(entry.RelPath, params.AgentMap, params.TeamSet) + agentID, teamID, chatID, scope, strippedPath := inferOwnerFromPath(entry.RelPath, params.AgentMap, params.TeamSet) if scope == "" { // Unknown agent key or invalid team UUID — skip. result.Skipped++ @@ -94,6 +94,7 @@ func RescanWorkspace(ctx context.Context, params RescanParams, vs store.VaultSto TenantID: params.TenantID, AgentID: agentID, TeamID: teamID, + ChatID: chatID, Scope: scope, Path: relPath, Title: InferTitle(relPath), @@ -148,33 +149,40 @@ func RescanWorkspace(ctx context.Context, params RescanParams, vs store.VaultSto } // inferOwnerFromPath parses a tenant-relative path to determine ownership. -// Returns: agentID (*string), teamID (*string), scope (string), strippedPath (string). +// Returns: agentID (*string), teamID (*string), chatID (*string), scope, strippedPath. // // Path patterns (checked in order): // -// teams/{team_uuid}/rest/of/path → teamID=uuid, scope="team", path=full relPath -// agents/{agent_key}/rest/of/path → agentID=lookup(key), scope="personal", path=full relPath (legacy) -// {agent_key}/rest/of/path → agentID=lookup(key), scope="personal", path=full relPath (workspace layout) -// anything/else → scope="shared", path unchanged +// teams/{team_uuid}/{chat}/... → teamID=uuid, chatID=chat, scope="team" +// teams/{team_uuid}/file.md → teamID=uuid, chatID=nil (team-wide), scope="team" +// agents/{agent_key}/... → agentID=lookup(key), scope="personal" (legacy prefix) +// {agent_key}/... → agentID=lookup(key), scope="personal" (workspace layout) // -// The full relPath is always preserved in strippedPath for DB storage so enrichment -// workers can locate files via filepath.Join(workspace, path). +// Chat segments starting with "." (e.g. ".goclaw") are config dirs, not real chats — chatID stays nil. +// The full relPath is preserved in strippedPath for DB storage so enrichment workers +// can locate files via filepath.Join(workspace, path). // Returns scope="" to signal the file should be skipped (unknown agent or invalid team). -func inferOwnerFromPath(relPath string, agentMap map[string]string, teamSet map[string]bool) (agentID *string, teamID *string, scope string, strippedPath string) { - // Team paths: teams/{uuid}/... +func inferOwnerFromPath(relPath string, agentMap map[string]string, teamSet map[string]bool) (agentID *string, teamID *string, chatID *string, scope string, strippedPath string) { + // Team paths: teams/{uuid}/[chat/]... if strings.HasPrefix(relPath, "teams/") { rest := relPath[len("teams/"):] id, remainder, hasSlash := strings.Cut(rest, "/") if !hasSlash || id == "" || strings.Contains(remainder, "..") { - return nil, nil, "", relPath + return nil, nil, nil, "", relPath } if _, parseErr := uuid.Parse(id); parseErr != nil { - return nil, nil, "", relPath + return nil, nil, nil, "", relPath } if !teamSet[id] { - return nil, nil, "", relPath + return nil, nil, nil, "", relPath } - return nil, &id, "team", relPath + // Extract chat segment (second path component after team uuid) if present + // and not a config/hidden dir. Paths without a chat segment stay team-wide. + if chatSeg, _, hasChat := strings.Cut(remainder, "/"); hasChat && chatSeg != "" && !strings.HasPrefix(chatSeg, ".") { + cid := chatSeg + return nil, &id, &cid, "team", relPath + } + return nil, &id, nil, "team", relPath } // Agent paths: agents/{key}/... (legacy prefix) or {key}/... (actual workspace layout) @@ -183,10 +191,10 @@ func inferOwnerFromPath(relPath string, agentMap map[string]string, teamSet map[ key, _, hasSlash := strings.Cut(rest, "/") if hasSlash && key != "" && !strings.Contains(relPath, "..") { if agentUUID, ok := agentMap[key]; ok { - return &agentUUID, nil, "personal", relPath + return &agentUUID, nil, nil, "personal", relPath } } - return nil, nil, "", relPath + return nil, nil, nil, "", relPath } // Root-level agent_key match: {agent_key}/... @@ -194,12 +202,12 @@ func inferOwnerFromPath(relPath string, agentMap map[string]string, teamSet map[ firstSeg, _, hasSlash := strings.Cut(relPath, "/") if hasSlash && firstSeg != "" { if agentUUID, ok := agentMap[firstSeg]; ok { - return &agentUUID, nil, "personal", relPath + return &agentUUID, nil, nil, "personal", relPath } } // Everything else is shared (root-level files, unknown folders) - return nil, nil, "shared", relPath + return nil, nil, nil, "shared", relPath } // InferDocType guesses doc_type from path conventions. diff --git a/internal/vault/rescan_test.go b/internal/vault/rescan_test.go index 0d991e2e..c4f4fcf7 100644 --- a/internal/vault/rescan_test.go +++ b/internal/vault/rescan_test.go @@ -98,7 +98,7 @@ func TestInferOwnerFromPath(t *testing.T) { for _, tt := range tests { t.Run(tt.path, func(t *testing.T) { - gotAgentID, gotTeamID, gotScope, gotPath := inferOwnerFromPath(tt.path, agentMap, teamSet) + gotAgentID, gotTeamID, _, gotScope, gotPath := inferOwnerFromPath(tt.path, agentMap, teamSet) if gotScope != tt.wantScope { t.Errorf("scope = %q, want %q", gotScope, tt.wantScope) diff --git a/migrations/000056_vault_chat_id.up.sql b/migrations/000056_vault_chat_id.up.sql index 4edadb44..a34edb82 100644 --- a/migrations/000056_vault_chat_id.up.sql +++ b/migrations/000056_vault_chat_id.up.sql @@ -7,13 +7,60 @@ CREATE INDEX IF NOT EXISTS idx_vault_docs_team_chat ON vault_documents(team_id, chat_id) WHERE team_id IS NOT NULL; --- Backfill chat_id for isolated teams. Two path layouts exist: +-- ----------------------------------------------------------------------------- +-- Backfill 1: team-scoped docs (scope='team', team_id set). +-- Two path layouts: -- master tenant: teams///... -- non-master tenant: tenants//teams///... -- Chat segments starting with '.' (e.g. '.goclaw') are config dirs, not real chats — skip. +-- ----------------------------------------------------------------------------- UPDATE vault_documents vd SET chat_id = (regexp_match(vd.path, '^(?:tenants/[^/]+/)?teams/[^/]+/([^/]+)/'))[1] FROM agent_teams t WHERE vd.team_id = t.id AND (t.settings->>'workspace_scope' IS NULL OR t.settings->>'workspace_scope' != 'shared') AND vd.path ~ '^(?:tenants/[^/]+/)?teams/[^/]+/[^.][^/]*/'; + +-- ----------------------------------------------------------------------------- +-- Backfill 2: legacy docs from before team scope (team_id IS NULL) with chat +-- identifiers embedded in their path. Without chat_id these leak across chats +-- in isolated-team search because the `searchChatFilter` predicate cannot +-- distinguish them. +-- +-- Path layouts handled (ordered most-specific → most-general in COALESCE): +-- telegram/group_telegram_/... (nested legacy) +-- /telegram/group_telegram_/. (agent-owned nested) +-- group_telegram_/... (bare legacy) +-- /telegram//... or leading telegram//... +-- tenants//ws//... (non-master tenant WS) +-- ws//... or /ws//... (WS direct) +-- /delegate//... (delegated task) +-- //group__/... (legacy bot channel) +-- ///... (bot + numeric/ws chat) +-- +-- Chat IDs can be numeric (Telegram), `system`, user handles, etc. +-- Only populate when chat_id IS NULL so interceptor-stamped values are preserved. +-- ----------------------------------------------------------------------------- +UPDATE vault_documents +SET chat_id = COALESCE( + (regexp_match(path, '^telegram/group_telegram_(-?[0-9]+)/'))[1], + (regexp_match(path, '/telegram/group_telegram_(-?[0-9]+)/'))[1], + (regexp_match(path, '^group_telegram_(-?[0-9]+)/'))[1], + (regexp_match(path, '/group_telegram_(-?[0-9]+)/'))[1], + (regexp_match(path, '^telegram/(-?[0-9a-zA-Z_-]+)/'))[1], + (regexp_match(path, '/telegram/([a-zA-Z0-9_-]+)/'))[1], + (regexp_match(path, '^tenants/[^/]+/ws/([^/]+)/'))[1], + (regexp_match(path, '^ws/([^/]+)/'))[1], + (regexp_match(path, '^[^/]+/ws/([^/]+)/'))[1], + (regexp_match(path, '^[^/]+/delegate/([^/]+)/'))[1], + (regexp_match(path, '^[^/]+/[^/]+/group_[^/]+_(-?[0-9]+)/'))[1], + (regexp_match(path, '^[^/]+/[^/]+/([a-zA-Z0-9_-]+)/'))[1] +) +WHERE chat_id IS NULL + AND team_id IS NULL + AND ( + path ~ '(^|/)(group_telegram_|telegram/)' + OR path ~ '^(([^/]+/)?(tenants/[^/]+/)?)?ws/[^/]+/' + OR path ~ '^[^/]+/delegate/[^/]+/' + OR path ~ '^[^/]+/[^/]+/(group_[^/]+_-?[0-9]+|[0-9]+)/' + );