From 8893cdbf8b2fab2a2005eb7e8e4bfc4b04b0168a Mon Sep 17 00:00:00 2001 From: viettranx Date: Sun, 12 Apr 2026 07:17:54 +0700 Subject: [PATCH] fix(store/pg): session tenant isolation + vault scan arity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two unrelated integration-test failures, both real source bugs: 1. Session ListPaged/ListPagedRich leaked cross-tenant data. buildSessionFilter only honored opts.TenantID and never read tenant from ctx — so callers relying on ctx scoping (the common case) got no tenant filter at all. List() already had the correct pattern using store.TenantIDFromContext(ctx) gated by !store.IsCrossTenant(ctx). Replicate inside buildSessionFilter; thread ctx into the helper; opts.TenantID still wins as admin override. Fixes TestStoreSession_ListPaged_Pagination/tenant_isolation and TestStoreSession_ListPagedRich_TenantIsolation. 2. Vault GetDocument / GetDocumentByBasename panicked with "sql: expected 15 destination arguments in Scan, not 14". Migration 000047 added `path_basename` as a generated column; the SELECT lists were updated but the Scan arg lists were not. GetDocumentByID had the correct 15-target pattern — copy it. vaultDocRow.PathBasename field already exists. Audit sweep of vault_links.go + vault_documents_enrichment.go confirms no other drift. Fixes TestStoreVault_UpsertAndGetDocument. --- internal/store/pg/sessions_list.go | 23 ++++++++++++++++++----- internal/store/pg/vault_documents.go | 8 ++++++-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/internal/store/pg/sessions_list.go b/internal/store/pg/sessions_list.go index 7e061ad3..3050ada9 100644 --- a/internal/store/pg/sessions_list.go +++ b/internal/store/pg/sessions_list.go @@ -16,7 +16,14 @@ import ( // buildSessionFilter builds a dynamic WHERE clause from SessionListOpts. // Returns the WHERE string (with leading " WHERE ") and the positional args. // The tableAlias is prepended to column names (e.g. "s" → "s.session_key"). -func buildSessionFilter(opts store.SessionListOpts, tableAlias string) (string, []any) { +// +// Tenant isolation precedence: +// 1. opts.TenantID (if set) wins — admin tooling override path. +// 2. Else if ctx is NOT cross-tenant, fall back to store.TenantIDFromContext(ctx). +// This matches the canonical pattern in List() above and prevents +// silent cross-tenant reads from callers that rely on ctx scoping. +// 3. Else (cross-tenant ctx, no opts override) no tenant filter. +func buildSessionFilter(ctx context.Context, opts store.SessionListOpts, tableAlias string) (string, []any) { prefix := "" if tableAlias != "" { prefix = tableAlias + "." @@ -41,9 +48,15 @@ func buildSessionFilter(opts store.SessionListOpts, tableAlias string) (string, args = append(args, opts.UserID) idx++ } - if opts.TenantID != uuid.Nil { + + // Resolve tenant filter — opts override beats ctx. + tenantID := opts.TenantID + if tenantID == uuid.Nil && !store.IsCrossTenant(ctx) { + tenantID = store.TenantIDFromContext(ctx) + } + if tenantID != uuid.Nil { conditions = append(conditions, fmt.Sprintf("%stenant_id = $%d", prefix, idx)) - args = append(args, opts.TenantID) + args = append(args, tenantID) idx++ } _ = idx // consumed @@ -102,7 +115,7 @@ func (s *PGSessionStore) ListPaged(ctx context.Context, opts store.SessionListOp } offset := max(opts.Offset, 0) - where, whereArgs := buildSessionFilter(opts, "") + where, whereArgs := buildSessionFilter(ctx, opts, "") // Count total var total int @@ -137,7 +150,7 @@ func (s *PGSessionStore) ListPagedRich(ctx context.Context, opts store.SessionLi } offset := max(opts.Offset, 0) - where, whereArgs := buildSessionFilter(opts, "s") + where, whereArgs := buildSessionFilter(ctx, opts, "s") // Count total var total int diff --git a/internal/store/pg/vault_documents.go b/internal/store/pg/vault_documents.go index 13aca493..fe5a453b 100644 --- a/internal/store/pg/vault_documents.go +++ b/internal/store/pg/vault_documents.go @@ -174,9 +174,11 @@ func (s *PGVaultStore) GetDocument(ctx context.Context, tenantID, agentID, path } var row vaultDocRow + // Scan order MUST match SELECT order above: 15 columns including + // path_basename (generated column added in migration 000047). err = s.db.QueryRowContext(ctx, q, args...).Scan( &row.ID, &row.TenantID, &row.AgentID, &row.TeamID, &row.Scope, &row.CustomScope, - &row.Path, &row.Title, &row.DocType, &row.ContentHash, &row.Summary, + &row.Path, &row.PathBasename, &row.Title, &row.DocType, &row.ContentHash, &row.Summary, &row.MetaJSON, &row.CreatedAt, &row.UpdatedAt) if err != nil { return nil, err @@ -259,9 +261,11 @@ func (s *PGVaultStore) GetDocumentByBasename(ctx context.Context, tenantID, agen } q += " LIMIT 1" var row vaultDocRow + // Scan order MUST match SELECT order above: 15 columns including + // path_basename (generated column added in migration 000047). err = s.db.QueryRowContext(ctx, q, args...).Scan( &row.ID, &row.TenantID, &row.AgentID, &row.TeamID, &row.Scope, &row.CustomScope, - &row.Path, &row.Title, &row.DocType, &row.ContentHash, &row.Summary, + &row.Path, &row.PathBasename, &row.Title, &row.DocType, &row.ContentHash, &row.Summary, &row.MetaJSON, &row.CreatedAt, &row.UpdatedAt) if err != nil { return nil, err