From 7cfcbbf9db382b706e7fd1f8dab07310ec65941d Mon Sep 17 00:00:00 2001 From: viettranx Date: Thu, 16 Apr 2026 12:55:50 +0700 Subject: [PATCH] fix(vault): include shared docs in agent read paths (#917) - Patch vault_search, ListDocuments, CountDocuments, ListTreeEntries to include shared docs (agent_id IS NULL) for agents in the tenant - Keep DELETE/GetDocument/GetByBasename strict (auth-intent) - Add CHECK invariant vault_documents_scope_consistency (PG migration 000055 NOT VALID, SQLite triggers v24) to prevent future drift - Update docs and changelog Affects PostgreSQL and SQLite (desktop edition). --- docs/17-changelog.md | 1 + docs/24-knowledge-vault.md | 21 + internal/store/pg/vault_documents.go | 14 +- internal/store/sqlitestore/schema.go | 32 +- internal/store/sqlitestore/schema.sql | 8 +- .../sqlitestore/schema_migration_test.go | 74 ++++ internal/store/sqlitestore/vault_documents.go | 10 +- internal/upgrade/version.go | 2 +- ...055_vault_scope_consistency_check.down.sql | 1 + ...00055_vault_scope_consistency_check.up.sql | 16 + .../sqlite_vault_shared_docs_test.go | 169 ++++++++ .../sqlite_vault_visibility_matrix_test.go | 338 ++++++++++++++++ .../integration/v3_vault_scope_check_test.go | 81 ++++ tests/integration/v3_vault_store_ext_test.go | 364 ++++++++++++++++++ tests/integration/v3_vault_store_test.go | 29 ++ .../v3_vault_visibility_matrix_test.go | 266 +++++++++++++ tests/invariants/tenant_isolation_test.go | 2 +- 17 files changed, 1412 insertions(+), 16 deletions(-) create mode 100644 migrations/000055_vault_scope_consistency_check.down.sql create mode 100644 migrations/000055_vault_scope_consistency_check.up.sql create mode 100644 tests/integration/sqlite_vault_shared_docs_test.go create mode 100644 tests/integration/sqlite_vault_visibility_matrix_test.go create mode 100644 tests/integration/v3_vault_scope_check_test.go create mode 100644 tests/integration/v3_vault_visibility_matrix_test.go diff --git a/docs/17-changelog.md b/docs/17-changelog.md index 93faf3ab..25baf84a 100644 --- a/docs/17-changelog.md +++ b/docs/17-changelog.md @@ -52,6 +52,7 @@ Integration / chaos / RBAC / tracing coverage + user documentation + copy-paste ### Fixed - **`PGHookStore.GetByID` + `SqliteHookStore.GetByID`** now enforce tenant scope: non-master callers only see own + global rows. Previously returned any row by UUID which leaked cross-tenant. - **Existing pipeline integration tests** now call `security.SetAllowLoopbackForTest(true)` via shared helper; no longer flake on SSRF block for httptest endpoints. +- **Vault**: Shared docs (`scope='shared'`, `agent_id=NULL`) are now visible to agents via `vault_search`, `ListDocuments`, `CountDocuments`. Previously excluded by strict `agent_id = $N` predicate after migration 000046 made `agent_id` nullable. Also fixes single-team filter to include shared docs. Adds CHECK invariant (migration 000055) to prevent future scope/ownership drift. Affects PostgreSQL and SQLite (desktop edition). (#917) ### Testing - All hook integration tests pass under `TEST_DATABASE_URL` PG container (7 new test files, 18 new top-level tests) diff --git a/docs/24-knowledge-vault.md b/docs/24-knowledge-vault.md index 8fb7cf92..e4804a40 100644 --- a/docs/24-knowledge-vault.md +++ b/docs/24-knowledge-vault.md @@ -46,6 +46,27 @@ Documents are scoped by **tenant** (isolation), **agent** (per-agent namespace), - **team**: Team workspace documents (team_context_files) - **shared**: Cross-tenant shared knowledge (future) +### Document Scope & Ownership + +| scope | agent_id | team_id | Visibility | +|-------|----------|---------|------------| +| personal | set | NULL | Owning agent only (within tenant) | +| team | NULL | set | Members of the team (within tenant) | +| shared | NULL | NULL | All agents within the tenant | +| custom | any | any | User-defined via `custom_scope` | + +#### DB Invariant (migration 000055) + +A CHECK constraint enforces the above table: `vault_documents_scope_consistency`. Rejects inserts/updates that violate the scope × ownership relationship. `scope='custom'` is unconstrained (user-defined scopes). + +#### Agent Read Semantics + +`vault_search`, `ListDocuments`, `CountDocuments` return: +- Docs owned by the agent (`agent_id = `) +- PLUS shared docs (`agent_id IS NULL`) + +Within a team context (RunContext with TeamID set), results also include team-scoped docs for that team. Tenant isolation (`tenant_id = `) is always enforced. + --- ## 2. Data Model diff --git a/internal/store/pg/vault_documents.go b/internal/store/pg/vault_documents.go index cb3e0067..fe6dd806 100644 --- a/internal/store/pg/vault_documents.go +++ b/internal/store/pg/vault_documents.go @@ -27,7 +27,7 @@ func appendTeamFilter(q string, args []any, p int, teamID *string, teamIDs []str q += " AND (team_id IS NULL OR team_id IN (" + strings.Join(ph, ",") + "))" } else if teamID != nil { if *teamID != "" { - q += fmt.Sprintf(" AND team_id = $%d", p) + q += fmt.Sprintf(" AND (team_id = $%d OR team_id IS NULL)", p) args = append(args, parseUUIDOrNil(*teamID)) p++ } else { @@ -332,7 +332,7 @@ func (s *PGVaultStore) ListDocuments(ctx context.Context, tenantID, agentID stri if err != nil { return nil, fmt.Errorf("vault list documents: agent: %w", err) } - q += fmt.Sprintf(" AND agent_id = $%d", p) + q += fmt.Sprintf(" AND (agent_id = $%d OR agent_id IS NULL)", p) args = append(args, aid) p++ } @@ -391,7 +391,7 @@ func (s *PGVaultStore) CountDocuments(ctx context.Context, tenantID, agentID str if err != nil { return 0, fmt.Errorf("vault count documents: agent: %w", err) } - q += fmt.Sprintf(" AND agent_id = $%d", p) + q += fmt.Sprintf(" AND (agent_id = $%d OR agent_id IS NULL)", p) args = append(args, aid) p++ } @@ -526,7 +526,7 @@ func (tf searchTeamFilter) append(q string, args []any, p int) (string, []any, i } q += " AND (team_id IS NULL OR team_id IN (" + strings.Join(ph, ",") + "))" } else if tf.teamID != nil { - q += fmt.Sprintf(" AND team_id = $%d", p) + q += fmt.Sprintf(" AND (team_id = $%d OR team_id IS NULL)", p) args = append(args, *tf.teamID) p++ } else { @@ -544,7 +544,7 @@ func (s *PGVaultStore) ftsSearch(ctx context.Context, query string, tenantID uui p := 3 if agentID != nil { - q += fmt.Sprintf(" AND agent_id = $%d", p) + q += fmt.Sprintf(" AND (agent_id = $%d OR agent_id IS NULL)", p) args = append(args, *agentID) p++ } @@ -582,7 +582,7 @@ func (s *PGVaultStore) vectorSearch(ctx context.Context, embedding []float32, te p := 3 if agentID != nil { - q += fmt.Sprintf(" AND agent_id = $%d", p) + q += fmt.Sprintf(" AND (agent_id = $%d OR agent_id IS NULL)", p) args = append(args, *agentID) p++ } @@ -771,7 +771,7 @@ func appendTreeFilters(q string, args []any, p int, opts store.VaultTreeOptions) if opts.AgentID != "" { aid, err := parseUUID(opts.AgentID) if err == nil { - q += fmt.Sprintf(" AND agent_id = $%d", p) + q += fmt.Sprintf(" AND (agent_id = $%d OR agent_id IS NULL)", p) args = append(args, aid) p++ } diff --git a/internal/store/sqlitestore/schema.go b/internal/store/sqlitestore/schema.go index 75a20ec5..04738efd 100644 --- a/internal/store/sqlitestore/schema.go +++ b/internal/store/sqlitestore/schema.go @@ -16,7 +16,7 @@ var schemaSQL string // SchemaVersion is the current SQLite schema version. // Bump this when adding new migration steps below. -const SchemaVersion = 23 +const SchemaVersion = 24 // migrations maps version → SQL to apply when upgrading FROM that version. // schema.sql always represents the LATEST full schema (for fresh DBs). @@ -466,6 +466,36 @@ WHERE context_pruning IS NOT NULL 20: `SELECT 1;`, 21: `SELECT 1;`, 22: `SELECT 1;`, + + // Version 23 → 24: vault_documents scope/ownership consistency triggers. + // Mirrors PG migration 000055 CHECK constraint; SQLite cannot add CHECK via + // ALTER TABLE so we use BEFORE INSERT + BEFORE UPDATE triggers instead. + // fresh DBs get the inline CHECK in schema.sql; existing DBs get triggers. + 23: `CREATE TRIGGER IF NOT EXISTS trg_vault_docs_scope_consistency_ins + BEFORE INSERT ON vault_documents + FOR EACH ROW + WHEN NOT ( + (NEW.scope='personal' AND NEW.agent_id IS NOT NULL AND NEW.team_id IS NULL) OR + (NEW.scope='team' AND NEW.team_id IS NOT NULL AND NEW.agent_id IS NULL) OR + (NEW.scope='shared' AND NEW.agent_id IS NULL AND NEW.team_id IS NULL) OR + NEW.scope='custom' + ) + BEGIN + SELECT RAISE(ABORT, 'vault_documents_scope_consistency violation'); + END; + +CREATE TRIGGER IF NOT EXISTS trg_vault_docs_scope_consistency_upd + BEFORE UPDATE OF scope, agent_id, team_id ON vault_documents + FOR EACH ROW + WHEN NOT ( + (NEW.scope='personal' AND NEW.agent_id IS NOT NULL AND NEW.team_id IS NULL) OR + (NEW.scope='team' AND NEW.team_id IS NOT NULL AND NEW.agent_id IS NULL) OR + (NEW.scope='shared' AND NEW.agent_id IS NULL AND NEW.team_id IS NULL) OR + NEW.scope='custom' + ) + BEGIN + SELECT RAISE(ABORT, 'vault_documents_scope_consistency violation'); + END;`, } // addHooksTables is the SQLite incremental migration for schema v19 → v20. diff --git a/internal/store/sqlitestore/schema.sql b/internal/store/sqlitestore/schema.sql index 1e948755..680dae87 100644 --- a/internal/store/sqlitestore/schema.sql +++ b/internal/store/sqlitestore/schema.sql @@ -1531,7 +1531,13 @@ CREATE TABLE IF NOT EXISTS vault_documents ( summary TEXT NOT NULL DEFAULT '', metadata TEXT DEFAULT '{}', created_at TEXT DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - updated_at TEXT DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) + updated_at TEXT DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + CONSTRAINT vault_documents_scope_consistency CHECK ( + (scope = 'personal' AND agent_id IS NOT NULL AND team_id IS NULL) OR + (scope = 'team' AND team_id IS NOT NULL AND agent_id IS NULL) OR + (scope = 'shared' AND agent_id IS NULL AND team_id IS NULL) OR + scope = 'custom' + ) ); -- SQLite prohibits expressions in inline UNIQUE constraints; use a unique index instead. CREATE UNIQUE INDEX IF NOT EXISTS idx_vault_docs_unique_path diff --git a/internal/store/sqlitestore/schema_migration_test.go b/internal/store/sqlitestore/schema_migration_test.go index addd3abe..671367b4 100644 --- a/internal/store/sqlitestore/schema_migration_test.go +++ b/internal/store/sqlitestore/schema_migration_test.go @@ -125,6 +125,80 @@ func TestEnsureSchema_MigrationV11_SeedsAgentFiles(t *testing.T) { } } +// TestSQLiteSchemaUpgrade_23_to_24 verifies the v23→24 migration creates both +// scope-consistency triggers on an existing DB. +func TestSQLiteSchemaUpgrade_23_to_24(t *testing.T) { + db := openTestDBAtVersion(t, 23) + + if err := EnsureSchema(db); err != nil { + t.Fatalf("EnsureSchema (v23→24) failed: %v", err) + } + + var version int + db.QueryRow("SELECT version FROM schema_version LIMIT 1").Scan(&version) + if version != SchemaVersion { + t.Errorf("schema version = %d, want %d", version, SchemaVersion) + } + + // Verify both triggers exist in sqlite_master. + for _, trigName := range []string{ + "trg_vault_docs_scope_consistency_ins", + "trg_vault_docs_scope_consistency_upd", + } { + var count int + db.QueryRow( + `SELECT COUNT(*) FROM sqlite_master WHERE type='trigger' AND name=?`, trigName, + ).Scan(&count) + if count != 1 { + t.Errorf("trigger %q not found after migration", trigName) + } + } +} + +// TestSQLiteVaultStore_UpsertTriggerEnforcesCheck verifies the v24 triggers +// fire on both the INSERT path and the UPDATE path (UPSERT ON CONFLICT). +func TestSQLiteVaultStore_UpsertTriggerEnforcesCheck(t *testing.T) { + db := openTestDB(t) + if err := EnsureSchema(db); err != nil { + t.Fatalf("EnsureSchema: %v", err) + } + + // Seed required FK rows: tenant + agent. + tenantID := "00000000-0000-0000-0000-000000000001" + agentID := "00000000-0000-0000-0000-000000000002" + db.Exec(`INSERT INTO tenants (id, name, slug, status) VALUES (?, 'T', 't', 'active')`, tenantID) + db.Exec(`INSERT INTO agents (id, agent_key, display_name, status, tenant_id, owner_id, model, provider) + VALUES (?, 'agt', 'A', 'active', ?, 'owner', 'gpt-4o', 'openai')`, agentID, tenantID) + + // 1. Valid INSERT (personal + agent_id set) must succeed. + _, err := db.Exec( + `INSERT INTO vault_documents (id, tenant_id, agent_id, team_id, scope, path, path_basename, title, doc_type, content_hash) + VALUES ('doc-1', ?, ?, NULL, 'personal', '/a/b.md', 'b.md', 'T', 'note', 'h1')`, + tenantID, agentID) + if err != nil { + t.Fatalf("valid INSERT failed: %v", err) + } + + // 2. Invalid fresh INSERT (personal + agent_id NULL) must abort. + _, err = db.Exec( + `INSERT INTO vault_documents (id, tenant_id, agent_id, team_id, scope, path, path_basename, title, doc_type, content_hash) + VALUES ('doc-2', ?, NULL, NULL, 'personal', '/a/c.md', 'c.md', 'T2', 'note', 'h2')`, + tenantID) + if err == nil { + t.Fatal("expected INSERT to fail scope_consistency check, but it succeeded") + } + + // 3. UPSERT that would make scope inconsistent must abort on UPDATE path. + _, err = db.Exec( + `INSERT INTO vault_documents (id, tenant_id, agent_id, team_id, scope, path, path_basename, title, doc_type, content_hash) + VALUES ('doc-1', ?, NULL, NULL, 'personal', '/a/b.md', 'b.md', 'T-upd', 'note', 'h1') + ON CONFLICT(id) DO UPDATE SET agent_id = NULL, scope = 'personal'`, + tenantID) + if err == nil { + t.Fatal("expected UPSERT to fail scope_consistency check on UPDATE path, but it succeeded") + } +} + func openTestDB(t *testing.T) *sql.DB { t.Helper() db, err := OpenDB(filepath.Join(t.TempDir(), "test.db")) diff --git a/internal/store/sqlitestore/vault_documents.go b/internal/store/sqlitestore/vault_documents.go index 25a11fdf..2055558c 100644 --- a/internal/store/sqlitestore/vault_documents.go +++ b/internal/store/sqlitestore/vault_documents.go @@ -25,7 +25,7 @@ func sqliteAppendTeamFilter(q string, args []any, teamID *string, teamIDs []stri } } else if teamID != nil { if *teamID != "" { - q += " AND team_id = ?" + q += " AND (team_id = ? OR team_id IS NULL)" args = append(args, *teamID) } else { q += " AND team_id IS NULL" @@ -212,7 +212,7 @@ func (s *SQLiteVaultStore) ListDocuments(ctx context.Context, tenantID, agentID args := []any{tenantID} if agentID != "" { - q += " AND agent_id = ?" + q += " AND (agent_id = ? OR agent_id IS NULL)" args = append(args, agentID) } q, args = sqliteAppendTeamFilter(q, args, opts.TeamID, opts.TeamIDs) @@ -263,7 +263,7 @@ func (s *SQLiteVaultStore) CountDocuments(ctx context.Context, tenantID, agentID args := []any{tenantID} if agentID != "" { - q += " AND agent_id = ?" + q += " AND (agent_id = ? OR agent_id IS NULL)" args = append(args, agentID) } q, args = sqliteAppendTeamFilter(q, args, opts.TeamID, opts.TeamIDs) @@ -362,7 +362,7 @@ func (s *SQLiteVaultStore) Search(ctx context.Context, opts store.VaultSearchOpt args := []any{opts.TenantID, pattern, pattern} if opts.AgentID != "" { - q += " AND agent_id = ?" + q += " AND (agent_id = ? OR agent_id IS NULL)" args = append(args, opts.AgentID) } @@ -501,7 +501,7 @@ func (s *SQLiteVaultStore) ListTreeEntries(ctx context.Context, tenantID string, func sqliteAppendTreeFilters(q string, args []any, opts store.VaultTreeOptions) (string, []any) { if opts.AgentID != "" { - q += " AND agent_id = ?" + q += " AND (agent_id = ? OR agent_id IS NULL)" args = append(args, opts.AgentID) } q, args = sqliteAppendTeamFilter(q, args, opts.TeamID, opts.TeamIDs) diff --git a/internal/upgrade/version.go b/internal/upgrade/version.go index 0c64f086..d7787a6c 100644 --- a/internal/upgrade/version.go +++ b/internal/upgrade/version.go @@ -2,4 +2,4 @@ package upgrade // RequiredSchemaVersion is the schema migration version this binary requires. // Bump this whenever adding a new SQL migration file. -const RequiredSchemaVersion uint = 54 +const RequiredSchemaVersion uint = 55 diff --git a/migrations/000055_vault_scope_consistency_check.down.sql b/migrations/000055_vault_scope_consistency_check.down.sql new file mode 100644 index 00000000..f006e074 --- /dev/null +++ b/migrations/000055_vault_scope_consistency_check.down.sql @@ -0,0 +1 @@ +ALTER TABLE vault_documents DROP CONSTRAINT IF EXISTS vault_documents_scope_consistency; diff --git a/migrations/000055_vault_scope_consistency_check.up.sql b/migrations/000055_vault_scope_consistency_check.up.sql new file mode 100644 index 00000000..65653833 --- /dev/null +++ b/migrations/000055_vault_scope_consistency_check.up.sql @@ -0,0 +1,16 @@ +-- Scope/ownership invariant for vault_documents. +-- personal → agent_id NOT NULL, team_id NULL +-- team → team_id NOT NULL, agent_id NULL +-- shared → both NULL +-- custom → no constraint (user-defined scopes) +ALTER TABLE vault_documents + ADD CONSTRAINT vault_documents_scope_consistency + CHECK ( + (scope = 'personal' AND agent_id IS NOT NULL AND team_id IS NULL) OR + (scope = 'team' AND team_id IS NOT NULL AND agent_id IS NULL) OR + (scope = 'shared' AND agent_id IS NULL AND team_id IS NULL) OR + scope = 'custom' + ) NOT VALID; + +-- Ops step (run after audit cleanup): +-- ALTER TABLE vault_documents VALIDATE CONSTRAINT vault_documents_scope_consistency; diff --git a/tests/integration/sqlite_vault_shared_docs_test.go b/tests/integration/sqlite_vault_shared_docs_test.go new file mode 100644 index 00000000..4022f78a --- /dev/null +++ b/tests/integration/sqlite_vault_shared_docs_test.go @@ -0,0 +1,169 @@ +//go:build sqliteonly + +package integration + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/store/sqlitestore" +) + +// newSQLiteTestStores creates a SQLite-backed store for integration tests. +func newSQLiteTestStores(t *testing.T) *store.Stores { + t.Helper() + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "test.db") + skillDir := filepath.Join(tmpDir, "skills") + os.MkdirAll(skillDir, 0o755) + + stores, err := sqlitestore.NewSQLiteStores(store.StoreConfig{ + SQLitePath: dbPath, + SkillsStorageDir: skillDir, + EncryptionKey: "test-key-32-bytes-long-for-aes!!", + }) + if err != nil { + t.Fatalf("NewSQLiteStores: %v", err) + } + t.Cleanup(func() { stores.DB.Close() }) + return stores +} + +// TestSQLiteVaultStore_Search_SharedDocsVisible verifies that Search returns +// shared (agent_id=NULL) documents alongside personal (agent_id=X) documents +// when AgentID is specified. +func TestSQLiteVaultStore_Search_SharedDocsVisible(t *testing.T) { + stores := newSQLiteTestStores(t) + + tenantID := uuid.Must(uuid.NewV7()) + agentID := uuid.Must(uuid.NewV7()) + agentIDStr := agentID.String() + + // Seed tenant + agent. + stores.DB.Exec(`INSERT INTO tenants (id, name, slug, status) VALUES (?, 'T', 'srch', 'active')`, tenantID.String()) + stores.DB.Exec( + `INSERT INTO agents (id, agent_key, display_name, status, tenant_id, owner_id, model, provider) + VALUES (?, 'srch-agt', 'A', 'active', ?, 'owner', 'gpt-4o', 'openai')`, + agentIDStr, tenantID.String()) + + ctx := store.WithTenantID(context.Background(), tenantID) + + // Personal doc (agent-owned). + personalDoc := &store.VaultDocument{ + TenantID: tenantID.String(), + AgentID: &agentIDStr, + Scope: "personal", + Path: "agents/a1/srch-personal.md", + Title: "Personal Search Doc", + DocType: "note", + ContentHash: "ph1", + } + if err := stores.Vault.UpsertDocument(ctx, personalDoc); err != nil { + t.Fatalf("UpsertDocument personal: %v", err) + } + + // Shared doc (agent_id=NULL, scope=shared). + sharedDoc := &store.VaultDocument{ + TenantID: tenantID.String(), + AgentID: nil, + Scope: "shared", + Path: "shared/srch-shared.md", + Title: "Shared Search Doc", + DocType: "note", + ContentHash: "sh1", + } + if err := stores.Vault.UpsertDocument(ctx, sharedDoc); err != nil { + t.Fatalf("UpsertDocument shared: %v", err) + } + + results, err := stores.Vault.Search(ctx, store.VaultSearchOptions{ + Query: "Search Doc", + AgentID: agentIDStr, + TenantID: tenantID.String(), + }) + if err != nil { + t.Fatalf("Search: %v", err) + } + + found := map[string]bool{} + for _, r := range results { + found[r.Document.Path] = true + } + + if !found["agents/a1/srch-personal.md"] { + t.Errorf("Search: personal doc not found in results %v", results) + } + if !found["shared/srch-shared.md"] { + t.Errorf("Search: shared doc (agent_id=NULL) not found in results — BUG: shared docs excluded") + } +} + +// TestSQLiteVaultStore_List_IncludesShared verifies that ListDocuments returns +// shared (agent_id=NULL) documents alongside personal (agent_id=X) documents +// when agentID is specified. +func TestSQLiteVaultStore_List_IncludesShared(t *testing.T) { + stores := newSQLiteTestStores(t) + + tenantID := uuid.Must(uuid.NewV7()) + agentID := uuid.Must(uuid.NewV7()) + agentIDStr := agentID.String() + + // Seed tenant + agent. + stores.DB.Exec(`INSERT INTO tenants (id, name, slug, status) VALUES (?, 'T', 'list', 'active')`, tenantID.String()) + stores.DB.Exec( + `INSERT INTO agents (id, agent_key, display_name, status, tenant_id, owner_id, model, provider) + VALUES (?, 'list-agt', 'A', 'active', ?, 'owner', 'gpt-4o', 'openai')`, + agentIDStr, tenantID.String()) + + ctx := store.WithTenantID(context.Background(), tenantID) + + // Personal doc. + personalDoc := &store.VaultDocument{ + TenantID: tenantID.String(), + AgentID: &agentIDStr, + Scope: "personal", + Path: "agents/a1/list-personal.md", + Title: "Personal List Doc", + DocType: "note", + ContentHash: "lph1", + } + if err := stores.Vault.UpsertDocument(ctx, personalDoc); err != nil { + t.Fatalf("UpsertDocument personal: %v", err) + } + + // Shared doc. + sharedDoc := &store.VaultDocument{ + TenantID: tenantID.String(), + AgentID: nil, + Scope: "shared", + Path: "shared/list-shared.md", + Title: "Shared List Doc", + DocType: "note", + ContentHash: "lsh1", + } + if err := stores.Vault.UpsertDocument(ctx, sharedDoc); err != nil { + t.Fatalf("UpsertDocument shared: %v", err) + } + + docs, err := stores.Vault.ListDocuments(ctx, tenantID.String(), agentIDStr, store.VaultListOptions{Limit: 50}) + if err != nil { + t.Fatalf("ListDocuments: %v", err) + } + + found := map[string]bool{} + for _, d := range docs { + found[d.Path] = true + } + + if !found["agents/a1/list-personal.md"] { + t.Errorf("ListDocuments: personal doc not found in results %v", docs) + } + if !found["shared/list-shared.md"] { + t.Errorf("ListDocuments: shared doc (agent_id=NULL) not found in results — BUG: shared docs excluded") + } +} diff --git a/tests/integration/sqlite_vault_visibility_matrix_test.go b/tests/integration/sqlite_vault_visibility_matrix_test.go new file mode 100644 index 00000000..44fda37c --- /dev/null +++ b/tests/integration/sqlite_vault_visibility_matrix_test.go @@ -0,0 +1,338 @@ +//go:build sqlite || sqliteonly + +package integration + +import ( + "context" + "sort" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// seedSQLiteAgent inserts an agent into an existing tenant in the SQLite test DB. +func seedSQLiteAgent(t *testing.T, stores *store.Stores, tenantID string) string { + t.Helper() + agentID := uuid.Must(uuid.NewV7()).String() + key := "sqla-" + agentID[:8] + stores.DB.Exec( + `INSERT INTO agents (id, agent_key, display_name, status, tenant_id, owner_id, model, provider) + VALUES (?, ?, 'B', 'active', ?, 'owner', 'gpt-4o', 'openai')`, + agentID, key, tenantID) + return agentID +} + +// seedSQLiteTeam creates a team with two members in the SQLite test DB. +// Returns teamID and the member agent's ID (ownerID is agentA). +func seedSQLiteTeam(t *testing.T, stores *store.Stores, tenantID, ownerAgentID string) (teamID, memberAgentID string) { + t.Helper() + teamID = uuid.Must(uuid.NewV7()).String() + memberAgentID = uuid.Must(uuid.NewV7()).String() + memberKey := "sqlm-" + memberAgentID[:8] + + stores.DB.Exec( + `INSERT INTO agents (id, agent_key, display_name, status, tenant_id, owner_id, model, provider) + VALUES (?, ?, 'M', 'active', ?, 'owner', 'gpt-4o', 'openai')`, + memberAgentID, memberKey, tenantID) + + stores.DB.Exec( + `INSERT INTO agent_teams (id, tenant_id, name, lead_agent_id, status, settings, created_by) + VALUES (?, ?, ?, ?, 'active', '{"version":2}', 'test')`, + teamID, tenantID, "test-team-"+teamID[:8], ownerAgentID) + + for _, m := range []struct{ id, role string }{{ownerAgentID, "lead"}, {memberAgentID, "member"}} { + stores.DB.Exec( + `INSERT INTO agent_team_members (team_id, agent_id, tenant_id, role) VALUES (?, ?, ?, ?)`, + teamID, m.id, tenantID, m.role) + } + return teamID, memberAgentID +} + +// sqliteListPaths returns sorted paths from Vault.ListDocuments. +func sqliteListPaths(t *testing.T, stores *store.Stores, ctx context.Context, tenantID, agentID string) []string { + t.Helper() + docs, err := stores.Vault.ListDocuments(ctx, tenantID, agentID, store.VaultListOptions{Limit: 100}) + if err != nil { + t.Fatalf("ListDocuments: %v", err) + } + paths := make([]string, 0, len(docs)) + for _, d := range docs { + paths = append(paths, d.Path) + } + sort.Strings(paths) + return paths +} + +// sqliteSearchPaths returns sorted paths from Vault.Search. +// query must be non-empty; SQLite Search returns nil on empty query. +func sqliteSearchPaths(t *testing.T, stores *store.Stores, ctx context.Context, tenantID, agentID, query string, teamID *string) []string { + t.Helper() + if query == "" { + t.Fatal("sqliteSearchPaths: query must be non-empty for SQLite LIKE search") + } + results, err := stores.Vault.Search(ctx, store.VaultSearchOptions{ + Query: query, + TenantID: tenantID, + AgentID: agentID, + TeamID: teamID, + MaxResults: 100, + }) + if err != nil { + t.Fatalf("Search(%q): %v", query, err) + } + paths := make([]string, 0, len(results)) + for _, r := range results { + paths = append(paths, r.Document.Path) + } + sort.Strings(paths) + return paths +} + +// sqliteTreePaths returns sorted paths from Vault.ListTreeEntries at the given root. +func sqliteTreePaths(t *testing.T, stores *store.Stores, ctx context.Context, tenantID string, opts store.VaultTreeOptions) []string { + t.Helper() + entries, err := stores.Vault.ListTreeEntries(ctx, tenantID, opts) + if err != nil { + t.Fatalf("ListTreeEntries: %v", err) + } + paths := make([]string, 0, len(entries)) + for _, e := range entries { + paths = append(paths, e.Path) + } + sort.Strings(paths) + return paths +} + +// sqliteContainsPath reports whether target is in paths. +func sqliteContainsPath(paths []string, target string) bool { + for _, p := range paths { + if p == target { + return true + } + } + return false +} + +// TestSQLiteVaultStore_VisibilityMatrix mirrors a subset of the PG matrix for SQLite. +// SQLite uses LIKE-based search (no FTS), so Search results may be narrower. +func TestSQLiteVaultStore_VisibilityMatrix(t *testing.T) { + stores := newSQLiteTestStores(t) + + tenantID := uuid.Must(uuid.NewV7()).String() + agentA := uuid.Must(uuid.NewV7()).String() + + // Seed tenant + agentA. + stores.DB.Exec(`INSERT INTO tenants (id, name, slug, status) VALUES (?, 'TenantA', 'ta', 'active')`, tenantID) + stores.DB.Exec( + `INSERT INTO agents (id, agent_key, display_name, status, tenant_id, owner_id, model, provider) + VALUES (?, 'matrix-a', 'A', 'active', ?, 'owner', 'gpt-4o', 'openai')`, + agentA, tenantID) + + agentB := seedSQLiteAgent(t, stores, tenantID) + teamID, teamMember := seedSQLiteTeam(t, stores, tenantID, agentA) + + ctx := store.WithTenantID(context.Background(), uuid.MustParse(tenantID)) + + const ( + pathPersonal = "sqmatrix/personal.md" + pathShared = "sqmatrix/shared.md" + pathTeam = "sqmatrix/team.md" + ) + + // Seed personal doc (agentA-owned). + if err := stores.Vault.UpsertDocument(ctx, &store.VaultDocument{ + TenantID: tenantID, AgentID: &agentA, Scope: "personal", + Path: pathPersonal, Title: "Personal A", DocType: "note", ContentHash: "sq-pa1", + Summary: "personal sqlite doc", + }); err != nil { + t.Fatalf("upsert personal: %v", err) + } + + // Seed shared doc (agent_id=NULL). + if err := stores.Vault.UpsertDocument(ctx, &store.VaultDocument{ + TenantID: tenantID, AgentID: nil, Scope: "shared", + Path: pathShared, Title: "Shared Doc", DocType: "note", ContentHash: "sq-sh1", + Summary: "shared sqlite doc", + }); err != nil { + t.Fatalf("upsert shared: %v", err) + } + + // Seed team doc (team_id set, agent_id=NULL). + if err := stores.Vault.UpsertDocument(ctx, &store.VaultDocument{ + TenantID: tenantID, AgentID: nil, TeamID: &teamID, Scope: "team", + Path: pathTeam, Title: "Team Doc", DocType: "note", ContentHash: "sq-tm1", + Summary: "team sqlite doc", + }); err != nil { + t.Fatalf("upsert team: %v", err) + } + + type matrixCase struct { + name string + getPaths func() []string + wantAll []string + wantNone []string + } + + cases := []matrixCase{ + { + name: "personal_visible_to_owner_via_list", + getPaths: func() []string { + return sqliteListPaths(t, stores, ctx, tenantID, agentA) + }, + wantAll: []string{pathPersonal}, + }, + { + name: "personal_invisible_to_other_agent_same_tenant", + getPaths: func() []string { + return sqliteListPaths(t, stores, ctx, tenantID, agentB) + }, + wantNone: []string{pathPersonal}, + }, + { + name: "shared_visible_to_owner_via_list", + getPaths: func() []string { + return sqliteListPaths(t, stores, ctx, tenantID, agentA) + }, + wantAll: []string{pathShared}, + }, + { + name: "shared_visible_to_other_agent_via_list", + getPaths: func() []string { + return sqliteListPaths(t, stores, ctx, tenantID, agentB) + }, + wantAll: []string{pathShared}, + }, + { + name: "shared_visible_via_list_tree_agentA", + getPaths: func() []string { + root := sqliteTreePaths(t, stores, ctx, tenantID, store.VaultTreeOptions{AgentID: agentA, Path: ""}) + children := sqliteTreePaths(t, stores, ctx, tenantID, store.VaultTreeOptions{AgentID: agentA, Path: "sqmatrix"}) + return append(root, children...) + }, + wantAll: []string{"sqmatrix/shared.md"}, + }, + { + name: "team_doc_visible_to_member_via_search", + getPaths: func() []string { + return sqliteSearchPaths(t, stores, ctx, tenantID, teamMember, "sqmatrix", &teamID) + }, + wantAll: []string{pathTeam}, + }, + { + // personal-only search (TeamID="") must not return team-scoped docs. + name: "team_doc_invisible_in_personal_only_search", + getPaths: func() []string { + personalOnly := "" + return sqliteSearchPaths(t, stores, ctx, tenantID, agentB, "sqmatrix", &personalOnly) + }, + wantNone: []string{pathTeam}, + }, + { + name: "shared_visible_to_team_member_with_teamID_via_search", + getPaths: func() []string { + return sqliteSearchPaths(t, stores, ctx, tenantID, teamMember, "sqmatrix", &teamID) + }, + wantAll: []string{pathShared}, + }, + { + name: "shared_visible_via_list_tree_with_teamID", + getPaths: func() []string { + root := sqliteTreePaths(t, stores, ctx, tenantID, store.VaultTreeOptions{AgentID: teamMember, TeamID: &teamID, Path: ""}) + children := sqliteTreePaths(t, stores, ctx, tenantID, store.VaultTreeOptions{AgentID: teamMember, TeamID: &teamID, Path: "sqmatrix"}) + return append(root, children...) + }, + wantAll: []string{"sqmatrix/shared.md"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := tc.getPaths() + t.Logf("paths: %v", got) + for _, want := range tc.wantAll { + if !sqliteContainsPath(got, want) { + t.Errorf("want %q in results %v", want, got) + } + } + for _, forbid := range tc.wantNone { + if sqliteContainsPath(got, forbid) { + t.Errorf("path %q must NOT appear in results %v", forbid, got) + } + } + }) + } +} + +// TestSQLiteVaultStore_ScopeCheckTrigger verifies the SQLite BEFORE INSERT trigger +// (trg_vault_docs_scope_consistency_ins) fires RAISE(ABORT) for invalid scope/ownership +// combinations and accepts valid ones. +func TestSQLiteVaultStore_ScopeCheckTrigger(t *testing.T) { + stores := newSQLiteTestStores(t) + + tenantID := uuid.Must(uuid.NewV7()).String() + agentID := uuid.Must(uuid.NewV7()).String() + suffix := uuid.Must(uuid.NewV7()).String()[:8] + + stores.DB.Exec(`INSERT INTO tenants (id, name, slug, status) VALUES (?, 'T', 'sc', 'active')`, tenantID) + stores.DB.Exec( + `INSERT INTO agents (id, agent_key, display_name, status, tenant_id, owner_id, model, provider) + VALUES (?, 'sc-agt', 'A', 'active', ?, 'owner', 'gpt-4o', 'openai')`, + agentID, tenantID) + + cases := []struct { + name string + query string + args []any + wantErr bool + }{ + { + name: "reject_personal_null_agent_id", + query: `INSERT INTO vault_documents (id, tenant_id, scope, path, title, doc_type, content_hash) + VALUES (?, ?, 'personal', ?, 'bad', 'note', 'h1')`, + args: []any{uuid.Must(uuid.NewV7()).String(), tenantID, "sc/bad-personal-" + suffix}, + wantErr: true, + }, + { + name: "reject_team_null_team_id", + query: `INSERT INTO vault_documents (id, tenant_id, scope, path, title, doc_type, content_hash) + VALUES (?, ?, 'team', ?, 'bad', 'note', 'h2')`, + args: []any{uuid.Must(uuid.NewV7()).String(), tenantID, "sc/bad-team-" + suffix}, + wantErr: true, + }, + { + name: "reject_shared_with_agent_id", + query: `INSERT INTO vault_documents (id, tenant_id, agent_id, scope, path, title, doc_type, content_hash) + VALUES (?, ?, ?, 'shared', ?, 'bad', 'note', 'h3')`, + args: []any{uuid.Must(uuid.NewV7()).String(), tenantID, agentID, "sc/bad-shared-" + suffix}, + wantErr: true, + }, + { + name: "accept_custom_scope_null_agent_id", + query: `INSERT INTO vault_documents (id, tenant_id, scope, path, title, doc_type, content_hash) + VALUES (?, ?, 'custom', ?, 'ok', 'note', 'h4')`, + args: []any{uuid.Must(uuid.NewV7()).String(), tenantID, "sc/ok-custom-" + suffix}, + wantErr: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := stores.DB.Exec(tc.query, tc.args...) + if tc.wantErr { + if err == nil { + t.Error("expected scope_consistency trigger error, got nil") + return + } + if !strings.Contains(err.Error(), "scope_consistency") { + t.Errorf("expected error containing 'scope_consistency', got: %v", err) + } + } else { + if err != nil { + t.Errorf("expected no error, got: %v", err) + } + } + }) + } +} diff --git a/tests/integration/v3_vault_scope_check_test.go b/tests/integration/v3_vault_scope_check_test.go new file mode 100644 index 00000000..c02c584d --- /dev/null +++ b/tests/integration/v3_vault_scope_check_test.go @@ -0,0 +1,81 @@ +//go:build integration + +package integration + +import ( + "testing" + + "github.com/google/uuid" +) + +// TestStoreVault_ScopeCheck verifies the vault_documents_scope_consistency CHECK constraint +// (added by migration 000055 NOT VALID) rejects invalid scope/ownership combinations on +// new inserts while accepting valid ones. +// +// NOT VALID means existing rows are not scanned but new writes are still gated — the CHECK +// fires on INSERT/UPDATE regardless of VALID status. +func TestStoreVault_ScopeCheck(t *testing.T) { + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + tid := tenantID.String() + aid := agentID.String() + + // unique path suffix per case to avoid conflicts with other test runs. + suffix := uuid.New().String()[:8] + + cases := []struct { + name string + query string + args []any + wantErr bool + }{ + { + name: "reject_personal_with_null_agent_id", + // scope='personal' requires agent_id NOT NULL + query: `INSERT INTO vault_documents + (id, tenant_id, scope, path, title, doc_type, content_hash) + VALUES ($1, $2, 'personal', $3, 'bad', 'note', 'h1')`, + args: []any{uuid.New().String(), tid, "scope-check/bad-personal-" + suffix + ".md"}, + wantErr: true, + }, + { + name: "reject_team_with_null_team_id", + // scope='team' requires team_id NOT NULL + query: `INSERT INTO vault_documents + (id, tenant_id, scope, path, title, doc_type, content_hash) + VALUES ($1, $2, 'team', $3, 'bad', 'note', 'h2')`, + args: []any{uuid.New().String(), tid, "scope-check/bad-team-" + suffix + ".md"}, + wantErr: true, + }, + { + name: "reject_shared_with_non_null_agent_id", + // scope='shared' requires agent_id IS NULL + query: `INSERT INTO vault_documents + (id, tenant_id, agent_id, scope, path, title, doc_type, content_hash) + VALUES ($1, $2, $3, 'shared', $4, 'bad', 'note', 'h3')`, + args: []any{uuid.New().String(), tid, aid, "scope-check/bad-shared-" + suffix + ".md"}, + wantErr: true, + }, + { + name: "accept_custom_scope_with_null_agent_id", + // scope='custom' has no constraint — should succeed + query: `INSERT INTO vault_documents + (id, tenant_id, scope, path, title, doc_type, content_hash) + VALUES ($1, $2, 'custom', $3, 'ok', 'note', 'h4')`, + args: []any{uuid.New().String(), tid, "scope-check/ok-custom-" + suffix + ".md"}, + wantErr: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := db.Exec(tc.query, tc.args...) + if tc.wantErr && err == nil { + t.Errorf("expected constraint violation error, got nil") + } + if !tc.wantErr && err != nil { + t.Errorf("expected no error, got: %v", err) + } + }) + } +} diff --git a/tests/integration/v3_vault_store_ext_test.go b/tests/integration/v3_vault_store_ext_test.go index 17ab2e3e..7b74e5cf 100644 --- a/tests/integration/v3_vault_store_ext_test.go +++ b/tests/integration/v3_vault_store_ext_test.go @@ -3,8 +3,10 @@ package integration import ( + "context" "database/sql" "errors" + "sort" "testing" "github.com/google/uuid" @@ -355,3 +357,365 @@ func TestStoreVault_ListDocuments_Pagination(t *testing.T) { } }) } + +// ---- Characterization tests: shared doc visibility (RED on unpatched code) ---- + +// TestStoreVault_List_IncludesShared asserts ListDocuments returns shared (agent_id=NULL) docs +// alongside personal docs for the same tenant+agent. +// BUG: currently fails — strict "AND agent_id = $N" excludes NULL rows. +func TestStoreVault_List_IncludesShared(t *testing.T) { + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + ctx := tenantCtx(tenantID) + vs := newVaultStore(db) + + tid := tenantID.String() + aid := agentID.String() + + // Seed personal doc (agent_id = A1). + personal := makeVaultDoc(tid, aid, "agents/a1/alpha.md", "personal alpha doc") + personal.Summary = "personal alpha text" + if err := vs.UpsertDocument(ctx, personal); err != nil { + t.Fatalf("upsert personal: %v", err) + } + + // Seed shared doc (agent_id = NULL). + shared := makeSharedVaultDoc(tid, "shared/alpha.md", "shared alpha doc") + shared.Summary = "shared alpha text" + if err := vs.UpsertDocument(ctx, shared); err != nil { + t.Fatalf("upsert shared: %v", err) + } + + docs, err := vs.ListDocuments(ctx, tid, aid, store.VaultListOptions{Limit: 50}) + if err != nil { + t.Fatalf("ListDocuments: %v", err) + } + + paths := make([]string, 0, len(docs)) + for _, d := range docs { + paths = append(paths, d.Path) + } + sort.Strings(paths) + t.Logf("ListDocuments returned %d docs: %v", len(docs), paths) + + foundShared := false + foundPersonal := false + for _, d := range docs { + if d.Path == "shared/alpha.md" { + foundShared = true + } + if d.Path == "agents/a1/alpha.md" { + foundPersonal = true + } + } + + if !foundPersonal { + t.Error("ListDocuments: personal doc agents/a1/alpha.md not found") + } + if !foundShared { + t.Errorf("ListDocuments: shared doc shared/alpha.md not found — BUG: shared docs excluded (agent_id=NULL filtered out)") + } +} + +// TestStoreVault_Count_IncludesShared asserts CountDocuments counts shared docs. +// BUG: currently fails — shared docs excluded from count. +func TestStoreVault_Count_IncludesShared(t *testing.T) { + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + ctx := tenantCtx(tenantID) + vs := newVaultStore(db) + + tid := tenantID.String() + aid := agentID.String() + + personal := makeVaultDoc(tid, aid, "agents/a1/cnt-alpha.md", "personal alpha doc") + personal.Summary = "personal alpha text" + if err := vs.UpsertDocument(ctx, personal); err != nil { + t.Fatalf("upsert personal: %v", err) + } + + shared := makeSharedVaultDoc(tid, "shared/cnt-alpha.md", "shared alpha doc") + shared.Summary = "shared alpha text" + if err := vs.UpsertDocument(ctx, shared); err != nil { + t.Fatalf("upsert shared: %v", err) + } + + n, err := vs.CountDocuments(ctx, tid, aid, store.VaultListOptions{}) + if err != nil { + t.Fatalf("CountDocuments: %v", err) + } + t.Logf("CountDocuments = %d", n) + + if n < 2 { + t.Errorf("CountDocuments = %d, want >= 2 — BUG: shared docs (agent_id=NULL) excluded from count", n) + } +} + +// TestStoreVault_Search_SharedDocsVisible asserts Search (FTS) returns shared docs. +// BUG: currently fails — "AND agent_id = $N" in ftsSearch excludes NULL rows. +func TestStoreVault_Search_SharedDocsVisible(t *testing.T) { + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + ctx := tenantCtx(tenantID) + vs := newVaultStore(db) + + tid := tenantID.String() + aid := agentID.String() + + personal := makeVaultDoc(tid, aid, "agents/a1/srch-alpha.md", "personal alpha doc") + personal.Summary = "personal alpha text" + if err := vs.UpsertDocument(ctx, personal); err != nil { + t.Fatalf("upsert personal: %v", err) + } + + shared := makeSharedVaultDoc(tid, "shared/srch-alpha.md", "shared alpha doc") + shared.Summary = "shared alpha text" + if err := vs.UpsertDocument(ctx, shared); err != nil { + t.Fatalf("upsert shared: %v", err) + } + + results, err := vs.Search(ctx, store.VaultSearchOptions{ + Query: "alpha", + TenantID: tid, + AgentID: aid, + MaxResults: 20, + }) + if err != nil { + t.Fatalf("Search: %v", err) + } + + paths := make([]string, 0, len(results)) + for _, r := range results { + paths = append(paths, r.Document.Path) + } + sort.Strings(paths) + t.Logf("Search returned %d results: %v", len(results), paths) + + foundShared := false + foundPersonal := false + for _, r := range results { + if r.Document.Path == "shared/srch-alpha.md" { + foundShared = true + } + if r.Document.Path == "agents/a1/srch-alpha.md" { + foundPersonal = true + } + } + + if !foundPersonal { + t.Error("Search: personal doc agents/a1/srch-alpha.md not found") + } + if !foundShared { + t.Errorf("Search: shared doc shared/srch-alpha.md not found — BUG: shared docs (agent_id=NULL) excluded from FTS results") + } +} + +// TestStoreVault_Tree_SharedDocsVisible asserts ListTreeEntries includes shared docs. +// BUG: currently fails — appendTreeFilters adds "AND agent_id = $N" (PG line 774). +func TestStoreVault_Tree_SharedDocsVisible(t *testing.T) { + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + ctx := tenantCtx(tenantID) + vs := newVaultStore(db) + + tid := tenantID.String() + aid := agentID.String() + + personal := makeVaultDoc(tid, aid, "agents/a1/tree-alpha.md", "personal alpha doc") + personal.Summary = "personal alpha text" + if err := vs.UpsertDocument(ctx, personal); err != nil { + t.Fatalf("upsert personal: %v", err) + } + + shared := makeSharedVaultDoc(tid, "shared/tree-alpha.md", "shared alpha doc") + shared.Summary = "shared alpha text" + if err := vs.UpsertDocument(ctx, shared); err != nil { + t.Fatalf("upsert shared: %v", err) + } + + entries, err := vs.ListTreeEntries(ctx, tid, store.VaultTreeOptions{AgentID: aid, Path: ""}) + if err != nil { + t.Fatalf("ListTreeEntries: %v", err) + } + + names := make([]string, 0, len(entries)) + for _, e := range entries { + names = append(names, e.Path) + } + sort.Strings(names) + t.Logf("ListTreeEntries returned %d entries: %v", len(entries), names) + + foundSharedFolder := false + for _, e := range entries { + // The shared doc is at shared/tree-alpha.md; at root level we expect "shared" folder entry. + if e.Path == "shared" || e.Path == "shared/tree-alpha.md" { + foundSharedFolder = true + } + } + + if !foundSharedFolder { + t.Errorf("ListTreeEntries: 'shared' folder/file not found — BUG: shared docs (agent_id=NULL) excluded from tree (appendTreeFilters strict agent filter)") + } +} + +// TestStoreVault_Search_TeamAgentSeesShared asserts that a team-context Search +// returns shared docs in addition to personal + team docs. +// BUG: currently fails — searchTeamFilter.append uses strict "AND team_id = $N" +// when teamID is set, which excludes shared docs (team_id=NULL, agent_id=NULL). +// NOTE: Search uses opts.TeamID directly (not RunContext). +func TestStoreVault_Search_TeamAgentSeesShared(t *testing.T) { + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + ctx := tenantCtx(tenantID) + vs := newVaultStore(db) + + tid := tenantID.String() + aid := agentID.String() + + teamID, _ := seedTeam(t, db, tenantID, agentID) + teamIDStr := teamID.String() + + // Personal doc (agent_id = A1, team_id = NULL). + personal := makeVaultDoc(tid, aid, "agents/a1/team-alpha.md", "personal alpha doc") + personal.Summary = "personal alpha text" + if err := vs.UpsertDocument(ctx, personal); err != nil { + t.Fatalf("upsert personal: %v", err) + } + + // Team doc (agent_id = NULL, team_id = T1). + team := makeTeamVaultDoc(tid, teamIDStr, "teams/t1/team-alpha.md", "team alpha doc") + team.Summary = "team alpha text" + if err := vs.UpsertDocument(ctx, team); err != nil { + t.Fatalf("upsert team: %v", err) + } + + // Shared doc (agent_id = NULL, team_id = NULL). + shared := makeSharedVaultDoc(tid, "shared/team-alpha.md", "shared alpha doc") + shared.Summary = "shared alpha text" + if err := vs.UpsertDocument(ctx, shared); err != nil { + t.Fatalf("upsert shared: %v", err) + } + + results, err := vs.Search(ctx, store.VaultSearchOptions{ + Query: "alpha", + TenantID: tid, + AgentID: aid, + TeamID: &teamIDStr, + MaxResults: 20, + }) + if err != nil { + t.Fatalf("Search with TeamID: %v", err) + } + + paths := make([]string, 0, len(results)) + for _, r := range results { + paths = append(paths, r.Document.Path) + } + sort.Strings(paths) + t.Logf("Search (TeamID=%s) returned %d results: %v", teamIDStr, len(results), paths) + + foundPersonal := false + foundTeam := false + foundShared := false + for _, r := range results { + switch r.Document.Path { + case "agents/a1/team-alpha.md": + foundPersonal = true + case "teams/t1/team-alpha.md": + foundTeam = true + case "shared/team-alpha.md": + foundShared = true + } + } + + if !foundPersonal { + t.Error("Search: personal doc agents/a1/team-alpha.md not found") + } + if !foundTeam { + t.Error("Search: team doc teams/t1/team-alpha.md not found") + } + if !foundShared { + t.Errorf("Search: shared doc shared/team-alpha.md not found — BUG: strict team_id filter excludes shared docs (team_id=NULL, agent_id=NULL)") + } +} + +// ---- Guard tests (must PASS now and stay green through all phases) ---- + +// TestStoreVault_Delete_RemainsStrict asserts that DeleteDocument with a specific agentID +// does NOT delete shared docs (agent_id=NULL) — the strict agent filter protects shared data. +func TestStoreVault_Delete_RemainsStrict(t *testing.T) { + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + ctx := tenantCtx(tenantID) + vs := newVaultStore(db) + + tid := tenantID.String() + aid := agentID.String() + + shared := makeSharedVaultDoc(tid, "shared/del-guard.md", "shared alpha doc") + if err := vs.UpsertDocument(ctx, shared); err != nil { + t.Fatalf("upsert shared: %v", err) + } + + // Attempt to delete the shared doc using agentID — should NOT match (agent_id=NULL ≠ agentID). + if err := vs.DeleteDocument(ctx, tid, aid, "shared/del-guard.md"); err != nil { + t.Fatalf("DeleteDocument: %v", err) + } + + // Verify shared doc still exists via raw SQL (bypasses store agent filter). + var count int + err := db.QueryRowContext(ctx, + `SELECT COUNT(*) FROM vault_documents WHERE tenant_id = $1 AND path = $2 AND agent_id IS NULL`, + tenantID, "shared/del-guard.md").Scan(&count) + if err != nil { + t.Fatalf("raw count query: %v", err) + } + if count != 1 { + t.Errorf("shared doc deleted by agent-scoped DeleteDocument — guard broken (count=%d, want 1)", count) + } +} + +// TestStoreVault_Search_CrossTenantIsolation asserts that shared docs from tenant A +// are NOT visible to a search from tenant B. +func TestStoreVault_Search_CrossTenantIsolation(t *testing.T) { + db := testDB(t) + tenantA, agentA := seedTenantAgent(t, db) + tenantB, agentB := seedTenantAgent(t, db) + ctxA := tenantCtx(tenantA) + vs := newVaultStore(db) + + tidA := tenantA.String() + tidB := tenantB.String() + aidA := agentA.String() + aidB := agentB.String() + + // Seed shared doc in tenant A. + shared := makeSharedVaultDoc(tidA, "shared/iso-alpha.md", "shared alpha doc") + shared.Summary = "shared alpha text" + if err := vs.UpsertDocument(ctxA, shared); err != nil { + t.Fatalf("upsert shared tenantA: %v", err) + } + + // Search from tenant B — must return 0 results for "alpha". + results, err := vs.Search(context.Background(), store.VaultSearchOptions{ + Query: "alpha", + TenantID: tidB, + AgentID: aidB, + MaxResults: 20, + }) + if err != nil { + t.Fatalf("Search tenantB: %v", err) + } + + t.Logf("Cross-tenant search returned %d results (want 0)", len(results)) + + // Filter to only shared paths to catch leaks. + for _, r := range results { + if r.Document.TenantID == tidA { + t.Errorf("cross-tenant isolation broken: tenant B search returned tenant A doc %q", r.Document.Path) + } + } + + // Confirm agent A doc is isolated as a sanity baseline. + _ = aidA +} diff --git a/tests/integration/v3_vault_store_test.go b/tests/integration/v3_vault_store_test.go index 4571f2dc..8c5782be 100644 --- a/tests/integration/v3_vault_store_test.go +++ b/tests/integration/v3_vault_store_test.go @@ -30,6 +30,35 @@ func makeVaultDoc(tenantID, agentID, path, title string) *store.VaultDocument { } } +// makeSharedVaultDoc builds a shared (agent_id=NULL, scope='shared') vault document. +// Used to seed characterization tests that verify shared docs are visible to agents. +func makeSharedVaultDoc(tenantID, path, title string) *store.VaultDocument { + return &store.VaultDocument{ + TenantID: tenantID, + AgentID: nil, // NULL — shared doc + TeamID: nil, + Scope: "shared", + Path: path, + Title: title, + DocType: "note", + ContentHash: "abc123", + } +} + +// makeTeamVaultDoc builds a team-scoped vault document (agent_id=NULL, team_id set, scope='team'). +func makeTeamVaultDoc(tenantID, teamID, path, title string) *store.VaultDocument { + return &store.VaultDocument{ + TenantID: tenantID, + AgentID: nil, // NULL for team docs + TeamID: &teamID, + Scope: "team", + Path: path, + Title: title, + DocType: "note", + ContentHash: "abc123", + } +} + func TestStoreVault_UpsertAndGetDocument(t *testing.T) { db := testDB(t) tenantID, agentID := seedTenantAgent(t, db) diff --git a/tests/integration/v3_vault_visibility_matrix_test.go b/tests/integration/v3_vault_visibility_matrix_test.go new file mode 100644 index 00000000..5e591d46 --- /dev/null +++ b/tests/integration/v3_vault_visibility_matrix_test.go @@ -0,0 +1,266 @@ +//go:build integration + +package integration + +import ( + "context" + "database/sql" + "sort" + "testing" + + "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// seedAgentInTenant inserts an agent into an existing tenant for same-tenant multi-agent tests. +func seedAgentInTenant(t *testing.T, db *sql.DB, tenantID uuid.UUID) uuid.UUID { + t.Helper() + agentID := uuid.New() + key := "ta-" + agentID.String()[:8] + _, err := db.Exec( + `INSERT INTO agents (id, tenant_id, agent_key, agent_type, status, provider, model, owner_id) + VALUES ($1, $2, $3, 'predefined', 'active', 'test', 'test-model', 'test-owner') + ON CONFLICT DO NOTHING`, + agentID, tenantID, key) + if err != nil { + t.Fatalf("seedAgentInTenant: %v", err) + } + t.Cleanup(func() { + db.Exec("DELETE FROM agents WHERE id = $1", agentID) + }) + return agentID +} + +// vaultListPaths calls ListDocuments and returns sorted paths. +func vaultListPaths(t *testing.T, vs interface { + ListDocuments(ctx context.Context, tenantID, agentID string, opts store.VaultListOptions) ([]store.VaultDocument, error) +}, ctx context.Context, tenantID, agentID string) []string { + t.Helper() + docs, err := vs.ListDocuments(ctx, tenantID, agentID, store.VaultListOptions{Limit: 100}) + if err != nil { + t.Fatalf("ListDocuments(%s): %v", agentID, err) + } + paths := make([]string, 0, len(docs)) + for _, d := range docs { + paths = append(paths, d.Path) + } + sort.Strings(paths) + return paths +} + +// vaultSearchPaths calls Search with optional teamID and returns sorted paths. +func vaultSearchPaths(t *testing.T, vs interface { + Search(ctx context.Context, opts store.VaultSearchOptions) ([]store.VaultSearchResult, error) +}, ctx context.Context, tenantID, agentID string, teamID *string) []string { + t.Helper() + results, err := vs.Search(ctx, store.VaultSearchOptions{ + TenantID: tenantID, + AgentID: agentID, + TeamID: teamID, + MaxResults: 100, + }) + if err != nil { + t.Fatalf("Search(%s): %v", agentID, err) + } + paths := make([]string, 0, len(results)) + for _, r := range results { + paths = append(paths, r.Document.Path) + } + sort.Strings(paths) + return paths +} + +// vaultTreeRootPaths calls ListTreeEntries at root and returns sorted entry paths. +func vaultTreeRootPaths(t *testing.T, vs interface { + ListTreeEntries(ctx context.Context, tenantID string, opts store.VaultTreeOptions) ([]store.VaultTreeEntry, error) +}, ctx context.Context, tenantID string, opts store.VaultTreeOptions) []string { + t.Helper() + entries, err := vs.ListTreeEntries(ctx, tenantID, opts) + if err != nil { + t.Fatalf("ListTreeEntries: %v", err) + } + paths := make([]string, 0, len(entries)) + for _, e := range entries { + paths = append(paths, e.Path) + } + sort.Strings(paths) + return paths +} + +// containsPath reports whether target appears in the sorted paths slice. +func containsPath(paths []string, target string) bool { + for _, p := range paths { + if p == target { + return true + } + } + return false +} + +// TestStoreVault_VisibilityMatrix covers 9 matrix rows: +// personal/shared/team visibility by owner, other-agent, cross-tenant, team-member, non-member. +func TestStoreVault_VisibilityMatrix(t *testing.T) { + db := testDB(t) + vs := newVaultStore(db) + + // Tenant A: agentA (owner), agentBinA (same-tenant non-team), teamMember. + tenantA, agentA := seedTenantAgent(t, db) + agentBinA := seedAgentInTenant(t, db, tenantA) + teamID, teamMember := seedTeam(t, db, tenantA, agentA) + + // Tenant X: cross-tenant viewer. + tenantX, agentX := seedTenantAgent(t, db) + + tidA := tenantA.String() + tidX := tenantX.String() + aidA := agentA.String() + aidB := agentBinA.String() + aidMember := teamMember.String() + aidX := agentX.String() + teamIDStr := teamID.String() + + ctxA := tenantCtx(tenantA) + ctxX := tenantCtx(tenantX) + + // Seed docs in tenantA. + const ( + pathPersonal = "matrix/personal.md" + pathShared = "matrix/shared.md" + pathTeam = "matrix/team.md" + ) + + personal := makeVaultDoc(tidA, aidA, pathPersonal, "Personal A") + personal.Summary = "personal owner doc" + if err := vs.UpsertDocument(ctxA, personal); err != nil { + t.Fatalf("upsert personal: %v", err) + } + + shared := makeSharedVaultDoc(tidA, pathShared, "Shared Doc") + shared.Summary = "shared tenant doc" + if err := vs.UpsertDocument(ctxA, shared); err != nil { + t.Fatalf("upsert shared: %v", err) + } + + team := makeTeamVaultDoc(tidA, teamIDStr, pathTeam, "Team Doc") + team.Summary = "team scoped doc" + if err := vs.UpsertDocument(ctxA, team); err != nil { + t.Fatalf("upsert team: %v", err) + } + + type matrixCase struct { + name string + getPaths func() []string + wantAll []string // all must appear + wantNone []string // none must appear + } + + cases := []matrixCase{ + { + // Row 1: personal doc visible to its owner via ListDocuments. + name: "personal_visible_to_owner", + getPaths: func() []string { + return vaultListPaths(t, vs, ctxA, tidA, aidA) + }, + wantAll: []string{pathPersonal}, + }, + { + // Row 2: personal doc NOT visible to another agent in same tenant. + name: "personal_invisible_to_other_agent_same_tenant", + getPaths: func() []string { + return vaultListPaths(t, vs, ctxA, tidA, aidB) + }, + wantNone: []string{pathPersonal}, + }, + { + // Row 3: shared doc visible to any agent in tenant via ListDocuments. + name: "shared_visible_to_any_agent_via_list", + getPaths: func() []string { + return vaultListPaths(t, vs, ctxA, tidA, aidA) + }, + wantAll: []string{pathShared}, + }, + { + // Row 4: shared doc visible via ListTreeEntries(agentID=A). + name: "shared_visible_via_list_tree_agentA", + getPaths: func() []string { + // ListTreeEntries returns top-level entries; "matrix" folder should appear. + paths := vaultTreeRootPaths(t, vs, ctxA, tidA, store.VaultTreeOptions{AgentID: aidA, Path: ""}) + // Drill into "matrix" to check shared.md is there. + children := vaultTreeRootPaths(t, vs, ctxA, tidA, store.VaultTreeOptions{AgentID: aidA, Path: "matrix"}) + return append(paths, children...) + }, + wantAll: []string{"matrix/shared.md"}, + }, + { + // Row 5: shared doc from tenantA NOT visible in cross-tenant search. + name: "shared_invisible_cross_tenant", + getPaths: func() []string { + return vaultSearchPaths(t, vs, ctxX, tidX, aidX, nil) + }, + wantNone: []string{pathShared, pathPersonal, pathTeam}, + }, + { + // Row 6: team doc visible to a member of that team (Search with TeamID). + name: "team_doc_visible_to_member", + getPaths: func() []string { + return vaultSearchPaths(t, vs, ctxA, tidA, aidMember, &teamIDStr) + }, + wantAll: []string{pathTeam}, + }, + { + // Row 7: team doc NOT visible in personal-only search (TeamID="", means team_id IS NULL). + // The store layer filters by team_id when a TeamID is provided; passing an empty string + // means "personal only" (team_id IS NULL). Membership enforcement is a handler concern. + name: "team_doc_invisible_in_personal_only_search", + getPaths: func() []string { + personalOnly := "" + return vaultSearchPaths(t, vs, ctxA, tidA, aidB, &personalOnly) + }, + wantNone: []string{pathTeam}, + }, + { + // Row 8: shared doc visible to team member when RunContext.TeamID=T. + name: "shared_visible_to_team_member_with_teamID", + getPaths: func() []string { + return vaultSearchPaths(t, vs, ctxA, tidA, aidMember, &teamIDStr) + }, + wantAll: []string{pathShared}, + }, + { + // Row 9: shared doc visible via ListTreeEntries with TeamID=T. + name: "shared_visible_via_list_tree_with_teamID", + getPaths: func() []string { + paths := vaultTreeRootPaths(t, vs, ctxA, tidA, store.VaultTreeOptions{ + AgentID: aidMember, + TeamID: &teamIDStr, + Path: "", + }) + children := vaultTreeRootPaths(t, vs, ctxA, tidA, store.VaultTreeOptions{ + AgentID: aidMember, + TeamID: &teamIDStr, + Path: "matrix", + }) + return append(paths, children...) + }, + wantAll: []string{"matrix/shared.md"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := tc.getPaths() + t.Logf("paths: %v", got) + + for _, want := range tc.wantAll { + if !containsPath(got, want) { + t.Errorf("want %q in results %v", want, got) + } + } + for _, forbid := range tc.wantNone { + if containsPath(got, forbid) { + t.Errorf("path %q must NOT appear in results %v", forbid, got) + } + } + }) + } +} diff --git a/tests/invariants/tenant_isolation_test.go b/tests/invariants/tenant_isolation_test.go index 5e5fcecd..85f1b18f 100644 --- a/tests/invariants/tenant_isolation_test.go +++ b/tests/invariants/tenant_isolation_test.go @@ -341,7 +341,7 @@ func TestTenantIsolation_VaultStore(t *testing.T) { docID := uuid.New() _, err := db.Exec( `INSERT INTO vault_documents (id, tenant_id, title, path, doc_type, content_hash, scope, created_at, updated_at) - VALUES ($1, $2, 'secret-doc', '/tmp/secret.md', 'document', 'abc123', 'personal', NOW(), NOW())`, + VALUES ($1, $2, 'secret-doc', '/tmp/secret.md', 'document', 'abc123', 'custom', NOW(), NOW())`, docID, tenantA) if err != nil { t.Fatalf("create vault doc: %v", err)