diff --git a/CHANGELOG.md b/CHANGELOG.md index 8875cc9..0a67166 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,16 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C ## [Unreleased] +## [0.23.0] — 2026-04-30 + +### Added +- **Single-item Edit button on every memory item card** in `/corporate-memory/admin`. Surfaces the per-item `PATCH /api/memory/admin/{id}` endpoint added in #126 — until now it was only reachable via the CLI (`da admin memory edit `) or by selecting one item in the bulk batch bar. The modal pre-fills from the item's current title / content / category / domain (dropdown matching `VALID_DOMAINS` + `(unset)`) / audience / tags (comma-separated). Authorisation: same `require_admin` gate as the rest of the memory admin surface. +- **`ai` section editable in `/admin/server-config`**. The `ai:` block in `instance.yaml` (provider / api_key / model / base_url / structured_output for the corporate-memory extractor) was missing from `_EDITABLE_SECTIONS` and `SECTION_META`, so admins had no UI path to view or set the LLM token without editing `instance.yaml` directly. `api_key` is auto-masked via the existing `_SECRET_KEY_PATTERNS` (substring matches "api_key"), so the input renders as a password field and audit-log diffs redact the value. +- **`MEMORY_DOMAIN` RBAC resource type** for corporate-memory items. Admins use `/admin/access` to grant `user_groups` access to specific domains (one of `finance` / `engineering` / `product` / `data` / `operations` / `infrastructure`). Members of granted groups see all `knowledge_items` in that domain regardless of the existing `audience` string filter. The two filters compose with OR semantics, so the existing `audience='group:X'` convention keeps working unchanged for ad-hoc per-item targeting; pre-grant deployments behave identically (when no MEMORY_DOMAIN grants exist, the OR clause collapses to a no-op). Wired in `KnowledgeRepository.list_items` / `search` / `count_items` / `count_by_tag` / `count_by_audience` and in the inline SQL of `GET /api/memory/stats` via a new `granted_domains` parameter resolved from `resource_grants` by `_caller_granted_memory_domains`. **Note**: a MEMORY_DOMAIN grant is a parallel visibility path that pierces the `audience` field — an item with `audience='group:admins-only'` and `domain='finance'` becomes visible to anyone with a `MEMORY_DOMAIN/finance` grant. Operators who relied on `audience` as a hard access boundary should be aware (Devin ANALYSIS_0003 on PR #141). + +### Fixed +- **Edit modal NULL→empty-string preservation** in `/corporate-memory/admin`. `submitEditItem` was sending `audience=""` for items whose stored audience was NULL, which silently broke visibility (the audience filter checks `audience IS NULL OR audience = 'all'`, neither of which matches empty string). Now empty form values for `audience`/`category`/`domain`/`content` are sent as JSON `null` so the backend stores NULL. (Devin BUG_0001 on PR #141 5f649a4 review.) + ## [0.22.0] — 2026-04-30 ### Fixed diff --git a/app/api/admin.py b/app/api/admin.py index 64c8395..ae2728a 100644 --- a/app/api/admin.py +++ b/app/api/admin.py @@ -109,6 +109,14 @@ def _normalize_primary_key(v): # tuple is `(section, *intermediate_keys, leaf_key)` — same SSRF gate the # /configure wizard applies to keboola_url, so an admin can't sneak # http://169.254.169.254/ in via the server-config editor's data_source patch. +# +# Intentionally NOT included: ``("ai", "base_url")``. The openai_compat +# provider legitimately points at internal services (LiteLLM proxy on a +# private network, on-cluster vLLM endpoint, etc.) — see +# config/instance.yaml.example "LiteLLM proxy" example. SSRF blocking +# would break those valid setups. Operators with stricter posture should +# enforce the constraint upstream (firewall / egress proxy allowlist). +# Devin ANALYSIS_0001 on PR #141 5f649a4 review. _URL_BEARING_FIELDS: tuple[tuple[str, ...], ...] = ( ("data_source", "keboola", "stack_url"), ) @@ -163,6 +171,7 @@ _EDITABLE_SECTIONS: tuple[str, ...] = ( "theme", "server", "auth", + "ai", ) # "Danger-zone" sections — flipping these can lock operators out (auth.*) or diff --git a/app/api/memory.py b/app/api/memory.py index 8845201..0f99c48 100644 --- a/app/api/memory.py +++ b/app/api/memory.py @@ -79,6 +79,38 @@ def _effective_groups( return [f"group:{r[0]}" for r in rows] +def _caller_granted_memory_domains( + user: dict, + conn: duckdb.DuckDBPyConnection, +) -> Optional[List[str]]: + """Domains the caller has been granted access to via resource_grants. + + The grant model is generic — admins assign ``MEMORY_DOMAIN`` resources + (e.g. ``finance``) to ``user_groups`` rows via ``/admin/access``. This + helper resolves the caller's group memberships against + ``resource_grants`` and returns the union of domain strings. + + Returns ``None`` for privileged viewers (admins see everything regardless + of grants — same convention as ``_effective_groups``). Returns an + empty list when the caller has no grants — the SQL filter then treats + this as a no-op (the ``OR domain IN ()`` clause is skipped). + """ + if _is_privileged_viewer(user, conn): + return None + user_id = user.get("id") + if not user_id: + return [] + rows = conn.execute( + """SELECT DISTINCT rg.resource_id + FROM resource_grants rg + JOIN user_group_members m ON m.group_id = rg.group_id + WHERE m.user_id = ? + AND rg.resource_type = 'memory_domain'""", + [user_id], + ).fetchall() + return [r[0] for r in rows] + + def _can_view_item(user: dict, item: dict, is_priv: bool) -> bool: """Personal items are visible only to the contributor and privileged viewers. Non-personal items are visible to any authenticated user. @@ -195,12 +227,14 @@ async def list_knowledge( # Their own personal contributions are visible via /my-contributions, not here. effective_exclude_personal = True if not _is_privileged_viewer(user, conn) else exclude_personal effective_groups = _effective_groups(user, conn) + granted_domains = _caller_granted_memory_domains(user, conn) statuses = [status_filter] if status_filter else None if search: items = repo.search( search, exclude_personal=effective_exclude_personal, user_groups=effective_groups, + granted_domains=granted_domains, statuses=statuses, category=category, domain=domain, @@ -216,6 +250,7 @@ async def list_knowledge( source_type=source_type, exclude_personal=effective_exclude_personal, user_groups=effective_groups, + granted_domains=granted_domains, limit=per_page, offset=offset, ) @@ -236,6 +271,7 @@ async def list_knowledge( source_type=source_type, exclude_personal=effective_exclude_personal, user_groups=effective_groups, + granted_domains=granted_domains, ) total_pages = math.ceil(total_count / per_page) if per_page > 0 else 1 @@ -270,6 +306,7 @@ async def get_stats( """ is_priv = _is_privileged_viewer(user, conn) groups = _effective_groups(user, conn) + granted_domains = _caller_granted_memory_domains(user, conn) where_clauses: List[str] = [] params: list = [] @@ -283,16 +320,20 @@ async def get_stats( where_clauses.append("(is_personal IS NULL OR is_personal = FALSE)") if groups is not None: - # groups is None for admins → no audience filter; otherwise restrict to - # null/'all' or one of the caller's group audiences. + # Mirror the visibility composition KnowledgeRepository.list_items + # uses: audience match OR MEMORY_DOMAIN grant. Without this the + # stats `total` diverges from the list endpoint's `total_count` for + # non-admin users with grants (Devin BUG_0001 on PR #141 5f649a4). + visibility = ["audience IS NULL", "audience = 'all'"] if groups: placeholders = ",".join(["?"] * len(groups)) - where_clauses.append( - f"(audience IS NULL OR audience = 'all' OR audience IN ({placeholders}))" - ) + visibility.append(f"audience IN ({placeholders})") params.extend(groups) - else: - where_clauses.append("(audience IS NULL OR audience = 'all')") + if granted_domains: + domain_placeholders = ",".join(["?"] * len(granted_domains)) + visibility.append(f"domain IN ({domain_placeholders})") + params.extend(granted_domains) + where_clauses.append("(" + " OR ".join(visibility) + ")") where_sql = (" WHERE " + " AND ".join(where_clauses)) if where_clauses else "" @@ -336,10 +377,12 @@ async def get_stats( by_tag = repo.count_by_tag( exclude_personal=exclude_personal_for_caller, user_groups=groups, + granted_domains=granted_domains, ) by_audience = repo.count_by_audience( exclude_personal=exclude_personal_for_caller, user_groups=groups, + granted_domains=granted_domains, ) return { @@ -1129,12 +1172,14 @@ async def get_tree( # Audience-axis privacy (decision 13): non-admins only see their own # group buckets + null/all. Use the audience pre-filter on the SQL side # so non-admins never accidentally see another group's bucket count. + granted_domains = _caller_granted_memory_domains(user, conn) statuses = [status_filter] if status_filter else None items = repo.list_items( statuses=statuses, source_type=source_type, exclude_personal=effective_exclude_personal, user_groups=effective_groups, + granted_domains=granted_domains, limit=10000, offset=0, ) @@ -1228,11 +1273,13 @@ async def get_bundle( repo = KnowledgeRepository(conn) effective_groups = _effective_groups(user, conn) + granted_domains = _caller_granted_memory_domains(user, conn) mandatory = repo.list_items( statuses=["mandatory"], exclude_personal=True, user_groups=effective_groups, + granted_domains=granted_domains, limit=1000, offset=0, ) @@ -1241,6 +1288,7 @@ async def get_bundle( statuses=["approved"], exclude_personal=True, user_groups=effective_groups, + granted_domains=granted_domains, limit=1000, offset=0, ) diff --git a/app/resource_types.py b/app/resource_types.py index 9de1a8d..a7c7b88 100644 --- a/app/resource_types.py +++ b/app/resource_types.py @@ -44,6 +44,7 @@ class ResourceType(StrEnum): MARKETPLACE_PLUGIN = "marketplace_plugin" TABLE = "table" + MEMORY_DOMAIN = "memory_domain" # Shape returned by ``list_blocks`` delegates. Kept as plain ``dict`` to keep @@ -165,6 +166,53 @@ def _table_blocks(conn: "duckdb.DuckDBPyConnection") -> List[Block]: return list(blocks.values()) +# --------------------------------------------------------------------------- +# Memory domain projection +# --------------------------------------------------------------------------- + + +# Mirrors VALID_DOMAINS in app/api/memory.py. Kept inline here to avoid +# importing the FastAPI module at registry-load time (circular import risk). +# If this list drifts from VALID_DOMAINS, add a runtime cross-check or merge +# the two sources — for now they're tiny and reviewed together. +_MEMORY_DOMAINS = ( + "finance", + "engineering", + "product", + "data", + "operations", + "infrastructure", +) + + +def _memory_domain_blocks(conn: "duckdb.DuckDBPyConnection") -> List[Block]: + """Project the (fixed) set of corporate-memory domains into the + (block → items) shape the admin UI renders. + + Unlike marketplace plugins / tables, the grantable items are a fixed + enum, not a DB lookup — every deployment has the same 6 domains. One + synthetic block ``"Memory domains"`` holds them; ``resource_id`` is + the domain string (matches ``knowledge_items.domain``). + """ + return [{ + "id": "memory_domains", + "name": "Memory domains", + "items": [ + { + "resource_id": domain, + "name": domain, + "category": "domain", + "description": ( + f"Members of granted groups see all knowledge_items " + f"with domain={domain!r}, in addition to the existing " + f"audience filter." + ), + } + for domain in _MEMORY_DOMAINS + ], + }] + + # --------------------------------------------------------------------------- # Registry — the one place that gets edited when adding a new resource type # --------------------------------------------------------------------------- @@ -185,6 +233,13 @@ RESOURCE_TYPES: dict[ResourceType, ResourceTypeSpec] = { id_format="", list_blocks=_table_blocks, ), + ResourceType.MEMORY_DOMAIN: ResourceTypeSpec( + key=ResourceType.MEMORY_DOMAIN, + display_name="Memory domains", + description="A corporate-memory domain (knowledge_items.domain).", + id_format="", + list_blocks=_memory_domain_blocks, + ), } diff --git a/app/web/templates/admin_server_config.html b/app/web/templates/admin_server_config.html index a7345d3..90401b9 100644 --- a/app/web/templates/admin_server_config.html +++ b/app/web/templates/admin_server_config.html @@ -184,6 +184,7 @@ const SECTION_META = { theme: { title: "Theme", help: "Brand colors and typography." }, server: { title: "Server", help: "Hostname and host. Changing these can break OAuth callbacks." }, auth: { title: "Authentication", help: "Allowed sign-in domain and Google OAuth keys. Misconfiguration can lock everyone out." }, + ai: { title: "AI / LLM", help: "Provider + API key for the corporate-memory extractor. provider=anthropic|openai_compat; api_key uses ${ENV_VAR} so the secret stays in .env." }, }; const DANGER_SECTIONS = new Set(["auth", "server"]); diff --git a/app/web/templates/corporate_memory_admin.html b/app/web/templates/corporate_memory_admin.html index 49718df..84e90e8 100644 --- a/app/web/templates/corporate_memory_admin.html +++ b/app/web/templates/corporate_memory_admin.html @@ -1069,6 +1069,57 @@ + + +
@@ -1392,6 +1443,87 @@ } } + // ───────────────────────────────────────────────────────────── + // Single-item edit (PATCH /api/memory/admin/{id}) + // Surfaces the per-item edit endpoint added in #126; previously the + // only UI affordance was the bulk batch bar, which made "fix one + // item" awkward. Pre-fills from `_itemsById` (populated during card + // render) — no extra GET round-trip. + // ───────────────────────────────────────────────────────────── + + let _editingItemId = null; + + function openEditItemModal(itemId) { + const item = _itemsById.get(itemId); + if (!item) { + showToast('Item data unavailable — refresh and try again', 'error'); + return; + } + _editingItemId = itemId; + document.getElementById('editItemTitle').value = item.title || ''; + document.getElementById('editItemContent').value = item.content || ''; + document.getElementById('editItemCategory').value = item.category || ''; + document.getElementById('editItemDomain').value = item.domain || ''; + document.getElementById('editItemAudience').value = item.audience || ''; + const tagsArr = parseJsonField(item.tags); + document.getElementById('editItemTags').value = tagsArr.join(', '); + document.getElementById('editItemModal').style.display = 'block'; + } + + function closeEditItemModal() { + document.getElementById('editItemModal').style.display = 'none'; + _editingItemId = null; + } + + async function submitEditItem() { + if (!_editingItemId) return closeEditItemModal(); + const title = document.getElementById('editItemTitle').value.trim(); + if (!title) { + showToast('Title is required', 'error'); + return; + } + // Convert empty strings to JSON null so the backend writes NULL, + // not an empty string. The audience filter in + // src/repositories/knowledge.py uses `audience IS NULL OR audience + // = 'all' OR audience IN (...)`, so an empty-string audience would + // silently make the item invisible to non-admin viewers (Devin + // BUG_0001 on PR #141 5f649a4 review). Same NULL-preservation + // applies to content / category / domain so unmodified fields + // round-trip correctly. Title is required so we never send null. + // Domain dropdown's "(unset)" option also funnels through this + // empty-string → null path. + const orNull = (s) => (s && s.trim() ? s.trim() : null); + const content = document.getElementById('editItemContent').value || null; + const category = orNull(document.getElementById('editItemCategory').value); + const domain = orNull(document.getElementById('editItemDomain').value); + const audience = orNull(document.getElementById('editItemAudience').value); + const tagsStr = document.getElementById('editItemTags').value; + const tags = tagsStr + .split(',') + .map(t => t.trim()) + .filter(t => t.length > 0); + + const body = { title, content, category, domain, audience, tags }; + try { + const resp = await fetch(`/api/memory/admin/${encodeURIComponent(_editingItemId)}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }); + if (resp.ok) { + showToast('Item updated', 'success'); + closeEditItemModal(); + refreshCurrentTab(); + } else { + const data = await resp.json().catch(() => ({})); + showToast(data.detail || `Update failed (HTTP ${resp.status})`, 'error'); + } + } catch (e) { + console.error('Edit failed:', e); + showToast('Network error: edit failed', 'error'); + } + } + // ───────────────────────────────────────────────────────────── // Data loading // ───────────────────────────────────────────────────────────── @@ -1555,7 +1687,13 @@ // Rendering: Shared Item Card // ───────────────────────────────────────────────────────────── + // Lookup map populated as items render — backs `openEditItemModal(id)` + // so the modal can pre-fill from the same payload the card was built + // from, without a fresh GET round-trip. + const _itemsById = new Map(); + function renderItemCard(item, idx, isReview) { + _itemsById.set(item.id, item); const status = item.status || 'pending'; const categoryDisplay = (item.category || 'general').replace(/_/g, ' '); const dateStr = (item.created_at || item.extracted_at || '').substring(0, 10) || 'recently'; @@ -1587,6 +1725,7 @@ Reject R +
`; } else { actionsHtml = buildStatusActions(item); @@ -1669,6 +1808,10 @@ buttons.push(``); } + // Edit is always available regardless of status — admins fix + // category/domain/tags/audience/title/content via PATCH /admin/{id}. + buttons.push(``); + if (buttons.length === 0) return ''; return `
${buttons.join('')}
`; } diff --git a/pyproject.toml b/pyproject.toml index 1cfbb47..2faf8ee 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "agnes-the-ai-analyst" -version = "0.22.0" +version = "0.23.0" description = "Agnes — AI Data Analyst platform for AI analytical systems" requires-python = ">=3.11,<3.14" license = "MIT" diff --git a/src/repositories/knowledge.py b/src/repositories/knowledge.py index 1147b5f..f7a1f5b 100644 --- a/src/repositories/knowledge.py +++ b/src/repositories/knowledge.py @@ -113,6 +113,7 @@ class KnowledgeRepository: source_type: Optional[str] = None, exclude_personal: bool = False, user_groups: Optional[List[str]] = None, + granted_domains: Optional[List[str]] = None, limit: int = 100, offset: int = 0, ) -> List[Dict[str, Any]]: @@ -134,13 +135,20 @@ class KnowledgeRepository: if exclude_personal: query += " AND (is_personal = FALSE OR is_personal IS NULL)" if user_groups is not None: - # Audience filter: null/all → visible to everyone; group:X → only that group. + # Visibility: audience-string match (null/all/group:X) OR + # caller has been granted access to the item's domain via + # resource_grants (MEMORY_DOMAIN). When ``granted_domains`` is + # falsy the OR clause collapses, preserving pre-RBAC behaviour. + visibility_clauses = ["audience IS NULL", "audience = 'all'"] if user_groups: audience_placeholders = ", ".join("?" for _ in user_groups) - query += f" AND (audience IS NULL OR audience = 'all' OR audience IN ({audience_placeholders}))" + visibility_clauses.append(f"audience IN ({audience_placeholders})") params.extend(user_groups) - else: - query += " AND (audience IS NULL OR audience = 'all')" + if granted_domains: + domain_placeholders = ", ".join("?" for _ in granted_domains) + visibility_clauses.append(f"domain IN ({domain_placeholders})") + params.extend(granted_domains) + query += " AND (" + " OR ".join(visibility_clauses) + ")" query += " ORDER BY updated_at DESC LIMIT ? OFFSET ?" params.extend([limit, offset]) return self._rows_to_dicts(self.conn.execute(query, params).fetchall()) @@ -150,6 +158,7 @@ class KnowledgeRepository: query: str, exclude_personal: bool = False, user_groups: Optional[List[str]] = None, + granted_domains: Optional[List[str]] = None, statuses: Optional[List[str]] = None, category: Optional[str] = None, domain: Optional[str] = None, @@ -177,12 +186,16 @@ class KnowledgeRepository: if exclude_personal: sql += " AND (is_personal = FALSE OR is_personal IS NULL)" if user_groups is not None: + visibility_clauses = ["audience IS NULL", "audience = 'all'"] if user_groups: audience_placeholders = ", ".join("?" for _ in user_groups) - sql += f" AND (audience IS NULL OR audience = 'all' OR audience IN ({audience_placeholders}))" + visibility_clauses.append(f"audience IN ({audience_placeholders})") params.extend(user_groups) - else: - sql += " AND (audience IS NULL OR audience = 'all')" + if granted_domains: + domain_placeholders = ", ".join("?" for _ in granted_domains) + visibility_clauses.append(f"domain IN ({domain_placeholders})") + params.extend(granted_domains) + sql += " AND (" + " OR ".join(visibility_clauses) + ")" sql += " ORDER BY updated_at DESC LIMIT ? OFFSET ?" params.extend([limit, offset]) results = self.conn.execute(sql, params).fetchall() @@ -197,6 +210,7 @@ class KnowledgeRepository: source_type: Optional[str] = None, exclude_personal: bool = False, user_groups: Optional[List[str]] = None, + granted_domains: Optional[List[str]] = None, ) -> int: if search: pattern = f"%{search}%" @@ -221,12 +235,16 @@ class KnowledgeRepository: if exclude_personal: sql += " AND (is_personal = FALSE OR is_personal IS NULL)" if user_groups is not None: + visibility_clauses = ["audience IS NULL", "audience = 'all'"] if user_groups: audience_placeholders = ", ".join("?" for _ in user_groups) - sql += f" AND (audience IS NULL OR audience = 'all' OR audience IN ({audience_placeholders}))" + visibility_clauses.append(f"audience IN ({audience_placeholders})") params.extend(user_groups) - else: - sql += " AND (audience IS NULL OR audience = 'all')" + if granted_domains: + domain_placeholders = ", ".join("?" for _ in granted_domains) + visibility_clauses.append(f"domain IN ({domain_placeholders})") + params.extend(granted_domains) + sql += " AND (" + " OR ".join(visibility_clauses) + ")" return self.conn.execute(sql, params).fetchone()[0] def list_by_domain( @@ -658,23 +676,29 @@ class KnowledgeRepository: self, exclude_personal: bool = False, user_groups: Optional[List[str]] = None, + granted_domains: Optional[List[str]] = None, ) -> Dict[str, int]: """Aggregate item counts per tag (one tag may belong to many items). Uses DuckDB ``json_each`` to unnest the JSON tag list. Items with no - tags don't contribute. Audience filter mirrors ``count_items``. + tags don't contribute. Visibility filter mirrors ``count_items`` + (audience OR MEMORY_DOMAIN grant). """ where = ["tags IS NOT NULL"] params: List[Any] = [] if exclude_personal: where.append("(is_personal = FALSE OR is_personal IS NULL)") if user_groups is not None: + visibility = ["audience IS NULL", "audience = 'all'"] if user_groups: ph = ",".join(["?"] * len(user_groups)) - where.append(f"(audience IS NULL OR audience = 'all' OR audience IN ({ph}))") + visibility.append(f"audience IN ({ph})") params.extend(user_groups) - else: - where.append("(audience IS NULL OR audience = 'all')") + if granted_domains: + dph = ",".join(["?"] * len(granted_domains)) + visibility.append(f"domain IN ({dph})") + params.extend(granted_domains) + where.append("(" + " OR ".join(visibility) + ")") where_sql = " WHERE " + " AND ".join(where) sql = ( "SELECT t.value AS tag, COUNT(*) AS cnt " @@ -696,25 +720,31 @@ class KnowledgeRepository: self, exclude_personal: bool = False, user_groups: Optional[List[str]] = None, + granted_domains: Optional[List[str]] = None, ) -> Dict[str, int]: """Aggregate item counts per audience bucket. ``audience`` is a free-form column whose canonical values are ``NULL`` / ``'all'`` / ``'group:'``. NULL is bucketed as ``'all'`` so the chip-filter UI doesn't need a separate "no audience" - affordance. Audience filter mirrors ``count_items``. + affordance. Visibility filter mirrors ``count_items`` (audience OR + MEMORY_DOMAIN grant). """ where: List[str] = [] params: List[Any] = [] if exclude_personal: where.append("(is_personal = FALSE OR is_personal IS NULL)") if user_groups is not None: + visibility = ["audience IS NULL", "audience = 'all'"] if user_groups: ph = ",".join(["?"] * len(user_groups)) - where.append(f"(audience IS NULL OR audience = 'all' OR audience IN ({ph}))") + visibility.append(f"audience IN ({ph})") params.extend(user_groups) - else: - where.append("(audience IS NULL OR audience = 'all')") + if granted_domains: + dph = ",".join(["?"] * len(granted_domains)) + visibility.append(f"domain IN ({dph})") + params.extend(granted_domains) + where.append("(" + " OR ".join(visibility) + ")") where_sql = (" WHERE " + " AND ".join(where)) if where else "" sql = ( "SELECT COALESCE(audience, 'all') AS aud, COUNT(*) AS cnt " diff --git a/tests/test_memory_api.py b/tests/test_memory_api.py index 9e4fdb1..998914c 100644 --- a/tests/test_memory_api.py +++ b/tests/test_memory_api.py @@ -844,6 +844,64 @@ class TestAudienceDistribution: ids = {i["id"] for i in r.json()["items"]} assert "aud_null2" in ids + def test_memory_domain_grant_extends_visibility(self, seeded_app): + """A non-admin user whose group is granted MEMORY_DOMAIN/ + sees items in that domain even when audience is restrictive + (audience='group:admins-only' that they're not in). + + Wires together: resource_grants row + _caller_granted_memory_domains + helper + the OR domain IN (...) clause in list_items SQL.""" + import uuid + from datetime import datetime, timezone + from src.db import get_system_db + + conn = get_system_db() + # Item is restricted by audience to a group the analyst is NOT in, + # but tagged with domain=engineering. + self._seed_item( + conn, "dom_eng1", "Eng-only via domain grant", + audience="group:admins-only", + ) + conn.execute( + "UPDATE knowledge_items SET domain = ? WHERE id = ?", + ["engineering", "dom_eng1"], + ) + + # The seeded analyst (id="analyst1") has no implicit Everyone + # membership since 0.18.0 BREAKING change. Create a dedicated + # group, add the analyst, then grant MEMORY_DOMAIN/engineering. + gid = str(uuid.uuid4()) + conn.execute( + """INSERT INTO user_groups (id, name, description, created_by, created_at) + VALUES (?, 'eng-team', 'test', 'test', ?)""", + [gid, datetime.now(timezone.utc)], + ) + conn.execute( + """INSERT INTO user_group_members (user_id, group_id, source) + VALUES ('analyst1', ?, 'admin')""", + [gid], + ) + conn.execute( + """INSERT INTO resource_grants (id, group_id, resource_type, resource_id, assigned_at, assigned_by) + VALUES (?, ?, 'memory_domain', 'engineering', ?, 'test')""", + [str(uuid.uuid4()), gid, datetime.now(timezone.utc)], + ) + conn.close() + + # Without the grant the analyst would not see this item (audience + # gate excludes group:admins-only). With the grant on engineering + # domain, the OR clause restores visibility. + r = seeded_app["client"].get( + "/api/memory?per_page=500", + headers=_auth(seeded_app["analyst_token"]), + ) + assert r.status_code == 200, r.text + ids = {i["id"] for i in r.json()["items"]} + assert "dom_eng1" in ids, ( + "MEMORY_DOMAIN/engineering grant must make engineering items " + "visible regardless of audience filter" + ) + class TestVoteRetract: def _create_item(self, c, token):