diff --git a/CHANGELOG.md b/CHANGELOG.md index c097beae..8399c5a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,15 @@ All notable changes to vouch are documented here. Format follows artifact the caller could not already retrieve, and it touches no write path. ### Fixed +- **`kb.detect_themes` no longer leaks claim and session ids the viewer + cannot retrieve** (#657): the detector filtered claims on status and + `approved_by` but never on `ArtifactScope`, so a private or cross-project + claim contributed its id — and the session that produced it — to a + returned `ThemeCluster`. `propose_theme` writes both lists into the theme + page body, so the leak became committed yaml on approval rather than + stopping at a response. `detect_themes` now filters through + `scoping.is_visible` like `kb.recall` and the salience sidebar already do, + and takes an optional `viewer` for callers that carry one. - **security: empty-quote receipts no longer clear the auto-approve gate** (#513 reopened, root-caused): `verify_receipt` and `verify_evidence` both guarded only on `quote is None`, not an empty string. An `Evidence` with diff --git a/src/vouch/themes.py b/src/vouch/themes.py index 2e970f1b..b9c50200 100644 --- a/src/vouch/themes.py +++ b/src/vouch/themes.py @@ -19,6 +19,7 @@ from .config_coerce import coerce_bool from .models import ClaimStatus, ProposalStatus from .proposals import ProposalError, propose_page +from .scoping import ViewerContext, is_visible, viewer_from from .storage import KBStore logger = logging.getLogger(__name__) @@ -96,11 +97,15 @@ def detect_themes( min_sessions: int | None = None, min_claims: int | None = None, top_k: int | None = None, + viewer: ViewerContext | None = None, ) -> DetectResult: """Detect recurring entity clusters across sessions. Pure read-only operation. Returns ranked clusters without persisting - anything. Excludes archived, superseded, redacted, and pending claims. + anything. Excludes archived, superseded, redacted, and pending claims, + and anything the viewer cannot retrieve — ``viewer`` defaults to the + config-resolved context, so a KB read with no explicit viewer reads as + its own project (see ``scoping.viewer_from``). """ cfg = _load_theme_config(store) if not cfg["enabled"]: @@ -113,11 +118,20 @@ def detect_themes( # Collect approved claims that reference entities and belong to sessions. claims = store.list_claims() # Also exclude pending (working) — only look at review-gated claims. + # Viewer-filtered for the same reason search/recall/salience are: a + # cluster is a list of claim ids and session ids, so an unfiltered scan + # hands a private or cross-project claim's id — and the session that + # produced it — to a viewer that cannot retrieve the claim itself. The + # leak is durable rather than advisory: `propose_theme` writes those ids + # into a theme page body that then lands in the KB on approval. + if viewer is None: + viewer = viewer_from(config_path=store.config_path) eligible = [ c for c in claims if c.status not in _EXCLUDED_STATUSES and c.entities and c.approved_by is not None + and is_visible(c.scope, viewer) ] # Map each claim to its session(s) via decided proposals. diff --git a/tests/test_themes.py b/tests/test_themes.py index 2703fefa..88302e35 100644 --- a/tests/test_themes.py +++ b/tests/test_themes.py @@ -8,7 +8,9 @@ from vouch import sessions as sess_mod from vouch import themes +from vouch.models import ArtifactScope, Visibility from vouch.proposals import ProposalError, approve, propose_claim, propose_entity +from vouch.scoping import ViewerContext, is_visible, viewer_from from vouch.storage import KBStore @@ -123,6 +125,66 @@ def test_detect_themes_excludes_archived(store: KBStore) -> None: assert len(result.clusters) == 0 +def test_detect_themes_excludes_claims_the_viewer_cannot_see( + store: KBStore, +) -> None: + """A claim outside the viewer's scope must not reach a cluster.""" + _seed_multi_session_claims(store) + # Re-stamp every claim private to an agent nobody is reading as. + for claim in store.list_claims(): + claim.scope = ArtifactScope(visibility=Visibility.PRIVATE, agent="alice") + store.update_claim(claim) + + viewer = viewer_from(config_path=store.config_path) + assert not [c for c in store.list_claims() if is_visible(c.scope, viewer)] + + result = themes.detect_themes(store, min_sessions=1, min_claims=1) + assert result.clusters == [] + + +def test_detect_themes_honors_explicit_viewer(store: KBStore) -> None: + """The claim's own agent still sees it; a different agent does not.""" + _seed_multi_session_claims(store) + for claim in store.list_claims(): + claim.scope = ArtifactScope(visibility=Visibility.PRIVATE, agent="alice") + store.update_claim(claim) + + alice = themes.detect_themes( + store, min_sessions=2, min_claims=2, + viewer=ViewerContext(agent="alice"), + ) + assert [c.entities for c in alice.clusters], "alice owns these claims" + + bob = themes.detect_themes( + store, min_sessions=1, min_claims=1, viewer=ViewerContext(agent="bob"), + ) + assert bob.clusters == [] + + +def test_detect_themes_scope_leak_does_not_reach_a_theme_page( + store: KBStore, +) -> None: + """Regression: the detector fed private claim ids into a durable page. + + `propose_theme` writes `cluster.claim_ids` and `cluster.session_ids` into + the page body, so an unfiltered scan leaked them past the review gate as + committed yaml rather than as an advisory read. + """ + _seed_multi_session_claims(store) + private_ids = set() + for claim in store.list_claims(): + if "session-mgmt" in claim.entities: + continue + claim.scope = ArtifactScope(visibility=Visibility.PRIVATE, agent="alice") + store.update_claim(claim) + private_ids.add(claim.id) + assert private_ids + + result = themes.detect_themes(store, min_sessions=1, min_claims=1) + for cluster in result.clusters: + assert not private_ids & set(cluster.claim_ids) + + def test_detect_themes_disabled_config(store: KBStore) -> None: """When themes.enabled=false in config, returns empty.""" import yaml