fix(scopes): close auth and observed-position gaps, paginate membership
Review response for #884. Security: - gate `SessionCreate.scopes` behind a workspace-level key; the session-create route is self-authorizing, so a peer- or session-scoped token could mint scope peers and join sessions to scopes it had no access to via `POST /scopes` - refuse a reserved-but-nonexistent name in two observed positions that used the permissive guard: chat `target` and session-context `peer_target`. Both let a caller act on `scope.X` before it existed, then create the scope Facade: - exclude scope peers from `GET /sessions/{id}/peers` and refuse the membership -config read for a real scope, matching its write side - replace `GET /scopes/{id}/sessions` with `POST /scopes/{id}/sessions/list` returning `Page[Session]`; the add route now returns 204. Membership was unbounded on both, while every other list surface paginates - rename `crud.get_scope` to `get_scope_or_raise` Tests: - add a missing-name axis to the route-policy table (`Case.refuse_missing`), which is what surfaced the two guard gaps above - delete 14 hand-written tests the table now enumerates; 52 -> 39 functions in test_scopes.py with more cases covered - tighten the squatter assertion from `!= 422` to `< 400`, which was passing on 5xx - assert the FastAPI-internals traversal still derives positions, so a framework upgrade can't silently empty the suite Docs: - drop internal ticket and RFC references from the published OpenAPI descriptions and surrounding comments; state the behavior instead - move implementation reasoning out of the `PUT /peers/{id}` docstring, which FastAPI publishes, into a comment
This commit is contained in:
parent
3359a3771b
commit
de428780a6
|
|
@ -50,8 +50,8 @@ from .representation import (
|
|||
from .scope import (
|
||||
add_sessions_to_scope,
|
||||
get_or_create_scopes,
|
||||
get_scope,
|
||||
get_scope_session_names,
|
||||
get_scope_or_raise,
|
||||
get_scope_sessions,
|
||||
get_scopes,
|
||||
remove_session_from_scope,
|
||||
)
|
||||
|
|
@ -137,8 +137,8 @@ __all__ = [
|
|||
# Scope
|
||||
"add_sessions_to_scope",
|
||||
"get_or_create_scopes",
|
||||
"get_scope",
|
||||
"get_scope_session_names",
|
||||
"get_scope_or_raise",
|
||||
"get_scope_sessions",
|
||||
"get_scopes",
|
||||
"remove_session_from_scope",
|
||||
# Session
|
||||
|
|
|
|||
|
|
@ -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, reject_scope_peers
|
||||
from src.crud.peer import get_peer, reject_scope_observed
|
||||
from src.crud.session import get_session
|
||||
from src.dependencies import tracked_db
|
||||
from src.embedding_client import embedding_client
|
||||
|
|
@ -958,7 +958,14 @@ async def create_observations(
|
|||
# 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(
|
||||
#
|
||||
# The strict variant because this is an observed position, though defence in
|
||||
# depth rather than the active guard: the loop above resolves every peer, so a
|
||||
# reserved name that does not exist yet already 404s before reaching here. If
|
||||
# that validation ever stops covering observed_id, this still refuses the
|
||||
# pre-seeding case instead of persisting a conclusion that a later-created
|
||||
# scope would retroactively own.
|
||||
await reject_scope_observed(
|
||||
db,
|
||||
workspace_name,
|
||||
{obs.observed_id for obs in observations},
|
||||
|
|
|
|||
|
|
@ -72,7 +72,8 @@ def _reject_impossible_peer_names(names: Collection[str]) -> None:
|
|||
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"
|
||||
f"Peer name(s) {too_long} must be at most "
|
||||
+ f"{PEER_NAME_MAX_LENGTH} characters"
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -117,7 +118,7 @@ def scope_peer_clause() -> ColumnElement[bool]:
|
|||
)
|
||||
|
||||
|
||||
async def _reserved_name_candidates(names: Iterable[str]) -> list[str]:
|
||||
def _reserved_name_candidates(names: Iterable[str]) -> list[str]:
|
||||
"""Materialize ``names`` once and return the reserved-prefix ones, sorted.
|
||||
|
||||
Materializing up front matters: callers pass generators (the message-author
|
||||
|
|
@ -167,7 +168,7 @@ async def reject_scope_observed(
|
|||
Raises:
|
||||
ValidationException: On a real scope or a missing reserved name.
|
||||
"""
|
||||
candidates = await _reserved_name_candidates(names)
|
||||
candidates = _reserved_name_candidates(names)
|
||||
if not candidates:
|
||||
return
|
||||
|
||||
|
|
@ -212,7 +213,7 @@ async def scope_peer_names(
|
|||
Raises:
|
||||
ValidationException: On a NUL byte or an over-length name.
|
||||
"""
|
||||
candidates = await _reserved_name_candidates(names)
|
||||
candidates = _reserved_name_candidates(names)
|
||||
if not candidates:
|
||||
return set()
|
||||
|
||||
|
|
@ -272,6 +273,8 @@ async def get_or_create_peers(
|
|||
|
||||
Raises:
|
||||
ConflictException: If we fail to get or create the peers
|
||||
ValidationException: On an impossible name (NUL byte, over-length), or a
|
||||
reserved-prefix or non-conforming name on the create path
|
||||
"""
|
||||
|
||||
await get_or_create_workspace(db, schemas.WorkspaceCreate(name=workspace_name))
|
||||
|
|
|
|||
|
|
@ -8,8 +8,8 @@ and never speaks.
|
|||
See ``src/utils/scopes.py`` for the namespace helpers.
|
||||
|
||||
Membership only affects messages ingested *after* a session is added to a
|
||||
scope; backfill of pre-existing documents and reconciliation on removal land
|
||||
in a follow-up (DEV-1999).
|
||||
scope. Conclusions already derived are neither backfilled on add nor
|
||||
reconciled on removal.
|
||||
"""
|
||||
|
||||
from logging import getLogger
|
||||
|
|
@ -200,7 +200,7 @@ async def get_scopes(
|
|||
return stmt.order_by(models.Peer.created_at.asc(), models.Peer.id.asc())
|
||||
|
||||
|
||||
async def get_scope(
|
||||
async def get_scope_or_raise(
|
||||
db: AsyncSession,
|
||||
workspace_name: str,
|
||||
scope_name: str,
|
||||
|
|
@ -232,31 +232,35 @@ async def get_scope(
|
|||
return peer
|
||||
|
||||
|
||||
async def get_scope_session_names(
|
||||
db: AsyncSession,
|
||||
async def get_scope_sessions(
|
||||
workspace_name: str,
|
||||
scope_name: str,
|
||||
) -> list[str]:
|
||||
reverse: bool = False,
|
||||
) -> Select[tuple[models.Session]]:
|
||||
"""
|
||||
List the IDs of the active sessions that are members of a scope.
|
||||
Build a query for the active sessions that are members of a scope.
|
||||
|
||||
Membership is unbounded — a scope may span every session in a workspace — so
|
||||
this returns a query for the caller to paginate rather than a materialized
|
||||
list. Callers must check the scope exists themselves (``get_scope_or_raise``);
|
||||
an unknown scope yields an empty page here, not a 404.
|
||||
|
||||
Ordered by membership age, with the session id as a unique tiebreaker:
|
||||
``session_peers`` has a composite primary key and no id of its own, so
|
||||
``joined_at`` alone is not a stable pagination key.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
workspace_name: Name of the workspace
|
||||
scope_name: Unprefixed scope name
|
||||
reverse: Whether to return newest memberships first
|
||||
|
||||
Returns:
|
||||
Names of the scope's member sessions, oldest membership first
|
||||
|
||||
Raises:
|
||||
ResourceNotFoundException: If the scope does not exist
|
||||
Select for the scope's member sessions
|
||||
"""
|
||||
await get_scope(db, workspace_name, scope_name)
|
||||
|
||||
stmt = (
|
||||
select(models.SessionPeer.session_name)
|
||||
select(models.Session)
|
||||
.join(
|
||||
models.Session,
|
||||
models.SessionPeer,
|
||||
(models.Session.name == models.SessionPeer.session_name)
|
||||
& (models.Session.workspace_name == models.SessionPeer.workspace_name),
|
||||
)
|
||||
|
|
@ -264,10 +268,12 @@ async def get_scope_session_names(
|
|||
.where(models.SessionPeer.peer_name == scope_peer_name(scope_name))
|
||||
.where(models.SessionPeer.left_at.is_(None))
|
||||
.where(models.Session.is_active == True) # noqa: E712
|
||||
.order_by(models.SessionPeer.joined_at.asc())
|
||||
)
|
||||
result = await db.execute(stmt)
|
||||
return [row[0] for row in result.all()]
|
||||
if reverse:
|
||||
return stmt.order_by(
|
||||
models.SessionPeer.joined_at.desc(), models.Session.id.desc()
|
||||
)
|
||||
return stmt.order_by(models.SessionPeer.joined_at.asc(), models.Session.id.asc())
|
||||
|
||||
|
||||
async def add_sessions_to_scope(
|
||||
|
|
@ -275,14 +281,15 @@ async def add_sessions_to_scope(
|
|||
workspace_name: str,
|
||||
scope_name: str,
|
||||
session_names: list[str],
|
||||
) -> list[str]:
|
||||
) -> None:
|
||||
"""
|
||||
Add sessions to a scope by creating observer memberships for its peer.
|
||||
|
||||
Each membership is a ``session_peers`` row for the scope peer with
|
||||
``observe_others=true, observe_me=false`` — exactly what a hand-built
|
||||
observer peer would carry. Membership only affects messages ingested
|
||||
after this call (backfill is DEV-1999).
|
||||
observer peer would carry. No backfill happens here: membership only affects
|
||||
messages ingested after this call, and conclusions already derived are left
|
||||
as they are.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
|
|
@ -290,9 +297,6 @@ async def add_sessions_to_scope(
|
|||
scope_name: Unprefixed scope name
|
||||
session_names: Names of existing sessions to add
|
||||
|
||||
Returns:
|
||||
Names of all the scope's member sessions after the addition
|
||||
|
||||
Raises:
|
||||
ResourceNotFoundException: If the scope or any named session does not
|
||||
exist
|
||||
|
|
@ -301,7 +305,7 @@ async def add_sessions_to_scope(
|
|||
# `scopes` path, so a module-level import would be circular.
|
||||
from .session import upsert_session_peers
|
||||
|
||||
await get_scope(db, workspace_name, scope_name)
|
||||
await get_scope_or_raise(db, workspace_name, scope_name)
|
||||
|
||||
requested = set(session_names)
|
||||
result = await db.execute(
|
||||
|
|
@ -328,8 +332,6 @@ async def add_sessions_to_scope(
|
|||
|
||||
await db.commit()
|
||||
|
||||
return await get_scope_session_names(db, workspace_name, scope_name)
|
||||
|
||||
|
||||
async def remove_session_from_scope(
|
||||
db: AsyncSession,
|
||||
|
|
@ -341,8 +343,8 @@ async def remove_session_from_scope(
|
|||
Remove a session from a scope by ending the scope peer's membership.
|
||||
|
||||
Ends the membership the same way the generic remove-peer path does (sets
|
||||
``left_at``). Reconciliation of documents derived while the session was a
|
||||
member lands in a follow-up (DEV-1999).
|
||||
``left_at``). Conclusions derived while the session was a member are left in
|
||||
place — nothing reconciles them.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
|
|
@ -356,7 +358,7 @@ async def remove_session_from_scope(
|
|||
# Lazy import for the same circular-import reason as add_sessions_to_scope.
|
||||
from .session import remove_peers_from_session
|
||||
|
||||
await get_scope(db, workspace_name, scope_name)
|
||||
await get_scope_or_raise(db, workspace_name, scope_name)
|
||||
|
||||
await remove_peers_from_session(
|
||||
db,
|
||||
|
|
|
|||
|
|
@ -321,7 +321,7 @@ async def get_or_create_session(
|
|||
# Add the session to any requested scopes: create-or-get each scope peer
|
||||
# and record an observer membership (observe_others=true, observe_me=false).
|
||||
# No backfill happens here — membership only affects messages ingested
|
||||
# after this point (backfill is DEV-1999).
|
||||
# after this point.
|
||||
scopes_result = None
|
||||
if session.scopes:
|
||||
scopes_result = await get_or_create_scopes(
|
||||
|
|
@ -919,6 +919,13 @@ async def get_peers_from_session(
|
|||
workspace_name: Name of the workspace
|
||||
session_name: Name of the session
|
||||
|
||||
Scope peers are excluded: a scope's membership is the facade's internal
|
||||
observer wiring, and this is the generic peer surface. Listing them here
|
||||
would show a caller a peer named ``scope.<name>`` with ``observe_others``
|
||||
set, which is exactly the mechanic the facade exists to hide. Mirrors the
|
||||
``kind``-less default of ``crud.peer.get_peers``; the scopes routes expose
|
||||
membership from the other direction.
|
||||
|
||||
Returns:
|
||||
Paginated list of Peer objects in the session
|
||||
"""
|
||||
|
|
@ -935,6 +942,10 @@ async def get_peers_from_session(
|
|||
.where(models.SessionPeer.session_name == session_name)
|
||||
.where(models.Peer.workspace_name == workspace_name)
|
||||
.where(models.SessionPeer.left_at.is_(None)) # Only active peers
|
||||
# models.Peer is already in the FROM via the join above, so the clause
|
||||
# composes directly — no correlated exists() as in the SessionPeer-only
|
||||
# UPDATE statements elsewhere in this module.
|
||||
.where(~scope_peer_clause())
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -1164,7 +1175,7 @@ async def _get_or_add_peers_to_session(
|
|||
|
||||
# Scope memberships carry observe_others=True but do not count against the
|
||||
# limit. The limit bounds per-observer deriver fan-out for real peers; a scope
|
||||
# costs document rows, not LLM calls (Scopes RFC §5.2), and counting them would
|
||||
# costs document rows, not LLM calls, and counting them would
|
||||
# cap scopes-per-session at SESSION_OBSERVERS_LIMIT and surface as an
|
||||
# observer-shaped 400 through a facade that hides observers entirely.
|
||||
scopes_being_added = await scope_peer_names(db, workspace_name, peer_names.keys())
|
||||
|
|
@ -1270,7 +1281,14 @@ async def get_peer_config(
|
|||
|
||||
Raises:
|
||||
ResourceNotFoundException: If the session or peer does not exist
|
||||
ValidationException: If the peer is a scope
|
||||
"""
|
||||
# A scope's membership config belongs to the facade, not the caller — the
|
||||
# write path refuses it in set_peer_config below, and reading it back is the
|
||||
# same internal wiring by another route. Checked on the resolved row, so a
|
||||
# legacy peer merely occupying the reserved name keeps working.
|
||||
_reject_resolved_scope_peers([await get_peer(db, workspace_name, peer_id)])
|
||||
|
||||
# Get row from session_peer table
|
||||
stmt = select(models.SessionPeer).where(
|
||||
models.SessionPeer.workspace_name == workspace_name,
|
||||
|
|
|
|||
|
|
@ -25,8 +25,8 @@ 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.
|
||||
one exists to query. Querying *from* a scope's perspective is a read-side
|
||||
surface that does not exist yet, and not something the raw peer routes expose.
|
||||
|
||||
Raises:
|
||||
ValidationException: If any participant is a scope.
|
||||
|
|
|
|||
|
|
@ -142,13 +142,17 @@ async def update_peer(
|
|||
):
|
||||
"""Update a Peer's metadata and/or configuration.
|
||||
|
||||
Three-way on the reserved namespace, all enforced inside ``crud.update_peer``
|
||||
on the resolved row so there is no check-then-use window: a real scope is
|
||||
refused (this route replaces `configuration` wholesale, so it must never touch
|
||||
a facade-managed peer); an existing *unflagged* peer that merely occupies the
|
||||
namespace is a normal peer and updates fine; and a reserved-prefix name that
|
||||
does not exist is refused by create-path validation rather than being minted.
|
||||
Returns 422 if the peer is a scope — use the scopes routes to manage scopes.
|
||||
"""
|
||||
# Three-way on the reserved namespace, all enforced inside ``crud.update_peer``
|
||||
# on the resolved row so there is no check-then-use window: a real scope is
|
||||
# refused (this route replaces `configuration` wholesale, so it must never touch
|
||||
# a facade-managed peer); an existing *unflagged* peer that merely occupies the
|
||||
# namespace is a normal peer and updates fine; and a reserved-prefix name that
|
||||
# does not exist is refused by create-path validation rather than being minted.
|
||||
#
|
||||
# Kept out of the docstring deliberately: FastAPI publishes that into the
|
||||
# OpenAPI description, and callers need the contract, not the mechanism.
|
||||
updated_peer = await crud.update_peer(
|
||||
db, workspace_name=workspace_id, peer_name=peer_id, peer=peer
|
||||
)
|
||||
|
|
@ -217,14 +221,19 @@ async def chat(
|
|||
"""
|
||||
# Scope peers are never observed, so no representation of them exists to
|
||||
# query. Covers the path-level observer too: a scope `peer_id` no longer
|
||||
# errors out downstream now that crud.get_peer takes a plain name, and a
|
||||
# scope peer as the path-level observer is a Phase 2b concern.
|
||||
# errors out downstream now that crud.get_peer takes a plain name, and
|
||||
# querying from a scope's perspective is a read-side surface that does not
|
||||
# exist yet.
|
||||
scope_candidates = [
|
||||
n for n in (peer_id, options.target) if n is not None and is_scope_peer_name(n)
|
||||
]
|
||||
if scope_candidates:
|
||||
async with tracked_db("peers.chat.scope_check", read_only=True) as s_db:
|
||||
await crud.reject_scope_peers(
|
||||
# Strict variant, matching the representation route: `target` is an
|
||||
# observed position and nothing here creates the peer, so a reserved
|
||||
# name that does not exist yet must be refused rather than answered
|
||||
# and then turned into a scope.
|
||||
await crud.reject_scope_observed(
|
||||
s_db,
|
||||
workspace_id,
|
||||
scope_candidates,
|
||||
|
|
@ -401,15 +410,23 @@ async def get_representation(
|
|||
try:
|
||||
embedding: list[float] | None = None
|
||||
if options.search_query:
|
||||
with (
|
||||
suppress(Exception),
|
||||
embedding_call_purpose(
|
||||
try:
|
||||
with embedding_call_purpose(
|
||||
EmbeddingCallPurpose.SEARCH_MEMORY.value,
|
||||
workspace_name=workspace_id,
|
||||
parent_category="api",
|
||||
),
|
||||
):
|
||||
embedding = await embedding_client.embed(options.search_query)
|
||||
):
|
||||
embedding = await embedding_client.embed(options.search_query)
|
||||
except Exception:
|
||||
# Swallowed on purpose (see include_semantic_query below), but not
|
||||
# silently: without this a provider outage degrades every search
|
||||
# request to derived+recent retrieval with no signal anywhere.
|
||||
logger.warning(
|
||||
"Representation search embedding failed for workspace %s,"
|
||||
+ " degrading to non-semantic retrieval",
|
||||
workspace_id,
|
||||
exc_info=True,
|
||||
)
|
||||
|
||||
observed = options.target if options.target is not None else peer_id
|
||||
# Re-check and read in one short session, opened only now — after the
|
||||
|
|
|
|||
|
|
@ -8,9 +8,9 @@ that keeps the observer/observed mechanics hidden.
|
|||
All scopes routes require a workspace-level (or admin) key: scopes are an
|
||||
app-level admin surface, so peer- and session-scoped keys are rejected.
|
||||
|
||||
Note: backfill of pre-existing documents and reconciliation on removal land
|
||||
in a follow-up (DEV-1999) — for now, scope membership only affects messages
|
||||
ingested *after* the membership change.
|
||||
Note: scope membership only affects messages ingested *after* the membership
|
||||
change. Conclusions already derived are neither backfilled on add nor
|
||||
reconciled on removal.
|
||||
"""
|
||||
|
||||
import logging
|
||||
|
|
@ -85,12 +85,13 @@ async def get_scope(
|
|||
db: AsyncSession = read_db,
|
||||
):
|
||||
"""Get a single Scope by ID."""
|
||||
return await crud.get_scope(db, workspace_id, scope_id)
|
||||
return await crud.get_scope_or_raise(db, workspace_id, scope_id)
|
||||
|
||||
|
||||
@router.post(
|
||||
"/{scope_id}/sessions",
|
||||
response_model=schemas.ScopeSessions,
|
||||
status_code=204,
|
||||
response_model=None,
|
||||
dependencies=[Depends(require_auth(workspace_name="workspace_id"))],
|
||||
)
|
||||
async def add_sessions_to_scope(
|
||||
|
|
@ -104,19 +105,19 @@ async def add_sessions_to_scope(
|
|||
"""
|
||||
Add Sessions to a Scope.
|
||||
|
||||
All named sessions must already exist (404 otherwise). Returns the scope's
|
||||
full member session list after the addition.
|
||||
All named sessions must already exist (404 otherwise). Adding a session that
|
||||
is already a member is a no-op. List the resulting membership with
|
||||
`POST /scopes/{scope_id}/sessions/list`.
|
||||
|
||||
Note: membership only affects messages ingested after this call — backfill
|
||||
of pre-existing documents lands in a follow-up (DEV-1999).
|
||||
Note: membership applies only to messages ingested after this call;
|
||||
conclusions already derived are not backfilled.
|
||||
"""
|
||||
session_ids = await crud.add_sessions_to_scope(
|
||||
await crud.add_sessions_to_scope(
|
||||
db,
|
||||
workspace_name=workspace_id,
|
||||
scope_name=scope_id,
|
||||
session_names=body.session_ids,
|
||||
)
|
||||
return schemas.ScopeSessions(session_ids=session_ids)
|
||||
|
||||
|
||||
@router.delete(
|
||||
|
|
@ -134,8 +135,8 @@ async def remove_session_from_scope(
|
|||
"""
|
||||
Remove a Session from a Scope.
|
||||
|
||||
Note: documents already derived while the session was a member are left in
|
||||
place — reconciliation on removal lands in a follow-up (DEV-1999).
|
||||
Note: conclusions already derived while the session was a member are left in
|
||||
place.
|
||||
"""
|
||||
await crud.remove_session_from_scope(
|
||||
db,
|
||||
|
|
@ -145,16 +146,27 @@ async def remove_session_from_scope(
|
|||
)
|
||||
|
||||
|
||||
@router.get(
|
||||
"/{scope_id}/sessions",
|
||||
response_model=schemas.ScopeSessions,
|
||||
@router.post(
|
||||
"/{scope_id}/sessions/list",
|
||||
response_model=Page[schemas.Session],
|
||||
dependencies=[Depends(require_auth(workspace_name="workspace_id"))],
|
||||
)
|
||||
async def get_scope_sessions(
|
||||
workspace_id: str = Path(...),
|
||||
scope_id: str = Path(...),
|
||||
reverse: bool = Query(False, description="Whether to reverse the order of results"),
|
||||
db: AsyncSession = read_db,
|
||||
):
|
||||
"""Get the IDs of the Sessions that are members of a Scope."""
|
||||
session_ids = await crud.get_scope_session_names(db, workspace_id, scope_id)
|
||||
return schemas.ScopeSessions(session_ids=session_ids)
|
||||
"""Get the Sessions that are members of a Scope, paginated.
|
||||
|
||||
Ordered by how long each session has been a member, oldest first.
|
||||
"""
|
||||
# Distinguishes an empty scope from one that does not exist; the query itself
|
||||
# returns an empty page either way.
|
||||
await crud.get_scope_or_raise(db, workspace_id, scope_id)
|
||||
return await apaginate(
|
||||
db,
|
||||
await crud.get_scope_sessions(
|
||||
workspace_name=workspace_id, scope_name=scope_id, reverse=reverse
|
||||
),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -318,6 +318,16 @@ async def get_or_create_session(
|
|||
)
|
||||
session.name = jwt_params.s
|
||||
|
||||
# The `scopes` field does what the scopes routes do — create scope peers and
|
||||
# attach memberships — so it needs their authorization: workspace-level or
|
||||
# admin only. Checked here rather than through `require_auth(...)` because
|
||||
# that closure only resolves path and query params, never the body, so a
|
||||
# declarative gate cannot see this field.
|
||||
if session.scopes and not (
|
||||
jwt_params.ad or (jwt_params.p is None and jwt_params.s is None)
|
||||
):
|
||||
raise AuthenticationException("Scope membership requires a workspace-level key")
|
||||
|
||||
# Scope peers may not be added through the generic peers mapping; use the
|
||||
# `scopes` field (which handles scope-peer creation and observer config).
|
||||
if session.peer_names:
|
||||
|
|
@ -746,10 +756,12 @@ async def get_session_context(
|
|||
)
|
||||
|
||||
# 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.
|
||||
# formed of a scope. peer_perspective (the observer) is left alone: a scope is
|
||||
# a legitimate perspective, and the read-side scope surface will build on that.
|
||||
if peer_target is not None:
|
||||
await crud.reject_scope_peers(
|
||||
# Strict variant: an observed position that creates nothing, so a reserved
|
||||
# name which does not exist yet must be refused too.
|
||||
await crud.reject_scope_observed(
|
||||
db,
|
||||
workspace_id,
|
||||
[peer_target],
|
||||
|
|
|
|||
|
|
@ -38,7 +38,6 @@ from src.schemas.api import (
|
|||
ScheduleDreamRequest,
|
||||
Scope,
|
||||
ScopeCreate,
|
||||
ScopeSessions,
|
||||
ScopeSessionsAdd,
|
||||
Session,
|
||||
SessionBase,
|
||||
|
|
@ -139,7 +138,6 @@ __all__ = [
|
|||
"ScheduleDreamRequest",
|
||||
"Scope",
|
||||
"ScopeCreate",
|
||||
"ScopeSessions",
|
||||
"ScopeSessionsAdd",
|
||||
"Session",
|
||||
"SessionBase",
|
||||
|
|
|
|||
|
|
@ -401,10 +401,9 @@ class SessionCreate(SessionBase):
|
|||
max_length=100,
|
||||
description=(
|
||||
"Optional list of (unprefixed) scope names to add this session to. "
|
||||
"Each scope is created if it does not exist yet. Note: scope "
|
||||
"membership only affects messages ingested after the session is "
|
||||
"added to the scope; backfill of pre-existing documents lands in a "
|
||||
"follow-up (DEV-1999)."
|
||||
"Each scope is created if it does not exist yet. Membership applies "
|
||||
"only to messages ingested after the session is added to the scope; "
|
||||
"conclusions already derived are not backfilled."
|
||||
),
|
||||
)
|
||||
|
||||
|
|
@ -565,12 +564,6 @@ class ScopeSessionsAdd(BaseModel):
|
|||
)
|
||||
|
||||
|
||||
class ScopeSessions(BaseModel):
|
||||
"""IDs of the sessions that are currently members of a scope."""
|
||||
|
||||
session_ids: list[str]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Conclusion schemas
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -143,8 +143,8 @@ class TraceContentEvent(BaseEvent):
|
|||
# Tool calls in a unified {id, name, input} shape (provider-agnostic).
|
||||
tool_calls: list[dict[str, Any]] = Field(default_factory=list)
|
||||
# Tags Honcho-authored content (system prompts, scaffold) so tenant-facing
|
||||
# views can withhold globally-shared content (the §6.3 access invariant —
|
||||
# dedup is global, the content store has no tenant column).
|
||||
# views can withhold globally-shared content: dedup is global, and the
|
||||
# content store has no tenant column.
|
||||
honcho_authored: bool = False
|
||||
|
||||
def get_resource_id(self) -> str:
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
"""Tests for the card_refresh dream type (DEV-2000, Scopes RFC prerequisite).
|
||||
"""Tests for the card_refresh dream type.
|
||||
|
||||
Covers:
|
||||
- queue plumbing: payload roundtrip, work-unit key isolation from omni,
|
||||
|
|
|
|||
|
|
@ -35,14 +35,16 @@ enumerate; a gap there would not be caught here.
|
|||
"""
|
||||
|
||||
from collections.abc import Callable, Iterator
|
||||
from dataclasses import dataclass, field
|
||||
from dataclasses import dataclass
|
||||
|
||||
import pytest
|
||||
from fastapi.routing import APIRoute
|
||||
from fastapi.testclient import TestClient
|
||||
from httpx import Response
|
||||
from nanoid import generate as generate_nanoid
|
||||
from pydantic import BaseModel
|
||||
from pydantic.fields import FieldInfo
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from src import models
|
||||
|
|
@ -74,7 +76,7 @@ _KEY_POSITION = "body_peer_keys"
|
|||
_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]
|
||||
Builder = Callable[[TestClient, str, str, str], Response]
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
|
|
@ -87,6 +89,19 @@ class Case:
|
|||
refuse: bool
|
||||
reason: str = ""
|
||||
build: Builder | None = None
|
||||
# REFUSE cases only. Whether a reserved name that does NOT YET EXIST is also
|
||||
# refused — the third axis, and the one that is not derivable from `refuse`.
|
||||
# It follows from the guard the call site picked:
|
||||
#
|
||||
# validate_no_scope_peer_names name-only, no DB refuses missing
|
||||
# reject_scope_observed strict on the observed refuses missing
|
||||
# reject_scope_peers flag-based, permissive allows missing
|
||||
#
|
||||
# Permissive is correct where something downstream still stops it (the create
|
||||
# path validates new names) or where the name simply resolves to nothing (404
|
||||
# before any guard runs). Each False therefore needs `missing_reason`.
|
||||
refuse_missing: bool | None = None
|
||||
missing_reason: str = ""
|
||||
# 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
|
||||
|
|
@ -95,7 +110,6 @@ class Case:
|
|||
# ALLOW cases only: builder plus the status a real scope must receive, so the
|
||||
# suite proves legitimate observer positions keep working.
|
||||
allow_status: tuple[int, ...] = ()
|
||||
_: tuple[()] = field(default=(), repr=False)
|
||||
|
||||
@property
|
||||
def key(self) -> tuple[str, str, str]:
|
||||
|
|
@ -255,6 +269,10 @@ def _b_peer_config(c: TestClient, ws: str, s: str, p: str):
|
|||
)
|
||||
|
||||
|
||||
def _b_peer_config_get(c: TestClient, ws: str, s: str, p: str):
|
||||
return c.get(f"/v3/workspaces/{ws}/sessions/{s}/peers/{p}/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"
|
||||
|
|
@ -271,16 +289,48 @@ _READ_ONLY_OK = (
|
|||
POLICY: tuple[Case, ...] = (
|
||||
# ---- observed position: a scope must never be the subject ----
|
||||
Case(
|
||||
"POST", f"{_W}/conclusions", "observed_id", True, build=_b_conclusion_observed
|
||||
"POST",
|
||||
f"{_W}/conclusions",
|
||||
"observed_id",
|
||||
True,
|
||||
refuse_missing=False,
|
||||
missing_reason=(
|
||||
"Every observer and observed peer is resolved before the scope check, "
|
||||
"so a name that does not exist is a 404 and no conclusion is written. "
|
||||
"The guard is still the strict variant, for if that ever changes."
|
||||
),
|
||||
build=_b_conclusion_observed,
|
||||
),
|
||||
Case(
|
||||
"POST",
|
||||
f"{_W}/schedule_dream",
|
||||
"observed",
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_dream_observed,
|
||||
),
|
||||
Case(
|
||||
"PUT",
|
||||
f"{_W}/peers/{{peer_id}}/card",
|
||||
"target",
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_card_target,
|
||||
),
|
||||
Case(
|
||||
"POST",
|
||||
f"{_W}/peers/{{peer_id}}/chat",
|
||||
"target",
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_chat_target,
|
||||
),
|
||||
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,
|
||||
refuse_missing=True,
|
||||
build=_b_repr_target,
|
||||
),
|
||||
Case(
|
||||
|
|
@ -288,6 +338,7 @@ POLICY: tuple[Case, ...] = (
|
|||
f"{_W}/sessions/{{session_id}}/context",
|
||||
"peer_target",
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_session_context_target,
|
||||
),
|
||||
# ---- observer position: legitimately a scope ----
|
||||
|
|
@ -363,6 +414,7 @@ POLICY: tuple[Case, ...] = (
|
|||
f"{_W}/peers",
|
||||
_KEY_POSITION,
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_create_peer,
|
||||
schema_level=True,
|
||||
skip_squatter=(
|
||||
|
|
@ -371,13 +423,28 @@ POLICY: tuple[Case, ...] = (
|
|||
"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(
|
||||
"PUT",
|
||||
f"{_W}/peers/{{peer_id}}",
|
||||
"peer_id",
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_update_peer,
|
||||
),
|
||||
Case(
|
||||
"POST",
|
||||
f"{_W}/sessions",
|
||||
"peer_names",
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_session_create,
|
||||
),
|
||||
Case(
|
||||
"POST",
|
||||
f"{_W}/sessions/{{session_id}}/messages",
|
||||
"peer_name",
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_message,
|
||||
),
|
||||
Case(
|
||||
|
|
@ -385,6 +452,7 @@ POLICY: tuple[Case, ...] = (
|
|||
f"{_W}/sessions/{{session_id}}/messages/upload",
|
||||
"peer_id",
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_upload,
|
||||
),
|
||||
Case(
|
||||
|
|
@ -392,6 +460,7 @@ POLICY: tuple[Case, ...] = (
|
|||
f"{_W}/sessions/{{session_id}}/peers",
|
||||
_KEY_POSITION,
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_add_peers,
|
||||
),
|
||||
Case(
|
||||
|
|
@ -399,6 +468,7 @@ POLICY: tuple[Case, ...] = (
|
|||
f"{_W}/sessions/{{session_id}}/peers",
|
||||
_KEY_POSITION,
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_set_peers,
|
||||
),
|
||||
Case(
|
||||
|
|
@ -406,6 +476,12 @@ POLICY: tuple[Case, ...] = (
|
|||
f"{_W}/sessions/{{session_id}}/peers",
|
||||
_KEY_POSITION,
|
||||
True,
|
||||
refuse_missing=False,
|
||||
missing_reason=(
|
||||
"Removal creates nothing and a name that does not exist has no "
|
||||
"membership row, so the request is a no-op. Refusing here would give a "
|
||||
"reserved name a different removal result than any other absent peer."
|
||||
),
|
||||
build=_b_remove_peers,
|
||||
),
|
||||
Case(
|
||||
|
|
@ -413,14 +489,33 @@ POLICY: tuple[Case, ...] = (
|
|||
f"{_W}/sessions/{{session_id}}/peers/{{peer_id}}/config",
|
||||
"peer_id",
|
||||
True,
|
||||
refuse_missing=False,
|
||||
missing_reason=(
|
||||
"The peer is resolved before the scope check, so a name that does not "
|
||||
"exist is a 404 and never reaches the guard. Nothing is created, so "
|
||||
"there is no window for the name to be claimed here."
|
||||
),
|
||||
build=_b_peer_config,
|
||||
),
|
||||
Case(
|
||||
"GET",
|
||||
f"{_W}/sessions/{{session_id}}/peers/{{peer_id}}/config",
|
||||
"peer_id",
|
||||
True,
|
||||
refuse_missing=False,
|
||||
missing_reason=(
|
||||
"Same resolution order as the write side of this route: an absent peer "
|
||||
"is a 404 before the scope check, and a read creates nothing."
|
||||
),
|
||||
build=_b_peer_config_get,
|
||||
),
|
||||
# ---- path peer on the dialectic surface ----
|
||||
Case(
|
||||
"POST",
|
||||
f"{_W}/peers/{{peer_id}}/chat",
|
||||
"peer_id",
|
||||
True,
|
||||
refuse_missing=True,
|
||||
build=_b_chat_observer,
|
||||
),
|
||||
Case(
|
||||
|
|
@ -428,16 +523,10 @@ POLICY: tuple[Case, ...] = (
|
|||
f"{_W}/peers/{{peer_id}}/representation",
|
||||
"peer_id",
|
||||
True,
|
||||
refuse_missing=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
|
||||
),
|
||||
|
|
@ -457,8 +546,8 @@ POLICY: tuple[Case, ...] = (
|
|||
"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."
|
||||
"Read-only. The read-side scope surface is not implemented yet; the "
|
||||
"`scope` option on the context routes will own it when it lands."
|
||||
),
|
||||
),
|
||||
Case(
|
||||
|
|
@ -468,7 +557,7 @@ POLICY: tuple[Case, ...] = (
|
|||
False,
|
||||
reason=(
|
||||
"Read-only, and empty for a scope now that nothing can write knowledge "
|
||||
"about one. Phase 2b (DEV-1998) owns this surface."
|
||||
"about one. The read-side scope surface is not implemented yet."
|
||||
),
|
||||
),
|
||||
Case(
|
||||
|
|
@ -487,8 +576,8 @@ POLICY: tuple[Case, ...] = (
|
|||
"peer_id",
|
||||
False,
|
||||
reason=(
|
||||
"Mints a scoped JWT rather than touching a peer. Scope-bound keys are "
|
||||
"Phase 3 (DEV-2002)."
|
||||
"Mints a scoped JWT rather than touching a peer, so no peer row is read "
|
||||
"or written. Keys cannot be bound to a scope yet."
|
||||
),
|
||||
),
|
||||
)
|
||||
|
|
@ -587,6 +676,19 @@ def test_every_peer_position_is_classified():
|
|||
derived = _derived_positions()
|
||||
classified = set(_BY_KEY)
|
||||
|
||||
# _peer_positions walks FastAPI/Pydantic internals (route.dependant, its
|
||||
# *_params lists, field_info.annotation). An upgrade that reshapes any of them
|
||||
# would make derivation silently return nothing, and every assertion below
|
||||
# would then pass vacuously. Anchor on a position that must always be found.
|
||||
assert (
|
||||
"POST",
|
||||
f"{_W}/sessions/{{session_id}}/messages",
|
||||
"peer_name",
|
||||
) in derived, (
|
||||
"derived no peer positions for a route that certainly has one — the "
|
||||
"FastAPI internals _peer_positions() traverses have probably changed shape"
|
||||
)
|
||||
|
||||
unclassified = derived - classified
|
||||
assert not unclassified, (
|
||||
"peer positions with no scope policy: "
|
||||
|
|
@ -603,12 +705,25 @@ def test_policy_entries_are_well_formed():
|
|||
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"
|
||||
# The missing-name axis is not derivable from `refuse`, so it must be
|
||||
# stated rather than defaulted — that gap is what this field closes.
|
||||
assert case.refuse_missing is not None, (
|
||||
f"{case.key} refuses a real scope but does not say whether a "
|
||||
"reserved name that does not exist yet is also refused"
|
||||
)
|
||||
if not case.refuse_missing:
|
||||
assert (
|
||||
len(case.missing_reason.strip()) > 30
|
||||
), f"{case.key} tolerates a missing reserved name; say why"
|
||||
else:
|
||||
assert len(case.reason.strip()) > 30, f"{case.key} needs a real reason"
|
||||
assert bool(case.build) == bool(case.allow_status), (
|
||||
f"{case.key}: an allow case needs a builder and an expected "
|
||||
"allow_status together, or neither"
|
||||
)
|
||||
assert (
|
||||
case.refuse_missing is None
|
||||
), f"{case.key}: refuse_missing applies to REFUSE cases only"
|
||||
|
||||
|
||||
_REFUSING = tuple(case for case in POLICY if case.refuse)
|
||||
|
|
@ -630,6 +745,26 @@ def _setup(client: TestClient, workspace: str) -> tuple[str, str]:
|
|||
return session_name, str(generate_nanoid())
|
||||
|
||||
|
||||
def _real_scope(
|
||||
client: TestClient, workspace: str, session_name: str, scope_name: str
|
||||
) -> str:
|
||||
"""Create a scope, attach the session to it, and return its backing peer name."""
|
||||
assert (
|
||||
client.post(
|
||||
f"/v3/workspaces/{workspace}/scopes", json={"id": scope_name}
|
||||
).status_code
|
||||
== 201
|
||||
)
|
||||
assert (
|
||||
client.post(
|
||||
f"/v3/workspaces/{workspace}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [session_name]},
|
||||
).status_code
|
||||
== 204
|
||||
)
|
||||
return scope_peer_name(scope_name)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("case", _REFUSING, ids=lambda c: f"{c.method}:{c.position}")
|
||||
def test_refusing_position_rejects_a_real_scope(
|
||||
client: TestClient,
|
||||
|
|
@ -639,31 +774,18 @@ def test_refusing_position_rejects_a_real_scope(
|
|||
"""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",
|
||||
json={"session_ids": [session_name]},
|
||||
).status_code
|
||||
== 200
|
||||
)
|
||||
backing = _real_scope(client, test_workspace.name, session_name, scope_name)
|
||||
|
||||
assert case.build is not None
|
||||
result = case.build(client, test_workspace.name, session_name, backing)
|
||||
status = getattr(result, "status_code", None)
|
||||
status = result.status_code
|
||||
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 body would also produce one.
|
||||
detail = str(getattr(result, "text", ""))
|
||||
detail = result.text
|
||||
if case.schema_level:
|
||||
assert (
|
||||
"pattern" in detail
|
||||
|
|
@ -692,28 +814,14 @@ def test_allowing_position_accepts_a_real_scope(
|
|||
"""
|
||||
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",
|
||||
json={"session_ids": [session_name]},
|
||||
).status_code
|
||||
== 200
|
||||
)
|
||||
backing = _real_scope(client, test_workspace.name, session_name, scope_name)
|
||||
|
||||
assert case.build is not None
|
||||
result = case.build(client, test_workspace.name, session_name, backing)
|
||||
status = getattr(result, "status_code", None)
|
||||
assert status in case.allow_status, (
|
||||
assert result.status_code in case.allow_status, (
|
||||
f"{case.method} {case.path} refused a scope in the legitimate position "
|
||||
f"{case.position!r}: expected {case.allow_status}, got {status} — "
|
||||
f"{getattr(result, 'text', '')[:200]}"
|
||||
f"{case.position!r}: expected {case.allow_status}, got "
|
||||
f"{result.status_code} — {result.text[:200]}"
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -752,9 +860,62 @@ async def test_refusing_position_allows_unflagged_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"{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"
|
||||
# Deliberately not `!= 422`: that also passes on a 5xx, so a guard regressing
|
||||
# into an unhandled error (the psycopg DataError path this feature defends
|
||||
# against) would keep this green.
|
||||
assert result.status_code < 400, (
|
||||
f"{case.method} {case.path} did not accept an unflagged squatter in "
|
||||
f"position {case.position!r} (got {result.status_code}) — a 422 means the "
|
||||
"guard is keying off the name prefix rather than the scope flag; anything "
|
||||
f"else means the request blew up. Body: {result.text[:200]}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("case", _REFUSING, ids=lambda c: f"{c.method}:{c.position}")
|
||||
async def test_refusing_position_and_a_missing_reserved_name(
|
||||
client: TestClient,
|
||||
db_session: AsyncSession,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
case: Case,
|
||||
):
|
||||
"""The third axis: a reserved name that does not exist yet.
|
||||
|
||||
Neither of the other two tests reaches it — both resolve an existing subject.
|
||||
A permissive guard here is sometimes correct (the create path refuses the name
|
||||
itself, or it simply resolves to nothing), which is why the expected verdict is
|
||||
declared per case rather than assumed.
|
||||
|
||||
What is NOT negotiable in either direction is that the request must not MINT
|
||||
the reserved name. Minting it would let any caller squat a scope name before
|
||||
the workspace owner can create it, and would leave a peer that the facade can
|
||||
never adopt.
|
||||
"""
|
||||
test_workspace, _ = sample_data
|
||||
session_name, _ = _setup(client, test_workspace.name)
|
||||
missing = scope_peer_name(str(generate_nanoid()))
|
||||
|
||||
assert case.build is not None
|
||||
result = case.build(client, test_workspace.name, session_name, missing)
|
||||
|
||||
if case.refuse_missing:
|
||||
assert result.status_code == 422, (
|
||||
f"{case.method} {case.path} accepted a not-yet-existing reserved name "
|
||||
f"in position {case.position!r} (got {result.status_code}) — it could "
|
||||
f"become a scope later. Body: {result.text[:200]}"
|
||||
)
|
||||
else:
|
||||
assert result.status_code != 422, (
|
||||
f"{case.method} {case.path} refused a missing reserved name in position "
|
||||
f"{case.position!r}, but the policy says it tolerates one "
|
||||
f"({case.missing_reason!r}) — update the policy or the guard"
|
||||
)
|
||||
|
||||
minted = await db_session.scalar(
|
||||
select(models.Peer)
|
||||
.where(models.Peer.workspace_name == test_workspace.name)
|
||||
.where(models.Peer.name == missing)
|
||||
)
|
||||
assert minted is None, (
|
||||
f"{case.method} {case.path} minted the reserved name {missing!r} from "
|
||||
f"position {case.position!r} — the scope namespace is now squatted"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -58,6 +58,30 @@ def _create_session(
|
|||
return session_name
|
||||
|
||||
|
||||
def _add_sessions(
|
||||
client: TestClient,
|
||||
workspace_name: str,
|
||||
scope_name: str,
|
||||
session_names: list[str],
|
||||
) -> int:
|
||||
"""Add sessions to a scope via the facade; returns the status code (204 on success)."""
|
||||
return client.post(
|
||||
f"/v3/workspaces/{workspace_name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": session_names},
|
||||
).status_code
|
||||
|
||||
|
||||
def _scope_sessions(
|
||||
client: TestClient, workspace_name: str, scope_name: str
|
||||
) -> list[str]:
|
||||
"""Names of a scope's member sessions, oldest membership first."""
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{workspace_name}/scopes/{scope_name}/sessions/list"
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
return [item["id"] for item in response.json()["items"]]
|
||||
|
||||
|
||||
async def _get_session_peer(
|
||||
db_session: AsyncSession,
|
||||
workspace_name: str,
|
||||
|
|
@ -226,7 +250,61 @@ def test_scopes_routes_require_workspace_level_key(
|
|||
client.headers["Authorization"] = (
|
||||
f"Bearer {create_jwt(JWTParams(w=test_workspace.name, s='some-session'))}"
|
||||
)
|
||||
assert client.get(f"{scopes_url}/{scope_name}/sessions").status_code == 401
|
||||
assert client.post(f"{scopes_url}/{scope_name}/sessions/list").status_code == 401
|
||||
|
||||
|
||||
def test_session_create_scopes_requires_workspace_level_key(
|
||||
client: TestClient,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
):
|
||||
"""`scopes` on session create is the scopes routes by another door.
|
||||
|
||||
It creates scope peers and attaches memberships, so it carries the same
|
||||
workspace-level requirement. Session create is otherwise a self-authorizing
|
||||
route that accepts peer- and session-scoped keys, which is exactly why this
|
||||
needs its own check rather than the route's auth dependency.
|
||||
"""
|
||||
test_workspace, test_peer = sample_data
|
||||
monkeypatch.setattr(settings.AUTH, "USE_AUTH", True)
|
||||
monkeypatch.setattr(settings.AUTH, "JWT_SECRET", "test-secret")
|
||||
sessions_url = f"/v3/workspaces/{test_workspace.name}/sessions"
|
||||
scopes_url = f"/v3/workspaces/{test_workspace.name}/scopes"
|
||||
scope_name = str(generate_nanoid())
|
||||
|
||||
def create_with_scope(session_name: str) -> int:
|
||||
return client.post(
|
||||
sessions_url, json={"id": session_name, "scopes": [scope_name]}
|
||||
).status_code
|
||||
|
||||
def listed_scopes() -> set[str]:
|
||||
response = client.post(f"{scopes_url}/list")
|
||||
assert response.status_code == 200
|
||||
return {item["id"] for item in response.json()["items"]}
|
||||
|
||||
# Peer-scoped key: rejected
|
||||
client.headers["Authorization"] = (
|
||||
f"Bearer {create_jwt(JWTParams(w=test_workspace.name, p=test_peer.name))}"
|
||||
)
|
||||
assert create_with_scope(str(generate_nanoid())) == 401
|
||||
|
||||
# Session-scoped key: rejected. The session name must match the token's `s`
|
||||
# claim, or the handler's own session check would 401 first and this would
|
||||
# pass without ever reaching the scopes gate.
|
||||
own_session = str(generate_nanoid())
|
||||
client.headers["Authorization"] = (
|
||||
f"Bearer {create_jwt(JWTParams(w=test_workspace.name, s=own_session))}"
|
||||
)
|
||||
assert create_with_scope(own_session) == 401
|
||||
|
||||
# Workspace-scoped key: allowed. The scope is absent until this call lands,
|
||||
# which proves the rejected requests did not mint it on their way out.
|
||||
client.headers["Authorization"] = (
|
||||
f"Bearer {create_jwt(JWTParams(w=test_workspace.name))}"
|
||||
)
|
||||
assert scope_name not in listed_scopes()
|
||||
assert create_with_scope(str(generate_nanoid())) == 201
|
||||
assert scope_name in listed_scopes()
|
||||
|
||||
|
||||
def test_peer_create_rejects_reserved_prefix(
|
||||
|
|
@ -283,89 +361,6 @@ def test_peers_list_kind_filtering(
|
|||
assert {test_peer.name, backing_peer_name} <= names
|
||||
|
||||
|
||||
def test_scope_peer_cannot_author_messages(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
test_workspace, _ = sample_data
|
||||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
session_name = _create_session(client, test_workspace.name)
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/messages",
|
||||
json={
|
||||
"messages": [
|
||||
{
|
||||
"peer_id": scope_peer_name(scope_name),
|
||||
"content": "I should not speak",
|
||||
}
|
||||
]
|
||||
},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
assert SCOPE_PEER_PREFIX in response.json()["detail"]
|
||||
|
||||
|
||||
def test_scope_peer_cannot_be_chat_target(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
"""Chat validation happens before any LLM work, so this is safe to exercise."""
|
||||
test_workspace, test_peer = sample_data
|
||||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{test_peer.name}/chat",
|
||||
json={"query": "what do you know?", "target": scope_peer_name(scope_name)},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
|
||||
|
||||
def test_scope_peer_cannot_be_representation_target(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
test_workspace, test_peer = sample_data
|
||||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{test_peer.name}/representation",
|
||||
json={"target": scope_peer_name(scope_name)},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
|
||||
|
||||
def test_generic_session_peer_routes_reject_scope_peers(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
"""Scope membership is managed only via the scopes facade."""
|
||||
test_workspace, _ = sample_data
|
||||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
backing_peer_name = scope_peer_name(scope_name)
|
||||
session_name = _create_session(client, test_workspace.name)
|
||||
|
||||
base = f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}"
|
||||
|
||||
response = client.post(f"{base}/peers", json={backing_peer_name: {}})
|
||||
assert response.status_code == 422
|
||||
assert "scopes" in response.json()["detail"]
|
||||
|
||||
response = client.put(f"{base}/peers", json={backing_peer_name: {}})
|
||||
assert response.status_code == 422
|
||||
|
||||
response = client.request("DELETE", f"{base}/peers", json=[backing_peer_name])
|
||||
assert response.status_code == 422
|
||||
|
||||
# Session creation with a scope peer in the generic peers mapping is also
|
||||
# rejected; the `scopes` field is the supported path.
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions",
|
||||
json={"id": str(generate_nanoid()), "peers": {backing_peer_name: {}}},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
|
||||
|
||||
async def test_scope_sessions_add_list_remove(
|
||||
client: TestClient,
|
||||
db_session: AsyncSession,
|
||||
|
|
@ -381,11 +376,13 @@ async def test_scope_sessions_add_list_remove(
|
|||
scope_base = f"/v3/workspaces/{workspace_name}/scopes/{scope_name}"
|
||||
|
||||
# Add both sessions
|
||||
response = client.post(
|
||||
f"{scope_base}/sessions", json={"session_ids": [session_1, session_2]}
|
||||
assert (
|
||||
_add_sessions(client, workspace_name, scope_name, [session_1, session_2]) == 204
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert set(response.json()["session_ids"]) == {session_1, session_2}
|
||||
assert set(_scope_sessions(client, workspace_name, scope_name)) == {
|
||||
session_1,
|
||||
session_2,
|
||||
}
|
||||
|
||||
# Membership rows carry the observer shape: observe_others on, observe_me off
|
||||
session_peer = await _get_session_peer(
|
||||
|
|
@ -396,18 +393,11 @@ async def test_scope_sessions_add_list_remove(
|
|||
assert session_peer.configuration["observe_others"] is True
|
||||
assert session_peer.configuration["observe_me"] is False
|
||||
|
||||
# List memberships
|
||||
response = client.get(f"{scope_base}/sessions")
|
||||
assert response.status_code == 200
|
||||
assert set(response.json()["session_ids"]) == {session_1, session_2}
|
||||
|
||||
# Remove one membership (soft delete, like the generic remove-peer path)
|
||||
response = client.delete(f"{scope_base}/sessions/{session_1}")
|
||||
assert response.status_code == 204
|
||||
|
||||
response = client.get(f"{scope_base}/sessions")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["session_ids"] == [session_2]
|
||||
assert _scope_sessions(client, workspace_name, scope_name) == [session_2]
|
||||
|
||||
db_session.expire_all()
|
||||
session_peer = await _get_session_peer(
|
||||
|
|
@ -425,11 +415,15 @@ def test_scope_sessions_add_missing_session_404(
|
|||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
existing_session = _create_session(client, test_workspace.name)
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [existing_session, str(generate_nanoid())]},
|
||||
assert (
|
||||
_add_sessions(
|
||||
client,
|
||||
test_workspace.name,
|
||||
scope_name,
|
||||
[existing_session, str(generate_nanoid())],
|
||||
)
|
||||
== 404
|
||||
)
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
def test_scope_sessions_routes_404_on_unknown_scope(
|
||||
|
|
@ -437,14 +431,13 @@ def test_scope_sessions_routes_404_on_unknown_scope(
|
|||
):
|
||||
test_workspace, _ = sample_data
|
||||
session_name = _create_session(client, test_workspace.name)
|
||||
scope_base = f"/v3/workspaces/{test_workspace.name}/scopes/{generate_nanoid()}"
|
||||
unknown_scope = str(generate_nanoid())
|
||||
scope_base = f"/v3/workspaces/{test_workspace.name}/scopes/{unknown_scope}"
|
||||
|
||||
response = client.post(
|
||||
f"{scope_base}/sessions", json={"session_ids": [session_name]}
|
||||
assert (
|
||||
_add_sessions(client, test_workspace.name, unknown_scope, [session_name]) == 404
|
||||
)
|
||||
assert response.status_code == 404
|
||||
|
||||
assert client.get(f"{scope_base}/sessions").status_code == 404
|
||||
assert client.post(f"{scope_base}/sessions/list").status_code == 404
|
||||
assert client.delete(f"{scope_base}/sessions/{session_name}").status_code == 404
|
||||
|
||||
|
||||
|
|
@ -483,11 +476,7 @@ async def test_session_create_with_scopes(
|
|||
assert session_peer.configuration["observe_me"] is False
|
||||
|
||||
# And the memberships show up through the facade
|
||||
response = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_a}/sessions"
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json()["session_ids"] == [session_name]
|
||||
assert _scope_sessions(client, test_workspace.name, scope_a) == [session_name]
|
||||
|
||||
|
||||
def test_session_create_rejects_prefixed_scope_names(
|
||||
|
|
@ -530,11 +519,9 @@ async def test_scope_membership_equals_hand_built_observer(
|
|||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
scope_session = _create_session(client, test_workspace.name)
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [scope_session]},
|
||||
assert (
|
||||
_add_sessions(client, test_workspace.name, scope_name, [scope_session]) == 204
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
hand_built = await _get_session_peer(
|
||||
db_session, test_workspace.name, observer_session, observer_name
|
||||
|
|
@ -565,11 +552,7 @@ async def test_scope_peer_observes_ingested_messages(
|
|||
json={test_peer.name: {}},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [session_name]},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert _add_sessions(client, test_workspace.name, scope_name, [session_name]) == 204
|
||||
|
||||
# Ingest a message from the real peer and run the deriver enqueue fan-out
|
||||
message = models.Message(
|
||||
|
|
@ -731,42 +714,6 @@ async def test_forged_configuration_kind_does_not_make_a_scope(
|
|||
assert response.status_code in [200, 201], response.text
|
||||
|
||||
|
||||
def test_update_peer_rejects_reserved_prefix(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
"""PUT on a reserved-namespace name is a clean 422, never a 500."""
|
||||
test_workspace, _ = sample_data
|
||||
|
||||
response = client.put(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{scope_peer_name('therapy')}",
|
||||
json={"metadata": {"k": "v"}},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
assert SCOPE_PEER_PREFIX in response.json()["detail"]
|
||||
|
||||
|
||||
async def test_scope_peer_cannot_be_chat_or_representation_observer(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
"""The path-level peer_id is guarded, not just `target`."""
|
||||
test_workspace, _ = sample_data
|
||||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
backing = scope_peer_name(scope_name)
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{backing}/chat",
|
||||
json={"query": "what do you know?"},
|
||||
)
|
||||
assert response.status_code == 422, response.text
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{backing}/representation",
|
||||
json={},
|
||||
)
|
||||
assert response.status_code == 422, response.text
|
||||
|
||||
|
||||
def test_internal_metadata_never_in_peer_response(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
|
|
@ -848,96 +795,6 @@ def test_message_author_cannot_create_invalid_peer_name(
|
|||
assert bad_name not in [p["id"] for p in response.json()["items"]]
|
||||
|
||||
|
||||
def test_message_author_cannot_squat_scope_namespace(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
"""The reserved namespace must not be reachable via the message author path.
|
||||
|
||||
Without create-path validation this minted an unflagged `scope.<name>` peer,
|
||||
which then permanently 409-blocked creating that scope — a denial of service
|
||||
on the namespace by any caller who can post a message.
|
||||
"""
|
||||
test_workspace, _ = sample_data
|
||||
scope_name = str(generate_nanoid())
|
||||
session_name = _create_session(client, test_workspace.name)
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/messages",
|
||||
json={
|
||||
"messages": [{"peer_id": scope_peer_name(scope_name), "content": "hello"}]
|
||||
},
|
||||
)
|
||||
assert response.status_code == 422, response.text
|
||||
assert SCOPE_PEER_PREFIX in response.json()["detail"]
|
||||
|
||||
# The scope name is still available
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
|
||||
|
||||
def test_session_peer_map_cannot_squat_scope_namespace(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
"""Same hole, via the session peer mapping rather than a message author."""
|
||||
test_workspace, _ = sample_data
|
||||
scope_name = str(generate_nanoid())
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions",
|
||||
json={
|
||||
"id": str(generate_nanoid()),
|
||||
"peers": {scope_peer_name(scope_name): {}},
|
||||
},
|
||||
)
|
||||
assert response.status_code == 422, response.text
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
|
||||
|
||||
async def test_unflagged_squatter_can_still_be_updated(
|
||||
client: TestClient,
|
||||
db_session: AsyncSession,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
):
|
||||
"""An existing unflagged peer in the namespace is a normal peer.
|
||||
|
||||
Three-way behavior on PUT /peers/{id}: a real scope is refused, an existing
|
||||
unflagged squatter updates fine, and a missing reserved-prefix name is
|
||||
refused by create-path validation rather than being minted.
|
||||
"""
|
||||
test_workspace, _ = sample_data
|
||||
squatter = scope_peer_name(str(generate_nanoid()))
|
||||
db_session.add(models.Peer(workspace_name=test_workspace.name, name=squatter))
|
||||
await db_session.commit()
|
||||
|
||||
# existing unflagged peer -> allowed
|
||||
response = client.put(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{squatter}",
|
||||
json={"metadata": {"k": "v"}},
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
assert response.json()["metadata"] == {"k": "v"}
|
||||
|
||||
# real scope -> refused
|
||||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
response = client.put(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{scope_peer_name(scope_name)}",
|
||||
json={"metadata": {"k": "v"}},
|
||||
)
|
||||
assert response.status_code == 422, response.text
|
||||
|
||||
# missing reserved-prefix name -> refused, not created
|
||||
missing = scope_peer_name(str(generate_nanoid()))
|
||||
response = client.put(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{missing}",
|
||||
json={"metadata": {"k": "v"}},
|
||||
)
|
||||
assert response.status_code == 422, response.text
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/list", json={"kind": "all"}
|
||||
)
|
||||
assert missing not in [p["id"] for p in response.json()["items"]]
|
||||
|
||||
|
||||
async def test_resolved_scope_peer_rejected_at_membership_upsert(
|
||||
client: TestClient,
|
||||
db_session: AsyncSession,
|
||||
|
|
@ -968,26 +825,25 @@ async def test_resolved_scope_peer_rejected_at_membership_upsert(
|
|||
)
|
||||
|
||||
|
||||
def test_set_peer_config_cannot_disable_a_scope(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
async def test_set_peer_config_cannot_disable_a_scope(
|
||||
client: TestClient,
|
||||
db_session: AsyncSession,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
):
|
||||
"""A scope's membership config belongs to the facade, not the caller.
|
||||
|
||||
`observe_others=false` would silently stop all fan-out into the scope, and
|
||||
`observe_me=true` would make Honcho form a representation *of* a scope.
|
||||
|
||||
Verified against the row rather than the config route, because that read is
|
||||
refused for a scope too — see the policy table.
|
||||
"""
|
||||
test_workspace, _ = sample_data
|
||||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
backing = scope_peer_name(scope_name)
|
||||
session_name = _create_session(client, test_workspace.name)
|
||||
assert (
|
||||
client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [session_name]},
|
||||
).status_code
|
||||
== 200
|
||||
)
|
||||
assert _add_sessions(client, test_workspace.name, scope_name, [session_name]) == 204
|
||||
|
||||
response = client.put(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers/{backing}/config",
|
||||
|
|
@ -996,62 +852,54 @@ def test_set_peer_config_cannot_disable_a_scope(
|
|||
assert response.status_code == 422, response.text
|
||||
|
||||
# Observer semantics intact
|
||||
current = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers/{backing}/config"
|
||||
membership = await _get_session_peer(
|
||||
db_session, test_workspace.name, session_name, backing
|
||||
)
|
||||
assert current.status_code == 200
|
||||
assert current.json() == {"observe_me": False, "observe_others": True}
|
||||
assert membership is not None
|
||||
assert membership.configuration == {"observe_me": False, "observe_others": True}
|
||||
|
||||
|
||||
async def test_unflagged_squatter_config_still_settable(
|
||||
client: TestClient,
|
||||
db_session: AsyncSession,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
):
|
||||
"""The set_peer_config guard is flag-based, so a squatter is unaffected."""
|
||||
test_workspace, _ = sample_data
|
||||
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 = _create_session(client, test_workspace.name)
|
||||
assert (
|
||||
client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers",
|
||||
json={squatter: {}},
|
||||
).status_code
|
||||
== 200
|
||||
)
|
||||
|
||||
response = client.put(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers/{squatter}/config",
|
||||
json={"observe_others": False, "observe_me": True},
|
||||
)
|
||||
assert response.status_code == 204, response.text
|
||||
|
||||
|
||||
def test_generic_session_remove_cannot_detach_a_scope(
|
||||
def test_session_peers_listing_excludes_scopes(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
"""Scope membership must end through the scopes routes, which reconcile."""
|
||||
test_workspace, _ = sample_data
|
||||
"""The generic session-peer surface must not expose the facade's observer.
|
||||
|
||||
Without the filter this listing returns a peer literally named
|
||||
`scope.<name>` carrying `observe_others=true` — the exact mechanic the
|
||||
facade exists to hide. The membership-config read is refused for the same
|
||||
reason; ordinary members are unaffected by both.
|
||||
"""
|
||||
test_workspace, test_peer = sample_data
|
||||
base = f"/v3/workspaces/{test_workspace.name}"
|
||||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
backing = scope_peer_name(scope_name)
|
||||
session_name = _create_session(client, test_workspace.name)
|
||||
|
||||
assert (
|
||||
client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [session_name]},
|
||||
f"{base}/sessions/{session_name}/peers", json={test_peer.name: {}}
|
||||
).status_code
|
||||
== 200
|
||||
)
|
||||
assert _add_sessions(client, test_workspace.name, scope_name, [session_name]) == 204
|
||||
|
||||
response = client.request(
|
||||
"DELETE",
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers",
|
||||
json=[scope_peer_name(scope_name)],
|
||||
listed = client.get(f"{base}/sessions/{session_name}/peers")
|
||||
assert listed.status_code == 200
|
||||
names = {item["id"] for item in listed.json()["items"]}
|
||||
assert test_peer.name in names
|
||||
assert backing not in names
|
||||
|
||||
assert (
|
||||
client.get(f"{base}/sessions/{session_name}/peers/{backing}/config").status_code
|
||||
== 422
|
||||
)
|
||||
assert (
|
||||
client.get(
|
||||
f"{base}/sessions/{session_name}/peers/{test_peer.name}/config"
|
||||
).status_code
|
||||
== 200
|
||||
)
|
||||
assert response.status_code == 422, response.text
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad_name", ["", "a" * 513])
|
||||
|
|
@ -1075,10 +923,9 @@ def test_degenerate_peer_names_are_422_not_500(
|
|||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Observed-position and pre-seeding guards. A scope may be an observer but must
|
||||
# never be observed — and "observed" includes a reserved name that does not yet
|
||||
# exist, since nothing on these paths creates the peer and the state would
|
||||
# retroactively describe the scope once created.
|
||||
# Detach-by-omission. The replacement routes never name the scope, so there is
|
||||
# no peer position for the route-policy table to classify — the caller detaches
|
||||
# by leaving it out. Preservation has to be flag-based, not request-based.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
|
|
@ -1094,13 +941,7 @@ def test_generic_replacement_preserves_scope_membership(
|
|||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
session_name = _create_session(client, test_workspace.name)
|
||||
assert (
|
||||
client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [session_name]},
|
||||
).status_code
|
||||
== 200
|
||||
)
|
||||
assert _add_sessions(client, test_workspace.name, scope_name, [session_name]) == 204
|
||||
|
||||
# Replacement naming only an ordinary peer must succeed...
|
||||
response = client.put(
|
||||
|
|
@ -1110,11 +951,7 @@ def test_generic_replacement_preserves_scope_membership(
|
|||
assert response.status_code == 200, response.text
|
||||
|
||||
# ...while leaving the scope's membership intact.
|
||||
response = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions"
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json()["session_ids"] == [session_name]
|
||||
assert _scope_sessions(client, test_workspace.name, scope_name) == [session_name]
|
||||
|
||||
|
||||
async def test_empty_replacement_preserves_scope_membership(
|
||||
|
|
@ -1131,10 +968,7 @@ async def test_empty_replacement_preserves_scope_membership(
|
|||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers",
|
||||
json={test_peer.name: {}},
|
||||
)
|
||||
client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [session_name]},
|
||||
)
|
||||
_add_sessions(client, test_workspace.name, scope_name, [session_name])
|
||||
|
||||
assert (
|
||||
client.put(
|
||||
|
|
@ -1144,10 +978,7 @@ async def test_empty_replacement_preserves_scope_membership(
|
|||
== 200
|
||||
)
|
||||
|
||||
response = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions"
|
||||
)
|
||||
assert response.json()["session_ids"] == [session_name]
|
||||
assert _scope_sessions(client, test_workspace.name, scope_name) == [session_name]
|
||||
|
||||
# The other half of the docstring: without this the test passes even if the
|
||||
# empty replacement became a no-op for ordinary peers too.
|
||||
|
|
@ -1192,45 +1023,15 @@ async def test_replacement_still_removes_unflagged_squatter(
|
|||
assert session_peer.left_at is not None, "squatter should be replaced normally"
|
||||
|
||||
|
||||
def test_peer_card_cannot_be_preseeded_for_a_future_scope(
|
||||
client: TestClient, sample_data: tuple[Workspace, Peer]
|
||||
):
|
||||
"""A card keyed on a not-yet-existing reserved name is refused.
|
||||
|
||||
Only the observer is resolved when writing a card, so without this the card
|
||||
persists and starts describing a real scope the moment one is created.
|
||||
"""
|
||||
test_workspace, test_peer = sample_data
|
||||
future = str(generate_nanoid())
|
||||
|
||||
response = client.put(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{test_peer.name}"
|
||||
+ f"/card?target={scope_peer_name(future)}",
|
||||
json={"peer_card": ["pre-seeded"]},
|
||||
)
|
||||
assert response.status_code == 422, response.text
|
||||
|
||||
# And the scope name is still free to create
|
||||
assert _create_scope(client, test_workspace.name, future).status_code == 201
|
||||
|
||||
|
||||
async def test_peer_card_target_squatter_still_allowed(
|
||||
client: TestClient,
|
||||
db_session: AsyncSession,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
):
|
||||
"""An existing unflagged squatter remains a valid card subject."""
|
||||
test_workspace, test_peer = sample_data
|
||||
squatter = scope_peer_name(str(generate_nanoid()))
|
||||
db_session.add(models.Peer(workspace_name=test_workspace.name, name=squatter))
|
||||
await db_session.commit()
|
||||
|
||||
response = client.put(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{test_peer.name}"
|
||||
+ f"/card?target={squatter}",
|
||||
json={"peer_card": ["ordinary"]},
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
# ---------------------------------------------------------------------------
|
||||
# Observed-position guards below the HTTP surface. A scope may be an observer
|
||||
# but must never be observed — and "observed" includes a reserved name that does
|
||||
# not yet exist, since nothing on these paths creates the peer and the state
|
||||
# would retroactively describe the scope once created. The route-level half of
|
||||
# this is enumerated in test_scope_route_policy.py; these cover the crud entry
|
||||
# points that no route table reaches, plus the side effects a status code alone
|
||||
# would not catch.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
async def test_set_peer_card_guard_covers_internal_callers(
|
||||
|
|
@ -1254,10 +1055,12 @@ async def test_dream_cannot_be_queued_for_a_future_scope(
|
|||
db_session: AsyncSession,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
):
|
||||
"""A dream naming a not-yet-existing reserved observed peer is refused.
|
||||
"""A refused dream leaves no queue row behind.
|
||||
|
||||
The route's own precheck cannot catch this — the peer is not flagged yet — so
|
||||
the check has to sit in the transaction that inserts the queue item.
|
||||
The 422 itself is enumerated in test_scope_route_policy.py; what that cannot
|
||||
see is the side effect. The route's own precheck cannot catch this — the peer
|
||||
is not flagged yet — so the check has to sit in the transaction that inserts
|
||||
the queue item, and a check placed after the insert would still 422.
|
||||
"""
|
||||
test_workspace, test_peer = sample_data
|
||||
future = scope_peer_name(str(generate_nanoid()))
|
||||
|
|
@ -1316,35 +1119,6 @@ def test_prefixed_nul_name_is_422_not_500(
|
|||
assert response.status_code == 422, response.text
|
||||
|
||||
|
||||
async def test_representation_rechecks_after_early_check(
|
||||
client: TestClient,
|
||||
db_session: AsyncSession,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
):
|
||||
"""The representation read refuses a reserved name that could become a scope.
|
||||
|
||||
The early name check passes anything not yet flagged, and the read happens in
|
||||
a later session — so a reserved name that does not exist yet must be refused
|
||||
rather than left to become a scope before the read.
|
||||
"""
|
||||
test_workspace, _ = sample_data
|
||||
future = scope_peer_name(str(generate_nanoid()))
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{future}/representation", json={}
|
||||
)
|
||||
assert response.status_code == 422, response.text
|
||||
|
||||
# An existing unflagged squatter still reads normally.
|
||||
squatter = scope_peer_name(str(generate_nanoid()))
|
||||
db_session.add(models.Peer(workspace_name=test_workspace.name, name=squatter))
|
||||
await db_session.commit()
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/peers/{squatter}/representation", json={}
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Observer limit. Scope memberships carry observe_others=True but must not
|
||||
# consume SESSION_OBSERVERS_LIMIT: that would cap scopes-per-session at the
|
||||
|
|
@ -1365,18 +1139,16 @@ def test_scopes_do_not_count_toward_observer_limit(
|
|||
|
||||
for scope_name in scope_names:
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [session_name]},
|
||||
assert (
|
||||
_add_sessions(client, test_workspace.name, scope_name, [session_name])
|
||||
== 204
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
|
||||
# Every membership is live, and the session-create path agrees.
|
||||
for scope_name in scope_names:
|
||||
response = client.get(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions"
|
||||
)
|
||||
assert response.json()["session_ids"] == [session_name]
|
||||
assert _scope_sessions(client, test_workspace.name, scope_name) == [
|
||||
session_name
|
||||
]
|
||||
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions",
|
||||
|
|
@ -1396,13 +1168,7 @@ def test_observer_limit_still_applies_to_real_peers(
|
|||
session_name = _create_session(client, test_workspace.name)
|
||||
scope_name = str(generate_nanoid())
|
||||
assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
|
||||
assert (
|
||||
client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
|
||||
json={"session_ids": [session_name]},
|
||||
).status_code
|
||||
== 200
|
||||
)
|
||||
assert _add_sessions(client, test_workspace.name, scope_name, [session_name]) == 204
|
||||
|
||||
observers = {
|
||||
str(generate_nanoid()): {"observe_others": True}
|
||||
|
|
|
|||
Loading…
Reference in New Issue