feat(db): ADR-5 and the get_connection(scope=) seam for row-level security (P2-1b-i) by jwvanderstam · Pull Request #404 · jwvanderstam/LocalChat

jwvanderstam · 2026-09-29T07:27:45Z

First of three PRs for the application half of P2-1b (ROADMAP updated with the sequence). No behaviour change.

ADR-5: workspace isolation is enforced in the database

Decision: the scope is passed to get_connection(scope=) by the method that already holds it. It is not bound in a contextvar, because that route drops silently at SSE, threadpool and worker boundaries. ALL_WORKSPACES paths stay on the owner role.
Commits to: shared-schema tenancy as the security boundary, and operator = installation owner. The ADR names the second as the small-workgroup assumption.
Does not commit to: a single node. The ADR names the constraints that actually tie LocalChat to one process: M7, ADR-2, and in-process state.
Revisit when: tenant admins must be separated from platform operators, or operators must not be able to read content. At that point the second-pool design plugs into the same seam.

Code

The 18 by-id methods now call self.get_connection(scope=scope). get_connection accepts scope and doesn't act on it yet.
test_object_authorization_matrix.py gains TestScopedMethodsHandTheirScopeToTheConnection:
- every src/db method that takes a scope must hand that same scope to its connection;
- the scan must find exactly SCOPED_METHODS, so it can't pass vacuously or drift;
- a self-test checks that the scan actually recognises the offending shape.
Migration 0018: GRANT localchat_scoped TO CURRENT_USER. SET ROLE needs membership with SET. Compose connects as postgres, a superuser, so it would have worked by accident, but a managed database would have refused the switch.

Why the switch isn't turned on yet

34 methods, retrieval among them (search_similar_chunks, search_lexical_chunks, search_memories), still take workspace_id: str | None, where None means every workspace. Turning on RLS now would cover everything except the path document content leaves by. The order is:

P2-1b-ii converts those 34 methods to Scope.
P2-1b-iii turns the switch on and measures retrieval latency before and after.

Verification (local, Python 3.12 container, isolated from the live stack)

ruff, mypy, bandit: clean.
Fast suite: 3185 passed, 66 skipped, 0 failed.
test_row_level_security.py, test_migrations_apply.py and test_set_local_scope_mechanism.py against a throwaway pgvector pg16: 55 passed.
Non-vacuity: with 0018's grant turned into a no-op, the new grant test fails. Restored afterwards.

Found along the way (existing test, not changed here)

In test_row_level_security.py, _count uses conn.transaction() on a non-autocommit connection. When an implicit transaction is already open, that becomes a savepoint, and SET LOCAL ROLE stays in force after the savepoint is released. Later statements on that connection therefore run as localchat_scoped. That's why the new test checks session_user rather than current_user.

It's worth checking whether test_the_owner_can_see_the_seeded_row really runs as the owner in every test order. It passes today, but its guarantee depends on the order tests run in.
It belongs in P2-1b-iii, which relies on that test.

Not verified

End-to-end SET ROLE as a non-superuser identity. CI and the local run both use postgres. The grant follows PG16 semantics (GRANT defaults to SET TRUE), but the Scaleway identity hasn't been exercised.
If localchat_scoped was created by a different identity than the one that migrates, 0018 needs ADMIN OPTION on it and would fail. Roles are cluster-wide, so a shared managed cluster could hit this.

🤖 Generated with Claude Code

…urity (P2-1b-i) Records ADR-5: workspace isolation is enforced in the database. The scope is passed to get_connection() by the method that already holds it, not bound in a contextvar; ALL_WORKSPACES stays on the owner role. The ADR states what this commits to (shared-schema tenancy as the boundary, operator = installation owner) and what it does not (a single node). - The 18 by-id methods hand their scope to get_connection(scope=). - test_object_authorization_matrix fails any scoped method that does not, and asserts the scan finds exactly SCOPED_METHODS. - Migration 0018 grants the application identity SET on localchat_scoped; without it a managed database would refuse the role switch. No behaviour change: get_connection accepts the scope and does not act on it until the 34 methods still taking workspace_id: str | None are converted (P2-1b-ii), so retrieval is covered from the first day RLS enforces anything. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…onnection # Conflicts: # docs/ROADMAP.md

Copilot

Copilot review overview

🟡 Changes recommended

The role-membership test incorrectly assumes a single grant row and fails for the intended non-superuser PostgreSQL identity.

Review effort: Balanced
Findings: 1 · 3

Open (4)

Accept multiple membership grants with SET enabled · New
Clarify 18-method Scope conversion boundary · New
Correct inaccurate by-ID coverage description · New
Replace inaccurate by-ID classification · New

What changed in this PR

Introduces the explicit get_connection(scope=) seam needed for future database-enforced workspace isolation, without activating RLS switching yet.

Changes:

Forwards Scope through 18 scoped database methods.
Adds ADR-5, migration 0018, and supporting documentation.
Adds AST and PostgreSQL integration checks for scope propagation and role membership.

File	Description
`src/db/connection.py`	Adds the optional scope seam.
`src/db/documents.py`	Forwards document scopes.
`src/db/conversations.py`	Forwards conversation scopes.
`src/db/memories.py`	Forwards memory scopes.
`src/db/connectors.py`	Forwards connector scopes.
`src/db/annotations.py`	Forwards annotation scopes.
`migrations/versions/0018_p21b_grant_scoped_role.py`	Grants role-switch membership.
`tests/unit/test_object_authorization_matrix.py`	Enforces scope propagation statically.
`tests/unit/test_conversations_full.py`	Updates the connection test double.
`tests/integration/test_row_level_security.py`	Tests the role membership grant.
`docs/ADR.md`	Records ADR-5.
`docs/ROADMAP.md`	Documents the three-PR rollout.
`docs/MIGRATIONS.md`	Records migration `0018`.
`CHANGELOG.md`	Describes the new seam.
`.claude/rules/file-map.md`	Updates the module index.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        " WHERE r.rolname = %s AND u.rolname = session_user",
+        (ROLE,),
+    ).fetchall()
+    assert rows == [(True,)]


+  operator as installation owner) and what it does not (a single node). The 18 by-id methods
+  now hand their scope to the connection, and `test_object_authorization_matrix.py` fails any


+**Coverage, stated plainly.** The seam reaches the 18 methods that address an object by id.
+34 more — retrieval (`search_similar_chunks`, `search_lexical_chunks`, `search_memories`)
+among them — still take `workspace_id: str | None`, where `None` means every workspace: the


+  role. Deciding it surfaced the real gap: `scope` reaches only the 18 by-id methods, while 34
+  others, retrieval among them, still take `workspace_id: str | None` with `None` meaning every


sonarqubecloud · 2026-10-02T08:44:41Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
80.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ADR-5 (row-level security, #404) sits before ADR-6; the file-map row lists both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…tration is eval-gated (#412) * docs(adr): ADR-5: generation path is a deterministic pipeline; orchestration is eval-gated The code carries two answers to "who decides what gets retrieved". The vocabulary says agent (a "ReAct-style" aggregator, a planner that emits tools and hops, a multi-round tool loop); the wiring says pipeline (the planner's tools field is never read, multi-hop sub-questions run in parallel, and api_routes.py:346 disables tool calling whenever retrieval returned anything). ADR-1 ended the same kind of split in the deployment layer. ADR-5 records the pipeline as the product and licenses a bounded re-query as an experiment, off by default, built after P2-3 and shipped only if it beats the P2-3 baseline on multi-hop questions by a margin fixed before the run. Both alternatives were argued at full strength and rejected on the same test: each acts before the evidence exists. Deleting the planner and aggregator now would break the DEL-2 rule (defer, do not delete, when the measurement cannot yet be made); building a harness now has no evidence of lift, runs on the models ADR-4 leaves us, and its strongest stated motive is the one ADR-1's revisit clause excludes. A second revisit trigger (2026-12-31 if P2-3 has not started) keeps "let the eval decide" from becoming indefinite deferral. Found while writing it and listed first among the consequences: get_rag_context_multi_hop drops source_ids and additional_workspace_ids, so on the default path a source-narrowed chat asking a multi-hop question searches the whole workspace. Workspace isolation holds. No code changes in this commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018CuUHpys6UBronLzBdtAkS * docs(adr): renumber to ADR-6 — #404 takes ADR-5 for row-level security Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>

jwvanderstam added a commit that referenced this pull request Oct 2, 2026

docs(adr): renumber to ADR-6 — #404 takes ADR-5 for row-level security

c1eec59

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge remote-tracking branch 'origin/main' into feat/p2-1b-scope-to-c…

eb687a9

…onnection # Conflicts: # docs/ROADMAP.md

Copilot AI balanced review requested due to automatic review settings October 2, 2026 08:26

Copilot started reviewing on behalf of jwvanderstam October 2, 2026 08:27

View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

jwvanderstam merged commit 480ee8c into main Oct 2, 2026
16 checks passed

jwvanderstam deleted the feat/p2-1b-scope-to-connection branch October 2, 2026 08:59

jwvanderstam added a commit that referenced this pull request Oct 2, 2026

Merge origin/main into docs/adr-5-generation-path

ec5010d

ADR-5 (row-level security, #404) sits before ADR-6; the file-map row lists both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Navigation Menu

Repository navigation

feat(db): ADR-5 and the get_connection(scope=) seam for row-level security (P2-1b-i) - #404

feat(db): ADR-5 and the get_connection(scope=) seam for row-level security (P2-1b-i)#404
jwvanderstam merged 2 commits into
mainfrom
feat/p2-1b-scope-to-connection

jwvanderstam commented Sep 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

sonarqubecloud Bot commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		operator as installation owner) and what it does not (a single node). The 18 by-id methods
		now hand their scope to the connection, and `test_object_authorization_matrix.py` fails any

		role. Deciding it surfaced the real gap: `scope` reaches only the 18 by-id methods, while 34
		others, retrieval among them, still take `workspace_id: str \| None` with `None` meaning every

Conversation

jwvanderstam commented Sep 29, 2026

ADR-5: workspace isolation is enforced in the database

Code

Why the switch isn't turned on yet

Verification (local, Python 3.12 container, isolated from the live stack)

Found along the way (existing test, not changed here)

Not verified

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

sonarqubecloud Bot commented Oct 2, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants