From 733d86f9eee74845bb03dd16d38d44ee3da28a3a Mon Sep 17 00:00:00 2001 From: jwvanderstam Date: Sun, 8 Nov 2026 21:11:49 +0100 Subject: [PATCH 1/2] =?UTF-8?q?test(security):=20R-5b=20=E2=80=94=20every?= =?UTF-8?q?=20use=20of=20ALL=5FWORKSPACES=20carries=20a=20reason?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ALL_WORKSPACES is the one scope row-level security does not narrow, and both High findings of the QP-9 review were paths that passed it where a workspace user could reach them. A unit test now walks src/ and fails on a function that passes it without a recorded reason, and on a reason whose function no longer passes it. A comparison on the scope is a branch, not a use. The 42 functions that pass it today were each checked against every caller that can reach them: boot and background work, rows that name their own workspace, ids already proved in the caller's scope, the authorisation path itself, and admin-only routes. None let a workspace user reach another workspace's rows. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KkWPqAcWbxfqz1aCP6Qsdx --- .claude/rules/architecture.md | 2 +- .claude/rules/file-map.md | 1 + CHANGELOG.md | 1 + SECURITY.md | 2 +- docs/ROADMAP.md | 2 +- .../unit/test_all_workspaces_has_a_reason.py | 148 ++++++++++++++++++ 6 files changed, 153 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_all_workspaces_has_a_reason.py diff --git a/.claude/rules/architecture.md b/.claude/rules/architecture.md index 11dd3ca7..79648000 100644 --- a/.claude/rules/architecture.md +++ b/.claude/rules/architecture.md @@ -23,7 +23,7 @@ Row-level security is forced on the ten workspace tables (ADR-5, migration `0023 - Every repository method that touches one of those tables takes `scope=` and opens its connection with `get_connection(scope=...)`. A ratchet holds unscoped methods at zero. - The scope is the workspace the route authorised, from `get_scope(request)`. A repository never picks it. -- `ALL_WORKSPACES` is for installation-wide paths only, such as admin totals and the sync worker. Every use needs a reason that holds for every caller who can reach it. Both High findings of the QP-9 review were `ALL_WORKSPACES` paths that a workspace user could reach. +- `ALL_WORKSPACES` is for installation-wide paths only, such as admin totals and the sync worker. Every use needs a reason that holds for every caller who can reach it, recorded in `tests/unit/test_all_workspaces_has_a_reason.py`, which fails without one. Both High findings of the QP-9 review were `ALL_WORKSPACES` paths that a workspace user could reach. - An id that arrives in a body or a header is looked up in the caller's scope before it is used, just like an id in the path. A miss is a 404. - A schema change is an Alembic migration in `migrations/versions/` — a new column, index, table, backfill or policy. `_ensure_extensions_and_tables()` in `src/db/connection.py` only creates the base schema a fresh database starts from. See [docs/MIGRATIONS.md](../../docs/MIGRATIONS.md). (This line said the opposite until 2026-10-06, eighteen migrations after it stopped being true.) - Migrations are additive: `CREATE TABLE IF NOT EXISTS`, `ALTER TABLE ... ADD COLUMN IF NOT EXISTS`. Never destructive. diff --git a/.claude/rules/file-map.md b/.claude/rules/file-map.md index 8a221bc0..e64c6fdf 100644 --- a/.claude/rules/file-map.md +++ b/.claude/rules/file-map.md @@ -132,6 +132,7 @@ Full module index for LocalChat. **Keep this current** — update in the same co | `docs/CONFIGURATION.md` | Configuration reference — every env var, default and effect (lifted out of README) | | `docs/MIGRATIONS.md` | Migration docs — how to apply, write, and roll back | | `tests/integration/test_object_authorization_over_the_wire.py` | P2-2b — a plain user holding workspace A addresses workspace B's objects over HTTP against a real Postgres, and every route must refuse. The route list is derived from `app.openapi()` (not `app.routes`, which this FastAPI version leaves unflattened as `_IncludedRouter` with `path=None`), an unclassified path parameter fails rather than silently dropping its routes, and the fixture proves its own objects are reachable by their owner — a 404 is a pass here, so an empty fixture would be a perfect green run testing nothing. Runs as an editor and as an owner of A (R-5), and probes ids named in a body or header beside a control with the caller's own | +| `tests/unit/test_all_workspaces_has_a_reason.py` | R-5b (QP-9 review) — every function in `src/` passing `ALL_WORKSPACES` has a recorded reason that holds for every caller; a new use without one fails, and so does a reason whose use is gone. A comparison on the scope is not a use | | `tests/integration/test_chunk_statistics_scope.py` | R-1 (QP-9 review) — document statistics count and sample only the caller's workspace's live chunks, with a second workspace and a retired document beside it | | `tests/integration/test_migrations_apply.py` | TQ-5b — the migration chain applies against a real database, lands on the single head, and a second run does nothing | | `tests/integration/test_set_local_scope_mechanism.py` | P2-1b's precondition, made executable — a transaction-local scope plus RLS returns zero rows when unset, does not survive `COMMIT`, and is not inherited by the next transaction; the properties `hnsw.ef_search` lacks | diff --git a/CHANGELOG.md b/CHANGELOG.md index 1840adf3..c58bff3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ OpenAI, `https://api.openai.com/v1`) and drop any `provider/` prefix from `CLOUD `CLOUD_PRICE_OUTPUT_PER_MTOK`; without both, only tokens are counted. A provider that is not OpenAI-shaped (Anthropic's native API, Vertex) is no longer reachable, as ADR-4 accepted. +- **Every installation-wide scope carries a reason (R-5b).** A unit test walks `src/` and fails on a function that passes `ALL_WORKSPACES` without a recorded reason, or on a reason whose function no longer passes it. The 42 functions that do today were each checked against every caller that can reach them; none was a workspace user reaching another workspace's rows. ## [3.5.0] — 2026-10-31 diff --git a/SECURITY.md b/SECURITY.md index 4ce76631..f39f3002 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -305,7 +305,7 @@ fails on the code before it. | R-2 | High | A connector's delete event retired every live document with that filename, in every workspace | **Fixed** (#506) — `tests/integration/test_connector_delete_scope.py` | | R-3 | Medium | Chat appended both turns to any `conversation_id`, in any workspace | **Fixed** (#508) — `tests/unit/test_chat_conversation_is_authorised.py`, `tests/integration/test_save_message_scope.py` | | R-4 | Medium | Access outlived a deactivated or retired account, and a retired workspace | **Fixed** (#509, #511) — `tests/integration/test_access_ends_with_the_account.py`, `TestASessionEndsWithItsAccount`. A password change still leaves open sessions running: item 13 above | -| R-5 | Medium | The object-authorisation matrix ran as an editor only and probed path ids only | **Fixed** (#510): runs as an editor and an owner, probes body and header ids. The ratchet over repository methods that use `ALL_WORKSPACES` is **open** | +| R-5 | Medium | The object-authorisation matrix ran as an editor only and probed path ids only | **Fixed** (#510): runs as an editor and an owner, probes body and header ids. R-5b: every function in `src/` that passes `ALL_WORKSPACES` carries a recorded reason, and a new one without a reason fails — `tests/unit/test_all_workspaces_has_a_reason.py` | | R-6 | Medium | `GET /settings` rendered installation-wide admin statistics for any caller | **Fixed** (#507) — `tests/unit/test_settings_page_stats_are_admin_only.py` | | R-7 | Low | A connector's sync history and manual sync took the id alone | **Fixed** (#510) — `TestAConnectorOutsideTheScopeIsNotFound` | | R-8 | Low | Feedback and annotations accept conversation and chunk ids from another workspace; a viewer can make any document un-purgeable | **Fixed** — every id a rating or annotation names is looked up in the caller's scope first, the global tier's chunks included except for a workspace API key; the message id too, which the review did not name. `tests/integration/test_object_authorization_over_the_wire.py`, `tests/integration/test_rating_ids_are_in_scope.py` | diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index c99fa414..267d7826 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -1923,7 +1923,7 @@ From `SECURITY.md`'s QP-9 register, in this order: Each id is now looked up in the caller's scope first. 2. Item 13: a password change ends the account's other sessions (`tokens_valid_after`). 3. R-10: admin user routes take raw dicts; the last-admin count ignores `is_active`. -4. R-5b: a ratchet over the `ALL_WORKSPACES` call sites, each with a recorded reason. +4. ✅ R-5b: a ratchet over the `ALL_WORKSPACES` call sites, each with a recorded reason. 5. R-9, R-11, R-12: purge preconditions, the image's file ownership, GraphRAG's scope. #### QP-11 — Replace `litellm` (ADR-4) ✅ (built 2026-11-07, taken ahead of the rest of QP-10) diff --git a/tests/unit/test_all_workspaces_has_a_reason.py b/tests/unit/test_all_workspaces_has_a_reason.py new file mode 100644 index 00000000..de84a742 --- /dev/null +++ b/tests/unit/test_all_workspaces_has_a_reason.py @@ -0,0 +1,148 @@ +"""R-5b (QP-9 review) — every use of ``ALL_WORKSPACES`` carries a reason. + +``ALL_WORKSPACES`` is the one scope row-level security does not narrow, and both High +findings of the QP-9 review were paths that passed it where a workspace user could reach +them. This walks ``src/`` and fails on a function that passes it without an entry below, +and on an entry whose function no longer does. A reason must hold for every caller who +can reach the function, not only for the one that prompted it. + +A comparison (``scope is ALL_WORKSPACES``) is a branch on the scope, not a use of it, and +is not counted. +""" + +from __future__ import annotations + +import ast +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.unit + +_SRC = Path(__file__).resolve().parents[2] / "src" + +_NO_WORKSPACE_YET = ( + "the row names its own workspace; ALL_WORKSPACES only when it has none, a row no " + "scoped transaction admits" +) +_ADMIN_PURGE = "reached only from an admin-only purge route; the row must be retired first (R-9)" +_SYNC_WORKER = "called only by the SyncWorker, with the id of a connector it loaded itself" + +#: ``module:function`` -> why an installation-wide scope is right for every caller. +REASONS: dict[str, str] = { + # Boot and background work: no request, so no caller to scope by. + "app_bootstrap.py:_init_database_service": "boot-time log of the installation's document total", + "connectors/registry.py:ConnectorRegistry.load_from_db": + "boot loads every enabled connector; each then syncs into its own workspace", + "connectors/worker.py:SyncWorker._reingest_loop": + "the worker re-ingests stale documents of every workspace, each into its own", + "db/connectors.py:ConnectorsRepository.interval": _SYNC_WORKER, + "db/connectors.py:ConnectorsRepository.set_sync_status": _SYNC_WORKER, + "db/documents.py:DocumentsRepository.mark_ingested": + "SyncWorker bookkeeping, by the id of the document it has just re-ingested", + "db/feedback.py:FeedbackRepository.export_pairs": + "training data for the reranker, which is installation-wide; reached from the " + "weekly job and the admin-only retrain", + # A row that names its own workspace. + "db/connectors.py:ConnectorsRepository.create": _NO_WORKSPACE_YET, + "db/conversations.py:ConversationsRepository.create": _NO_WORKSPACE_YET, + "db/conversations.py:ConversationsRepository.create_with_message": _NO_WORKSPACE_YET, + "db/memories.py:MemoriesRepository.insert": _NO_WORKSPACE_YET, + "rag/processor.py:DocumentProcessor.ingest_document": _NO_WORKSPACE_YET, + "db/documents.py:DocumentsRepository.exists": + "with no workspace it matches only documents without one (IS NOT DISTINCT FROM)", + "memory/extractor.py:MemoryExtractor._try_persist_memory_item": + "a memory with no workspace dedupes against all; the answer is a yes or no", + "db/workspaces.py:WorkspacesRepository.create": + "a new workspace has no scope yet; its creator becomes owner in the same transaction", + # Ids the caller already obtained, or proved, in its own scope. + "db/feedback.py:FeedbackRepository.insert": + "every id the rating names was looked up in the caller's scope first (R-8)", + "db/memories.py:MemoriesRepository.record_use": "the ids a scoped memory search just returned", + "db/memories.py:MemoriesRepository.mark_extracted": + "the id of a conversation the extraction batch read in its own scope", + "db/workspace_keys.py:WorkspaceKeysRepository._touch": "the id `resolve` has just matched", + "db/contributions.py:ContributionsRepository.archive": + "withdraws only rows whose source_project_id is the workspace the route's " + "ws:owner guard admitted; those rows have no workspace_id, being global", + # Authentication and authorisation: they establish the scope, so none exists yet. + "db/workspace_keys.py:WorkspaceKeysRepository.resolve": + "finding a key's workspace is the point; the hash comparison is constant-time", + "db/workspaces.py:WorkspaceMembersRepository.role": "the authorisation check itself", + "db/workspaces.py:WorkspaceMembersRepository.for_user": + "a caller's own memberships, read by the guard and by the workspace switcher", + "security_fastapi.py:check_workspace_access": + "an admin, by the database's role, who names no workspace acts installation-wide", + "routes_fastapi/connector_routes.py:receive_webhook": + "a public receiver; the connector id plus its mandatory secret is the credential", + # Membership changes, after the route authorised the workspace. + "db/workspaces.py:WorkspaceMembersRepository.add": + "the route admitted a global admin or the workspace's owner first", + "db/workspaces.py:WorkspaceMembersRepository.remove": + "the route admitted a global admin or the workspace's owner first", + # Global-admin only. + "db/connectors.py:ConnectorsRepository.purge": _ADMIN_PURGE, + "db/conversations.py:ConversationsRepository.purge": _ADMIN_PURGE, + "db/documents.py:DocumentsRepository.purge": _ADMIN_PURGE, + "db/users.py:UsersRepository.purge": _ADMIN_PURGE, + "db/workspaces.py:WorkspacesRepository.purge": _ADMIN_PURGE, + "db/documents.py:DocumentsRepository.purge_all": + "admin-only, and touches only documents already retired", + "db/users.py:UsersRepository.list_all": "the admin-only user list", + "db/workspaces.py:WorkspacesRepository.list_all": + "every workspace, shown only to a global admin by the database's role", + "db/feedback.py:FeedbackRepository.stats": "GET /api/feedback/stats, admin-only", + "db/feedback.py:FeedbackRepository.trend": "GET /api/feedback/stats, admin-only", + "db/feedback.py:FeedbackRepository.stale_chunks": "GET /api/feedback/stats, admin-only", + "db/contributions.py:ContributionsRepository.contribute": + "the global tier belongs to no workspace; contributing needs a global admin (GKB-2)", + "db/contributions.py:ContributionsRepository.list_all": + "GET /api/gkb/documents, admin-only", + "services/admin_stats.py:_collect_document_stats": + "admin statistics, gathered only for a caller the database says is an admin (R-6)", + # Errs toward the safe answer. + "db/documents.py:DocumentsRepository.any_local_only_sources": + "a yes or no that can only block the cloud fallback, never allow it", +} + + +def _uses() -> set[str]: + """``module:function`` for every function that passes ALL_WORKSPACES as a value.""" + found: set[str] = set() + for path in sorted(_SRC.rglob("*.py")): + module = path.relative_to(_SRC).as_posix() + if module == "utils/scope.py": + continue + tree = ast.parse(path.read_text(encoding="utf-8")) + compared = { + id(node) + for cmp in ast.walk(tree) if isinstance(cmp, ast.Compare) + and all(isinstance(op, (ast.Is, ast.IsNot)) for op in cmp.ops) + for node in [cmp.left, *cmp.comparators] + } + + def visit(node: ast.AST, qualname: list[str], module: str = module, compared: set[int] = compared) -> None: + for child in ast.iter_child_nodes(node): + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + visit(child, [*qualname, child.name]) + continue + if isinstance(child, ast.Name) and child.id == "ALL_WORKSPACES" and id(child) not in compared: + found.add(f"{module}:{'.'.join(qualname) or ''}") + visit(child, qualname) + + visit(tree, []) + return found + + +def test_every_use_of_all_workspaces_has_a_reason(): + assert sorted(_uses() - REASONS.keys()) == [] + + +def test_no_reason_outlives_its_use(): + """A function that stopped passing ALL_WORKSPACES takes its entry with it.""" + assert sorted(REASONS.keys() - _uses()) == [] + + +def test_a_comparison_is_not_counted_as_a_use(): + """`scope is ALL_WORKSPACES` in get_connection is a branch, and has no entry.""" + assert "db/connection.py:DatabaseConnection.get_connection" not in _uses() From c4149bda6e533d98d852915e23dbfcc2c0487245 Mon Sep 17 00:00:00 2001 From: jwvanderstam Date: Sun, 8 Nov 2026 21:41:38 +0100 Subject: [PATCH 2/2] =?UTF-8?q?test(security):=20R-5b=20=E2=80=94=20walk?= =?UTF-8?q?=20mcp=5Fservers=20and=20plugins,=20follow=20attribute=20uses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the separate review of #528: the walk covered src/ only and matched only the bare name. It now walks every directory whose code imports src (src/, mcp_servers/, plugins/), counts scope.ALL_WORKSPACES, and fails on an import that renames it. The two MCP list_sources tools get reasons. Three reasons are corrected: the purge reason no longer cites R-9 (not on main yet), feedback.insert says correct_doc_ids is unchecked, and for_user and any_local_only_sources name the rest of what they do. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KkWPqAcWbxfqz1aCP6Qsdx --- .claude/rules/file-map.md | 2 +- CHANGELOG.md | 2 +- SECURITY.md | 2 +- .../unit/test_all_workspaces_has_a_reason.py | 154 +++++++++++------- 4 files changed, 99 insertions(+), 61 deletions(-) diff --git a/.claude/rules/file-map.md b/.claude/rules/file-map.md index e64c6fdf..574ffcb4 100644 --- a/.claude/rules/file-map.md +++ b/.claude/rules/file-map.md @@ -132,7 +132,7 @@ Full module index for LocalChat. **Keep this current** — update in the same co | `docs/CONFIGURATION.md` | Configuration reference — every env var, default and effect (lifted out of README) | | `docs/MIGRATIONS.md` | Migration docs — how to apply, write, and roll back | | `tests/integration/test_object_authorization_over_the_wire.py` | P2-2b — a plain user holding workspace A addresses workspace B's objects over HTTP against a real Postgres, and every route must refuse. The route list is derived from `app.openapi()` (not `app.routes`, which this FastAPI version leaves unflattened as `_IncludedRouter` with `path=None`), an unclassified path parameter fails rather than silently dropping its routes, and the fixture proves its own objects are reachable by their owner — a 404 is a pass here, so an empty fixture would be a perfect green run testing nothing. Runs as an editor and as an owner of A (R-5), and probes ids named in a body or header beside a control with the caller's own | -| `tests/unit/test_all_workspaces_has_a_reason.py` | R-5b (QP-9 review) — every function in `src/` passing `ALL_WORKSPACES` has a recorded reason that holds for every caller; a new use without one fails, and so does a reason whose use is gone. A comparison on the scope is not a use | +| `tests/unit/test_all_workspaces_has_a_reason.py` | R-5b (QP-9 review) — every function in `src/`, `mcp_servers/` and `plugins/` passing `ALL_WORKSPACES` (bare or as `scope.ALL_WORKSPACES`) has a recorded reason that holds for every caller; a new use without one fails, so does a reason whose use is gone, and so does importing it under another name. A comparison on the scope is not a use | | `tests/integration/test_chunk_statistics_scope.py` | R-1 (QP-9 review) — document statistics count and sample only the caller's workspace's live chunks, with a second workspace and a retired document beside it | | `tests/integration/test_migrations_apply.py` | TQ-5b — the migration chain applies against a real database, lands on the single head, and a second run does nothing | | `tests/integration/test_set_local_scope_mechanism.py` | P2-1b's precondition, made executable — a transaction-local scope plus RLS returns zero rows when unset, does not survive `COMMIT`, and is not inherited by the next transaction; the properties `hnsw.ef_search` lacks | diff --git a/CHANGELOG.md b/CHANGELOG.md index c58bff3d..03749fd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ OpenAI, `https://api.openai.com/v1`) and drop any `provider/` prefix from `CLOUD `CLOUD_PRICE_OUTPUT_PER_MTOK`; without both, only tokens are counted. A provider that is not OpenAI-shaped (Anthropic's native API, Vertex) is no longer reachable, as ADR-4 accepted. -- **Every installation-wide scope carries a reason (R-5b).** A unit test walks `src/` and fails on a function that passes `ALL_WORKSPACES` without a recorded reason, or on a reason whose function no longer passes it. The 42 functions that do today were each checked against every caller that can reach them; none was a workspace user reaching another workspace's rows. +- **Every installation-wide scope carries a reason (R-5b).** A unit test walks `src/`, `mcp_servers/` and `plugins/` and fails on a function that passes `ALL_WORKSPACES` without a recorded reason, on a reason whose function no longer passes it, and on an import that renames it. The 44 functions that pass it today were each checked against every caller that can reach them; none lets a workspace user reach another workspace's rows. ## [3.5.0] — 2026-10-31 diff --git a/SECURITY.md b/SECURITY.md index f39f3002..6a908672 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -305,7 +305,7 @@ fails on the code before it. | R-2 | High | A connector's delete event retired every live document with that filename, in every workspace | **Fixed** (#506) — `tests/integration/test_connector_delete_scope.py` | | R-3 | Medium | Chat appended both turns to any `conversation_id`, in any workspace | **Fixed** (#508) — `tests/unit/test_chat_conversation_is_authorised.py`, `tests/integration/test_save_message_scope.py` | | R-4 | Medium | Access outlived a deactivated or retired account, and a retired workspace | **Fixed** (#509, #511) — `tests/integration/test_access_ends_with_the_account.py`, `TestASessionEndsWithItsAccount`. A password change still leaves open sessions running: item 13 above | -| R-5 | Medium | The object-authorisation matrix ran as an editor only and probed path ids only | **Fixed** (#510): runs as an editor and an owner, probes body and header ids. R-5b: every function in `src/` that passes `ALL_WORKSPACES` carries a recorded reason, and a new one without a reason fails — `tests/unit/test_all_workspaces_has_a_reason.py` | +| R-5 | Medium | The object-authorisation matrix ran as an editor only and probed path ids only | **Fixed** (#510): runs as an editor and an owner, probes body and header ids. R-5b: every function in `src/`, `mcp_servers/` and `plugins/` that passes `ALL_WORKSPACES` carries a recorded reason, and a new one without a reason fails — `tests/unit/test_all_workspaces_has_a_reason.py` | | R-6 | Medium | `GET /settings` rendered installation-wide admin statistics for any caller | **Fixed** (#507) — `tests/unit/test_settings_page_stats_are_admin_only.py` | | R-7 | Low | A connector's sync history and manual sync took the id alone | **Fixed** (#510) — `TestAConnectorOutsideTheScopeIsNotFound` | | R-8 | Low | Feedback and annotations accept conversation and chunk ids from another workspace; a viewer can make any document un-purgeable | **Fixed** — every id a rating or annotation names is looked up in the caller's scope first, the global tier's chunks included except for a workspace API key; the message id too, which the review did not name. `tests/integration/test_object_authorization_over_the_wire.py`, `tests/integration/test_rating_ids_are_in_scope.py` | diff --git a/tests/unit/test_all_workspaces_has_a_reason.py b/tests/unit/test_all_workspaces_has_a_reason.py index de84a742..146b58dd 100644 --- a/tests/unit/test_all_workspaces_has_a_reason.py +++ b/tests/unit/test_all_workspaces_has_a_reason.py @@ -2,12 +2,17 @@ ``ALL_WORKSPACES`` is the one scope row-level security does not narrow, and both High findings of the QP-9 review were paths that passed it where a workspace user could reach -them. This walks ``src/`` and fails on a function that passes it without an entry below, -and on an entry whose function no longer does. A reason must hold for every caller who -can reach the function, not only for the one that prompted it. +them. This walks every directory whose code imports ``src`` — ``src/``, ``mcp_servers/`` +and ``plugins/`` — and fails on a function that passes it without an entry below, and on +an entry whose function no longer does. A reason must hold for every caller who can reach +the function, not only for the one that prompted it. A comparison (``scope is ALL_WORKSPACES``) is a branch on the scope, not a use of it, and -is not counted. +is not counted. ``scope.ALL_WORKSPACES`` is counted like the bare name, and importing it +under another name fails outright, since the walk could not follow it. + +The entries pin the function, not who calls it: a new route reaching an admin-only +function is caught by its guard's tests and the permission matrix, not here. """ from __future__ import annotations @@ -19,101 +24,123 @@ pytestmark = pytest.mark.unit -_SRC = Path(__file__).resolve().parents[2] / "src" +_ROOT = Path(__file__).resolve().parents[2] +#: plugins/ is absent in the core-without-plugins job, and walking nothing is right there. +_WALKED = ("src", "mcp_servers", "plugins") _NO_WORKSPACE_YET = ( "the row names its own workspace; ALL_WORKSPACES only when it has none, a row no " "scoped transaction admits" ) -_ADMIN_PURGE = "reached only from an admin-only purge route; the row must be retired first (R-9)" +_ADMIN_PURGE = "reached only from an admin-only purge route" +_MCP_LIST = ( + "an MCP tool behind the MCP_AUTH_TOKEN bearer, installation-wide by design; the " + "application never calls it" +) _SYNC_WORKER = "called only by the SyncWorker, with the id of a connector it loaded itself" #: ``module:function`` -> why an installation-wide scope is right for every caller. REASONS: dict[str, str] = { # Boot and background work: no request, so no caller to scope by. - "app_bootstrap.py:_init_database_service": "boot-time log of the installation's document total", - "connectors/registry.py:ConnectorRegistry.load_from_db": + "src/app_bootstrap.py:_init_database_service": "boot-time log of the installation's document total", + "src/connectors/registry.py:ConnectorRegistry.load_from_db": "boot loads every enabled connector; each then syncs into its own workspace", - "connectors/worker.py:SyncWorker._reingest_loop": + "src/connectors/worker.py:SyncWorker._reingest_loop": "the worker re-ingests stale documents of every workspace, each into its own", - "db/connectors.py:ConnectorsRepository.interval": _SYNC_WORKER, - "db/connectors.py:ConnectorsRepository.set_sync_status": _SYNC_WORKER, - "db/documents.py:DocumentsRepository.mark_ingested": + "src/db/connectors.py:ConnectorsRepository.interval": _SYNC_WORKER, + "src/db/connectors.py:ConnectorsRepository.set_sync_status": _SYNC_WORKER, + "src/db/documents.py:DocumentsRepository.mark_ingested": "SyncWorker bookkeeping, by the id of the document it has just re-ingested", - "db/feedback.py:FeedbackRepository.export_pairs": + "src/db/feedback.py:FeedbackRepository.export_pairs": "training data for the reranker, which is installation-wide; reached from the " "weekly job and the admin-only retrain", # A row that names its own workspace. - "db/connectors.py:ConnectorsRepository.create": _NO_WORKSPACE_YET, - "db/conversations.py:ConversationsRepository.create": _NO_WORKSPACE_YET, - "db/conversations.py:ConversationsRepository.create_with_message": _NO_WORKSPACE_YET, - "db/memories.py:MemoriesRepository.insert": _NO_WORKSPACE_YET, - "rag/processor.py:DocumentProcessor.ingest_document": _NO_WORKSPACE_YET, - "db/documents.py:DocumentsRepository.exists": + "src/db/connectors.py:ConnectorsRepository.create": _NO_WORKSPACE_YET, + "src/db/conversations.py:ConversationsRepository.create": _NO_WORKSPACE_YET, + "src/db/conversations.py:ConversationsRepository.create_with_message": _NO_WORKSPACE_YET, + "src/db/memories.py:MemoriesRepository.insert": _NO_WORKSPACE_YET, + "src/rag/processor.py:DocumentProcessor.ingest_document": _NO_WORKSPACE_YET, + "src/db/documents.py:DocumentsRepository.exists": "with no workspace it matches only documents without one (IS NOT DISTINCT FROM)", - "memory/extractor.py:MemoryExtractor._try_persist_memory_item": + "src/memory/extractor.py:MemoryExtractor._try_persist_memory_item": "a memory with no workspace dedupes against all; the answer is a yes or no", - "db/workspaces.py:WorkspacesRepository.create": + "src/db/workspaces.py:WorkspacesRepository.create": "a new workspace has no scope yet; its creator becomes owner in the same transaction", # Ids the caller already obtained, or proved, in its own scope. - "db/feedback.py:FeedbackRepository.insert": - "every id the rating names was looked up in the caller's scope first (R-8)", - "db/memories.py:MemoriesRepository.record_use": "the ids a scoped memory search just returned", - "db/memories.py:MemoriesRepository.mark_extracted": + "src/db/feedback.py:FeedbackRepository.insert": + "the conversation, message and chunk ids were looked up in the caller's scope first " + "(R-8); correct_doc_ids is stored unchecked, and nothing reads it", + "src/db/memories.py:MemoriesRepository.record_use": "the ids a scoped memory search just returned", + "src/db/memories.py:MemoriesRepository.mark_extracted": "the id of a conversation the extraction batch read in its own scope", - "db/workspace_keys.py:WorkspaceKeysRepository._touch": "the id `resolve` has just matched", - "db/contributions.py:ContributionsRepository.archive": + "src/db/workspace_keys.py:WorkspaceKeysRepository._touch": "the id `resolve` has just matched", + "src/db/contributions.py:ContributionsRepository.archive": "withdraws only rows whose source_project_id is the workspace the route's " "ws:owner guard admitted; those rows have no workspace_id, being global", # Authentication and authorisation: they establish the scope, so none exists yet. - "db/workspace_keys.py:WorkspaceKeysRepository.resolve": + "src/db/workspace_keys.py:WorkspaceKeysRepository.resolve": "finding a key's workspace is the point; the hash comparison is constant-time", - "db/workspaces.py:WorkspaceMembersRepository.role": "the authorisation check itself", - "db/workspaces.py:WorkspaceMembersRepository.for_user": - "a caller's own memberships, read by the guard and by the workspace switcher", - "security_fastapi.py:check_workspace_access": + "src/db/workspaces.py:WorkspaceMembersRepository.role": "the authorisation check itself", + "src/db/workspaces.py:WorkspaceMembersRepository.for_user": + "a caller's own memberships, read by the guard and the workspace switcher; any " + "user's only through the admin-only user management", + "src/security_fastapi.py:check_workspace_access": "an admin, by the database's role, who names no workspace acts installation-wide", - "routes_fastapi/connector_routes.py:receive_webhook": + "src/routes_fastapi/connector_routes.py:receive_webhook": "a public receiver; the connector id plus its mandatory secret is the credential", # Membership changes, after the route authorised the workspace. - "db/workspaces.py:WorkspaceMembersRepository.add": + "src/db/workspaces.py:WorkspaceMembersRepository.add": "the route admitted a global admin or the workspace's owner first", - "db/workspaces.py:WorkspaceMembersRepository.remove": + "src/db/workspaces.py:WorkspaceMembersRepository.remove": "the route admitted a global admin or the workspace's owner first", # Global-admin only. - "db/connectors.py:ConnectorsRepository.purge": _ADMIN_PURGE, - "db/conversations.py:ConversationsRepository.purge": _ADMIN_PURGE, - "db/documents.py:DocumentsRepository.purge": _ADMIN_PURGE, - "db/users.py:UsersRepository.purge": _ADMIN_PURGE, - "db/workspaces.py:WorkspacesRepository.purge": _ADMIN_PURGE, - "db/documents.py:DocumentsRepository.purge_all": + "src/db/connectors.py:ConnectorsRepository.purge": _ADMIN_PURGE, + "src/db/conversations.py:ConversationsRepository.purge": _ADMIN_PURGE, + "src/db/documents.py:DocumentsRepository.purge": _ADMIN_PURGE, + "src/db/users.py:UsersRepository.purge": _ADMIN_PURGE, + "src/db/workspaces.py:WorkspacesRepository.purge": _ADMIN_PURGE, + "src/db/documents.py:DocumentsRepository.purge_all": "admin-only, and touches only documents already retired", - "db/users.py:UsersRepository.list_all": "the admin-only user list", - "db/workspaces.py:WorkspacesRepository.list_all": + "src/db/users.py:UsersRepository.list_all": "the admin-only user list", + "src/db/workspaces.py:WorkspacesRepository.list_all": "every workspace, shown only to a global admin by the database's role", - "db/feedback.py:FeedbackRepository.stats": "GET /api/feedback/stats, admin-only", - "db/feedback.py:FeedbackRepository.trend": "GET /api/feedback/stats, admin-only", - "db/feedback.py:FeedbackRepository.stale_chunks": "GET /api/feedback/stats, admin-only", - "db/contributions.py:ContributionsRepository.contribute": + "src/db/feedback.py:FeedbackRepository.stats": "GET /api/feedback/stats, admin-only", + "src/db/feedback.py:FeedbackRepository.trend": "GET /api/feedback/stats, admin-only", + "src/db/feedback.py:FeedbackRepository.stale_chunks": "GET /api/feedback/stats, admin-only", + "src/db/contributions.py:ContributionsRepository.contribute": "the global tier belongs to no workspace; contributing needs a global admin (GKB-2)", - "db/contributions.py:ContributionsRepository.list_all": + "src/db/contributions.py:ContributionsRepository.list_all": "GET /api/gkb/documents, admin-only", - "services/admin_stats.py:_collect_document_stats": + "src/services/admin_stats.py:_collect_document_stats": "admin statistics, gathered only for a caller the database says is an admin (R-6)", # Errs toward the safe answer. - "db/documents.py:DocumentsRepository.any_local_only_sources": - "a yes or no that can only block the cloud fallback, never allow it", + "src/db/documents.py:DocumentsRepository.any_local_only_sources": + "a yes or no that can only block the cloud fallback, never allow it; a namesake in " + "another workspace can block it, one bit that errs safe", + "mcp_servers/cloud_connectors/server.py:list_sources": _MCP_LIST, + "mcp_servers/local_docs/server.py:list_sources": _MCP_LIST, } +def _modules() -> list[tuple[str, ast.Module]]: + found = [] + for top in _WALKED: + for path in sorted((_ROOT / top).rglob("*.py")): + module = path.relative_to(_ROOT).as_posix() + if module != "src/utils/scope.py": + found.append((module, ast.parse(path.read_text(encoding="utf-8")))) + return found + + +def _is_the_scope(node: ast.AST) -> bool: + return (isinstance(node, ast.Name) and node.id == "ALL_WORKSPACES") or ( + isinstance(node, ast.Attribute) and node.attr == "ALL_WORKSPACES") + + def _uses() -> set[str]: """``module:function`` for every function that passes ALL_WORKSPACES as a value.""" found: set[str] = set() - for path in sorted(_SRC.rglob("*.py")): - module = path.relative_to(_SRC).as_posix() - if module == "utils/scope.py": - continue - tree = ast.parse(path.read_text(encoding="utf-8")) + for module, tree in _modules(): compared = { id(node) for cmp in ast.walk(tree) if isinstance(cmp, ast.Compare) @@ -126,7 +153,7 @@ def visit(node: ast.AST, qualname: list[str], module: str = module, compared: se if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): visit(child, [*qualname, child.name]) continue - if isinstance(child, ast.Name) and child.id == "ALL_WORKSPACES" and id(child) not in compared: + if _is_the_scope(child) and id(child) not in compared: found.add(f"{module}:{'.'.join(qualname) or ''}") visit(child, qualname) @@ -143,6 +170,17 @@ def test_no_reason_outlives_its_use(): assert sorted(REASONS.keys() - _uses()) == [] +def test_it_is_never_imported_under_another_name(): + """`import ALL_WORKSPACES as X` would hide every use of X from the walk above.""" + renamed = [ + module + for module, tree in _modules() + for node in ast.walk(tree) if isinstance(node, ast.ImportFrom) + for alias in node.names if alias.name == "ALL_WORKSPACES" and alias.asname + ] + assert renamed == [] + + def test_a_comparison_is_not_counted_as_a_use(): """`scope is ALL_WORKSPACES` in get_connection is a branch, and has no entry.""" - assert "db/connection.py:DatabaseConnection.get_connection" not in _uses() + assert "src/db/connection.py:DatabaseConnection.get_connection" not in _uses()