mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-13 22:15:47 +00:00
fix(store/pg): session tenant isolation + vault scan arity
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.
This commit is contained in:
1 parent
adf3eab01b
commit
8893cdbf8b
2 files changed
+24
-7
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user