diff --git a/cmd/gateway_vault_wiring.go b/cmd/gateway_vault_wiring.go index 9957fd74..9c07bfa9 100644 --- a/cmd/gateway_vault_wiring.go +++ b/cmd/gateway_vault_wiring.go @@ -36,6 +36,10 @@ func wireVault(stores *store.Stores, toolsReg *tools.Registry, workspace string, if stores.Episodic != nil { vaultReadTool.SetEpisodicStore(stores.Episodic) } + // Tenant slug fallback for runs whose context carries no slug (channels, cron). + if stores.Tenants != nil { + vaultReadTool.SetTenantStore(stores.Tenants) + } toolsReg.Register(vaultReadTool) // Build VaultSearchService: fan-out across vault + episodic + KG. diff --git a/internal/tools/vault_read.go b/internal/tools/vault_read.go index e00c3c05..befbacd0 100644 --- a/internal/tools/vault_read.go +++ b/internal/tools/vault_read.go @@ -22,6 +22,7 @@ type VaultReadTool struct { vaultStore store.VaultStore kgStore store.KnowledgeGraphStore episodicStore store.EpisodicStore + tenantStore store.TenantStore workspace string } @@ -74,9 +75,15 @@ func (t *VaultReadTool) SetKGStore(kg store.KnowledgeGraphStore) { t.kgStore = k // lookup when vault lookup misses. nil is safe. func (t *VaultReadTool) SetEpisodicStore(es store.EpisodicStore) { t.episodicStore = es } -// SetWorkspace injects the tenant workspace root (wired at boot). +// SetWorkspace injects the global workspace root (wired at boot). This is the +// base for every tenant — the tenant-scoped root is resolved per request in +// tenantWorkspace, since one tool instance serves all tenants. func (t *VaultReadTool) SetWorkspace(ws string) { t.workspace = ws } +// SetTenantStore injects an optional TenantStore used to resolve the tenant +// slug when the request context does not carry one. nil is safe. +func (t *VaultReadTool) SetTenantStore(ts store.TenantStore) { t.tenantStore = ts } + func (t *VaultReadTool) Name() string { return "vault_read" } func (t *VaultReadTool) Description() string { @@ -145,8 +152,8 @@ func (t *VaultReadTool) Execute(ctx context.Context, args map[string]any) *Resul return ErrorResult(fmt.Sprintf("vault_read does not support %s files — use read_image/read_audio/read_video/read_document", ext)) } - // Resolve path under tenant workspace root. - fullPath, err := t.resolvePath(doc.Path) + // Resolve path under the tenant workspace root. + fullPath, err := t.resolvePath(t.tenantWorkspace(ctx, tenantID), doc.Path) if err != nil { return ErrorResult(err.Error()) } @@ -344,13 +351,33 @@ func (t *VaultReadTool) allowed(ctx context.Context, doc *store.VaultDocument) b } } -// resolvePath joins the tenant workspace with the doc's relative path and -// enforces that the fully-resolved path remains strictly under the workspace. +// tenantWorkspace resolves the tenant-scoped workspace root for this request. +// Vault paths are stored relative to that root (e.g. "agents//note.md"), +// so joining them onto the global root silently misses every non-master tenant +// whose files live under /tenants//. Master tenant is a no-op +// inside TenantLayer and keeps using the base root. +func (t *VaultReadTool) tenantWorkspace(ctx context.Context, tenantID uuid.UUID) string { + slug := store.TenantSlugFromContext(ctx) + // Channel- and cron-originated runs may not carry the slug in context. + // Without it TenantLayer falls back to a UUID-named directory, which does + // not match the slug-named directory the write path creates on disk. + if slug == "" && tenantID != store.MasterTenantID && t.tenantStore != nil { + if tenant, err := t.tenantStore.GetTenant(ctx, tenantID); err == nil && tenant != nil { + slug = tenant.Slug + } + } + return ResolveWorkspace(t.workspace, TenantLayer(tenantID, slug)) +} + +// resolvePath joins the given workspace root with the doc's relative path and +// enforces that the fully-resolved path remains strictly under that root. +// Passing the tenant root (not the global one) also makes the boundary check +// double as cross-tenant isolation. // Symlinks are resolved via EvalSymlinks for defence-in-depth; if the file is // missing EvalSymlinks will fail and we fall back to the cleaned join (the // subsequent os.Open will surface the not-found error naturally). -func (t *VaultReadTool) resolvePath(relPath string) (string, error) { - wsClean := filepath.Clean(t.workspace) +func (t *VaultReadTool) resolvePath(root, relPath string) (string, error) { + wsClean := filepath.Clean(root) joined := filepath.Join(wsClean, filepath.FromSlash(relPath)) // Resolve symlinks where possible; on error (e.g. file missing) keep the diff --git a/internal/tools/vault_read_test.go b/internal/tools/vault_read_test.go index f80f3757..adeb8182 100644 --- a/internal/tools/vault_read_test.go +++ b/internal/tools/vault_read_test.go @@ -11,6 +11,7 @@ import ( "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/config" "github.com/nextlevelbuilder/goclaw/internal/store" ) @@ -61,17 +62,30 @@ func (f *fakeVaultStore) GetDocumentsByIDs(ctx context.Context, tenantID string, // newVaultReadTestTool builds a VaultReadTool with a temp workspace and a // fake store pre-seeded with docs. +// +// The tool is given the GLOBAL workspace root, mirroring boot wiring. The +// returned path is the TENANT-scoped root of the first seeded doc — the +// directory the tool resolves per request, and therefore where test content +// must be written. Tests set no slug on the context, so the tenant directory +// is UUID-named (see config.TenantWorkspace). func newVaultReadTestTool(t *testing.T, docs ...*store.VaultDocument) (*VaultReadTool, string) { t.Helper() - ws := t.TempDir() + base := t.TempDir() fake := &fakeVaultStore{byID: make(map[string]*store.VaultDocument)} for _, d := range docs { fake.byID[d.TenantID+":"+d.ID] = d } tool := NewVaultReadTool() tool.SetVaultStore(fake) - tool.SetWorkspace(ws) - return tool, ws + tool.SetWorkspace(base) + + contentRoot := base + if len(docs) > 0 { + if tid, err := uuid.Parse(docs[0].TenantID); err == nil { + contentRoot = config.TenantWorkspace(base, tid, "") + } + } + return tool, contentRoot } // writeFile creates a file under workspace with the given relative path. @@ -917,3 +931,118 @@ func TestVaultRead_TrulyMissingReturnsNotFound(t *testing.T) { t.Fatalf("expected document not found, got: %s", res.ForLLM) } } + +// --- 18. Regression: non-master tenant content lives under tenants//. +// The tool holds the GLOBAL root, so a global-root join misses the file and +// every non-master tenant reads ENOENT while vault_search (DB-only) still +// returns the doc. Mirrors the production failure on tenant "alpha". --- +func TestVaultRead_NonMasterTenant_ResolvesUnderTenantRoot(t *testing.T) { + tenantID := uuid.New() + agentID := uuid.New() + docID := uuid.New() + doc := &store.VaultDocument{ + ID: docID.String(), TenantID: tenantID.String(), + Scope: "shared", Path: "agents/ho-tro/Refund Flow.md", + Title: "Refund Flow", DocType: "note", + } + base := t.TempDir() + fake := &fakeVaultStore{byID: map[string]*store.VaultDocument{ + tenantID.String() + ":" + docID.String(): doc, + }} + tool := NewVaultReadTool() + tool.SetVaultStore(fake) + tool.SetWorkspace(base) + + // Written by the tenant-aware write path; slug comes from context. + tenantRoot := config.TenantWorkspace(base, tenantID, "alpha") + writeFile(t, tenantRoot, "agents/ho-tro/Refund Flow.md", "refund body") + + ctx := store.WithTenantSlug(makeCtx(tenantID, agentID), "alpha") + res := tool.Execute(ctx, map[string]any{"doc_id": docID.String()}) + if res.IsError { + t.Fatalf("tenant doc must resolve under tenants//, got: %s", res.ForLLM) + } + if !strings.Contains(res.ForLLM, "refund body") { + t.Fatalf("content missing: %s", res.ForLLM) + } +} + +// --- 19. Master tenant keeps using the base root (no tenants/ segment), so +// the fix above must not regress single-tenant and master installs. --- +func TestVaultRead_MasterTenant_UsesBaseRoot(t *testing.T) { + agentID := uuid.New() + docID := uuid.New() + doc := &store.VaultDocument{ + ID: docID.String(), TenantID: store.MasterTenantID.String(), + Scope: "shared", Path: "agents/ho-tro/note.md", + Title: "Note", DocType: "note", + } + base := t.TempDir() + fake := &fakeVaultStore{byID: map[string]*store.VaultDocument{ + store.MasterTenantID.String() + ":" + docID.String(): doc, + }} + tool := NewVaultReadTool() + tool.SetVaultStore(fake) + tool.SetWorkspace(base) + + // No tenants/ segment for master. + writeFile(t, base, "agents/ho-tro/note.md", "master body") + + res := tool.Execute(makeCtx(store.MasterTenantID, agentID), + map[string]any{"doc_id": docID.String()}) + if res.IsError { + t.Fatalf("master doc must resolve at base root, got: %s", res.ForLLM) + } + if !strings.Contains(res.ForLLM, "master body") { + t.Fatalf("content missing: %s", res.ForLLM) + } +} + +// --- 20. Slug absent from context (channel/cron runs) → fall back to the +// TenantStore lookup, since a UUID-named directory would miss the slug-named +// one the write path created. --- +func TestVaultRead_SlugMissingFromContext_FallsBackToTenantStore(t *testing.T) { + tenantID := uuid.New() + agentID := uuid.New() + docID := uuid.New() + doc := &store.VaultDocument{ + ID: docID.String(), TenantID: tenantID.String(), + Scope: "shared", Path: "agents/ho-tro/note.md", + Title: "Note", DocType: "note", + } + base := t.TempDir() + fake := &fakeVaultStore{byID: map[string]*store.VaultDocument{ + tenantID.String() + ":" + docID.String(): doc, + }} + tool := NewVaultReadTool() + tool.SetVaultStore(fake) + tool.SetWorkspace(base) + tool.SetTenantStore(&fakeTenantStoreRead{slug: "alpha", id: tenantID}) + + writeFile(t, config.TenantWorkspace(base, tenantID, "alpha"), + "agents/ho-tro/note.md", "cron body") + + // Context deliberately carries no slug. + res := tool.Execute(makeCtx(tenantID, agentID), + map[string]any{"doc_id": docID.String()}) + if res.IsError { + t.Fatalf("slug fallback must resolve the doc, got: %s", res.ForLLM) + } + if !strings.Contains(res.ForLLM, "cron body") { + t.Fatalf("content missing: %s", res.ForLLM) + } +} + +// fakeTenantStoreRead resolves a single tenant slug for the fallback path. +type fakeTenantStoreRead struct { + store.TenantStore + id uuid.UUID + slug string +} + +func (f *fakeTenantStoreRead) GetTenant(_ context.Context, id uuid.UUID) (*store.TenantData, error) { + if id != f.id { + return nil, os.ErrNotExist + } + return &store.TenantData{ID: f.id, Slug: f.slug}, nil +} diff --git a/tests/integration/v3_test_helper.go b/tests/integration/v3_test_helper.go index 12016275..ac856284 100644 --- a/tests/integration/v3_test_helper.go +++ b/tests/integration/v3_test_helper.go @@ -15,6 +15,7 @@ import ( "github.com/google/uuid" _ "github.com/jackc/pgx/v5/stdlib" + "github.com/nextlevelbuilder/goclaw/internal/config" "github.com/nextlevelbuilder/goclaw/internal/security" "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/nextlevelbuilder/goclaw/internal/store/pg" @@ -89,7 +90,7 @@ func seedTenantAgent(t *testing.T, db *sql.DB) (tenantID, agentID uuid.UUID) { _, err := db.Exec( `INSERT INTO tenants (id, name, slug, status) VALUES ($1, $2, $3, 'active') ON CONFLICT DO NOTHING`, - tenantID, "test-tenant-"+tenantID.String()[:8], "t"+tenantID.String()[:8]) + tenantID, "test-tenant-"+tenantID.String()[:8], tenantSlug(tenantID)) if err != nil { t.Fatalf("seed tenant: %v", err) } @@ -178,6 +179,34 @@ func tenantCtx(tenantID uuid.UUID) context.Context { return store.WithTenantID(context.Background(), tenantID) } +// tenantSlug returns the slug seedTenantAgent assigns to a seeded tenant. +// Single source of truth so on-disk layout in a test cannot drift from the seed. +func tenantSlug(tenantID uuid.UUID) string { + return "t" + tenantID.String()[:8] +} + +// tenantCtxSlug is tenantCtx plus the tenant slug. Real runs carry the slug in +// context, and tools that resolve a tenant-scoped workspace need it: without a +// slug config.TenantScopedDir deliberately falls back to an id-named directory +// (see its doc comment — an empty slug must never resolve to the tenants/ parent). +func tenantCtxSlug(tenantID uuid.UUID) context.Context { + return store.WithTenantSlug(tenantCtx(tenantID), tenantSlug(tenantID)) +} + +// tenantWorkspaceDir creates and returns the tenant-scoped workspace root under +// base — what config.TenantWorkspace resolves to for a seeded (non-master) tenant. +// Vault document paths are stored relative to THIS directory, not to base: base is +// the global workspace root wired once at boot, and one tool instance serves every +// tenant. Fixtures written straight to base are only reachable by the master tenant. +func tenantWorkspaceDir(t *testing.T, base string, tenantID uuid.UUID) string { + t.Helper() + dir := config.TenantWorkspace(base, tenantID, tenantSlug(tenantID)) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("mkdir tenant workspace: %v", err) + } + return dir +} + // userCtx returns a context with both tenant ID and user ID set. func userCtx(tenantID uuid.UUID, userID string) context.Context { ctx := store.WithTenantID(context.Background(), tenantID) diff --git a/tests/integration/vault_namespace_fix_test.go b/tests/integration/vault_namespace_fix_test.go index a3a01c2d..c810ca7b 100644 --- a/tests/integration/vault_namespace_fix_test.go +++ b/tests/integration/vault_namespace_fix_test.go @@ -35,12 +35,12 @@ func TestVaultNamespaceFix_thuyTienScenario(t *testing.T) { ws := t.TempDir() relPath := "KG_03_Danh_Muc_San_Pham.md" body := "Product catalog body" - if err := os.WriteFile(filepath.Join(ws, relPath), []byte(body), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), []byte(body), 0o644); err != nil { t.Fatalf("write file: %v", err) } userID := "kguser-" + agentID.String()[:8] - ctx := store.WithUserID(store.WithAgentID(tenantCtx(tenantID), agentID), userID) + ctx := store.WithUserID(store.WithAgentID(tenantCtxSlug(tenantID), agentID), userID) // Seed vault doc. vdoc := makeSharedVaultDoc(tenantID.String(), relPath, "KG_03 Danh Muc San Pham") diff --git a/tests/integration/vault_read_test.go b/tests/integration/vault_read_test.go index 1804ef55..6e60f1d2 100644 --- a/tests/integration/vault_read_test.go +++ b/tests/integration/vault_read_test.go @@ -27,13 +27,13 @@ func TestVaultRead_SharedAtWorkspaceRoot(t *testing.T) { ws := t.TempDir() body := strings.Repeat("KG_03 body line\n", 300) // ~4.8KB relPath := "KG_03_Test.md" - if err := os.WriteFile(filepath.Join(ws, relPath), []byte(body), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), []byte(body), 0o644); err != nil { t.Fatalf("write file: %v", err) } // Register doc directly (avoids spinning up the full rescan pipeline). doc := makeSharedVaultDoc(tenantID.String(), relPath, "KG_03 Test") - ctx := tenantCtx(tenantID) + ctx := tenantCtxSlug(tenantID) ctx = store.WithAgentID(ctx, agentID) if err := vs.UpsertDocument(ctx, doc); err != nil { t.Fatalf("UpsertDocument: %v", err) @@ -68,12 +68,12 @@ func TestVaultRead_PersonalCrossAgentDenied(t *testing.T) { ws := t.TempDir() relPath := "private.md" - if err := os.WriteFile(filepath.Join(ws, relPath), []byte("secret"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), []byte("secret"), 0o644); err != nil { t.Fatalf("write file: %v", err) } doc := makeVaultDoc(tenantID.String(), agentA.String(), relPath, "Private") - ctxA := store.WithAgentID(tenantCtx(tenantID), agentA) + ctxA := store.WithAgentID(tenantCtxSlug(tenantID), agentA) if err := vs.UpsertDocument(ctxA, doc); err != nil { t.Fatalf("UpsertDocument: %v", err) } @@ -83,7 +83,7 @@ func TestVaultRead_PersonalCrossAgentDenied(t *testing.T) { tool.SetWorkspace(ws) // Agent B reads → must be denied. - ctxB := store.WithAgentID(tenantCtx(tenantID), agentB) + ctxB := store.WithAgentID(tenantCtxSlug(tenantID), agentB) res := tool.Execute(ctxB, map[string]any{"doc_id": doc.ID}) if !res.IsError { t.Fatalf("expected access denied for cross-agent personal read, got content") @@ -102,7 +102,7 @@ func TestVaultRead_MediaRejected(t *testing.T) { ws := t.TempDir() relPath := "pic.png" - if err := os.WriteFile(filepath.Join(ws, relPath), []byte("pretend-png"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), []byte("pretend-png"), 0o644); err != nil { t.Fatalf("write file: %v", err) } @@ -117,7 +117,7 @@ func TestVaultRead_MediaRejected(t *testing.T) { ContentHash: "abc", Metadata: map[string]any{"mime_type": "image/png"}, } - ctx := store.WithAgentID(tenantCtx(tenantID), agentID) + ctx := store.WithAgentID(tenantCtxSlug(tenantID), agentID) if err := vs.UpsertDocument(ctx, doc); err != nil { t.Fatalf("UpsertDocument: %v", err) } @@ -144,12 +144,12 @@ func TestVaultRead_OversizeTruncation(t *testing.T) { ws := t.TempDir() relPath := "big.md" big := bytes.Repeat([]byte("a"), 300_000) - if err := os.WriteFile(filepath.Join(ws, relPath), big, 0o644); err != nil { + if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), big, 0o644); err != nil { t.Fatalf("write file: %v", err) } doc := makeSharedVaultDoc(tenantID.String(), relPath, "Big Doc") - ctx := store.WithAgentID(tenantCtx(tenantID), agentID) + ctx := store.WithAgentID(tenantCtxSlug(tenantID), agentID) if err := vs.UpsertDocument(ctx, doc); err != nil { t.Fatalf("UpsertDocument: %v", err) } @@ -194,11 +194,11 @@ func TestVaultRead_OutlinksScopeMatrix(t *testing.T) { ws := t.TempDir() tid := tenantID.String() - ctxSelf := store.WithAgentID(tenantCtx(tenantID), agentSelf) + ctxSelf := store.WithAgentID(tenantCtxSlug(tenantID), agentSelf) // Source file on disk + doc (shared so read is allowed). srcRel := "src.md" - if err := os.WriteFile(filepath.Join(ws, srcRel), []byte("src body"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), srcRel), []byte("src body"), 0o644); err != nil { t.Fatalf("write src: %v", err) } src := makeSharedVaultDoc(tid, srcRel, "Source")