RomanNum3ral
/
honcho
mirror of https://github.com/plastic-labs/honcho.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(scopes): refuse a scope in every observed position; enumerate per position 

Addresses a fourth review pass against 62681e4d. The headline finding is that the
previous commit's enumeration test had the wrong *model*, not a missing entry.

1. Manual conclusions could create knowledge about a scope. `POST /conclusions`
   validated only that observer_id and observed_id exist, so a scope as
   `observed_id` persisted a conclusion about a peer carrying observe_me=false and
   created an (observer, scope) collection for it. Confirmed: 201, and it read
   back. `POST /schedule_dream` had the same hole via `observed`.

   The fix is positional, because the invariant is:

       A scope may be an OBSERVER. A scope may never be OBSERVED.

   A scope as `observer_id` is how scoped conclusions are stored and must keep
   working (verified still 201); as `observed_id` it is now refused. Same split
   applied to schedule_dream `observed`, the peer-card `target` (which also covers
   a scope's self-card, since target-omitted collapses observed to peer_id), and
   session-context `peer_target`.

2. Chat target and both representation roles kept check-to-use races. Only the
   chat path-level observer was re-checked on its resolved row; the target was
   checked by name and then resolved without inspecting scope identity. Both are
   now checked at the dialectic preflight, where observer and observed are already
   resolved — an absent name has already failed by then, and an existing squatter
   cannot retroactively become a scope.

3. Generic membership removal was still racy. The adjacent SELECT narrowed the
   window but could not close it under READ COMMITTED. The UPDATE now carries its
   own correlated NOT EXISTS against scope_peer_clause(), so Postgres evaluates
   the exclusion as part of the statement and a scope committed after the advisory
   check still cannot be detached.

4. New-name validation ran after the name reached Postgres. A NUL byte passed the
   request schemas and PeerSpec, then raised psycopg.DataError inside the lookup —
   a 500. Values that cannot correspond to a stored row by construction (NUL
   bytes, over-length names) are now refused before the query.
   (Over-length names already returned 422; only the wasted query was real there.)

The enumeration test is rekeyed from (method, path) to (method, path, position).
A binary per-route verdict cannot express finding 1 at all: `POST /conclusions` is
one route with two positions and opposite verdicts. Detection widens to observer /
observed / target / peer_target / peer_perspective, which surfaced four routes the
previous version never saw — conclusions, schedule_dream, queue/status, and
session context.

Also registers `src.routers.workspaces.tracked_db` in the conftest patch list; the
new guard there would otherwise have run against the real configured database
instead of the per-test one.

Mutation-tested: disabling the conclusions observed-guard fails the positional
test naming that position; adding an unclassified `observed_id` param fails
enumeration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vineeth Voruganti 2026-07-29 14:57:09 -04:00
parent 62681e4db7
commit 14136e5bc4
9 changed files with 587 additions and 260 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/crud/document.py
View File

@ -15,7 +15,7 @@ from sqlalchemy.sql.functions import func
from src import models, schemas
from src.config import settings
from src.crud.collection import get_or_create_collection
from src.crud.peer import get_peer
from src.crud.peer import get_peer, reject_scope_peers
from src.crud.session import get_session
from src.dependencies import tracked_db
from src.embedding_client import embedding_client
@ -954,6 +954,17 @@ async def create_observations(
for peer_name in peers_to_validate:
await get_peer(db, workspace_name, peer_name)
# A scope may be an *observer* — that is how scoped conclusions are stored —
# but it must never be *observed*: scope peers carry observe_me=false and no
# representation is ever formed of one. Without this, a conclusion about a
# scope persists and a (observer, scope) collection is created for it.
await reject_scope_peers(
db,
workspace_name,
{obs.observed_id for obs in observations},
action="No conclusion is ever formed about a scope.",
)
# Get or create all collections
for observer, observed in collection_pairs:
await get_or_create_collection(

36
src/crud/peer.py
View File

@ -47,6 +47,31 @@ def peer_cache_key(workspace_name: str, peer_name: str) -> str:
)
def _reject_impossible_peer_names(names: Iterable[str]) -> None:
"""Reject names that cannot correspond to any stored row, before querying.
``PeerSpec`` accepts anything so existing names can be looked up, and the
full new-name rules run later on the insert path — but a couple of values
cannot be a legacy row *by construction*, and sending them to Postgres first
fails before that 422 can happen:
- NUL bytes: Postgres text cannot hold them, so psycopg raises DataError
during the lookup itself, surfacing as a 500.
- Over-length names: the ``peers.name`` CHECK caps them at
``PEER_NAME_MAX_LENGTH``, so no stored row can exceed it.
Raises:
ValidationException: On a NUL byte or an over-length name.
"""
if any("\x00" in name for name in names):
raise ValidationException("Peer name(s) must not contain NUL (0x00) bytes")
too_long = sorted({n for n in names if len(n) > PEER_NAME_MAX_LENGTH})
if too_long:
raise ValidationException(
f"Peer name(s) must be at most {PEER_NAME_MAX_LENGTH} characters"
)
def _validate_new_peer_names(names: list[str]) -> None:
"""Validate peer names that are about to be created.
@ -61,12 +86,8 @@ def _validate_new_peer_names(names: list[str]) -> None:
scopes_util.validate_no_scope_peer_names(
names, action="Use the scopes routes to create scopes."
)
too_long = sorted({n for n in names if len(n) > PEER_NAME_MAX_LENGTH})
if too_long:
raise ValidationException(
f"Peer name(s) must be at most {PEER_NAME_MAX_LENGTH} characters"
)
# RESOURCE_NAME_PATTERN's `+` already rejects the empty name.
# Length and NUL bytes are already refused before the lookup by
# _reject_impossible_peer_names; RESOURCE_NAME_PATTERN's `+` rejects empty.
offenders = sorted({n for n in names if not re.fullmatch(RESOURCE_NAME_PATTERN, n)})
if offenders:
raise ValidationException(
@ -156,6 +177,9 @@ async def get_or_create_peers(
await get_or_create_workspace(db, schemas.WorkspaceCreate(name=workspace_name))
peer_names = [p.name for p in peers]
# Before the lookup: these values cannot match a stored row and would fail
# inside the query itself rather than as a clean 422.
_reject_impossible_peer_names(peer_names)
stmt = (
select(models.Peer)
.where(models.Peer.workspace_name == workspace_name)

40
src/crud/session.py
View File

@ -7,7 +7,18 @@ from typing import cast as typing_cast
from cashews import NOT_NONE
from nanoid import generate as generate_nanoid
from sqlalchemy import Select, and_, case, cast, delete, func, insert, select, update
from sqlalchemy import (
Select,
and_,
case,
cast,
delete,
exists,
func,
insert,
select,
update,
)
from sqlalchemy.dialects.postgresql import insert as pg_insert
from sqlalchemy.engine import CursorResult
from sqlalchemy.exc import IntegrityError
@ -34,7 +45,12 @@ from src.utils.scopes import is_scope_peer, scope_peer_name
from src.utils.types import GetOrCreateResult
from src.vector_store import get_external_vector_store
from .peer import get_or_create_peers, get_peer, reject_scope_peers
from .peer import (
get_or_create_peers,
get_peer,
reject_scope_peers,
scope_peer_clause,
)
from .scope import SCOPE_MEMBERSHIP_CONFIG, get_or_create_scopes
from .workspace import get_or_create_workspace
@ -848,9 +864,9 @@ async def remove_peers_from_session(
await get_session(db, session_name, workspace_name)
# Scope membership is ended through the scopes routes, which also reconcile
# the scope's copies. Checked here rather than in the route so it is adjacent
# to the UPDATE below, leaving no window for a concurrently-created scope to
# be silently detached from its sessions.
# the scope's copies. Rejected up front for a clear 422 rather than a silent
# no-op — but this check alone is only advisory: under READ COMMITTED a scope
# can be created between it and the UPDATE below.
if not _allow_scope_peers:
await reject_scope_peers(
db,
@ -870,6 +886,20 @@ async def remove_peers_from_session(
)
.values(left_at=func.now())
)
if not _allow_scope_peers:
# Closes the window the advisory check above cannot: the exclusion is
# evaluated by Postgres as part of the UPDATE, so a scope committed after
# that check still cannot be detached here. Correlated rather than a join
# so the statement stays a plain UPDATE.
update_stmt = update_stmt.where(
~exists(
select(models.Peer.id)
.where(models.Peer.workspace_name == workspace_name)
.where(models.Peer.name == models.SessionPeer.peer_name)
.where(scope_peer_clause())
.correlate(models.SessionPeer)
)
)
await db.execute(update_stmt)
await db.commit()

51
src/dialectic/chat.py
View File

@ -10,15 +10,38 @@ from collections.abc import AsyncIterator
from pydantic import BaseModel
from src import crud
from src import crud, models
from src.config import ReasoningLevel
from src.dependencies import tracked_db
from src.dialectic.core import DialecticAgent
from src.exceptions import ValidationException
from src.utils.config_helpers import get_configuration
from src.utils.scopes import is_scope_peer
logger = logging.getLogger(__name__)
def _reject_scope_participants(*peers: models.Peer) -> None:
"""Refuse a dialectic run whose observer or observed is a scope peer.
A scope is a silent observer with ``observe_me=false``: no representation of
one exists to query, and a scope as the path-level observer is a Phase 2b
concern rather than something the raw peer routes expose.
Raises:
ValidationException: If any participant is a scope.
"""
offenders = sorted(
{p.name for p in peers if is_scope_peer(p.name, p.internal_metadata)}
)
if offenders:
raise ValidationException(
f"Peer name(s) {offenders} are scopes."
+ " No representation is formed of a scope, so a scope cannot be a"
+ " dialectic observer or target."
)
async def agentic_chat(
workspace_name: str,
session_name: str | None,
@ -48,9 +71,18 @@ async def agentic_chat(
"""
# Short-lived DB session for validation + config
async with tracked_db("dialectic.preflight", read_only=True) as db:
await crud.get_peer(db, workspace_name, observer)
observer_peer = await crud.get_peer(db, workspace_name, observer)
observed_peer = observer_peer
if observer != observed:
await crud.get_peer(db, workspace_name, observed)
observed_peer = await crud.get_peer(db, workspace_name, observed)
# Resolved-row scope check, not a name check. The routes reject scope
# names up front for a clear error, but that runs before resolution: a
# scope created in between would otherwise be used here as observer or
# target. Checking the rows we just resolved closes that window — an
# absent name already failed above, and an existing unflagged squatter
# cannot retroactively become a scope.
_reject_scope_participants(observer_peer, observed_peer)
session = None
if session_name:
@ -120,9 +152,18 @@ async def agentic_chat_stream(
"""
# Short-lived DB session for validation + config
async with tracked_db("dialectic.preflight", read_only=True) as db:
await crud.get_peer(db, workspace_name, observer)
observer_peer = await crud.get_peer(db, workspace_name, observer)
observed_peer = observer_peer
if observer != observed:
await crud.get_peer(db, workspace_name, observed)
observed_peer = await crud.get_peer(db, workspace_name, observed)
# Resolved-row scope check, not a name check. The routes reject scope
# names up front for a clear error, but that runs before resolution: a
# scope created in between would otherwise be used here as observer or
# target. Checking the rows we just resolved closes that window — an
# absent name already failed above, and an existing unflagged squatter
# cannot retroactively become a scope.
_reject_scope_participants(observer_peer, observed_peer)
session = None
if session_name:

13
src/routers/peers.py
View File

@ -494,6 +494,19 @@ async def set_peer_card(
# If no target specified, set the observer's own card
observed = target if target is not None else peer_id
# A scope may be the *observer* of a card — the Dreamer writes (scope, observed)
# cards, which is how scoped peer cards exist — but never the observed. With no
# target, observed collapses to peer_id, so this also refuses a scope's self-card.
await crud.reject_scope_peers(
db,
workspace_id,
[observed],
action=(
"No representation is formed of a scope, so a scope cannot be the"
" subject of a peer card."
),
)
await crud.set_peer_card(
db,
workspace_id,

14
src/routers/sessions.py
View File

@ -745,6 +745,20 @@ async def get_session_context(
"peer_target must be provided if peer_perspective is provided"
)
# peer_target is the *observed* peer, and no representation or card is ever
# formed of a scope. peer_perspective (the observer) is left alone: a scope
# is a legitimate perspective, which is what Phase 2b's `scope` option builds on.
if peer_target is not None:
await crud.reject_scope_peers(
db,
workspace_id,
[peer_target],
action=(
"No representation is formed of a scope, so a scope cannot be a"
" context target."
),
)
if not peer_target:
# No representation or card needed
summary, messages = await _get_session_context_task(

18
src/routers/workspaces.py
View File

@ -9,7 +9,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
from src import crud, schemas
from src.config import settings
from src.dependencies import db, read_db
from src.dependencies import db, read_db, tracked_db
from src.deriver.enqueue import enqueue_deletion, enqueue_dream
from src.exceptions import AuthenticationException
from src.security import JWTParams, require_auth
@ -225,6 +225,22 @@ async def schedule_dream(
observed = request.observed if request.observed is not None else request.observer
dream_type = request.dream_type
# A scope is a legitimate dream *observer* — the Dreamer consolidates scoped
# collections — but never the observed: no representation is formed of a scope,
# so such a dream would build knowledge about one.
async with tracked_db(
"workspaces.schedule_dream.scope_check", read_only=True
) as db:
await crud.reject_scope_peers(
db,
workspace_id,
[observed],
action=(
"No representation is formed of a scope, so a scope cannot be the"
" observed peer of a dream."
),
)
await enqueue_dream(
workspace_id,
observer=observer,

1
tests/conftest.py
View File

@ -840,6 +840,7 @@ def mock_tracked_db(request: pytest.FixtureRequest):
"src.deriver.consumer.tracked_db",
"src.deriver.enqueue.tracked_db",
"src.routers.peers.tracked_db",
"src.routers.workspaces.tracked_db",
"src.crud.representation.tracked_db",
"src.dreamer.orchestrator.tracked_db",
"src.dreamer.dream_scheduler.tracked_db",

661
tests/routes/test_scope_route_policy.py
View File

@ -1,33 +1,41 @@
"""Route-policy enumeration for the scopes facade.
"""Route-policy enumeration for the scopes facade, per peer *position*.
Three review passes over the scopes work each found the same *class* of defect —
a route nobody had checked, not logic that was subtly wrong. One of them
(`PUT /sessions/{id}/peers/{id}/config`, which let any caller set a scope to
``observe_others=false`` and silently stop all fan-out into it) predated the
work entirely: the guardrail set was assembled guardrail-by-guardrail rather
than derived from the route list. Sampling review cannot close that kind of gap.
Four review passes over the scopes work each found the same class of defect: a
place nobody had checked, rather than logic that was subtly wrong. The first
version of this module enumerated routes and classified each one guarded or
exempt — and that model was itself the fifth defect. A binary per-route verdict
cannot express the actual invariant, which is positional:
So this module enumerates instead. It derives every route through which a peer
name can reach the system, and requires each to be classified as either
GUARDED (a real scope is refused) or EXEMPT (with a stated reason). A newly
added peer-touching route fails `test_every_peer_touching_route_is_classified`
until someone consciously classifies it.
A scope may be an OBSERVER. A scope may never be OBSERVED.
Two invariants are then asserted *behaviorally* — by calling the routes, not by
inspecting annotations, because the guards deliberately live in crud (which is
what makes `messages/upload` guarded for free, via `crud.create_messages`):
`POST /conclusions` is the case that proves it: a scope as `observer_id` is how
scoped conclusions are stored and must work, while a scope as `observed_id`
persisted a conclusion about something that carries ``observe_me=false``. One
route, two positions, opposite verdicts. The same split applies to
`schedule_dream`, the peer-card routes, and session context.
1. A real scope peer is refused on every GUARDED route.
2. An *unflagged* peer merely occupying the reserved namespace is NOT a scope and
is unaffected. This is the half most easily broken by a well-meaning guard —
it regressed once already when `update_peer` used a name-based check.
So classification here is keyed by ``(method, path, position)``, where position is
the request parameter carrying the peer name. Every derived triple must appear in
`POLICY` as either REFUSE or ALLOW-with-a-reason; a new one fails
`test_every_peer_position_is_classified` until someone classifies it.
Each REFUSE case is then asserted behaviorally — by calling the route, because the
guards deliberately live in crud (which is what makes `messages/upload` guarded
for free via `crud.create_messages`) — in both directions:
1. a real scope is refused, and the rejection must actually name it, so an
unrelated 422 cannot pass the assertion;
2. an *unflagged* peer merely occupying the reserved namespace is NOT a scope and
is unaffected. That half regressed once already when `update_peer` keyed off
the name prefix.
Known limitation: this covers the HTTP surface only. Peer names also reach the
system through the deriver, dreamer, and queue, which have no route table to
enumerate; a guard gap there would not be caught here.
enumerate; a gap there would not be caught here.
"""
from collections.abc import Callable, Iterator
from dataclasses import dataclass, field
import pytest
from fastapi.routing import APIRoute
@ -42,176 +50,353 @@ from src.main import app
from src.models import Peer, Workspace
from src.utils.scopes import scope_peer_name
# Parameter and model-field names that carry a peer name.
_PEER_PARAM_NAMES = {"peer_id", "peer_name", "peer_names"}
# Request parameters and model fields that carry a peer name, in any position.
_PEER_PARAM_NAMES = {
"peer_id",
"peer_name",
"peer_names",
"observer",
"observer_id",
"observed",
"observed_id",
"target",
"peer_target",
"peer_perspective",
}
# Peer names arriving as dict keys or an aliased body field are invisible to
# parameter-name detection, so these paths are matched by shape instead. The
# position recorded for them is the body field or key role.
_KEY_POSITION = "body_peer_keys"
# The scopes router *is* the facade; scope peers are its whole subject.
_SCOPES_PREFIX = "/v3/workspaces/{workspace_id}/scopes"
# A builder places `peer` into one position of one route and returns the response.
Builder = Callable[[TestClient, str, str, str], object]
def _request_update_peer(client: TestClient, ws: str, _session: str, peer: str):
return client.put(
f"/v3/workspaces/{ws}/peers/{peer}", json={"metadata": {"k": "v"}}
@dataclass(frozen=True)
class Case:
"""Policy for one peer position on one route."""
method: str
path: str
position: str
refuse: bool
reason: str = ""
build: Builder | None = None
# Set when the 422 legitimately comes from request-schema validation rather
# than a scope guard, so the detail is pydantic's rather than ours.
schema_level: bool = False
# Set when the squatter direction cannot be asserted here, with why.
skip_squatter: str = ""
_: tuple[()] = field(default=(), repr=False)
@property
def key(self) -> tuple[str, str, str]:
return (self.method, self.path, self.position)
_W = "/v3/workspaces/{workspace_id}"
def _b_create_peer(c: TestClient, ws: str, _s: str, p: str):
return c.post(f"/v3/workspaces/{ws}/peers", json={"id": p})
def _b_update_peer(c: TestClient, ws: str, _s: str, p: str):
return c.put(f"/v3/workspaces/{ws}/peers/{p}", json={"metadata": {"k": "v"}})
def _b_chat_observer(c: TestClient, ws: str, _s: str, p: str):
return c.post(f"/v3/workspaces/{ws}/peers/{p}/chat", json={"query": "hi"})
def _b_chat_target(c: TestClient, ws: str, _s: str, p: str):
return c.post(
f"/v3/workspaces/{ws}/peers/{_OTHER}/chat", json={"query": "hi", "target": p}
)
def _request_create_peer(client: TestClient, ws: str, _session: str, peer: str):
return client.post(f"/v3/workspaces/{ws}/peers", json={"id": peer})
def _b_repr_observer(c: TestClient, ws: str, _s: str, p: str):
return c.post(f"/v3/workspaces/{ws}/peers/{p}/representation", json={})
def _request_chat(client: TestClient, ws: str, _session: str, peer: str):
return client.post(
f"/v3/workspaces/{ws}/peers/{peer}/chat", json={"query": "what do you know?"}
def _b_repr_target(c: TestClient, ws: str, _s: str, p: str):
return c.post(
f"/v3/workspaces/{ws}/peers/{_OTHER}/representation", json={"target": p}
)
def _request_representation(client: TestClient, ws: str, _session: str, peer: str):
return client.post(f"/v3/workspaces/{ws}/peers/{peer}/representation", json={})
def _b_card_target(c: TestClient, ws: str, _s: str, p: str):
return c.put(
f"/v3/workspaces/{ws}/peers/{_OTHER}/card?target={p}",
json={"peer_card": ["note"]},
)
def _request_create_session_with_peer(
client: TestClient, ws: str, _session: str, peer: str
):
return client.post(
def _b_conclusion_observed(c: TestClient, ws: str, _s: str, p: str):
return c.post(
f"/v3/workspaces/{ws}/conclusions",
json={
"conclusions": [
{
"observer_id": _OTHER,
"observed_id": p,
"content": "something",
"level": "explicit",
}
]
},
)
def _b_dream_observed(c: TestClient, ws: str, _s: str, p: str):
return c.post(
f"/v3/workspaces/{ws}/schedule_dream",
json={"observer": _OTHER, "observed": p, "dream_type": "omni"},
)
def _b_session_create(c: TestClient, ws: str, _s: str, p: str):
return c.post(
f"/v3/workspaces/{ws}/sessions",
json={"id": str(generate_nanoid()), "peers": {peer: {}}},
json={"id": str(generate_nanoid()), "peers": {p: {}}},
)
def _request_create_message(client: TestClient, ws: str, session: str, peer: str):
return client.post(
f"/v3/workspaces/{ws}/sessions/{session}/messages",
json={"messages": [{"peer_id": peer, "content": "hello"}]},
def _b_session_context_target(c: TestClient, ws: str, s: str, p: str):
return c.get(f"/v3/workspaces/{ws}/sessions/{s}/context?peer_target={p}")
def _b_message(c: TestClient, ws: str, s: str, p: str):
return c.post(
f"/v3/workspaces/{ws}/sessions/{s}/messages",
json={"messages": [{"peer_id": p, "content": "hello"}]},
)
def _request_upload_message(client: TestClient, ws: str, session: str, peer: str):
return client.post(
f"/v3/workspaces/{ws}/sessions/{session}/messages/upload",
data={"peer_id": peer},
def _b_upload(c: TestClient, ws: str, s: str, p: str):
return c.post(
f"/v3/workspaces/{ws}/sessions/{s}/messages/upload",
data={"peer_id": p},
files={"file": ("note.txt", b"hello there", "text/plain")},
)
def _request_add_session_peers(client: TestClient, ws: str, session: str, peer: str):
return client.post(f"/v3/workspaces/{ws}/sessions/{session}/peers", json={peer: {}})
def _b_add_peers(c: TestClient, ws: str, s: str, p: str):
return c.post(f"/v3/workspaces/{ws}/sessions/{s}/peers", json={p: {}})
def _request_set_session_peers(client: TestClient, ws: str, session: str, peer: str):
return client.put(f"/v3/workspaces/{ws}/sessions/{session}/peers", json={peer: {}})
def _b_set_peers(c: TestClient, ws: str, s: str, p: str):
return c.put(f"/v3/workspaces/{ws}/sessions/{s}/peers", json={p: {}})
def _request_remove_session_peers(client: TestClient, ws: str, session: str, peer: str):
return client.request(
"DELETE", f"/v3/workspaces/{ws}/sessions/{session}/peers", json=[peer]
)
def _b_remove_peers(c: TestClient, ws: str, s: str, p: str):
return c.request("DELETE", f"/v3/workspaces/{ws}/sessions/{s}/peers", json=[p])
def _request_set_peer_config(client: TestClient, ws: str, session: str, peer: str):
return client.put(
f"/v3/workspaces/{ws}/sessions/{session}/peers/{peer}/config",
def _b_peer_config(c: TestClient, ws: str, s: str, p: str):
return c.put(
f"/v3/workspaces/{ws}/sessions/{s}/peers/{p}/config",
json={"observe_others": False, "observe_me": True},
)
# Routes that must refuse a real scope peer. Each maps to a request builder so
# the guard is proven by calling it, not by trusting an annotation.
SCOPE_GUARDED_ROUTES: dict[tuple[str, str], Builder] = {
("POST", "/v3/workspaces/{workspace_id}/peers"): _request_create_peer,
("PUT", "/v3/workspaces/{workspace_id}/peers/{peer_id}"): _request_update_peer,
("POST", "/v3/workspaces/{workspace_id}/peers/{peer_id}/chat"): _request_chat,
(
"POST",
"/v3/workspaces/{workspace_id}/peers/{peer_id}/representation",
): _request_representation,
(
"POST",
"/v3/workspaces/{workspace_id}/sessions",
): _request_create_session_with_peer,
(
"POST",
"/v3/workspaces/{workspace_id}/sessions/{session_id}/messages",
): _request_create_message,
(
"POST",
"/v3/workspaces/{workspace_id}/sessions/{session_id}/messages/upload",
): _request_upload_message,
(
"POST",
"/v3/workspaces/{workspace_id}/sessions/{session_id}/peers",
): _request_add_session_peers,
(
"PUT",
"/v3/workspaces/{workspace_id}/sessions/{session_id}/peers",
): _request_set_session_peers,
(
"DELETE",
"/v3/workspaces/{workspace_id}/sessions/{session_id}/peers",
): _request_remove_session_peers,
(
"PUT",
"/v3/workspaces/{workspace_id}/sessions/{session_id}/peers/{peer_id}/config",
): _request_set_peer_config,
}
# A plain peer used for the *other* side of two-position routes, so the position
# under test is the only scope in the request. Created by the fixtures below.
_OTHER = "policy-counterparty"
# Routes a scope peer may legitimately reach. Every entry needs a reason: adding
# one is a deliberate decision that a scope in this position is harmless.
SCOPE_EXEMPT_ROUTES: dict[tuple[str, str], str] = {
("GET", "/v3/workspaces/{workspace_id}/peers/{peer_id}/card"): (
"peer_id is the *observer*. The Dreamer writes (scope, observed) cards, so "
"reading a scope's card of another peer is normal operation."
_OBSERVER_OK = (
"Observer position. A scope observing others is the entire mechanism scopes "
"are built on, so this must keep working."
)
_READ_ONLY_OK = (
"Read-only. Returns nothing meaningful for a scope rather than creating or "
"mutating knowledge about one."
)
POLICY: tuple[Case, ...] = (
# ---- observed position: a scope must never be the subject ----
Case(
"POST", f"{_W}/conclusions", "observed_id", True, build=_b_conclusion_observed
),
("PUT", "/v3/workspaces/{workspace_id}/peers/{peer_id}/card"): (
"Same: peer_id is the observer. See DEV-1998 follow-up for the narrower "
"question of a scope's *self* card (target omitted), which is junk data "
"rather than a leak since nothing forms a representation of a scope."
Case("POST", f"{_W}/schedule_dream", "observed", True, build=_b_dream_observed),
Case("PUT", f"{_W}/peers/{{peer_id}}/card", "target", True, build=_b_card_target),
Case("POST", f"{_W}/peers/{{peer_id}}/chat", "target", True, build=_b_chat_target),
Case(
"POST",
f"{_W}/peers/{{peer_id}}/representation",
"target",
True,
build=_b_repr_target,
),
("GET", "/v3/workspaces/{workspace_id}/peers/{peer_id}/context"): (
"Read-only. Scope-as-path-peer on the context routes is closed in Phase 2b "
"(DEV-1998), which owns the read-side scope surface."
),
("POST", "/v3/workspaces/{workspace_id}/peers/{peer_id}/search"): (
"Read-only, and empty by construction: a scope can never author messages, "
"so there is nothing to search."
),
("POST", "/v3/workspaces/{workspace_id}/peers/{peer_id}/sessions"): (
"Read-only. A scope legitimately has member sessions; this is the "
"observer-mechanics view of what GET /scopes/{id}/sessions exposes."
),
("GET", "/v3/workspaces/{workspace_id}/sessions/{session_id}/peers"): (
"Read-only membership listing, which includes the scope observer."
),
(
Case(
"GET",
"/v3/workspaces/{workspace_id}/sessions/{session_id}/peers/{peer_id}/config",
): "Read-only. The write side of this route IS guarded.",
("POST", "/v3/keys"): (
"Mints a scoped JWT rather than touching a peer. Scope-bound keys are "
"Phase 3 (DEV-2002)."
f"{_W}/sessions/{{session_id}}/context",
"peer_target",
True,
build=_b_session_context_target,
),
}
# ---- observer position: legitimately a scope ----
Case("POST", f"{_W}/conclusions", "observer_id", False, reason=_OBSERVER_OK),
Case("POST", f"{_W}/schedule_dream", "observer", False, reason=_OBSERVER_OK),
Case("PUT", f"{_W}/peers/{{peer_id}}/card", "peer_id", False, reason=_OBSERVER_OK),
Case("GET", f"{_W}/peers/{{peer_id}}/card", "peer_id", False, reason=_OBSERVER_OK),
Case(
"GET",
f"{_W}/sessions/{{session_id}}/context",
"peer_perspective",
False,
reason=_OBSERVER_OK,
),
Case("GET", f"{_W}/queue/status", "observer_id", False, reason=_OBSERVER_OK),
# ---- peer identity / membership mutation: never a scope ----
Case(
"POST",
f"{_W}/peers",
_KEY_POSITION,
True,
build=_b_create_peer,
schema_level=True,
skip_squatter=(
"Creating any name in the reserved namespace is refused whether flagged "
"or not — that is what reserving it means. Covered by "
"test_scopes.py::test_peer_create_rejects_reserved_prefix."
),
),
Case("PUT", f"{_W}/peers/{{peer_id}}", "peer_id", True, build=_b_update_peer),
Case("POST", f"{_W}/sessions", "peer_names", True, build=_b_session_create),
Case(
"POST",
f"{_W}/sessions/{{session_id}}/messages",
"peer_name",
True,
build=_b_message,
),
Case(
"POST",
f"{_W}/sessions/{{session_id}}/messages/upload",
"peer_id",
True,
build=_b_upload,
),
Case(
"POST",
f"{_W}/sessions/{{session_id}}/peers",
_KEY_POSITION,
True,
build=_b_add_peers,
),
Case(
"PUT",
f"{_W}/sessions/{{session_id}}/peers",
_KEY_POSITION,
True,
build=_b_set_peers,
),
Case(
"DELETE",
f"{_W}/sessions/{{session_id}}/peers",
_KEY_POSITION,
True,
build=_b_remove_peers,
),
Case(
"PUT",
f"{_W}/sessions/{{session_id}}/peers/{{peer_id}}/config",
"peer_id",
True,
build=_b_peer_config,
),
# ---- path peer on the dialectic surface ----
Case(
"POST",
f"{_W}/peers/{{peer_id}}/chat",
"peer_id",
True,
build=_b_chat_observer,
),
Case(
"POST",
f"{_W}/peers/{{peer_id}}/representation",
"peer_id",
True,
build=_b_repr_observer,
),
# ---- reads that neither create nor mutate knowledge about a scope ----
Case(
"GET",
f"{_W}/sessions/{{session_id}}/peers/{{peer_id}}/config",
"peer_id",
False,
reason="Read-only. The write side of this same route IS refused.",
),
Case(
"POST", f"{_W}/peers/{{peer_id}}/search", "peer_id", False, reason=_READ_ONLY_OK
),
Case(
"POST",
f"{_W}/peers/{{peer_id}}/sessions",
"peer_id",
False,
reason=(
"Read-only. A scope legitimately has member sessions; this is the "
"observer-mechanics view of GET /scopes/{scope_id}/sessions."
),
),
Case(
"GET",
f"{_W}/peers/{{peer_id}}/context",
"peer_id",
False,
reason=(
"Read-only. The read-side scope surface belongs to Phase 2b (DEV-1998), "
"which owns the `scope` option on the context routes."
),
),
Case(
"GET",
f"{_W}/peers/{{peer_id}}/context",
"target",
False,
reason=(
"Read-only, and empty for a scope now that nothing can write knowledge "
"about one. Phase 2b (DEV-1998) owns this surface."
),
),
Case(
"GET",
f"{_W}/peers/{{peer_id}}/card",
"target",
False,
reason=(
"Read-only. The write side (PUT with target) IS refused, so this can only "
"return pre-existing rows, never create them."
),
),
Case(
"POST",
"/v3/keys",
"peer_id",
False,
reason=(
"Mints a scoped JWT rather than touching a peer. Scope-bound keys are "
"Phase 3 (DEV-2002)."
),
),
)
# Routes excluded from the squatter check, with why. The guarded direction is
# still asserted for all of these.
_SQUATTER_CHECK_SKIPS: dict[tuple[str, str], str] = {
("POST", "/v3/workspaces/{workspace_id}/peers"): (
"Creating a reserved-prefix peer is refused for ANY name in the namespace, "
"flagged or not — that is the point of reserving it. Covered by "
"test_scopes.py::test_peer_create_rejects_reserved_prefix."
),
}
_BY_KEY = {case.key: case for case in POLICY}
# Routes whose 422 legitimately comes from request-schema validation rather than a
# scope guard, so the response detail is pydantic's rather than ours. Everything
# else must name the offending peer AND say why, or the test would happily pass on
# an unrelated 422 (a malformed body, say) and prove nothing.
_SCHEMA_LEVEL_REJECTION: dict[tuple[str, str], str] = {
("POST", "/v3/workspaces/{workspace_id}/peers"): (
"PeerCreate.name carries RESOURCE_NAME_PATTERN, which the reserved prefix "
"violates, so pydantic refuses it before the route body runs."
),
# Routes whose peer names arrive as dict keys or an aliased body field, invisible
# to parameter-name detection and therefore matched by path shape.
_KEY_POSITION_PATHS = {
("POST", f"{_W}/peers"),
("POST", f"{_W}/sessions/{{session_id}}/peers"),
("PUT", f"{_W}/sessions/{{session_id}}/peers"),
("DELETE", f"{_W}/sessions/{{session_id}}/peers"),
}
@ -227,19 +412,19 @@ def _nested_models(annotation: object, seen: set[object]) -> Iterator[type[BaseM
seen.add(model)
yield model
fields: dict[str, FieldInfo] = model.model_fields
for field in fields.values():
stack = [field.annotation]
for f in fields.values():
stack = [f.annotation]
while stack:
current = stack.pop()
yield from _nested_models(current, seen)
stack.extend(getattr(current, "__args__", ()) or ())
def _peer_param_names(route: APIRoute) -> set[str]:
"""Peer-name-carrying params anywhere in a route's dependant tree.
def _peer_positions(route: APIRoute) -> set[str]:
"""Peer-name-carrying parameter names anywhere in a route's dependant tree.
Walks sub-dependencies so `Form(...)` params behind a parser dependency are
seen (this is how `messages/upload` takes its `peer_id`), and descends into
seen — this is how `messages/upload` takes its `peer_id` — and descends into
request-body models so `MessageCreate.peer_name` is seen too.
"""
found: set[str] = set()
@ -266,95 +451,89 @@ def _peer_param_names(route: APIRoute) -> set[str]:
return found
def _peer_touching_routes() -> set[tuple[str, str]]:
"""Every (method, path) through which a peer name can reach the system.
Union of two signals, because neither alone is sufficient: parameter names
miss routes where peer names are dict *keys* (`POST /sessions/{id}/peers`
takes `dict[str, SessionPeerConfig]`), and path shape misses names carried in
a body or form field.
"""
found: set[tuple[str, str]] = set()
def _derived_positions() -> set[tuple[str, str, str]]:
"""Every (method, path, position) through which a peer name can be supplied."""
found: set[tuple[str, str, str]] = set()
for route in app.routes:
if not isinstance(route, APIRoute):
continue
path = route.path.rstrip("/") or route.path
if path.startswith(_SCOPES_PREFIX):
continue
by_shape = "{peer_id}" in path or path.endswith("/peers")
if not (by_shape or _peer_param_names(route)):
continue
positions = _peer_positions(route)
for method in route.methods or set():
if method not in ("HEAD", "OPTIONS"):
found.add((method, path))
if method in ("HEAD", "OPTIONS"):
continue
if (method, path) in _KEY_POSITION_PATHS:
found.add((method, path, _KEY_POSITION))
for position in positions:
found.add((method, path, position))
return found
def test_every_peer_touching_route_is_classified():
"""Each peer-touching route is either guarded or explicitly exempt.
def test_every_peer_position_is_classified():
"""Each (route, peer position) pair has an explicit scope policy.
A new route fails here until classified. If this fails for a route you added,
decide whether a scope peer reaching it is harmful: add it to
SCOPE_GUARDED_ROUTES with a request builder, or to SCOPE_EXEMPT_ROUTES with a
reason. Do not add a mutating route to the exempt set.
A new one fails here until classified. Decide whether a scope in that
*position* is harmful — the rule is that a scope may be an observer but never
observed — then add a Case with `refuse=True` and a builder, or `refuse=False`
and a reason.
"""
classified = set(SCOPE_GUARDED_ROUTES) | set(SCOPE_EXEMPT_ROUTES)
actual = _peer_touching_routes()
derived = _derived_positions()
classified = set(_BY_KEY)
unclassified = actual - classified
unclassified = derived - classified
assert not unclassified, (
"peer-touching routes with no scope policy: "
+ f"{sorted(unclassified)} — classify each as guarded or exempt"
"peer positions with no scope policy: "
+ f"{sorted(unclassified)} — classify each as refuse or allow"
)
stale = classified - actual
assert not stale, f"classified routes that no longer exist: {sorted(stale)}"
stale = classified - derived
assert not stale, f"classified positions that no longer exist: {sorted(stale)}"
def test_guarded_and_exempt_sets_are_disjoint():
overlap = set(SCOPE_GUARDED_ROUTES) & set(SCOPE_EXEMPT_ROUTES)
assert not overlap, f"routes classified both ways: {sorted(overlap)}"
def test_policy_entries_are_well_formed():
assert len(_BY_KEY) == len(POLICY), "duplicate (method, path, position) in POLICY"
for case in POLICY:
if case.refuse:
assert case.build is not None, f"{case.key} refuses but has no builder"
assert not case.reason, f"{case.key} refuses; reason is for allow cases"
else:
assert case.build is None, f"{case.key} allows but has a builder"
assert len(case.reason.strip()) > 30, f"{case.key} needs a real reason"
def test_exempt_routes_all_state_a_reason():
for route, reason in SCOPE_EXEMPT_ROUTES.items():
assert len(reason.strip()) > 30, f"{route} needs a real reason, got {reason!r}"
_REFUSING = tuple(case for case in POLICY if case.refuse)
def test_skip_list_only_covers_guarded_routes():
"""The exception lists must not drift out of the guarded set."""
for name, entries in (
("_SQUATTER_CHECK_SKIPS", _SQUATTER_CHECK_SKIPS),
("_SCHEMA_LEVEL_REJECTION", _SCHEMA_LEVEL_REJECTION),
):
unknown = set(entries) - set(SCOPE_GUARDED_ROUTES)
assert not unknown, f"{name} references non-guarded routes: {sorted(unknown)}"
@pytest.mark.parametrize(
("method", "path"),
sorted(SCOPE_GUARDED_ROUTES),
ids=lambda v: v if isinstance(v, str) else str(v),
)
def test_guarded_route_refuses_a_real_scope(
client: TestClient,
sample_data: tuple[Workspace, Peer],
method: str,
path: str,
):
"""Every guarded route refuses a peer that is a real scope."""
test_workspace, _ = sample_data
scope_name = str(generate_nanoid())
response = client.post(
f"/v3/workspaces/{test_workspace.name}/scopes", json={"id": scope_name}
)
assert response.status_code == 201, response.text
backing = scope_peer_name(scope_name)
def _setup(client: TestClient, workspace: str) -> tuple[str, str]:
"""Create the counterparty peer and a session, returning (session, scope name)."""
assert client.post(
f"/v3/workspaces/{workspace}/peers", json={"id": _OTHER}
).status_code in (200, 201)
session_name = str(generate_nanoid())
assert client.post(
f"/v3/workspaces/{test_workspace.name}/sessions", json={"id": session_name}
f"/v3/workspaces/{workspace}/sessions", json={"id": session_name}
).status_code in (200, 201)
return session_name, str(generate_nanoid())
@pytest.mark.parametrize("case", _REFUSING, ids=lambda c: f"{c.method}:{c.position}")
def test_refusing_position_rejects_a_real_scope(
client: TestClient,
sample_data: tuple[Workspace, Peer],
case: Case,
):
"""A real scope is refused in every position marked REFUSE."""
test_workspace, _ = sample_data
session_name, scope_name = _setup(client, test_workspace.name)
assert (
client.post(
f"/v3/workspaces/{test_workspace.name}/scopes", json={"id": scope_name}
).status_code
== 201
)
backing = scope_peer_name(scope_name)
assert (
client.post(
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
@ -363,55 +542,52 @@ def test_guarded_route_refuses_a_real_scope(
== 200
)
build = SCOPE_GUARDED_ROUTES[(method, path)]
result = build(client, test_workspace.name, session_name, backing)
assert case.build is not None
result = case.build(client, test_workspace.name, session_name, backing)
status = getattr(result, "status_code", None)
assert status == 422, f"{method} {path} accepted a scope peer (got {status})"
assert status == 422, (
f"{case.method} {case.path} accepted a scope in position "
f"{case.position!r} (got {status})"
)
# A 422 alone proves nothing — a malformed request body would also produce
# one. Require the rejection to actually be about this scope.
# A 422 alone proves nothing — a malformed body would also produce one.
detail = str(getattr(result, "text", ""))
if (method, path) in _SCHEMA_LEVEL_REJECTION:
assert "pattern" in detail, (
f"{method} {path} was expected to be refused by schema validation, "
f"but the detail does not mention the pattern: {detail[:200]}"
)
if case.schema_level:
assert (
"pattern" in detail
), f"{case.key} expected a schema-level refusal; detail: {detail[:200]}"
else:
assert "scope" in detail.lower() and backing in detail, (
f"{method} {path} returned 422 but not because of the scope — "
f"{case.key} returned 422 but not because of the scope; "
f"detail: {detail[:200]}"
)
@pytest.mark.parametrize(
("method", "path"),
sorted(set(SCOPE_GUARDED_ROUTES) - set(_SQUATTER_CHECK_SKIPS)),
ids=lambda v: v if isinstance(v, str) else str(v),
"case",
tuple(c for c in _REFUSING if not c.skip_squatter),
ids=lambda c: f"{c.method}:{c.position}",
)
async def test_guarded_route_allows_unflagged_squatter(
async def test_refusing_position_allows_unflagged_squatter(
client: TestClient,
db_session: AsyncSession,
sample_data: tuple[Workspace, Peer],
method: str,
path: str,
case: Case,
):
"""A peer merely occupying the reserved namespace is not a scope.
Peer names were length-validated only before migration d429de0e5338, so
`scope.production` is a possible real user name. Such a peer has only the name
half of the invariant and must keep working — guards that key off the prefix
alone lock a tenant out of its own data.
half of the invariant and must keep working — a guard keying off the prefix
alone locks a tenant out of its own data.
"""
test_workspace, _ = sample_data
session_name, _ = _setup(client, test_workspace.name)
squatter = scope_peer_name(str(generate_nanoid()))
db_session.add(models.Peer(workspace_name=test_workspace.name, name=squatter))
await db_session.commit()
session_name = str(generate_nanoid())
assert client.post(
f"/v3/workspaces/{test_workspace.name}/sessions", json={"id": session_name}
).status_code in (200, 201)
# Give the squatter a membership so config/removal routes have a row to act on.
# Give it a membership so config and removal have a row to act on.
assert (
client.post(
f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers",
@ -420,10 +596,11 @@ async def test_guarded_route_allows_unflagged_squatter(
== 200
)
build = SCOPE_GUARDED_ROUTES[(method, path)]
result = build(client, test_workspace.name, session_name, squatter)
assert case.build is not None
result = case.build(client, test_workspace.name, session_name, squatter)
status = getattr(result, "status_code", None)
assert status != 422, (
f"{method} {path} refused an unflagged squatter (got {status}) — "
"the guard is keying off the name prefix rather than the scope flag"
f"{case.method} {case.path} refused an unflagged squatter in position "
f"{case.position!r} (got {status}) — the guard is keying off the name "
"prefix rather than the scope flag"
)