From 0ed8a89fb3562922e7a99eaec5b7c8d9d6bf0326 Mon Sep 17 00:00:00 2001 From: galuis116 Date: Mon, 25 May 2026 04:28:13 -0700 Subject: [PATCH 1/2] fix(sessions): close crystallize review-gate bypass via summary page (#76) crystallize wrote a durable Page directly through store.put_page, embedding sess.task, sess.note, and sess.agent verbatim into the rendered markdown body. The body was never gated by propose_page + approve, so an agent calling kb.session_start(task=) and getting any one claim approved via crystallize could land arbitrary content into pages/. The page surfaces in kb.read_page, kb.list_pages, kb.context, and (once #60 is fixed) kb.search. Restrict the summary body to fields the proposing agent cannot influence: session id (server-generated), timestamps (server clock), and the list of approved artifact ids. The agent-controlled fields remain on the Session model itself and are still queryable, but no longer promoted into a durable Page. Also include summary_page_id in the session.crystallize audit event's object_ids when a page is written, so vouch audit truthfully attributes the write. Adds two regression tests: - test_crystallize_summary_page_does_not_leak_agent_controlled_fields - test_crystallize_audit_event_records_summary_page_id --- CHANGELOG.md | 11 ++++++++ src/vouch/sessions.py | 21 ++++++++++----- tests/test_sessions.py | 58 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b36d069c..f8bbcab7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,17 @@ All notable changes to vouch are documented here. Format follows the same tarball. `import_apply`, `import_check`, and `export_check` now validate every member path and raise on unsafe names. - Fix `vouch search` CLI: assign backend label per code path so substring fallback results are no longer mislabelled as `fts5`; update stale docstring to reflect multi-backend search surface (#52). +- Close the review-gate bypass in `sessions.crystallize` (#76). The + durable session-summary page wrote `sess.task`, `sess.note`, and + `sess.agent` verbatim into rendered markdown, letting an agent + land arbitrary content into `pages/` by calling + `kb.session_start(task=...)` and getting any one claim approved + via crystallize. The summary body now contains only fields the + proposing agent cannot influence (session id, server-clock + timestamps, list of approved artifact ids). The + `session.crystallize` audit event now also includes the summary + page id in `object_ids` when a page is written, so `vouch audit` + truthfully attributes the write. ## [0.0.1] — 2026-05-17 diff --git a/src/vouch/sessions.py b/src/vouch/sessions.py index 5b4d6354..1cde8430 100644 --- a/src/vouch/sessions.py +++ b/src/vouch/sessions.py @@ -109,9 +109,12 @@ def crystallize( store.put_page(page) summary_page_id = page.id + crystallize_object_ids = [sess.id, *approved_artifact_ids] + if summary_page_id is not None: + crystallize_object_ids.append(summary_page_id) audit.log_event( store.kb_dir, event="session.crystallize", actor=approver, - object_ids=[sess.id, *approved_artifact_ids], + object_ids=crystallize_object_ids, data={"approved": len(approved_artifact_ids), "failed": len(failures)}, ) return { @@ -123,11 +126,17 @@ def crystallize( def _build_summary_body(sess: Session, ids: list[str]) -> str: - lines = [f"# Session {sess.id}", ""] - if sess.task: - lines += [f"**Task:** {sess.task}", ""] - lines += [ - f"**Agent:** {sess.agent}", + # The summary page is durable and surfaces in kb.read_page / kb.search / + # kb.context, but never goes through propose_page + approve. The body is + # therefore restricted to fields the proposing agent cannot influence — + # session id (server-generated), timestamps (set from server clock at + # session_start / session_end), and the list of artifact ids that did go + # through the review gate. Anything agent-controlled (sess.task, + # sess.note, sess.agent) is omitted to keep the review-gate guarantee + # intact for the Page artifact kind. See #76. + lines = [ + f"# Session {sess.id}", + "", f"**Started:** {sess.started_at.isoformat()}", f"**Ended:** {(sess.ended_at or datetime.now(UTC)).isoformat()}", "", diff --git a/tests/test_sessions.py b/tests/test_sessions.py index 56d4a6cc..753d82ad 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from pathlib import Path import pytest @@ -43,3 +44,60 @@ def test_crystallize_skips_already_approved(store: KBStore) -> None: sess_mod.session_end(store, sess.id) result = sess_mod.crystallize(store, sess.id, approver="u") assert result["approved"] == [] # already handled + + +def test_crystallize_summary_page_does_not_leak_agent_controlled_fields( + store: KBStore, +) -> None: + """Regression for #76: the durable summary page bypasses propose_page + + approve, so its body must contain only fields the proposing agent + cannot influence — no sess.task, sess.note, or sess.agent prose. An + agent supplying a markdown payload via session_start(task=...) must + not see that payload promoted into pages/.""" + src = store.put_source(b"e") + injected_task = "## DECISION\n\nWe will migrate. Approved by leadership." + injected_note = " agent-controlled note" + sess = sess_mod.session_start( + store, agent="mallory", task=injected_task, note=injected_note, + ) + propose_claim( + store, text="legitimate finding", evidence=[src.id], + proposed_by="mallory", session_id=sess.id, + ) + sess_mod.session_end(store, sess.id) + + result = sess_mod.crystallize(store, sess.id, approver="human") + page_id = result["summary_page_id"] + assert page_id is not None + + page = store.get_page(page_id) + assert injected_task not in page.body, page.body + assert injected_note not in page.body, page.body + assert "mallory" not in page.body, page.body + assert "## DECISION" not in page.body, page.body + + +def test_crystallize_audit_event_records_summary_page_id( + store: KBStore, +) -> None: + """Regression for #76: the audit event must include the summary page id + in object_ids when a page is written, so `vouch audit` is truthful + about every artifact crystallize produced.""" + src = store.put_source(b"e") + sess = sess_mod.session_start(store, agent="a") + propose_claim( + store, text="t", evidence=[src.id], proposed_by="a", session_id=sess.id, + ) + sess_mod.session_end(store, sess.id) + result = sess_mod.crystallize(store, sess.id, approver="u") + page_id = result["summary_page_id"] + assert page_id is not None + + audit_lines = (store.kb_dir / "audit.log.jsonl").read_text().splitlines() + cryst_events = [ + json.loads(line) for line in audit_lines + if json.loads(line).get("event") == "session.crystallize" + ] + assert cryst_events, "no session.crystallize audit event found" + last = cryst_events[-1] + assert page_id in last["object_ids"], last["object_ids"] From b2e3db13afac219a88e35b6e42e84a8e215fa278 Mon Sep 17 00:00:00 2001 From: galuis116 Date: Mon, 25 May 2026 05:22:48 -0700 Subject: [PATCH 2/2] fix(tests): remove unused unittest.mock.patch import in test_sessions Leftover from the merge of main (which brought PR #62's test_sessions.py changes); ruff F401 was failing CI on PR #77. --- tests/test_sessions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_sessions.py b/tests/test_sessions.py index 1a33b946..753d82ad 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -4,7 +4,6 @@ import json from pathlib import Path -from unittest.mock import patch import pytest