diff --git a/docs/superpowers/plans/2026-08-20-source-code-protection.md b/docs/superpowers/plans/2026-08-20-source-code-protection.md new file mode 100644 index 0000000..d95693d --- /dev/null +++ b/docs/superpowers/plans/2026-08-20-source-code-protection.md @@ -0,0 +1,496 @@ +# Source-Code Protection Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Stop the agent from handing users complete source, while leaving `read_source` tool access and model reasoning unrestricted. + +**Architecture:** One new default-deny capability, `source:view`, gates the two channels that can leak raw source to the user — the thinking trace (suppressed in the chat SSE stream) and citation-fragment bodies (the `GET /api/graph/source` endpoint). The answer channel is protected by a system-prompt rule only (no code enforcement). Admin holds all capabilities and is unaffected. + +**Tech Stack:** Python (FastAPI, pytest), TypeScript/React (Vite), Timeplus streams for the auth store. + +**Spec:** `docs/superpowers/specs/2026-08-20-source-code-protection-design.md` + +## Global Constraints + +- Capability string is exactly `source:view` (backend constant `CAP_SOURCE_VIEW`, frontend `CAP.sourceView`). +- `source:view` is **NOT** in `DEFAULT_CAPABILITIES` — default-deny. Only `admin` (via `ALL_CAPABILITIES`) and explicit grants hold it. +- No `:manage` sibling — it is a bare grant like `chat`/`explore`; do not touch `_MANAGE_IMPLIES_VIEW`. +- Reference rows (`file:line`) stay visible to all chat users; only the source **body** is gated. +- Backend tests that touch Timeplus carry `pytestmark = requires_timeplus`; pure-unit tests do not. Follow the existing per-file convention. +- Every task ends with a passing test run and a commit. + +--- + +### Task 1: Add the `source:view` capability (backend model) + +**Files:** +- Modify: `src/tpk/auth.py:26-38` (capability constants + `ALL_CAPABILITIES`) +- Test: `tests/test_capabilities.py` (unit section, no store) + +**Interfaces:** +- Produces: `auth.CAP_SOURCE_VIEW = "source:view"`, present in `auth.ALL_CAPABILITIES`, absent from `auth.DEFAULT_CAPABILITIES`. Later tasks import `CAP_SOURCE_VIEW` from `tpk.auth`. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/test_capabilities.py` (import `CAP_SOURCE_VIEW` and `DEFAULT_CAPABILITIES` in the existing `from tpk.auth import (...)` block): + +```python +def test_source_view_capability_registered_and_default_deny(): + assert CAP_SOURCE_VIEW == "source:view" + assert CAP_SOURCE_VIEW in ALL_CAPABILITIES + # Default-deny: brand-new roles must not silently hold it. + assert CAP_SOURCE_VIEW not in DEFAULT_CAPABILITIES + # It is a bare grant (no :manage sibling), so expand is identity. + assert expand_capabilities([CAP_SOURCE_VIEW]) == {CAP_SOURCE_VIEW} + # Admin holds every capability, including this one. + admin = User("a", "", auth.ROLE_ADMIN) + assert CAP_SOURCE_VIEW in effective_capabilities(admin, None) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pytest tests/test_capabilities.py::test_source_view_capability_registered_and_default_deny -v` +Expected: FAIL with `ImportError` (cannot import `CAP_SOURCE_VIEW`) or `AttributeError`. + +- [ ] **Step 3: Write minimal implementation** + +In `src/tpk/auth.py`, add the constant after `CAP_USERS_MANAGE` (line 31): + +```python +CAP_USERS_MANAGE = "users:manage" +CAP_SOURCE_VIEW = "source:view" +``` + +Append it to `ALL_CAPABILITIES` (keep it last — canonical display order): + +```python +ALL_CAPABILITIES = [ + CAP_CHAT, CAP_EXPLORE, + CAP_CORPUS_VIEW, CAP_CORPUS_MANAGE, + CAP_USERS_VIEW, CAP_USERS_MANAGE, + CAP_SOURCE_VIEW, +] +``` + +Leave `DEFAULT_CAPABILITIES` and `_MANAGE_IMPLIES_VIEW` unchanged. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pytest tests/test_capabilities.py::test_source_view_capability_registered_and_default_deny -v` +Expected: PASS + +- [ ] **Step 5: Run the capability unit tests to confirm no regressions** + +Run: `pytest tests/test_capabilities.py -k "expand or effective or parse" -v` +Expected: PASS (existing `test_expand_capabilities_*`, `test_effective_capabilities_*` unaffected). + +- [ ] **Step 6: Commit** + +```bash +git add src/tpk/auth.py tests/test_capabilities.py +git commit -m "feat(auth): add default-deny source:view capability (#66)" +``` + +--- + +### Task 2: Gate `GET /api/graph/source` on `source:view` + +**Files:** +- Modify: `src/tpk/graph_api.py:85-94` (the `/source` route dependency) +- Test: `tests/test_graph_api.py` (update `scoped_hdr` fixture; add one test) + +**Interfaces:** +- Consumes: `auth.CAP_SOURCE_VIEW` (Task 1). +- Produces: `/api/graph/source` requires `source:view` (admin bypasses via `effective_caps`); `search`/`entity`/`neighbors` keep `CAP_EXPLORE`. + +- [ ] **Step 1: Write the failing test** + +The `scoped_hdr` fixture builds an `alpha-only` role with `capabilities=[auth.CAP_EXPLORE]`. Because `/source` will now require `source:view`, that fixture must also grant it so the existing scope test (`test_non_admin_source_out_of_scope_404`) still exercises scoping rather than the new cap gate. Update the fixture (`tests/test_graph_api.py:120-121`): + +```python + auth.upsert_role(client, auth.Role("alpha-only", ["alpha@v1"], + capabilities=[auth.CAP_EXPLORE, auth.CAP_SOURCE_VIEW]), prefix=prefix) +``` + +Then add a new test that a role with `explore` but **not** `source:view` is refused on `/source` yet still allowed on `/search`: + +```python +def test_source_requires_source_view_cap(kg_app): + """`/source` now requires source:view; a role with explore-only can still + search/entity/neighbors but is refused the raw source body.""" + c, client, prefix = kg_app + auth.upsert_role(client, auth.Role("explore-only", ["alpha@v1"], + capabilities=[auth.CAP_EXPLORE]), prefix=prefix) + auth.upsert_user(client, auth.User("exp", auth.hash_password("password-1"), + "explore-only"), prefix=prefix) + _eventually(lambda: c.post("/auth/login", + json={"username": "exp", "password": "password-1"}).status_code == 200) + token = c.post("/auth/login", json={"username": "exp", "password": "password-1"}).json()["token"] + hdr = {"Authorization": f"Bearer {token}"} + # search still works with explore alone... + assert c.get("/api/graph/search", params={"q": "AlphaWidget"}, headers=hdr).status_code == 200 + # ...but the source body is gated behind source:view. + assert c.get("/api/graph/source", params={ + "repo": "alpha@v1", "file_path": "w.py", "line_start": 1, "line_end": 2, + }, headers=hdr).status_code == 403 +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pytest tests/test_graph_api.py::test_source_requires_source_view_cap -v` +Expected: FAIL — returns 200 (or a 404 scope/file miss), not 403, because `/source` still only requires `CAP_EXPLORE`. + +- [ ] **Step 3: Write minimal implementation** + +In `src/tpk/graph_api.py`, change the `/source` route's dependency from `CAP_EXPLORE` to `CAP_SOURCE_VIEW` (line 88). Leave `search`/`entity`/`neighbors` on `CAP_EXPLORE`: + +```python + @router.get("/source") + async def source(repo: str, file_path: str, + line_start: int = Query(..., ge=1), line_end: int = Query(..., ge=1), + user=Depends(auth.require_cap(auth_mod.CAP_SOURCE_VIEW))): +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pytest tests/test_graph_api.py::test_source_requires_source_view_cap -v` +Expected: PASS + +- [ ] **Step 5: Run the full graph-api suite to confirm the fixture change is consistent** + +Run: `pytest tests/test_graph_api.py -v` +Expected: PASS. In particular `test_admin_source_returns_lines`, `test_admin_source_reads_beta_repo`, and `test_non_admin_source_out_of_scope_404` still pass (admin bypasses the cap; `scoped_hdr` now holds `source:view` so its 404 is a genuine scope block). `test_explore_capability_required` still passes (its `no-explore` role holds neither `explore` nor `source:view`, so `/source` is still 403). + +- [ ] **Step 6: Commit** + +```bash +git add src/tpk/graph_api.py tests/test_graph_api.py +git commit -m "feat(graph-api): require source:view for /api/graph/source (#66)" +``` + +--- + +### Task 3: Suppress `thinking` SSE events without `source:view` + +**Files:** +- Modify: `src/tpk/server.py:266-286` (compute the flag) and `:339-341` (gate the emit) +- Test: `tests/test_server.py` + +**Interfaces:** +- Consumes: `auth_mod.CAP_SOURCE_VIEW`, `auth_mod.expand_capabilities` (Task 1); the already-resolved `role` in the non-admin branch. +- Produces: the chat SSE stream omits every `{"type": "thinking"}` event when the caller lacks `source:view`; all other event types are unchanged. + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/test_server.py`. These monkeypatch `tpk.auth.get_role` so a non-admin role resolves without a real store, and give the stub a non-raising `_client` so the handler's role lookup reaches the patched `get_role`: + +```python +def _think_agent(): + # One thinking delta, then an answer token. + return FakeAgent([_think("secret internal reasoning"), _tok("the answer")]) + + +def _nonadmin_auth_with_role(monkeypatch, caps): + from tpk import auth as auth_mod + role = auth_mod.Role("r", [], capabilities=list(caps)) + monkeypatch.setattr(auth_mod, "get_role", lambda *a, **k: role) + a = _StubAuth(User("u", "", "r")) # non-admin user + a._client = lambda: None # reach patched get_role, don't raise + return a + + +def test_chat_suppresses_thinking_without_source_view(monkeypatch): + from tpk.auth import CAP_CHAT + auth = _nonadmin_auth_with_role(monkeypatch, [CAP_CHAT]) # no source:view + client = TestClient(create_app(agent=_think_agent(), auth=auth)) + events = _parse_sse(client.post("/chat", json={"message": "hi"}).text) + assert not any(e["type"] == "thinking" for e in events) + # The answer itself still streams. + assert any(e["type"] == "token" for e in events) + + +def test_chat_emits_thinking_with_source_view(monkeypatch): + from tpk.auth import CAP_CHAT, CAP_SOURCE_VIEW + auth = _nonadmin_auth_with_role(monkeypatch, [CAP_CHAT, CAP_SOURCE_VIEW]) + client = TestClient(create_app(agent=_think_agent(), auth=auth)) + events = _parse_sse(client.post("/chat", json={"message": "hi"}).text) + assert [e["text"] for e in events if e["type"] == "thinking"] == ["secret internal reasoning"] + + +def test_chat_admin_still_emits_thinking(): + # Default _StubAuth user is admin -> holds every capability. + client = TestClient(create_app(agent=_think_agent(), auth=_StubAuth())) + events = _parse_sse(client.post("/chat", json={"message": "hi"}).text) + assert any(e["type"] == "thinking" for e in events) +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pytest tests/test_server.py -k "thinking_without_source_view or thinking_with_source_view" -v` +Expected: FAIL — `test_chat_suppresses_thinking_without_source_view` finds a `thinking` event (gate not implemented yet). + +- [ ] **Step 3: Write minimal implementation** + +In `src/tpk/server.py`, compute `can_view_source` in the `/chat` handler. Initialize it to the admin default before the non-admin branch (near line 266, alongside `scope = None`): + +```python + scope = None + can_view_source = (user.role == auth_mod.ROLE_ADMIN) + turn_limit = 0 # effective daily token budget for this user (0 = unlimited) + if user.role != auth_mod.ROLE_ADMIN: +``` + +Inside the non-admin branch, after `scope = frozenset(role.entry_keys) if role else frozenset()` (line 281), set the flag from the same resolved `role` (fails closed to no cap when the role is missing/unreadable): + +```python + caps = auth_mod.expand_capabilities(role.capabilities) if role else set() + can_view_source = auth_mod.CAP_SOURCE_VIEW in caps +``` + +Then gate the emit at line 340-341: + +```python + thinking = _chunk_thinking(chunk) + if thinking and can_view_source: + yield _sse({"type": "thinking", "text": thinking}) +``` + +(`can_view_source` is captured by the inner `stream()` closure, exactly as `scope` already is.) + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `pytest tests/test_server.py -k "thinking" -v` +Expected: PASS — including the pre-existing `test_chat_streams_thinking_interleaved_with_tools`, `test_chat_streams_openai_reasoning_as_thinking`, and `test_chat_no_thinking_emits_no_thinking_events` (all use the default admin `_StubAuth`, so `can_view_source` is `True`). + +- [ ] **Step 5: Run the full server suite** + +Run: `pytest tests/test_server.py -v` +Expected: PASS + +- [ ] **Step 6: Commit** + +```bash +git add src/tpk/server.py tests/test_server.py +git commit -m "feat(server): suppress thinking trace without source:view (#66)" +``` + +--- + +### Task 4: Add the answer-protection rule to the system prompt + +**Files:** +- Modify: `src/tpk/agent.py:145-148` (append a rule to the numbered `Rules:` block) +- Test: `tests/test_agent.py` + +**Interfaces:** +- Produces: `system_prompt(repos)` output contains an explicit instruction not to reproduce complete source. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/test_agent.py` (`system_prompt` is already imported): + +```python +def test_system_prompt_forbids_dumping_complete_source(): + p = system_prompt(REPOS) + # The model may explain and quote minimally, but must not reproduce whole files. + assert "never reproduce complete" in p + assert "decline" in p +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pytest tests/test_agent.py::test_system_prompt_forbids_dumping_complete_source -v` +Expected: FAIL — the phrases are absent. + +- [ ] **Step 3: Write minimal implementation** + +In `src/tpk/agent.py`, add a new rule as item 7, immediately before the closing `"""` of the prompt (after the current rule 6 that ends `...only in your private reasoning.`): + +```python +6. Your last message MUST be a normal assistant reply containing the + answer text itself — never end the conversation on a tool call or with + an empty message, and never leave the answer only in your private + reasoning. +7. PROTECT SOURCE CODE. Read and search the code freely to ground your + answer, and quote only the SHORT snippets needed to explain a point — + but never reproduce complete or near-complete files, and never + reconstruct a whole file across several quotes. If the user asks you to + print, dump, export, or output the full contents of a file, decline and + offer to explain what it does or show the specific lines relevant to + their question instead.""" +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pytest tests/test_agent.py::test_system_prompt_forbids_dumping_complete_source -v` +Expected: PASS + +- [ ] **Step 5: Confirm the existing prompt test still passes** + +Run: `pytest tests/test_agent.py::test_system_prompt_contains_corpus_and_citation_rules -v` +Expected: PASS (the citation/grounding rules are untouched). + +- [ ] **Step 6: Commit** + +```bash +git add src/tpk/agent.py tests/test_agent.py +git commit -m "feat(agent): system-prompt rule against dumping complete source (#66)" +``` + +--- + +### Task 5: Frontend — expose `source:view` and gate citation previews + +**Files:** +- Modify: `web/src/capabilities.ts:5-22` (add `CAP.sourceView` + `CAPABILITY_OPTIONS` entry) +- Modify: `web/src/App.tsx:146` (pass the flag into `Chat`) +- Modify: `web/src/Chat.tsx` (`SourceCard` at `:150-187`, its call site at `:690`, and the `Chat` prop list at `:200-208`) + +**Interfaces:** +- Consumes: `me.capabilities` from `/auth/me` (already fetched in `App.tsx`). +- Produces: role editor lists "View source" so admins can grant it; `SourceCard` shows a restricted note instead of fetching the fragment when the user lacks the cap. (Server already withholds `thinking` events and 403s `/api/graph/source`, so this is UX, not the enforcement boundary.) + +> No automated frontend tests exist in `web/` (no vitest/jest, no `*.test.*`). Verification is a type-check/build plus manual check. Keep changes minimal. + +- [ ] **Step 1: Add the capability to `capabilities.ts`** + +In `web/src/capabilities.ts`, add to the `CAP` object: + +```javascript +export const CAP = { + chat: "chat", + explore: "explore", + corpusView: "corpus:view", + corpusManage: "corpus:manage", + usersView: "users:view", + usersManage: "users:manage", + sourceView: "source:view", +} as const; +``` + +And append to `CAPABILITY_OPTIONS` (keep it last, matching backend order): + +```javascript + { key: CAP.usersManage, label: "Users — manage", implies: CAP.usersView }, + { key: CAP.sourceView, label: "View source (thinking trace & citation code)" }, +``` + +Leave `MANAGE_IMPLIES_VIEW` unchanged (no `:manage` sibling). + +- [ ] **Step 2: Thread the flag from `App.tsx` into `Chat`** + +In `web/src/App.tsx`, update the `Chat` render (line 146) to pass the capability: + +```javascript + setChatPrefill(undefined)} + canViewSource={hasCap(me.capabilities, CAP.sourceView)} /> +``` + +(`hasCap` and `CAP` are already imported in `App.tsx`.) + +- [ ] **Step 3: Accept the prop and gate `SourceCard` in `Chat.tsx`** + +Add `canViewSource` to the `Chat` component's props (destructure at `:200-208`, and its type): + +```javascript +export default function Chat({ + initialInput, + onConsumeInitial, + canViewSource = false, +}: { + initialInput?: string; + onConsumeInitial?: () => void; + canViewSource?: boolean; +} = {}) { +``` + +Pass it to the `SourceCard` at line 690: + +```javascript + +``` + +Update `SourceCard` (`:150-161`) to skip the fetch and show a restricted note when the cap is absent: + +```javascript +function SourceCard({ n, source, canViewSource }: { n?: number; source: SourceEventPayload; canViewSource: boolean }) { + const [preview, setPreview] = useState( + canViewSource ? "loading" : "restricted", + ); + + useEffect(() => { + if (!canViewSource) { setPreview("restricted"); return; } + let cancelled = false; + setPreview("loading"); + readSource(source.repo, source.file_path, source.line_start, source.line_end) + .then((r) => { if (!cancelled) setPreview(r); }) + .catch(() => { if (!cancelled) setPreview("error"); }); + return () => { cancelled = true; }; + }, [canViewSource, source.repo, source.file_path, source.line_start, source.line_end]); +``` + +Add a branch in the render for the `"restricted"` state (alongside the `"loading"`/`"error"` branches at `:168-172`), so the card still shows the path + line range but not the body: + +```javascript + {preview === "loading" ? ( +
Loading preview…
+ ) : preview === "restricted" ? ( +
Source preview restricted for your role
+ ) : preview === "error" ? ( +
Preview unavailable
+ ) : ( +``` + +- [ ] **Step 4: Type-check and build** + +Run: `npm --prefix web run build` +Expected: PASS — `tsc -b` reports no type errors and `vite build` completes. (Fix any prop-type mismatch surfaced here.) + +- [ ] **Step 5: Manual verification (record result in the commit / PR)** + +- As an admin: Chat shows the thinking trace, and clicking a citation renders the code preview. The role editor (Users view) lists "View source (thinking trace & citation code)". +- Create a non-admin role with `chat` + `explore` but **not** `source:view`; log in as a user in it: no thinking trace appears, and a citation card shows "Source preview restricted for your role" with the path/line range still visible. + +- [ ] **Step 6: Commit** + +```bash +git add web/src/capabilities.ts web/src/App.tsx web/src/Chat.tsx +git commit -m "feat(web): gate thinking trace & citation preview behind source:view (#66)" +``` + +--- + +### Task 6: Full-suite verification + +**Files:** none (verification only). + +- [ ] **Step 1: Run the backend test suite** + +Run: `pytest tests/test_capabilities.py tests/test_graph_api.py tests/test_server.py tests/test_agent.py -v` +Expected: PASS. (These four files cover every backend change; run the full `pytest` if the environment has a live Timeplus for the `requires_timeplus` integration tests.) + +- [ ] **Step 2: Build the frontend** + +Run: `npm --prefix web run build` +Expected: PASS. + +- [ ] **Step 3: Open the PR** + +Push the branch and open a PR referencing #66, summarizing the three enforcement points (prompt, thinking gate, `/api/graph/source` cap) and the default-deny migration note (existing non-admin roles lose the thinking panel and citation previews until an admin grants `source:view`). + +--- + +## Self-Review + +**Spec coverage:** +- New `source:view` capability, default-deny, admin-held → Task 1. +- Enforcement point 1 (answer / prompt) → Task 4. +- Enforcement point 2 (thinking trace suppressed in stream) → Task 3. +- Enforcement point 3 (`/api/graph/source` requires the cap) → Task 2. +- Frontend cap exposure + citation/thinking gating → Task 5. +- Migration/behavior-change note surfaced in the PR → Task 6, Step 3. +- Testing list from the spec (expand includes cap; endpoint 403/200/admin; thinking suppressed/present; scope not bypassed) → Tasks 1–3. + +**Placeholder scan:** No TODO/TBD; every code and test step is concrete. + +**Type/name consistency:** `CAP_SOURCE_VIEW`/`"source:view"`/`CAP.sourceView` used consistently across tasks; `can_view_source` defined before use and captured by the `stream()` closure; `SourceCard`'s new `canViewSource` prop is threaded from `App.tsx` → `Chat` → `SourceCard`; the `"restricted"` preview state is added to both the `useState` union and the render branches. diff --git a/docs/superpowers/specs/2026-08-20-source-code-protection-design.md b/docs/superpowers/specs/2026-08-20-source-code-protection-design.md new file mode 100644 index 0000000..edf2cce --- /dev/null +++ b/docs/superpowers/specs/2026-08-20-source-code-protection-design.md @@ -0,0 +1,185 @@ +# Source-code protection design (#66) + +## Problem + +The knowledge agent ingests private source repos and reads raw source via the +`read_source` tool to answer questions. Reading is fine and must stay +unrestricted — the agent needs the code to answer well. The problem is at the +**user-facing output**: today the raw source can reach the user verbatim, +turning the Q&A surface into a source-download channel. + +Requirement: the agent may freely read code, but must not hand the user +complete source. + +## Source channels to the user + +There are exactly three ways source content reaches a user today: + +1. **The answer** — `token`/`done` SSE events (`server.py:345`, `:431`). The + model can paste source into its free-form answer text. +2. **The thinking trace** — `thinking` SSE event (`server.py:341`). Reasoning + text (Anthropic extended thinking; OpenAI-compatible reasoning models) can + quote source. +3. **Citation fragments** — the `source` SSE event (`server.py:410`) carries + only `repo/file_path/line_start/line_end` **references, not the body**. The + body reaches the user only when they click a citation, which calls + `GET /api/graph/source` (`graph_api.py:85`) — the only endpoint that returns + raw `lines`. Gated today by `CAP_EXPLORE` alone. + +The `read_source` tool's raw return value is **not** streamed to the client +(only `search_entities` emits a `tool_result` with a count; `read_source` emits +just the reference `source` event). So the tool output body never reaches the +user except through channel 1 or 3. + +## Decisions (locked) + +- **Answer channel: prompt only, always on.** No code enforcement, no + post-hoc redaction, no streaming line-cap. A system-prompt rule instructs the + model to explain and quote minimally and to decline full-file dumps. Applies + to every role, including admin (it is guidance, not a gate). +- **Thinking + citations: one role capability**, default-deny. +- **Dropped from the original issue (YAGNI):** aggregate per-conversation + output budget, streaming line-cap, post-hoc verbatim-diff redaction, per-repo + strictness knobs. Prompt-only + a single role gate meets the requirement. + +## Design + +### New capability: `source:view` + +Grants visibility of raw source detail — the thinking trace **and** +citation-fragment bodies. + +- Added to `auth.py`: new constant `CAP_SOURCE_VIEW = "source:view"`, appended + to `ALL_CAPABILITIES` (canonical UI/display order). No `:manage` sibling (it + is a bare grant like `chat`/`explore`), so `_MANAGE_IMPLIES_VIEW` is + untouched. +- **Not** added to `DEFAULT_CAPABILITIES` → default-deny. `admin` bypasses all + capability checks as usual. +- Added to `web/src/capabilities.ts` (`CAP`, `CAPABILITY_OPTIONS`) so it renders + in the role editor. + +> **Amended after review of the running build.** The original design kept the +> `file:line` reference rows visible to all chat users and gated only the body. +> In practice a restricted user still saw the reasoning trace and clickable +> citations, and — critically — the reference metadata was only hidden in the +> UI, still crossing the wire in the SSE stream. Decision revised to +> **answer-only** for users without `source:view`: the `/chat` stream carries +> only the answer (`token`/`done`/`error`), and the reasoning trace + all +> source/citation references are withheld **server-side** (not just hidden in +> the client). The server still records the full trace/sources to the audit +> log. See "Enforcement point 2" below. + +### Enforcement point 1 — answer (prompt) + +Extend the agent system prompt in `agent.py` (the numbered guidance block) with +a source-protection rule: explain behavior and structure, quote only short +illustrative snippets, never reproduce complete or near-complete files; if the +user asks to print/dump a whole file, decline and offer to explain instead. The +existing citation guidance (use `read_source` so files appear in the Sources +panel) is unchanged — reading and citing still happen; only bulk reproduction in +the answer is discouraged. + +### Enforcement point 2 — thinking trace (`server.py`) + +The chat stream handler already resolves the caller's `role` (see +`server.py:236`, `:274`). Compute a per-request flag: + +``` +can_view_source = (user.role == ROLE_ADMIN) or (CAP_SOURCE_VIEW in caps) +``` + +where `caps` is the expanded capability set for the role. When +`can_view_source` is false, the stream is **answer-only**: withhold every +non-answer event so the reasoning trace and all source references never cross +the wire. Concretely, gate these `yield _sse(...)` calls on `can_view_source`: + +- `thinking` — the extended-thinking deltas. +- `tool` (`on_tool_start`) and `tool_result` — tool names/inputs (a + `read_source` input *is* a file/line reference). +- `source` — the clickable citation reference. +- `done` — send `"sources": []` instead of the accumulated list. + +Only `token`, `done` (answer text), and `error` reach a restricted user. The +audit accumulators (`tool_calls`, `sources`) are still built and written to the +audit log — only the client stream is gated, not the server-side record. + +Why server-side and not just client hiding: the client re-labels the model's +between-tool narration (`token` text before a `tool` event) as a "thinking" +step, so hiding only `kind==="thinking"` rows still showed a trace; and any +client-only gate leaves the references visible to anyone reading the raw SSE +stream. Gating at emission is the actual boundary. + +### Enforcement point 3 — citation fragment (`graph_api.py`) + +`GET /api/graph/source` is the only endpoint returning raw `lines`. Its +dependency becomes `require_cap(CAP_SOURCE_VIEW)` — `source:view` **replaces** +`CAP_EXPLORE` here rather than being added to it. This is deliberate: `/source` +backs the **chat citation preview** (`SourceCard`), not just the Explorer, so a +chat user granted `source:view` but not `explore` must still reach it; +requiring `explore` as well would break citation previews for exactly that role. +`source:view` is the source-specific grant, so gating `/source` on it alone is +the correct condition. Without the cap the endpoint returns 403; the +`search`/`entity`/`neighbors` endpoints keep `CAP_EXPLORE` and are unaffected. + +Admin keeps working automatically: `require_cap` checks `effective_caps` +(`auth.py:390`), and `effective_caps` returns `set(ALL_CAPABILITIES)` for admin +(`auth.py:377`) — so adding `source:view` to `ALL_CAPABILITIES` grants it to +admin with no special-casing. + +### Frontend (`web/src/`) + +The app already fetches `capabilities` from `/auth/me` and gates UI with +`hasCap`. Thread the flag `hasCap(caps, CAP.sourceView)` into `Chat.tsx` as +`canViewSource`. These are defense-in-depth — the server now withholds the +events, so they matter only if a future stream change regresses: + +- **Reasoning trace:** `renderTrace` returns `null` when `canViewSource` is + false — no thinking steps, no tool-call rows. +- **Sources panel:** the citation side-panel (`activeSource`) is not rendered + when `canViewSource` is false. `SourceCard` also keeps its own guard, skipping + the `readSource` fetch and showing a restricted note. +- **Role editor (`Users.tsx`):** `CAPABILITY_OPTIONS` gains `source:view` so + admins can grant it. + +## Migration / behavior change + +Default-deny means existing non-admin roles lose the thinking panel and fragment +previews on upgrade until an admin grants `source:view`. This is the intended +protection posture (confirmed). No data migration is required — roles that +predate this cap simply don't hold it. Admin is unaffected. + +## Testing (TDD) + +Backend, mirroring existing auth/graph/server tests: + +- `expand_capabilities` includes `source:view` when granted; unknown/absent + behaves as before. +- `GET /api/graph/source` → 403 for a role without `source:view`; 200 for a role + with it; 200 for admin. +- Chat stream: `thinking` events are suppressed for a role without the cap and + present for a role with it (and for admin). Assert other event types + (`token`, `source`, `done`) are unaffected in both cases. +- Sanity: a role with `source:view` but the endpoint otherwise scoped still + respects role `entry_keys` scope (no scope bypass introduced). + +Frontend: follow existing `web/src` test conventions if present; otherwise a +manual check that thinking rows and fragment previews are hidden without the cap +and shown with it. + +## Out of scope + +- Any change to `read_source` tool access or what the model reads into context. +- Aggregate output budgets, streaming caps, redaction, per-repo strictness. +- Reworking how citations are represented for privileged users (unchanged); + restricted users simply receive no citation events. + +## Touched files + +- `src/tpk/auth.py` — new capability constant + `ALL_CAPABILITIES`. +- `src/tpk/agent.py` — system-prompt source-protection rule. +- `src/tpk/server.py` — compute `can_view_source`, gate `thinking` events. +- `src/tpk/graph_api.py` — require `source:view` on `/api/graph/source`. +- `web/src/capabilities.ts` — new cap + `CAPABILITY_OPTIONS`. +- `web/src/Chat.tsx` — gate thinking rows + `SourceCard` preview/click. +- `web/src/Users.tsx` — cap appears in role editor (via `CAPABILITY_OPTIONS`). +- Tests alongside the backend modules above. diff --git a/src/tpk/agent.py b/src/tpk/agent.py index 9d3abb9..ad466ac 100644 --- a/src/tpk/agent.py +++ b/src/tpk/agent.py @@ -145,7 +145,14 @@ def system_prompt(repos) -> str: 6. Your last message MUST be a normal assistant reply containing the answer text itself — never end the conversation on a tool call or with an empty message, and never leave the answer only in your private - reasoning.""" + reasoning. +7. PROTECT SOURCE CODE. Read and search the code freely to ground your + answer, and quote only the SHORT snippets needed to explain a point — + but never reproduce complete or near-complete files, and never + reconstruct a whole file across several quotes. If the user asks you to + print, dump, export, or output the full contents of a file, decline and + offer to explain what it does or show the specific lines relevant to + their question instead.""" def build_agent( diff --git a/src/tpk/auth.py b/src/tpk/auth.py index 250c5d2..e534491 100644 --- a/src/tpk/auth.py +++ b/src/tpk/auth.py @@ -29,12 +29,14 @@ CAP_CORPUS_MANAGE = "corpus:manage" CAP_USERS_VIEW = "users:view" CAP_USERS_MANAGE = "users:manage" +CAP_SOURCE_VIEW = "source:view" # Order is the canonical UI/display order. ALL_CAPABILITIES = [ CAP_CHAT, CAP_EXPLORE, CAP_CORPUS_VIEW, CAP_CORPUS_MANAGE, CAP_USERS_VIEW, CAP_USERS_MANAGE, + CAP_SOURCE_VIEW, ] _CAP_SET = frozenset(ALL_CAPABILITIES) # `:manage` grants its `:view` sibling for free. diff --git a/src/tpk/graph_api.py b/src/tpk/graph_api.py index 9f9ae08..9048ae7 100644 --- a/src/tpk/graph_api.py +++ b/src/tpk/graph_api.py @@ -85,7 +85,7 @@ async def neighbors(id: str, direction: str = "both", depth: int = 1, @router.get("/source") async def source(repo: str, file_path: str, line_start: int = Query(..., ge=1), line_end: int = Query(..., ge=1), - user=Depends(auth.require_cap(auth_mod.CAP_EXPLORE))): + user=Depends(auth.require_cap(auth_mod.CAP_SOURCE_VIEW))): try: lines = await _scoped(user, lambda: kg.read_source(repo, file_path, line_start, line_end)) except ValueError as exc: diff --git a/src/tpk/server.py b/src/tpk/server.py index 445c075..c12745f 100644 --- a/src/tpk/server.py +++ b/src/tpk/server.py @@ -264,6 +264,7 @@ async def chat(req: ChatRequest, user: User = Depends(auth.require_cap(auth_mod. audit_provider, audit_model = "", "" scope = None + role = None turn_limit = 0 # effective daily token budget for this user (0 = unlimited) if user.role != auth_mod.ROLE_ADMIN: try: @@ -305,6 +306,12 @@ async def chat(req: ChatRequest, user: User = Depends(auth.require_cap(auth_mod. }, ) + # Whether this user may see the reasoning trace + source references. + # Single source of truth: `effective_capabilities` already encodes + # admin -> all, missing/unreadable role -> none (fail closed), else the + # view-expanded role caps — the same helper the API/UI gates use. + can_view_source = auth_mod.CAP_SOURCE_VIEW in auth_mod.effective_capabilities(user, role) + async def stream(): # Set inside stream(), not the handler body: the generator runs # after `chat` returns (StreamingResponse drives it lazily), so @@ -337,12 +344,21 @@ async def stream(): # with tool calls in event order — the UI groups # consecutive deltas into a thinking step. thinking = _chunk_thinking(chunk) - if thinking: + if thinking and can_view_source: yield _sse({"type": "thinking", "text": thinking}) text = _chunk_text(chunk) if text: full.append(text) - yield _sse({"type": "token", "text": text}) + # Token deltas interleave the model's between-tool + # narration (reasoning) with answer text and can't + # be told apart mid-stream. For users without + # source:view the stream is answer-only: withhold + # token deltas entirely and deliver the answer in + # the `done` event (built from `full`/final_text), + # so no narration crosses the wire or lingers in + # the client buffer to surface on an error. + if can_view_source: + yield _sse({"type": "token", "text": text}) elif kind == "on_chat_model_end": # The last model turn's message is the authoritative # answer — token deltas can miss it entirely for @@ -355,13 +371,19 @@ async def stream(): # (the tool loop makes several) for the daily budget. turn_tokens += _usage_tokens(output) elif kind == "on_tool_start": - yield _sse( - { - "type": "tool", - "name": event.get("name", ""), - "input": event.get("data", {}).get("input", {}), - } - ) + # Trace/source events are withheld from users without + # source:view — the API stream carries only the answer + # (token/done/error). Reference metadata (tool names, + # file paths, line ranges) must not cross the wire, not + # just be hidden client-side. + if can_view_source: + yield _sse( + { + "type": "tool", + "name": event.get("name", ""), + "input": event.get("data", {}).get("input", {}), + } + ) elif kind == "on_tool_end": name = event.get("name", "") args = event.get("data", {}).get("input") or {} @@ -389,9 +411,12 @@ async def stream(): if isinstance(items, list): audit_entry["count"] = len(items) audit_entry["unit"] = "matches" - yield _sse({"type": "tool_result", "name": name, - "input": args, "count": len(items), - "unit": "matches"}) + # audit count is recorded above regardless; + # only the client event is cap-gated. + if can_view_source: + yield _sse({"type": "tool_result", "name": name, + "input": args, "count": len(items), + "unit": "matches"}) except Exception: logger.exception("failed to emit search tool_result; skipping") # Surface cited sources for the read_source rows. @@ -416,8 +441,12 @@ async def stream(): "line_end": line_end, "kind": "file", } + # `sources` is accumulated for the audit + # record below regardless; the client event + # (the clickable citation) is cap-gated. sources.append(source) - yield _sse(source) + if can_view_source: + yield _sse(source) except Exception: logger.exception( "failed to process on_tool_end source event; skipping" @@ -428,7 +457,10 @@ async def stream(): "or rephrase." ) answer_text = done_text - yield _sse({"type": "done", "text": done_text, "sources": sources}) + # Withhold the citation list from users without source:view — + # the done event carries the answer only. + yield _sse({"type": "done", "text": done_text, + "sources": sources if can_view_source else []}) except Exception as exc: # stream errors must reach the client # Log the full exception server-side; the client only gets # the exception's class name, never the raw message, which diff --git a/tests/test_agent.py b/tests/test_agent.py index b99b5ed..cfe1299 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -47,6 +47,13 @@ def test_system_prompt_contains_corpus_and_citation_rules(): assert "neighbors" in p and "path_between" in p +def test_system_prompt_forbids_dumping_complete_source(): + p = system_prompt(REPOS) + # The model may explain and quote minimally, but must not reproduce whole files. + assert "never reproduce complete" in p + assert "decline" in p + + def test_recursion_limit_constant(): assert RECURSION_LIMIT == 40 diff --git a/tests/test_capabilities.py b/tests/test_capabilities.py index c6b4537..16a027e 100644 --- a/tests/test_capabilities.py +++ b/tests/test_capabilities.py @@ -10,8 +10,8 @@ from tpk import auth from tpk.auth import ( ALL_CAPABILITIES, CAP_CHAT, CAP_CORPUS_MANAGE, CAP_CORPUS_VIEW, - CAP_EXPLORE, CAP_USERS_MANAGE, CAP_USERS_VIEW, Role, User, - effective_capabilities, expand_capabilities, + CAP_EXPLORE, CAP_SOURCE_VIEW, CAP_USERS_MANAGE, CAP_USERS_VIEW, + DEFAULT_CAPABILITIES, Role, User, effective_capabilities, expand_capabilities, ) from tpk.server import create_app @@ -47,6 +47,18 @@ def test_effective_capabilities_admin_and_scoped(): assert effective_capabilities(scoped, None) == set() +def test_source_view_capability_registered_and_default_deny(): + assert CAP_SOURCE_VIEW == "source:view" + assert CAP_SOURCE_VIEW in ALL_CAPABILITIES + # Default-deny: brand-new roles must not silently hold it. + assert CAP_SOURCE_VIEW not in DEFAULT_CAPABILITIES + # It is a bare grant (no :manage sibling), so expand is identity. + assert expand_capabilities([CAP_SOURCE_VIEW]) == {CAP_SOURCE_VIEW} + # Admin holds every capability, including this one. + admin = User("a", "", auth.ROLE_ADMIN) + assert CAP_SOURCE_VIEW in effective_capabilities(admin, None) + + # -- integration fixtures -------------------------------------------------- pytestmark = requires_timeplus diff --git a/tests/test_graph_api.py b/tests/test_graph_api.py index fb9b8ab..73d976f 100644 --- a/tests/test_graph_api.py +++ b/tests/test_graph_api.py @@ -118,7 +118,7 @@ def scoped_hdr(kg_app): (and anything else) is out of scope for it.""" c, client, prefix = kg_app auth.upsert_role(client, auth.Role("alpha-only", ["alpha@v1"], - capabilities=[auth.CAP_EXPLORE]), prefix=prefix) + capabilities=[auth.CAP_EXPLORE, auth.CAP_SOURCE_VIEW]), prefix=prefix) auth.upsert_user(client, auth.User("scoped", auth.hash_password("password-1"), "alpha-only"), prefix=prefix) _eventually(lambda: c.post("/auth/login", json={"username": "scoped", "password": "password-1"}).status_code == 200) token = c.post("/auth/login", json={"username": "scoped", "password": "password-1"}).json()["token"] @@ -263,3 +263,23 @@ def test_non_admin_source_out_of_scope_404(kg_app, scoped_hdr): "repo": "beta@v1", "file_path": "b.py", "line_start": 1, "line_end": 3, }, headers=scoped_hdr) assert r.status_code == 404 + + +def test_source_requires_source_view_cap(kg_app): + """`/source` now requires source:view; a role with explore-only can still + search/entity/neighbors but is refused the raw source body.""" + c, client, prefix = kg_app + auth.upsert_role(client, auth.Role("explore-only", ["r1"], + capabilities=[auth.CAP_EXPLORE]), prefix=prefix) + auth.upsert_user(client, auth.User("exp", auth.hash_password("password-1"), + "explore-only"), prefix=prefix) + _eventually(lambda: c.post("/auth/login", + json={"username": "exp", "password": "password-1"}).status_code == 200) + token = c.post("/auth/login", json={"username": "exp", "password": "password-1"}).json()["token"] + hdr = {"Authorization": f"Bearer {token}"} + # search still works with explore alone... + assert c.get("/api/graph/search", params={"q": "AlphaWidget"}, headers=hdr).status_code == 200 + # ...but the source body is gated behind source:view. + assert c.get("/api/graph/source", params={ + "repo": "r1", "file_path": "w.py", "line_start": 1, "line_end": 2, + }, headers=hdr).status_code == 403 diff --git a/tests/test_server.py b/tests/test_server.py index 553a8c3..4d72f50 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -552,3 +552,92 @@ def test_chat_sets_and_resets_role_scope(): resp = client.post("/chat", json={"message": "hi"}) assert resp.status_code == 200 assert ROLE_SCOPE.get() is None + + +# -- thinking suppression without source:view capability (Task 3) -- + + +def _think_agent(): + # One thinking delta, then an answer token. + return FakeAgent([_think("secret internal reasoning"), _tok("the answer")]) + + +def _nonadmin_auth_with_role(monkeypatch, caps): + from tpk import auth as auth_mod + role = auth_mod.Role("r", [], capabilities=list(caps)) + monkeypatch.setattr(auth_mod, "get_role", lambda *a, **k: role) + a = _StubAuth(User("u", "", "r")) # non-admin user + a._client = lambda: None # reach patched get_role, don't raise + return a + + +def test_chat_suppresses_thinking_without_source_view(monkeypatch): + from tpk.auth import CAP_CHAT + auth = _nonadmin_auth_with_role(monkeypatch, [CAP_CHAT]) # no source:view + client = TestClient(create_app(agent=_think_agent(), auth=auth)) + events = _parse_sse(client.post("/chat", json={"message": "hi"}).text) + assert not any(e["type"] == "thinking" for e in events) + # The answer is still delivered — via the done event, not token deltas + # (token narration is withheld too; see the answer-only test below). + done = next(e for e in events if e["type"] == "done") + assert done["text"] == "the answer" + + +def test_chat_emits_thinking_with_source_view(monkeypatch): + from tpk.auth import CAP_CHAT, CAP_SOURCE_VIEW + auth = _nonadmin_auth_with_role(monkeypatch, [CAP_CHAT, CAP_SOURCE_VIEW]) + client = TestClient(create_app(agent=_think_agent(), auth=auth)) + events = _parse_sse(client.post("/chat", json={"message": "hi"}).text) + assert [e["text"] for e in events if e["type"] == "thinking"] == ["secret internal reasoning"] + + +def test_chat_admin_still_emits_thinking(): + # Default _StubAuth user is admin -> holds every capability. + client = TestClient(create_app(agent=_think_agent(), auth=_StubAuth())) + events = _parse_sse(client.post("/chat", json={"message": "hi"}).text) + assert any(e["type"] == "thinking" for e in events) + + +def _source_agent(): + # A read_source tool call (server derives a `source` citation from it), + # then an answer token. + args = {"repo": "docs@main", "file_path": "a.md", "line_start": 1, "line_end": 5} + return FakeAgent([_tool("read_source", args), _tool_end("read_source", args), _tok("the answer")]) + + +def test_chat_withholds_source_and_tool_events_without_source_view(monkeypatch): + """Without source:view the API stream is answer-only — no thinking, tool, + tool_result, source, or even token (narration) events, and the done event's + sources list is empty. Client-side hiding is not enough; the reasoning and + references must not cross the wire. The answer arrives via the done event.""" + from tpk.auth import CAP_CHAT + auth = _nonadmin_auth_with_role(monkeypatch, [CAP_CHAT]) # no source:view + client = TestClient(create_app(agent=_source_agent(), auth=auth)) + events = _parse_sse(client.post("/chat", json={"message": "hi"}).text) + types = {e["type"] for e in events} + assert types <= {"done"}, f"restricted stream leaked event types: {types - {'done'}}" + done = next(e for e in events if e["type"] == "done") + assert done["sources"] == [] + # The answer is still delivered — just via the done event, not token deltas. + assert done["text"] == "the answer" + + +def test_chat_includes_source_and_tool_events_with_source_view(monkeypatch): + from tpk.auth import CAP_CHAT, CAP_SOURCE_VIEW + auth = _nonadmin_auth_with_role(monkeypatch, [CAP_CHAT, CAP_SOURCE_VIEW]) + client = TestClient(create_app(agent=_source_agent(), auth=auth)) + events = _parse_sse(client.post("/chat", json={"message": "hi"}).text) + types = {e["type"] for e in events} + assert "tool" in types and "source" in types + done = next(e for e in events if e["type"] == "done") + assert len(done["sources"]) == 1 + + +def test_chat_admin_gets_source_and_tool_events(): + # Default _StubAuth user is admin -> holds every capability. + client = TestClient(create_app(agent=_source_agent(), auth=_StubAuth())) + events = _parse_sse(client.post("/chat", json={"message": "hi"}).text) + types = {e["type"] for e in events} + assert "tool" in types and "source" in types + done = next(e for e in events if e["type"] == "done") + assert len(done["sources"]) == 1 diff --git a/web/src/App.tsx b/web/src/App.tsx index b563f40..9f07d00 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -143,7 +143,8 @@ export default function App() { final-review regression this fixes: App.tsx used to unmount Chat on every navigation. */}
- setChatPrefill(undefined)} /> + setChatPrefill(undefined)} + canViewSource={hasCap(me.capabilities, CAP.sourceView)} />
{view === "explorer" ? ( diff --git a/web/src/Chat.tsx b/web/src/Chat.tsx index c128e42..8f43284 100644 --- a/web/src/Chat.tsx +++ b/web/src/Chat.tsx @@ -147,6 +147,9 @@ function LightbulbIcon() { // citation while the answer is still streaming. // -------------------------------------------------------------------- +// Only rendered for users with source:view — the parent gates the whole +// Sources panel on `canViewSource`, and the server withholds source events +// from users without it, so this card never needs its own restricted state. function SourceCard({ n, source }: { n?: number; source: SourceEventPayload }) { const [preview, setPreview] = useState("loading"); @@ -200,11 +203,13 @@ type UsageInfo = export default function Chat({ initialInput, onConsumeInitial, + canViewSource = false, }: { // Task 5 (Explorer "Ask about this") will pass an initial question in and // get notified once Chat has consumed it into its input box. initialInput?: string; onConsumeInitial?: () => void; + canViewSource?: boolean; } = {}) { const [turns, setTurns] = useState([]); const [input, setInput] = useState(""); @@ -443,6 +448,11 @@ export default function Chat({ } function renderTrace(turn: Turn, idx: number) { + // Without source:view the reasoning trace (thinking + tool calls) is + // withheld entirely — restricted roles see only the answer. The server + // already omits these events from the stream (#66 follow-up); this is the + // matching client guard so a future stream change can't surface them. + if (!canViewSource) return null; if (turn.trace.length === 0) return null; const streaming = turn.status === "streaming"; const elapsedMs = turn.elapsedMs ?? Date.now() - turn.startedAt; @@ -673,7 +683,7 @@ export default function Chat({
- {activeSource && ( + {canViewSource && activeSource && (