Changeset 0.28.2.1 (#184)

2026-03-13 14:36:43 +00:00
parent f94243fbf3
commit 7803ba8adf
7 changed files with 491 additions and 62 deletions
--- 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).

 ---
--- 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.

--- 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)
 // ═══════════════════════════════════════════════
--- 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.
--- 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
--- 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,
--- 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,