mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-19 02:56:17 +00:00
* fix(gateway): scope runs read endpoints by data identity, not authorization identity Trusted internal callers are authorized as a synthetic internal user (id "default", or the make_safe_user_id-normalized owner with an owner header), while start_run stamps run rows with the raw trusted-owner value. list_runs / get_run / list_runs_page filtered by the authorization identity, so the store-side user filter never matched and internal callers always saw an empty runs list (or 404) for threads they are authorized to read. The three read endpoints now resolve their filter id through _run_scope_user_id: internal-role callers skip the per-user filter (thread visibility is already authorized by owner_check=True), and browser/API sessions keep the existing per-user filter unchanged. Regression tests cover both identities across the three endpoints (list, keyset page, single get) with a MemoryRunStore seeded with mixed-owner rows; without the fix the four internal-caller cases fail while the browser-session isolation case passes. * fix(gateway): route the message read endpoints through the same data-identity scoping Review follow-up on #5448: list_thread_messages and list_thread_messages_page resolved get_current_user and passed it as the data filter to the event-store scan, hidden-run lookups, turn-duration injection and the feedback queries — the same authorization-vs-data identity conflation fixed for the runs endpoints, leaving the #5437 empty-read symptom in place for lossy owner values. Both endpoints now resolve their filter id through _run_scope_user_id as well. Regression tests extend to the two message endpoints, asserting the resolved filter identity at the runs-store and feedback-repo boundaries (None for internal callers, the session user id for browser sessions). * fix(feedback): deterministic per-run collapse for unfiltered feedback reads Review follow-up on #5448: with _run_scope_user_id returning None for internal callers, the feedback lookups now receive an explicit-None user id, which skips the user_id WHERE in FeedbackRepository. On shared/NULL-owner threads several browser users can hold feedback on the same run, and list_by_thread_grouped / list_by_run_ids collapsed rows per run_id via a dict comprehension over unordered results — the feedback attached to the last AI message would be an arbitrary user's row. Both methods now order by created_at ASC with feedback_id as the tie-break, so the collapse deterministically keeps the most recently created feedback. _run_scope_user_id's docstring now documents that the resolved id also scopes feedback and event-store reads, not just run rows. Regression test seeds multi-user feedback on one run and asserts the collapse outcome is stable across repeated unfiltered reads. * docs(feedback): the collapse keeps the most recently written feedback created_at is refreshed on upsert, so the surviving row per run is the most recently written (created or updated), not the most recently created — align both docstrings with the ordering key's actual semantics.
318 lines
12 KiB
Python
318 lines
12 KiB
Python
"""Regression coverage for the runs/messages read endpoints' identity scoping (#5437).
|
|
|
|
Trusted internal callers are *authorized* as a synthetic internal user
|
|
(``system_role="internal"``) whose id is ``"default"`` or the
|
|
``make_safe_user_id``-normalized owner, while ``start_run`` stamps run rows
|
|
with the raw trusted-owner value. Filtering the reads by the authorization
|
|
identity therefore never matches the persisted rows. The read endpoints must
|
|
skip the per-user filter for internal callers — thread visibility is already
|
|
authorized by ``@require_permission(..., owner_check=True)`` — and keep it for
|
|
browser/API sessions.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
from uuid import UUID
|
|
|
|
import pytest
|
|
from fastapi import FastAPI, Request, Response
|
|
from fastapi.testclient import TestClient
|
|
from starlette.middleware.base import BaseHTTPMiddleware
|
|
|
|
from app.gateway.auth.models import User
|
|
from app.gateway.auth_disabled import AUTH_SOURCE_INTERNAL, AUTH_SOURCE_SESSION
|
|
from app.gateway.authz import AuthContext, Permissions
|
|
from app.gateway.internal_auth import INTERNAL_OWNER_USER_ID_HEADER_NAME, get_internal_user
|
|
from app.gateway.routers import thread_runs
|
|
from deerflow.runtime.events.store.memory import MemoryRunEventStore
|
|
from deerflow.runtime.runs.manager import RunManager
|
|
from deerflow.runtime.runs.store.memory import MemoryRunStore
|
|
|
|
THREAD_ID = "thread-scope"
|
|
BROWSER_USER_ID = UUID("00000000-0000-0000-0000-00000000000a")
|
|
# A lossy trusted-owner value: make_safe_user_id normalizes it to
|
|
# "feishu-owner-777-<digest>", which can never equal the raw value stamped on
|
|
# the run row — the exact mismatch class reported in #5437.
|
|
OWNER_RAW = "feishu:owner-777"
|
|
RUN_BROWSER = "run-browser-row"
|
|
RUN_OWNER = "run-owner-row"
|
|
|
|
_STUB_PERMISSIONS: list[str] = [
|
|
Permissions.THREADS_READ,
|
|
Permissions.RUNS_READ,
|
|
Permissions.RUNS_CANCEL,
|
|
]
|
|
|
|
|
|
class _ScopeAuthMiddleware(BaseHTTPMiddleware):
|
|
"""Stamp the same state trio production ``AuthMiddleware`` stamps."""
|
|
|
|
def __init__(self, app, *, user, auth_source: str) -> None:
|
|
super().__init__(app)
|
|
self._user = user
|
|
self._auth_source = auth_source
|
|
|
|
async def dispatch(self, request: Request, call_next) -> Response:
|
|
request.state.user = self._user
|
|
request.state.auth_source = self._auth_source
|
|
request.state.auth = AuthContext(user=self._user, permissions=list(_STUB_PERMISSIONS))
|
|
return await call_next(request)
|
|
|
|
|
|
class _PermissiveThreadStore:
|
|
"""Stands in for the thread store behind ``owner_check=True``."""
|
|
|
|
async def check_access(self, _thread_id: str, _user_id: str, *, require_existing: bool = False) -> bool:
|
|
return True
|
|
|
|
|
|
class _RecordingRunStore(MemoryRunStore):
|
|
"""Records the per-user filter identity each read resolves to.
|
|
|
|
``MemoryRunEventStore.list_messages`` ignores ``user_id`` (only the SQL
|
|
backends honor it), so the runs store is where the resolved filter id is
|
|
observable in-memory: hidden-run lookups and turn-duration injection both
|
|
flow through ``list_by_thread``/``get`` with the endpoint's filter id.
|
|
"""
|
|
|
|
def __init__(self) -> None:
|
|
super().__init__()
|
|
self.list_by_thread_user_ids: list[str | None] = []
|
|
self.get_user_ids: list[str | None] = []
|
|
|
|
async def list_by_thread(self, thread_id, *, user_id=None, **kwargs):
|
|
self.list_by_thread_user_ids.append(user_id)
|
|
return await super().list_by_thread(thread_id, user_id=user_id, **kwargs)
|
|
|
|
async def get(self, run_id, *, user_id=None, **kwargs):
|
|
self.get_user_ids.append(user_id)
|
|
return await super().get(run_id, user_id=user_id, **kwargs)
|
|
|
|
|
|
class _RecordingFeedbackRepo:
|
|
"""Records the per-user identity the feedback queries are scoped with."""
|
|
|
|
def __init__(self) -> None:
|
|
self.list_by_thread_user_ids: list[str | None] = []
|
|
self.list_by_run_ids_user_ids: list[str | None] = []
|
|
|
|
async def list_by_thread_grouped(self, thread_id, *, user_id=None):
|
|
self.list_by_thread_user_ids.append(user_id)
|
|
return {}
|
|
|
|
async def list_by_run_ids(self, thread_id, run_ids, *, user_id=None):
|
|
self.list_by_run_ids_user_ids.append(user_id)
|
|
return {}
|
|
|
|
|
|
def _browser_user() -> User:
|
|
return User(id=BROWSER_USER_ID, email="scope-test@example.com", password_hash="x", system_role="user")
|
|
|
|
|
|
def _internal_user(owner_raw: str | None):
|
|
# Mirrors AuthMiddleware + get_internal_user: the synthetic internal user
|
|
# carries the safe-spelled owner id, or "default" without an owner header.
|
|
return get_internal_user(owner_user_id=owner_raw)
|
|
|
|
|
|
def _seed_run(store: MemoryRunStore, run_id: str, *, user_id: str | None) -> None:
|
|
asyncio.run(store.put(run_id, thread_id=THREAD_ID, user_id=user_id, status="success"))
|
|
|
|
|
|
def _seed_message(event_store: MemoryRunEventStore, run_id: str, message_id: str) -> None:
|
|
asyncio.run(
|
|
event_store.put(
|
|
thread_id=THREAD_ID,
|
|
run_id=run_id,
|
|
event_type="llm.ai.response",
|
|
category="message",
|
|
content={"type": "ai", "id": message_id, "content": message_id, "additional_kwargs": {}},
|
|
metadata={},
|
|
)
|
|
)
|
|
|
|
|
|
def _make_app(
|
|
*,
|
|
user,
|
|
auth_source: str,
|
|
run_store: MemoryRunStore,
|
|
event_store: MemoryRunEventStore | None = None,
|
|
feedback_repo: _RecordingFeedbackRepo | None = None,
|
|
) -> TestClient:
|
|
app = FastAPI()
|
|
app.add_middleware(_ScopeAuthMiddleware, user=user, auth_source=auth_source)
|
|
app.state.thread_store = _PermissiveThreadStore()
|
|
app.state.run_manager = RunManager(store=run_store)
|
|
if event_store is not None:
|
|
app.state.run_event_store = event_store
|
|
if feedback_repo is not None:
|
|
app.state.feedback_repo = feedback_repo
|
|
app.include_router(thread_runs.router)
|
|
return TestClient(app)
|
|
|
|
|
|
@pytest.fixture()
|
|
def mixed_owner_store() -> MemoryRunStore:
|
|
store = MemoryRunStore()
|
|
_seed_run(store, RUN_BROWSER, user_id=str(BROWSER_USER_ID))
|
|
_seed_run(store, RUN_OWNER, user_id=OWNER_RAW)
|
|
return store
|
|
|
|
|
|
def test_internal_caller_lists_owner_stamped_runs(mixed_owner_store: MemoryRunStore) -> None:
|
|
client = _make_app(
|
|
user=_internal_user(OWNER_RAW),
|
|
auth_source=AUTH_SOURCE_INTERNAL,
|
|
run_store=mixed_owner_store,
|
|
)
|
|
|
|
with client:
|
|
response = client.get(
|
|
f"/api/threads/{THREAD_ID}/runs",
|
|
headers={INTERNAL_OWNER_USER_ID_HEADER_NAME: OWNER_RAW},
|
|
)
|
|
|
|
assert response.status_code == 200
|
|
assert {row["run_id"] for row in response.json()} == {RUN_BROWSER, RUN_OWNER}
|
|
|
|
|
|
def test_internal_caller_get_run_owner_stamped(mixed_owner_store: MemoryRunStore) -> None:
|
|
client = _make_app(
|
|
user=_internal_user(OWNER_RAW),
|
|
auth_source=AUTH_SOURCE_INTERNAL,
|
|
run_store=mixed_owner_store,
|
|
)
|
|
|
|
with client:
|
|
response = client.get(
|
|
f"/api/threads/{THREAD_ID}/runs/{RUN_OWNER}",
|
|
headers={INTERNAL_OWNER_USER_ID_HEADER_NAME: OWNER_RAW},
|
|
)
|
|
|
|
assert response.status_code == 200
|
|
assert response.json()["run_id"] == RUN_OWNER
|
|
|
|
|
|
def test_internal_caller_runs_page_owner_stamped(mixed_owner_store: MemoryRunStore) -> None:
|
|
client = _make_app(
|
|
user=_internal_user(OWNER_RAW),
|
|
auth_source=AUTH_SOURCE_INTERNAL,
|
|
run_store=mixed_owner_store,
|
|
)
|
|
|
|
with client:
|
|
response = client.get(
|
|
f"/api/threads/{THREAD_ID}/runs/page",
|
|
headers={INTERNAL_OWNER_USER_ID_HEADER_NAME: OWNER_RAW},
|
|
)
|
|
|
|
assert response.status_code == 200
|
|
assert {row["run_id"] for row in response.json()["data"]} == {RUN_BROWSER, RUN_OWNER}
|
|
assert response.json()["has_more"] is False
|
|
|
|
|
|
def test_internal_caller_without_owner_header_sees_authorized_thread_runs(mixed_owner_store: MemoryRunStore) -> None:
|
|
"""No owner header ⇒ synthetic id "default", which matches nothing either.
|
|
|
|
The thread is authorized via owner_check, so its runs stay listable.
|
|
"""
|
|
client = _make_app(
|
|
user=_internal_user(None),
|
|
auth_source=AUTH_SOURCE_INTERNAL,
|
|
run_store=mixed_owner_store,
|
|
)
|
|
|
|
with client:
|
|
response = client.get(f"/api/threads/{THREAD_ID}/runs")
|
|
|
|
assert response.status_code == 200
|
|
assert {row["run_id"] for row in response.json()} == {RUN_BROWSER, RUN_OWNER}
|
|
|
|
|
|
def test_browser_session_keeps_per_user_filter(mixed_owner_store: MemoryRunStore) -> None:
|
|
"""Browser sessions keep filtering by their own data identity."""
|
|
client = _make_app(
|
|
user=_browser_user(),
|
|
auth_source=AUTH_SOURCE_SESSION,
|
|
run_store=mixed_owner_store,
|
|
)
|
|
|
|
with client:
|
|
listed = client.get(f"/api/threads/{THREAD_ID}/runs")
|
|
cross_user = client.get(f"/api/threads/{THREAD_ID}/runs/{RUN_OWNER}")
|
|
|
|
assert listed.status_code == 200
|
|
assert [row["run_id"] for row in listed.json()] == [RUN_BROWSER]
|
|
assert cross_user.status_code == 404
|
|
|
|
|
|
def test_internal_caller_messages_skip_per_user_filter(mixed_owner_store: MemoryRunStore) -> None:
|
|
"""Internal callers read the authorized thread's messages unfiltered.
|
|
|
|
The observable filter identity is what reaches the scoped queries — the
|
|
runs store (hidden-run lookups, turn durations) and the feedback repo —
|
|
``None`` for internal callers.
|
|
"""
|
|
event_store = MemoryRunEventStore()
|
|
_seed_message(event_store, RUN_OWNER, "msg-owner")
|
|
_seed_message(event_store, RUN_BROWSER, "msg-browser")
|
|
run_store = _RecordingRunStore()
|
|
for run_id, user_id in ((RUN_BROWSER, str(BROWSER_USER_ID)), (RUN_OWNER, OWNER_RAW)):
|
|
_seed_run(run_store, run_id, user_id=user_id)
|
|
feedback_repo = _RecordingFeedbackRepo()
|
|
|
|
client = _make_app(
|
|
user=_internal_user(OWNER_RAW),
|
|
auth_source=AUTH_SOURCE_INTERNAL,
|
|
run_store=run_store,
|
|
event_store=event_store,
|
|
feedback_repo=feedback_repo,
|
|
)
|
|
|
|
with client:
|
|
response = client.get(
|
|
f"/api/threads/{THREAD_ID}/messages",
|
|
headers={INTERNAL_OWNER_USER_ID_HEADER_NAME: OWNER_RAW},
|
|
)
|
|
page = client.get(
|
|
f"/api/threads/{THREAD_ID}/messages/page",
|
|
headers={INTERNAL_OWNER_USER_ID_HEADER_NAME: OWNER_RAW},
|
|
)
|
|
|
|
assert response.status_code == 200
|
|
assert {row["content"]["id"] for row in response.json()} == {"msg-owner", "msg-browser"}
|
|
assert page.status_code == 200
|
|
assert {row["content"]["id"] for row in page.json()["data"]} == {"msg-owner", "msg-browser"}
|
|
assert run_store.list_by_thread_user_ids and all(uid is None for uid in run_store.list_by_thread_user_ids)
|
|
assert feedback_repo.list_by_thread_user_ids == [None]
|
|
assert feedback_repo.list_by_run_ids_user_ids == [None]
|
|
|
|
|
|
def test_browser_session_messages_keep_per_user_filter(mixed_owner_store: MemoryRunStore) -> None:
|
|
"""Browser sessions keep passing their own id to the messages pipeline."""
|
|
event_store = MemoryRunEventStore()
|
|
_seed_message(event_store, RUN_OWNER, "msg-owner")
|
|
_seed_message(event_store, RUN_BROWSER, "msg-browser")
|
|
run_store = _RecordingRunStore()
|
|
for run_id, user_id in ((RUN_BROWSER, str(BROWSER_USER_ID)), (RUN_OWNER, OWNER_RAW)):
|
|
_seed_run(run_store, run_id, user_id=user_id)
|
|
feedback_repo = _RecordingFeedbackRepo()
|
|
|
|
client = _make_app(
|
|
user=_browser_user(),
|
|
auth_source=AUTH_SOURCE_SESSION,
|
|
run_store=run_store,
|
|
event_store=event_store,
|
|
feedback_repo=feedback_repo,
|
|
)
|
|
|
|
with client:
|
|
response = client.get(f"/api/threads/{THREAD_ID}/messages")
|
|
|
|
assert response.status_code == 200
|
|
assert {row["content"]["id"] for row in response.json()} == {"msg-owner", "msg-browser"}
|
|
assert run_store.list_by_thread_user_ids and all(uid == str(BROWSER_USER_ID) for uid in run_store.list_by_thread_user_ids)
|
|
assert feedback_repo.list_by_thread_user_ids == [str(BROWSER_USER_ID)]
|