[jwvanderstam]

What

ADR-6 recorded a live defect on the default chat path: get_rag_context_multi_hop took neither source_ids nor additional_workspace_ids. So a chat narrowed to chosen documents searched the whole workspace whenever the planner judged a ≥ 7-word question multi-hop, and a chat spanning additional workspaces silently lost them. Workspace isolation itself held — scope was always passed.

Found while writing the test: multi-hop citations also lacked knowledge_tier and low_relevance, so a global contribution in a multi-hop answer was cited as a workspace document (GKB-3 regression on that path).

Change

• retrieve_contexts passes both restrictions to get_rag_context_multi_hop, which hands them to every sub-question's retrieve_context.
• One _citation() helper builds a source entry for both paths; multi-hop adds its combined_score.
• ADR-6's ⬜ item marked done; CHANGELOG Fixed; file-map row for the new test.

Tests

tests/unit/test_multi_hop_keeps_the_request_scope.py — 5 of its 6 tests fail on main; the sixth (the single-query path, as a control) passes both ways. The chat-related unit suites: 373 passed. ruff, mypy, lint-imports clean.

src/services/ is a security-critical path (CONTRIBUTING.md), so this gets the separate agent-session review before merging.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[jwvanderstam]

Separate agent-session review (CONTRIBUTING.md, QP-9)

Reviewed head: 8bc9c3a (base a04da4d). Diff only, with callers traced at the head.

No finding at Medium or above. The change narrows retrieval to what the caller already chose and was authorised for; it cannot widen what any caller reads.

Every sub-question's retrieve_context now gets the same five restrictions as the single-query path: scope, knowledge_scope, filename_filter, source_ids (new), additional_workspace_ids (new). Multi-hop citations come from the shared _citation() helper.

• source_ids is only an ANY(%s) filter inside the scoped query, and a non-empty one disables the global-tier branch (retrieval.py:481), matching the single-query path.
• additional_workspace_ids reach multi-hop only after _deny_additional → check_additional_workspace_access (api_routes.py:333), which refuses the whole request on one unauthorised id, before retrieval at :377. ChatService.contexts is the only caller of retrieve_contexts.
• Global tier / API keys: check_knowledge_scope_access (api_routes.py:336) still refuses a non-local scope to a key before retrieval.
• Threads: sub-questions run in a pool without the request contextvar, harmless since retrieve_context takes scope= explicitly.

Checklist

1. Identity — not touched. 2. Authorisation — not touched; relies on the existing guards at :309 and :333, verified. 3. Scope — authorised scope on every sub-question, no ALL_WORKSPACES. 4. RLS — not touched. 5. Integrity — not touched. 6. Input — still via ChatRequest; no SQL. 7. Secrets — not touched. 8. Image — not touched. 9. Tests — 5 of 6 fail on the base, the sixth is a control; exact-value assertions, both members of each set, two sub-questions driven.

Findings

• Low — the tests assert only source_ids and additional_workspace_ids per sub-question, not all five restrictions.
• Low — the tests do not pin AGGREGATOR_AGENT_ENABLED / MCP_ENABLED, so a differently configured environment could make them fail misleadingly or the control pass vacuously.
• Info — the aggregator path still drops the restrictions (ADR-6, latent while the flag is off).
• Info — outside the diff: GraphRAG's related_names is unscoped on this base (R-12), and the query-cache key carries no scope (dead today).

🤖 Generated with Claude Code

[jwvanderstam]

Thanks for the review. Nothing at Medium or above.

• Both Lows: fixed in d501da0. An autouse fixture sets AGGREGATOR_AGENT_ENABLED and MCP_ENABLED off, and a new test asserts all five restrictions on every sub-question call (with a document filter and knowledge_scope="hybrid").
• Aggregator path: left as is, recorded in ADR-6 as still open; latent while the flag is off.
• GraphRAG scope: that is R-12, fixed in fix(security): R-12 — GraphRAG's expansion reads only the caller's workspace #527. Query-cache key: fixed in fix(security): retrieval no longer consults the scope-blind query cache #533, which removes retrieval's use of the scope-blind cache.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud