From 7803ba8adfa4eae1c6289aa53d8777805054ec60 Mon Sep 17 00:00:00 2001 From: xcaliber Date: Fri, 13 Mar 2026 14:36:43 +0000 Subject: [PATCH] Changeset 0.28.2.1 (#184) --- docs/ICD/knowledge.md | 144 ++++++++-- docs/ROADMAP.md | 19 ++ server/handlers/integration_test.go | 322 ++++++++++++++++++++++- server/handlers/knowledge_bases.go | 38 ++- server/store/interfaces.go | 2 - server/store/postgres/knowledge_bases.go | 14 - server/store/sqlite/knowledge_bases.go | 14 - 7 files changed, 491 insertions(+), 62 deletions(-) diff --git a/docs/ICD/knowledge.md b/docs/ICD/knowledge.md index 29cd131..a0093ab 100644 --- a/docs/ICD/knowledge.md +++ b/docs/ICD/knowledge.md @@ -7,14 +7,25 @@ The **Knowledge Base** (KB) is the RAG system: upload documents → chunk ### KB CRUD ``` -GET /knowledge-bases → { "data": [KB objects] } +GET /knowledge-bases → { "data": [KB objects], "total": N } POST /knowledge-bases ← { "name", "description", "scope", "team_id" } GET /knowledge-bases/:id → KB object -PUT /knowledge-bases/:id ← partial update +PUT /knowledge-bases/:id ← partial update { "name", "description" } DELETE /knowledge-bases/:id ``` -KB object: +**Auth:** All endpoints require a valid JWT. `POST` additionally requires +the `kb.create` permission (checked via `RequirePermission` middleware). + +**Scope rules for `POST`:** + +| Scope | Requirement | +|-------|-------------| +| `personal` (default) | Any authenticated user with `kb.create` | +| `team` | `team_id` required; caller must be team admin (or system admin) | +| `global` | Caller must be system admin | + +KB object (as returned by CRUD endpoints): ```json { @@ -24,16 +35,21 @@ KB object: "scope": "personal|team|global", "owner_id": "uuid|null", "team_id": "uuid|null", - "discoverable": true, - "embedding_model": "text-embedding-3-small", - "chunk_size": 512, - "chunk_overlap": 50, + "embedding_config": {}, "document_count": 12, - "created_at": "...", - "updated_at": "..." + "chunk_count": 48, + "total_bytes": 245760, + "status": "active|processing|error", + "created_at": "2025-01-15T10:30:00Z", + "updated_at": "2025-01-15T10:30:00Z" } ``` +> **Note:** The `discoverable` field is stored in the DB and present on +> the raw model, but is **not** included in the `kbResponse` struct +> returned by CRUD endpoints. It is managed via the dedicated +> discoverable endpoints below. + ### Documents **Upload:** @@ -43,8 +59,31 @@ POST /knowledge-bases/:id/documents Content-Type: multipart/form-data ``` -Field: `file`. Supported: PDF, DOCX, TXT, MD, HTML, CSV, XLSX, PPTX. -Returns the document object with `status: "pending"`. +**Auth:** Requires `kb.write` permission (checked via `RequirePermission` +middleware). Also requires a configured embedding model role — returns +`412 Precondition Failed` if missing. + +Field: `file` (max 10 MB). Currently supported types: **TXT, MD, CSV, +HTML**. Binary formats (PDF, DOCX, XLSX, PPTX) are planned but require +an extraction sidecar that is not yet wired in. + +Returns the document object with HTTP `202 Accepted`: + +```json +{ + "id": "uuid", + "kb_id": "uuid", + "filename": "guide.md", + "content_type": "text/markdown", + "size_bytes": 12345, + "chunk_count": 0, + "status": "pending", + "error": null, + "uploaded_by": "uuid", + "created_at": "2025-01-15T10:30:00Z", + "updated_at": "2025-01-15T10:30:00Z" +} +``` **List:** @@ -52,7 +91,7 @@ Returns the document object with `status: "pending"`. GET /knowledge-bases/:id/documents ``` -Returns `{ "data": [document objects] }`. +Returns `{ "data": [document objects], "total": N }`. **Status** (poll during processing): @@ -60,7 +99,19 @@ Returns `{ "data": [document objects] }`. GET /knowledge-bases/:id/documents/:docId/status ``` -Status progression: `pending → chunking → embedding → ready | error`. +Returns a subset of the document object: + +```json +{ + "id": "uuid", + "status": "chunking", + "chunk_count": 0, + "error": null, + "filename": "guide.md" +} +``` + +Status progression: `pending → extracting → chunking → embedding → ready | error`. **Delete:** @@ -68,6 +119,9 @@ Status progression: `pending → chunking → embedding → ready | error`. DELETE /knowledge-bases/:id/documents/:docId ``` +Deletes the document, its chunks, and the stored file. Returns +`{ "deleted": true }`. + ### Search ``` @@ -77,20 +131,55 @@ POST /knowledge-bases/:id/search ```json { "query": "How do I configure SSO?", - "limit": 5 + "limit": 5, + "threshold": 0.3 } ``` -Returns `{ "results": [{ "content", "score", "document_id", "metadata" }] }`. +`limit` defaults to 5, max 20. `threshold` is the minimum cosine +similarity score (defaults to 0.3). + +Returns `{ "data": [...], "query": "...", "total": N }`: + +```json +{ + "data": [ + { + "content": "To configure SSO, navigate to...", + "source": "admin-guide.md", + "kb": "Product Docs", + "similarity": 0.87, + "metadata": {} + } + ], + "query": "How do I configure SSO?", + "total": 1 +} +``` ### Rebuild -Re-chunks and re-embeds all documents: +Re-chunks and re-embeds all documents. Skips documents whose extracted +text is missing (would need the original file from object store). ``` POST /knowledge-bases/:id/rebuild ``` +Returns `202 Accepted`: + +```json +{ + "message": "rebuild started", + "total_docs": 12, + "rebuilding": 10, + "skipped": 2 +} +``` + +Returns `503` if the ingestion pipeline is not configured, or `400` if +the KB has no documents. + ### Channel KB Bindings A channel can have KBs explicitly bound to it (in addition to any KBs @@ -123,6 +212,9 @@ PUT /channels/:id/knowledge-bases { "kb_ids": ["kb-uuid-1", "kb-uuid-2"] } ``` +Validates that the caller has access to every referenced KB before +setting the bindings. + ### Discoverable KBs The `discoverable` flag controls whether a KB appears in the user's KB @@ -136,9 +228,10 @@ GET /knowledge-bases-discoverable ``` Returns `{ "data": [KB objects] }` — only KBs the user can see -(personal + team + discoverable global). +(personal + team + discoverable global). Uses the same `kbResponse` +format as the CRUD endpoints. -**Toggle discoverability** (admin-enforced in handler): +**Toggle discoverability:** ``` PUT /knowledge-bases/:id/discoverable @@ -148,6 +241,10 @@ PUT /knowledge-bases/:id/discoverable { "discoverable": false } ``` +**Auth:** Requires `loadAndAuthorize` (KB must exist, caller must have +basic access). Additionally, only the KB **owner** or a **system admin** +may change the flag — all other callers receive `403 Forbidden`. + ### KB Resolution Chain When a completion fires, KBs are resolved in priority order: @@ -158,10 +255,17 @@ Persona KBs → Project KBs → Channel KBs → Personal KBs The `BuildKBHint` function assembles all applicable KBs into the system prompt, and the `kb_search` tool searches across all of them. +The `kb_search` tool additionally resolves project-bound KBs and +personal KBs at search time, so all user-accessible KBs are +searchable even if not explicitly listed in the hint. ### Persona KB Bindings -See §4.4. Persona-KB binding is the primary enterprise mechanism for -controlled KB access. +Managed via the Persona endpoints — see [personas.md](personas.md). +Persona-KB binding is the primary enterprise mechanism for controlled +KB access. The `auto_search` flag on persona KB bindings controls +whether top-K results are prepended to context automatically (planned, +see v0.28.4) vs available only through the `kb_search` tool (current +behavior). --- diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index a83d788..62cc250 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -84,6 +84,25 @@ Known ICD report failures to investigate (likely envelope shape mismatches): - [x] Remove dead `NotifTypeProjectInvite` constant - [ ] Notification handler + store tests (PG + SQLite) +**Knowledge ICD audit (cs1–cs4):** +- [x] `knowledge.md` corrected: KB object shape (6 field mismatches), search envelope + (`data` not `results`), search result fields, status progression (`extracting` + step), file type support (text-only, not binary), auth annotations +- [x] Test harness: add `RequirePermission` on `POST /knowledge-bases` and + `POST /:id/documents`, add missing `GET /:id/documents/:docId/status` and + `DELETE /:id/documents/:docId` routes +- [x] New tests: document status polling, document delete, update-empty-body 400, + permission denial for non-privileged user +- [x] P0 fix: `SetDiscoverable` authorization — `loadAndAuthorize` + owner/admin + check, audit log, cross-user security tests (cs2) +- [x] `ListDiscoverableKBs` response normalization — use `toKBResponse()`, + shape assertion test (cs3) +- [x] Dead code removal: `ListGlobal`/`ListForTeam` on KnowledgeBaseStore — + interface + PG + SQLite (cs4) +- [x] Move `CreateKB` team role check from raw `database.DB.QueryRow` to + `stores.Teams.IsTeamAdmin`; remove `database` import (cs4) +- [x] Team-scoped KB creation test: member denied, team admin succeeds (cs4) + ### v0.28.3 — Security Tier (ICD Runner Red Team) New `security` tier in ICD test runner. Multi-user fixtures already exist. diff --git a/server/handlers/integration_test.go b/server/handlers/integration_test.go index c30b5b8..14b4b35 100644 --- a/server/handlers/integration_test.go +++ b/server/handlers/integration_test.go @@ -289,13 +289,15 @@ func setupHarness(t *testing.T) *testHarness { // Knowledge Bases (nil storage/ingester/embedder — CRUD works, upload/rebuild return 503) kbH := NewKnowledgeBaseHandler(stores, nil, nil, nil) - protected.POST("/knowledge-bases", kbH.CreateKB) + protected.POST("/knowledge-bases", middleware.RequirePermission(authpkg.PermKBCreate, stores), kbH.CreateKB) protected.GET("/knowledge-bases", kbH.ListKBs) protected.GET("/knowledge-bases/:id", kbH.GetKB) protected.PUT("/knowledge-bases/:id", kbH.UpdateKB) protected.DELETE("/knowledge-bases/:id", kbH.DeleteKB) - protected.POST("/knowledge-bases/:id/documents", kbH.UploadDocument) + protected.POST("/knowledge-bases/:id/documents", middleware.RequirePermission(authpkg.PermKBWrite, stores), kbH.UploadDocument) protected.GET("/knowledge-bases/:id/documents", kbH.ListDocuments) + protected.GET("/knowledge-bases/:id/documents/:docId/status", kbH.GetDocumentStatus) + protected.DELETE("/knowledge-bases/:id/documents/:docId", kbH.DeleteDocument) protected.POST("/knowledge-bases/:id/search", kbH.SearchKB) protected.POST("/knowledge-bases/:id/rebuild", kbH.RebuildKB) protected.GET("/channels/:id/knowledge-bases", kbH.GetChannelKBs) @@ -2682,6 +2684,141 @@ func TestIntegration_ChannelKBLinking(t *testing.T) { } } +// ── KB Document Status + Delete ───────────────── + +func TestIntegration_KB_DocumentStatusAndDelete(t *testing.T) { + h := setupHarness(t) + adminID, adminToken := h.createAdminUser("admin", "admin@test.com") + + // Create a KB + w := h.request("POST", "/api/v1/knowledge-bases", adminToken, map[string]interface{}{ + "name": "Doc Test KB", + }) + if w.Code != http.StatusCreated { + t.Fatalf("create KB: want 201, got %d: %s", w.Code, w.Body.String()) + } + var kb map[string]interface{} + decode(w, &kb) + kbID := kb["id"].(string) + + // Seed a document directly (upload handler requires object store) + docID := uuid.NewString() + _, err := database.TestDB.Exec(dialectSQL( + `INSERT INTO kb_documents (id, kb_id, filename, content_type, size_bytes, storage_key, status, uploaded_by, created_at, updated_at) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)`), + docID, kbID, "test.md", "text/markdown", 1234, "kb/"+kbID+"/"+docID+"_test.md", + "chunking", adminID, time.Now().UTC().Format(time.RFC3339), time.Now().UTC().Format(time.RFC3339)) + if err != nil { + t.Fatalf("seed doc: %v", err) + } + + // GET document status + w = h.request("GET", fmt.Sprintf("/api/v1/knowledge-bases/%s/documents/%s/status", kbID, docID), adminToken, nil) + if w.Code != http.StatusOK { + t.Fatalf("get doc status: want 200, got %d: %s", w.Code, w.Body.String()) + } + var status map[string]interface{} + decode(w, &status) + if status["id"] != docID { + t.Fatalf("doc status: id mismatch: got %s, want %s", status["id"], docID) + } + if status["status"] != "chunking" { + t.Fatalf("doc status: want 'chunking', got %s", status["status"]) + } + if status["filename"] != "test.md" { + t.Fatalf("doc status: want filename 'test.md', got %s", status["filename"]) + } + + // GET status for wrong KB — should 404 + fakeKB := uuid.NewString() + w = h.request("GET", fmt.Sprintf("/api/v1/knowledge-bases/%s/documents/%s/status", fakeKB, docID), adminToken, nil) + if w.Code != http.StatusNotFound { + t.Fatalf("doc status wrong KB: want 404, got %d", w.Code) + } + + // GET status for wrong doc — should 404 + w = h.request("GET", fmt.Sprintf("/api/v1/knowledge-bases/%s/documents/%s/status", kbID, uuid.NewString()), adminToken, nil) + if w.Code != http.StatusNotFound { + t.Fatalf("doc status wrong doc: want 404, got %d", w.Code) + } + + // DELETE document + w = h.request("DELETE", fmt.Sprintf("/api/v1/knowledge-bases/%s/documents/%s", kbID, docID), adminToken, nil) + if w.Code != http.StatusOK { + t.Fatalf("delete doc: want 200, got %d: %s", w.Code, w.Body.String()) + } + var delResp map[string]interface{} + decode(w, &delResp) + if delResp["deleted"] != true { + t.Fatalf("delete doc: want deleted=true, got %v", delResp["deleted"]) + } + + // Verify gone + w = h.request("GET", fmt.Sprintf("/api/v1/knowledge-bases/%s/documents/%s/status", kbID, docID), adminToken, nil) + if w.Code != http.StatusNotFound { + t.Fatalf("deleted doc status: want 404, got %d", w.Code) + } + + // DELETE on wrong KB — should 404 + docID2 := uuid.NewString() + database.TestDB.Exec(dialectSQL( + `INSERT INTO kb_documents (id, kb_id, filename, content_type, size_bytes, storage_key, status, uploaded_by, created_at, updated_at) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)`), + docID2, kbID, "other.txt", "text/plain", 100, "placeholder", + "pending", adminID, time.Now().UTC().Format(time.RFC3339), time.Now().UTC().Format(time.RFC3339)) + + w = h.request("DELETE", fmt.Sprintf("/api/v1/knowledge-bases/%s/documents/%s", fakeKB, docID2), adminToken, nil) + if w.Code != http.StatusNotFound { + t.Fatalf("delete doc wrong KB: want 404, got %d", w.Code) + } +} + +// ── KB Update Empty Body ──────────────────────── + +func TestIntegration_KB_UpdateEmptyBody(t *testing.T) { + h := setupHarness(t) + _, adminToken := h.createAdminUser("admin", "admin@test.com") + + // Create KB + w := h.request("POST", "/api/v1/knowledge-bases", adminToken, map[string]interface{}{ + "name": "Empty Update KB", + }) + if w.Code != http.StatusCreated { + t.Fatalf("create KB: want 201, got %d: %s", w.Code, w.Body.String()) + } + var kb map[string]interface{} + decode(w, &kb) + kbID := kb["id"].(string) + + // Update with empty fields — should 400 + w = h.request("PUT", fmt.Sprintf("/api/v1/knowledge-bases/%s", kbID), adminToken, + map[string]interface{}{}) + if w.Code != http.StatusBadRequest { + t.Fatalf("empty update: want 400, got %d: %s", w.Code, w.Body.String()) + } +} + +// ── KB Permission Enforcement ─────────────────── + +func TestIntegration_KB_PermissionEnforcement(t *testing.T) { + h := setupHarness(t) + _, _ = h.createAdminUser("admin", "admin@test.com") + + database.TestDB.Exec("INSERT INTO platform_policies (key, value) VALUES ('allow_registration', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true'") + database.TestDB.Exec("INSERT INTO platform_policies (key, value) VALUES ('default_user_active', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true'") + + _, userToken := h.registerUser("normperm", "normperm@test.com", "password123") + + // Regular user without kb.create should get 403 + // (TruncateAll wipes the Everyone group, so no default permissions) + w := h.request("POST", "/api/v1/knowledge-bases", userToken, map[string]interface{}{ + "name": "Should Fail", + }) + if w.Code != http.StatusForbidden { + t.Fatalf("kb create without permission: want 403, got %d: %s", w.Code, w.Body.String()) + } +} + // ══════════════════════════════════════════════ // GROUPS + RESOURCE GRANTS (v0.16.0) // ══════════════════════════════════════════════ @@ -3132,6 +3269,19 @@ func TestIntegration_KB_CreateAuth(t *testing.T) { _ = adminToken database.TestDB.Exec("INSERT INTO platform_policies (key, value) VALUES ('default_user_active', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true'") + + // Seed Everyone group with kb.create so the permission middleware passes + // and we can test the handler-level scope checks below. + _, err := database.TestDB.Exec(dialectSQL(` + INSERT INTO groups (id, name, description, scope, created_by, source, permissions) + VALUES ($1, $2, $3, $4, NULL, $5, $6)`), + "00000000-0000-0000-0000-000000000001", "Everyone", + "Implicit group — all authenticated users receive these permissions.", + "global", "system", `["model.use","kb.read","kb.create","channel.create"]`) + if err != nil { + t.Fatalf("seed Everyone group: %v", err) + } + _, userToken := h.registerUser("kbuser", "kb@test.com", "password123") // Regular user should NOT be able to create global KBs @@ -3163,6 +3313,71 @@ func TestIntegration_KB_CreateAuth(t *testing.T) { } } +// ── KB Team Scope Creation ────────────────────── + +func TestIntegration_KB_TeamScopeCreate(t *testing.T) { + h := setupHarness(t) + _, adminToken := h.createAdminUser("admin", "admin@test.com") + + database.TestDB.Exec("INSERT INTO platform_policies (key, value) VALUES ('allow_registration', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true'") + database.TestDB.Exec("INSERT INTO platform_policies (key, value) VALUES ('default_user_active', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true'") + + // Seed Everyone group with kb.create + database.TestDB.Exec(dialectSQL(` + INSERT INTO groups (id, name, description, scope, created_by, source, permissions) + VALUES ($1, $2, $3, $4, NULL, $5, $6)`), + "00000000-0000-0000-0000-000000000001", "Everyone", + "Implicit group", "global", "system", + `["model.use","kb.read","kb.create","channel.create"]`) + + userID, userToken := h.registerUser("teamkb", "teamkb@test.com", "password123") + + // Create a team via admin + w := h.request("POST", "/api/v1/admin/teams", adminToken, map[string]string{ + "name": "KB Team", "description": "team for KB test", + }) + if w.Code != http.StatusCreated { + t.Fatalf("create team: want 201, got %d: %s", w.Code, w.Body.String()) + } + var team map[string]interface{} + decode(w, &team) + teamID := team["id"].(string) + + // Add user as regular member — should NOT be able to create team KB + w = h.request("POST", fmt.Sprintf("/api/v1/admin/teams/%s/members", teamID), adminToken, + map[string]string{"user_id": userID, "role": "member"}) + if w.Code != http.StatusOK && w.Code != http.StatusCreated { + t.Fatalf("add member: want 200/201, got %d: %s", w.Code, w.Body.String()) + } + + w = h.request("POST", "/api/v1/knowledge-bases", userToken, map[string]interface{}{ + "name": "Team KB by Member", "scope": "team", "team_id": teamID, + }) + if w.Code != http.StatusForbidden { + t.Fatalf("member create team KB: want 403, got %d: %s", w.Code, w.Body.String()) + } + + // Promote to team admin — should succeed + database.TestDB.Exec(dialectSQL( + `UPDATE team_members SET role = 'admin' WHERE team_id = $1 AND user_id = $2`), + teamID, userID) + + w = h.request("POST", "/api/v1/knowledge-bases", userToken, map[string]interface{}{ + "name": "Team KB by Admin", "scope": "team", "team_id": teamID, + }) + if w.Code != http.StatusCreated { + t.Fatalf("team admin create team KB: want 201, got %d: %s", w.Code, w.Body.String()) + } + var kbResp map[string]interface{} + decode(w, &kbResp) + if kbResp["scope"] != "team" { + t.Fatalf("team KB scope: want 'team', got %s", kbResp["scope"]) + } + if kbResp["team_id"] != teamID { + t.Fatalf("team KB team_id: want %s, got %s", teamID, kbResp["team_id"]) + } +} + // ── KB Discoverable Flag ── func TestIntegration_KB_Discoverable(t *testing.T) { @@ -3196,8 +3411,21 @@ func TestIntegration_KB_Discoverable(t *testing.T) { } found := false for _, kb := range kbs { - if kb.(map[string]interface{})["id"] == kbID { + kbMap := kb.(map[string]interface{}) + if kbMap["id"] == kbID { found = true + // Verify kbResponse shape (not raw model) + if _, ok := kbMap["status"]; !ok { + t.Fatal("discoverable response missing 'status' field (not using kbResponse?)") + } + if _, ok := kbMap["chunk_count"]; !ok { + t.Fatal("discoverable response missing 'chunk_count' field (not using kbResponse?)") + } + // created_at should be ISO format (kbResponse), not Go default with nanoseconds + ts, _ := kbMap["created_at"].(string) + if len(ts) > 0 && strings.Contains(ts, ".") { + t.Fatalf("discoverable created_at has nanoseconds (raw model leak): %s", ts) + } break } } @@ -3244,6 +3472,94 @@ func TestIntegration_KB_DirectAccessPolicy(t *testing.T) { _ = w } +// ── KB SetDiscoverable Auth Enforcement ───────── + +func TestIntegration_KB_SetDiscoverableAuth(t *testing.T) { + h := setupHarness(t) + adminID, adminToken := h.createAdminUser("admin", "admin@test.com") + _ = adminID + + database.TestDB.Exec("INSERT INTO platform_policies (key, value) VALUES ('allow_registration', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true'") + database.TestDB.Exec("INSERT INTO platform_policies (key, value) VALUES ('default_user_active', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true'") + + _, aliceToken := h.registerUser("alice", "alice@test.com", "password123") + _, bobToken := h.registerUser("bob", "bob@test.com", "password123") + + // Seed Everyone group with kb.create so users can create KBs + database.TestDB.Exec(dialectSQL(` + INSERT INTO groups (id, name, description, scope, created_by, source, permissions) + VALUES ($1, $2, $3, $4, NULL, $5, $6)`), + "00000000-0000-0000-0000-000000000001", "Everyone", + "Implicit group", "global", "system", + `["model.use","kb.read","kb.create","channel.create"]`) + + // Alice creates a personal KB + w := h.request("POST", "/api/v1/knowledge-bases", aliceToken, map[string]interface{}{ + "name": "Alice KB", + }) + if w.Code != http.StatusCreated { + t.Fatalf("alice create KB: want 201, got %d: %s", w.Code, w.Body.String()) + } + var aliceKB map[string]interface{} + decode(w, &aliceKB) + aliceKBID := aliceKB["id"].(string) + + // Alice CAN toggle her own KB + w = h.request("PUT", fmt.Sprintf("/api/v1/knowledge-bases/%s/discoverable", aliceKBID), + aliceToken, map[string]interface{}{"discoverable": false}) + if w.Code != http.StatusOK { + t.Fatalf("owner toggle discoverable: want 200, got %d: %s", w.Code, w.Body.String()) + } + + // Bob CANNOT toggle Alice's KB (not owner, not admin) + w = h.request("PUT", fmt.Sprintf("/api/v1/knowledge-bases/%s/discoverable", aliceKBID), + bobToken, map[string]interface{}{"discoverable": true}) + if w.Code != http.StatusNotFound { + // loadAndAuthorize returns 404 for personal KBs the user doesn't own + t.Fatalf("non-owner toggle discoverable: want 404, got %d: %s", w.Code, w.Body.String()) + } + + // Admin creates a global KB + w = h.request("POST", "/api/v1/knowledge-bases", adminToken, map[string]interface{}{ + "name": "Global KB", "scope": "global", + }) + if w.Code != http.StatusCreated { + t.Fatalf("admin create global KB: want 201, got %d: %s", w.Code, w.Body.String()) + } + var globalKB map[string]interface{} + decode(w, &globalKB) + globalKBID := globalKB["id"].(string) + + // Alice CANNOT toggle a global KB she doesn't own + w = h.request("PUT", fmt.Sprintf("/api/v1/knowledge-bases/%s/discoverable", globalKBID), + aliceToken, map[string]interface{}{"discoverable": false}) + if w.Code != http.StatusForbidden { + t.Fatalf("non-admin toggle global discoverable: want 403, got %d: %s", w.Code, w.Body.String()) + } + + // Admin CAN toggle the global KB + w = h.request("PUT", fmt.Sprintf("/api/v1/knowledge-bases/%s/discoverable", globalKBID), + adminToken, map[string]interface{}{"discoverable": false}) + if w.Code != http.StatusOK { + t.Fatalf("admin toggle global discoverable: want 200, got %d: %s", w.Code, w.Body.String()) + } + + // Admin CANNOT access Alice's personal KB (loadAndAuthorize returns 404 + // for personal KBs not owned by the caller — consistent with Get/Update/Delete) + w = h.request("PUT", fmt.Sprintf("/api/v1/knowledge-bases/%s/discoverable", aliceKBID), + adminToken, map[string]interface{}{"discoverable": true}) + if w.Code != http.StatusNotFound { + t.Fatalf("admin toggle alice personal KB: want 404 (not owner), got %d: %s", w.Code, w.Body.String()) + } + + // Non-existent KB returns 404 + w = h.request("PUT", fmt.Sprintf("/api/v1/knowledge-bases/%s/discoverable", uuid.NewString()), + adminToken, map[string]interface{}{"discoverable": false}) + if w.Code != http.StatusNotFound { + t.Fatalf("toggle nonexistent KB: want 404, got %d: %s", w.Code, w.Body.String()) + } +} + // ═══════════════════════════════════════════════ // Messages + Treepath tests (SQLite compat) // ═══════════════════════════════════════════════ diff --git a/server/handlers/knowledge_bases.go b/server/handlers/knowledge_bases.go index 428a398..7b72e0c 100644 --- a/server/handlers/knowledge_bases.go +++ b/server/handlers/knowledge_bases.go @@ -11,7 +11,6 @@ import ( "github.com/gin-gonic/gin" - "git.gobha.me/xcaliber/chat-switchboard/database" "git.gobha.me/xcaliber/chat-switchboard/knowledge" "git.gobha.me/xcaliber/chat-switchboard/models" "git.gobha.me/xcaliber/chat-switchboard/storage" @@ -150,11 +149,8 @@ func (h *KnowledgeBaseHandler) CreateKB(c *gin.Context) { // Verify user is team admin (or system admin) role, _ := c.Get("role") if role != "admin" { - var teamRole string - err := database.DB.QueryRow( - database.Q(`SELECT role FROM team_members WHERE team_id = $1 AND user_id = $2`), - req.TeamID, userID).Scan(&teamRole) - if err != nil || teamRole != "admin" { + isTA, err := h.stores.Teams.IsTeamAdmin(c.Request.Context(), req.TeamID, userID) + if err != nil || !isTA { c.JSON(http.StatusForbidden, gin.H{"error": "team admin access required to create team knowledge bases"}) return } @@ -764,8 +760,23 @@ func (h *KnowledgeBaseHandler) SetChannelKBs(c *gin.Context) { // ── Discoverable Management (v0.17.0) ──────────── // SetDiscoverable toggles KB visibility to users. +// Only the KB owner or a system admin may change this flag. func (h *KnowledgeBaseHandler) SetDiscoverable(c *gin.Context) { - kbID := c.Param("id") + kb, ok := h.loadAndAuthorize(c) + if !ok { + return + } + + // Require ownership or admin role to toggle discoverability + userID := getUserID(c) + role, _ := c.Get("role") + isAdmin := role == "admin" + isOwner := kb.OwnerID != nil && *kb.OwnerID == userID + + if !isAdmin && !isOwner { + c.JSON(http.StatusForbidden, gin.H{"error": "only the KB owner or a system admin can change discoverability"}) + return + } var req struct { Discoverable bool `json:"discoverable"` @@ -775,11 +786,15 @@ func (h *KnowledgeBaseHandler) SetDiscoverable(c *gin.Context) { return } - if err := h.stores.KnowledgeBases.SetDiscoverable(c.Request.Context(), kbID, req.Discoverable); err != nil { + if err := h.stores.KnowledgeBases.SetDiscoverable(c.Request.Context(), kb.ID, req.Discoverable); err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to update discoverability"}) return } + h.auditLog(c, "kb.discoverable", kb.ID, map[string]interface{}{ + "kb_name": kb.Name, "discoverable": req.Discoverable, + }) + c.JSON(http.StatusOK, gin.H{"status": "ok"}) } @@ -798,7 +813,12 @@ func (h *KnowledgeBaseHandler) ListDiscoverableKBs(c *gin.Context) { kbs = []models.KnowledgeBase{} } - c.JSON(http.StatusOK, gin.H{"data": kbs}) + items := make([]kbResponse, len(kbs)) + for i := range kbs { + items[i] = toKBResponse(&kbs[i]) + } + + c.JSON(http.StatusOK, gin.H{"data": items}) } // userCanAccess checks if a user can access a KB based on scope. diff --git a/server/store/interfaces.go b/server/store/interfaces.go index 38251d2..d7401ca 100644 --- a/server/store/interfaces.go +++ b/server/store/interfaces.go @@ -459,8 +459,6 @@ type KnowledgeBaseStore interface { // Scoped listing ListForUser(ctx context.Context, userID string, teamIDs []string) ([]models.KnowledgeBase, error) - ListGlobal(ctx context.Context) ([]models.KnowledgeBase, error) - ListForTeam(ctx context.Context, teamID string) ([]models.KnowledgeBase, error) ListPersonal(ctx context.Context, userID string) ([]models.KnowledgeBase, error) // Documents diff --git a/server/store/postgres/knowledge_bases.go b/server/store/postgres/knowledge_bases.go index 99f0985..1feae24 100644 --- a/server/store/postgres/knowledge_bases.go +++ b/server/store/postgres/knowledge_bases.go @@ -104,20 +104,6 @@ func (s *KnowledgeBaseStore) ListForUser(ctx context.Context, userID string, tea return queryKBs(ctx, q, args...) } -func (s *KnowledgeBaseStore) ListGlobal(ctx context.Context) ([]models.KnowledgeBase, error) { - return queryKBs(ctx, ` - SELECT id, name, description, scope, owner_id, team_id, embedding_config, - document_count, chunk_count, total_bytes, status, discoverable, created_at, updated_at - FROM knowledge_bases WHERE scope = 'global' ORDER BY name`) -} - -func (s *KnowledgeBaseStore) ListForTeam(ctx context.Context, teamID string) ([]models.KnowledgeBase, error) { - return queryKBs(ctx, ` - SELECT id, name, description, scope, owner_id, team_id, embedding_config, - document_count, chunk_count, total_bytes, status, discoverable, created_at, updated_at - FROM knowledge_bases WHERE team_id = $1 ORDER BY name`, teamID) -} - func (s *KnowledgeBaseStore) ListPersonal(ctx context.Context, userID string) ([]models.KnowledgeBase, error) { return queryKBs(ctx, ` SELECT id, name, description, scope, owner_id, team_id, embedding_config, diff --git a/server/store/sqlite/knowledge_bases.go b/server/store/sqlite/knowledge_bases.go index 9470a77..8fcf36e 100644 --- a/server/store/sqlite/knowledge_bases.go +++ b/server/store/sqlite/knowledge_bases.go @@ -114,20 +114,6 @@ func (s *KnowledgeBaseStore) ListForUser(ctx context.Context, userID string, tea return queryKBs(ctx, q, args...) } -func (s *KnowledgeBaseStore) ListGlobal(ctx context.Context) ([]models.KnowledgeBase, error) { - return queryKBs(ctx, ` - SELECT id, name, description, scope, owner_id, team_id, embedding_config, - document_count, chunk_count, total_bytes, status, discoverable, created_at, updated_at - FROM knowledge_bases WHERE scope = 'global' ORDER BY name`) -} - -func (s *KnowledgeBaseStore) ListForTeam(ctx context.Context, teamID string) ([]models.KnowledgeBase, error) { - return queryKBs(ctx, ` - SELECT id, name, description, scope, owner_id, team_id, embedding_config, - document_count, chunk_count, total_bytes, status, discoverable, created_at, updated_at - FROM knowledge_bases WHERE team_id = ? ORDER BY name`, teamID) -} - func (s *KnowledgeBaseStore) ListPersonal(ctx context.Context, userID string) ([]models.KnowledgeBase, error) { return queryKBs(ctx, ` SELECT id, name, description, scope, owner_id, team_id, embedding_config,