Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/rules/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions .claude/rules/file-map.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,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/`, `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_graph_expansion_scope.py` | R-12 (QP-9 review) — GraphRAG's 1-hop lookup returns a name that co-occurs in the caller's workspace, not one from another workspace's document or from a retired one |
| `tests/integration/test_ontology_reads_live_documents.py` | A workspace's ontology counts only its live documents and live chunks: a retired document and a chunk retired by a re-ingest leave it |
| `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 |
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,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/`, `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.
- **GraphRAG's query expansion reads only the caller's workspace (R-12).** Entities are shared across workspaces, so with `GRAPH_RAG_ENABLED` on, the 1-hop lookup added names that co-occurred in any workspace's documents, retired ones included, to the caller's query. A co-occurrence now counts only through a live document of the scope. GraphRAG is off by default.
- **A password change ends the account's other sessions (`SECURITY.md` item 13).** A
session opened before the change, on another device or by whoever knew the old password,
Expand Down
2 changes: 1 addition & 1 deletion SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,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 ends the sessions before it since item 13 was resolved |
| 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/`, `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` |
Expand Down
2 changes: 1 addition & 1 deletion docs/ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -1924,7 +1924,7 @@ From `SECURITY.md`'s QP-9 register, in this order:
2. ✅ Item 13: a password change ends the account's other sessions (`tokens_valid_after`,
migration 0024).
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)
Expand Down
186 changes: 186 additions & 0 deletions tests/unit/test_all_workspaces_has_a_reason.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
"""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 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. ``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

import ast
from pathlib import Path

import pytest

pytestmark = pytest.mark.unit

_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"
_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.
"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",
"src/connectors/worker.py:SyncWorker._reingest_loop":
"the worker re-ingests stale documents of every workspace, each into its own",
"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",
"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.
"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)",
"src/memory/extractor.py:MemoryExtractor._try_persist_memory_item":
"a memory with no workspace dedupes against all; the answer is a yes or no",
"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.
"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",
"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.
"src/db/workspace_keys.py:WorkspaceKeysRepository.resolve":
"finding a key's workspace is the point; the hash comparison is constant-time",
"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",
"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.
"src/db/workspaces.py:WorkspaceMembersRepository.add":
"the route admitted a global admin or the workspace's owner first",
"src/db/workspaces.py:WorkspaceMembersRepository.remove":
"the route admitted a global admin or the workspace's owner first",
# Global-admin only.
"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",
"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",
"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)",
"src/db/contributions.py:ContributionsRepository.list_all":
"GET /api/gkb/documents, admin-only",
"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.
"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 module, tree in _modules():
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 _is_the_scope(child) and id(child) not in compared:
found.add(f"{module}:{'.'.join(qualname) or '<module>'}")
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_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 "src/db/connection.py:DatabaseConnection.get_connection" not in _uses()
Loading