fix: allow read-only cross-user access to agent threads via Open in Web links (#1568)

The thread detail endpoint already returned metadata for non-owners, but
the transcript hydration endpoints (state, stream/events, history, pr-diff)
all asserted ownership and 404-ed. This caused the UI to redirect non-owners
back to /agents when they clicked an "Open in Web" link shared in Slack.

Dashboard login is already gated by ALLOWED_GITHUB_ORGS, so any logged-in
user is a trusted org member. This commit:
- Adds _thread_is_readable / _assert_thread_readable helpers that grant
  read access to any surfaced-source thread for authenticated users
- Relaxes read endpoints (state, stream/events, history, pr-diff, SSE
  stream) to use readable checks instead of ownership checks
- Keeps write endpoints (send message, cancel, delete, resolve, run
  commands) owner-only
- Adds an isOwner field to the thread summary so the frontend can render
  a read-only mode (hides the prompt bar, resolve/delete buttons)

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
Johannes du Plessis 2026-06-18 09:10:09 -07:00 • committed by GitHub
parent 98b824bd54
commit c07434a221
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 261 additions and 89 deletions

View file

@ -247,6 +247,22 @@ def _assert_thread_owner(metadata: dict[str, Any], login: str, email: str | None
raise HTTPException(404, "thread not found")
def _thread_is_readable(metadata: dict[str, Any]) -> bool:
"""Any surfaced-source thread is readable by authenticated users.
Dashboard login is already gated by ``ALLOWED_GITHUB_ORGS`` (see
``oauth.enforce_org_login_gate``), so any logged-in user is a trusted
org member. This lets teammates open "Open in Web" links shared in Slack
threads with read-only access.
"""
return _thread_source(metadata) in _SURFACED_SOURCES
def _assert_thread_readable(metadata: dict[str, Any]) -> None:
if not _thread_is_readable(metadata):
raise HTTPException(404, "thread not found")
def _metadata_repo(metadata: dict[str, Any]) -> tuple[str, str, str]:
owner = metadata.get("repo_owner")
name = metadata.get("repo_name")
@ -296,6 +312,8 @@ def _thread_summary(
*,
latest_run_status: str | None = None,
latest_run_id: str | None = None,
owner_login: str | None = None,
owner_email: str | None = None,
) -> dict[str, Any]:
metadata = thread.get("metadata") if isinstance(thread.get("metadata"), dict) else {}
owner, name, full_name = _metadata_repo(metadata)
@ -343,6 +361,7 @@ def _thread_summary(
),
"createdAt": int(created_at) if isinstance(created_at, (int, float)) else _now_ms(),
"updatedAt": int(updated_at) if isinstance(updated_at, (int, float)) else _now_ms(),
"isOwner": (_user_owns_thread(metadata, owner_login, owner_email) if owner_login else True),
"traceUrl": trace_url,
}
if isinstance(pr_number, int) and isinstance(pr_url, str):
@ -662,6 +681,7 @@ async def get_dashboard_thread(
raise HTTPException(404, "thread not found") from exc
metadata = thread.get("metadata") if isinstance(thread.get("metadata"), dict) else {}
_assert_thread_readable(metadata)
is_owner = _user_owns_thread(metadata, login, email)
# The transcript is hydrated client-side by the SDK (`StreamProvider` reads
@ -691,6 +711,8 @@ async def get_dashboard_thread(
thread,
latest_run_status=latest_run_status,
latest_run_id=latest_run_id,
owner_login=login,
owner_email=email,
)
@ -1096,10 +1118,35 @@ async def _authorized_thread(
return thread
async def _readable_thread(
thread_id: str, *, login: str | None = None, email: str | None = None
) -> dict[str, Any]:
"""Fetch a thread and assert it is readable by the requesting user.
Read access is granted to any authenticated org member for surfaced-source
threads; ``login``/``email`` are accepted for API parity but not required.
"""
try:
thread = await langgraph_client().threads.get(thread_id)
except Exception as exc: # noqa: BLE001
raise HTTPException(404, "thread not found") from exc
metadata = thread.get("metadata") if isinstance(thread.get("metadata"), dict) else {}
_assert_thread_readable(metadata)
return thread
async def _readable_thread_metadata(
thread_id: str, *, login: str | None = None, email: str | None = None
) -> dict[str, Any]:
thread = await _readable_thread(thread_id, login=login, email=email)
metadata = thread.get("metadata") if isinstance(thread.get("metadata"), dict) else {}
return metadata
async def get_dashboard_thread_state(
thread_id: str, login: str, *, email: str | None = None
) -> dict[str, Any]:
thread = await _authorized_thread(thread_id, login, email=email)
thread = await _readable_thread(thread_id, login=login, email=email)
metadata = thread.get("metadata") if isinstance(thread.get("metadata"), dict) else {}
state = await langgraph_client().threads.get_state(thread_id)
result = state if isinstance(state, dict) else dict(state)
@ -1127,7 +1174,7 @@ async def _github_token_for_login(login: str) -> str:
async def get_dashboard_thread_pr_diff(
thread_id: str, login: str, *, email: str | None = None
) -> dict[str, Any]:
metadata = await _authorized_thread_metadata(thread_id, login, email=email)
metadata = await _readable_thread_metadata(thread_id, login=login, email=email)
pr_number = metadata.get("pr_number")
_, _, full_name = _metadata_repo(metadata)
if not isinstance(pr_number, int) or not full_name:
@ -1162,7 +1209,7 @@ async def proxy_dashboard_thread_stream_events(
# Preflight here (not in the generator) so auth/content-type failures
# surface as real HTTP errors before the SSE response starts streaming.
_require_json_content_type(content_type)
await _authorized_thread_metadata(thread_id, login, email=email)
await _readable_thread_metadata(thread_id, login=login, email=email)
return _stream_thread_events(thread_id, body, content_type)
@ -1281,7 +1328,7 @@ async def proxy_dashboard_thread_history(
content_type: str = "application/json",
) -> tuple[int, bytes, str | None]:
_require_json_content_type(content_type)
await _authorized_thread_metadata(thread_id, login, email=email)
await _readable_thread_metadata(thread_id, login=login, email=email)
url = f"{langgraph_url().rstrip('/')}/threads/{thread_id}/history"
headers = _langgraph_proxy_headers(content_type=content_type)
async with httpx.AsyncClient(timeout=_PROXY_REQUEST_TIMEOUT) as client:
@ -1331,9 +1378,11 @@ async def stream_dashboard_thread(
thread_id: str, login: str, *, email: str | None = None, last_event_id: str | None = None
) -> AsyncIterator[str]:
try:
await langgraph_client().threads.get(thread_id)
thread = await langgraph_client().threads.get(thread_id)
except Exception as exc: # noqa: BLE001
raise HTTPException(404, "thread not found") from exc
metadata = thread.get("metadata") if isinstance(thread.get("metadata"), dict) else {}
_assert_thread_readable(metadata)
stream = await langgraph_client().threads.join_stream(
thread_id,

View file

@ -432,6 +432,8 @@ async def test_proxy_commands_rejects_non_object_body(monkeypatch) -> None:
async def test_proxy_endpoints_enforce_thread_ownership(monkeypatch) -> None:
"""Write endpoints (commands, run_cancel) still require thread ownership."""
class FakeThreads:
async def get(self, thread_id: str) -> dict[str, object]:
assert thread_id == "tid"
@ -445,24 +447,86 @@ async def test_proxy_endpoints_enforce_thread_ownership(monkeypatch) -> None:
monkeypatch.setattr(thread_api, "langgraph_client", lambda: FakeClient())
with pytest.raises(HTTPException) as exc_info:
await thread_api.get_dashboard_thread_state("tid", "intruder")
assert exc_info.value.status_code == 404
with pytest.raises(HTTPException) as exc_info:
await thread_api.proxy_dashboard_thread_commands("tid", "intruder", b"{}")
assert exc_info.value.status_code == 404
with pytest.raises(HTTPException) as exc_info:
await thread_api.proxy_dashboard_thread_history("tid", "intruder", b"{}")
assert exc_info.value.status_code == 404
with pytest.raises(HTTPException) as exc_info:
await thread_api.proxy_dashboard_thread_run_cancel("tid", "run-1", "intruder")
assert exc_info.value.status_code == 404
async def test_read_endpoints_accessible_by_non_owner(monkeypatch) -> None:
"""Read endpoints (state, stream, history) are accessible by any org member."""
class FakeThreads:
async def get(self, thread_id: str) -> dict[str, object]:
assert thread_id == "tid"
return {
"thread_id": "tid",
"metadata": {"source": "slack", "github_login": "owner"},
}
async def get_state(self, thread_id: str) -> dict[str, object]:
return {"values": {"messages": []}}
class FakeClient:
threads = FakeThreads()
monkeypatch.setattr(thread_api, "langgraph_client", lambda: FakeClient())
# Read endpoints succeed for non-owners (org members).
state = await thread_api.get_dashboard_thread_state("tid", "teammate")
assert "values" in state
# stream/events preflight should not raise.
await thread_api.proxy_dashboard_thread_stream_events(
"tid", "teammate", b"{}", content_type="application/json"
)
# history preflight should not raise; mock the proxied HTTP call.
class FakeResponse:
status_code = 200
content = b"{}"
headers = {"content-type": "application/json"}
class FakeAsyncClient:
def __init__(self, *a: object, **kw: object) -> None:
pass
async def __aenter__(self) -> "FakeAsyncClient":
return self
async def __aexit__(self, *a: object) -> None:
pass
async def post(self, *a: object, **kw: object) -> FakeResponse:
return FakeResponse()
monkeypatch.setattr(thread_api.httpx, "AsyncClient", FakeAsyncClient)
await thread_api.proxy_dashboard_thread_history("tid", "teammate", b"{}")
async def test_read_endpoints_reject_non_surfaced_source(monkeypatch) -> None:
"""Threads with an unknown source are not readable by anyone."""
class FakeThreads:
async def get(self, thread_id: str) -> dict[str, object]:
return {
"thread_id": "tid",
"metadata": {"source": "unknown-source", "github_login": "owner"},
}
async def get_state(self, thread_id: str) -> dict[str, object]:
return {"values": {"messages": []}}
class FakeClient:
threads = FakeThreads()
monkeypatch.setattr(thread_api, "langgraph_client", lambda: FakeClient())
with pytest.raises(HTTPException) as exc_info:
await thread_api.proxy_dashboard_thread_stream_events("tid", "intruder", b"{}")
await thread_api.get_dashboard_thread_state("tid", "owner")
assert exc_info.value.status_code == 404
@ -521,6 +585,49 @@ def test_thread_summary_defaults_to_not_resolved() -> None:
assert summary["resolvedAt"] is None
def test_thread_summary_is_owner_true_for_matching_login() -> None:
summary = thread_api._thread_summary(
{"thread_id": "tid", "metadata": {"source": "slack", "github_login": "octocat"}},
owner_login="octocat",
)
assert summary["isOwner"] is True
def test_thread_summary_is_owner_false_for_non_owner() -> None:
summary = thread_api._thread_summary(
{"thread_id": "tid", "metadata": {"source": "slack", "github_login": "octocat"}},
owner_login="teammate",
)
assert summary["isOwner"] is False
def test_thread_summary_is_owner_true_for_matching_email() -> None:
summary = thread_api._thread_summary(
{
"thread_id": "tid",
"metadata": {
"source": "slack",
"github_login": "octocat",
"triggering_user_email": "octo@example.com",
},
},
owner_login="someone-else",
owner_email="OCTO@example.com",
)
assert summary["isOwner"] is True
def test_thread_summary_is_owner_defaults_true_without_owner_login() -> None:
summary = thread_api._thread_summary(
{"thread_id": "tid", "metadata": {"source": "slack", "github_login": "octocat"}},
)
assert summary["isOwner"] is True
async def test_resolve_dashboard_thread_marks_resolved(monkeypatch) -> None:
updates: list[dict[str, object]] = []

View file

@ -24,6 +24,7 @@ export function AgentThreadView({ thread }: AgentThreadViewProps) {
const sendMessage = useSubmitAgentMessage(thread.id)
const stream = useAgentThreadStream()
const isMobile = useIsMobile()
const isReadOnly = thread.isOwner === false
const { models, defaultSelection } = useModelOptions()
const threadSelection = useMemo<ModelSelection | null>(() => {
@ -77,10 +78,42 @@ export function AgentThreadView({ thread }: AgentThreadViewProps) {
settingUpSandbox={settingUpSandbox}
contentWidthClass="max-w-3xl"
/>
<div className="shrink-0 px-4 pb-4">
<div className="mx-auto w-full min-w-0 max-w-3xl">
{!isReadOnly && (
<div className="shrink-0 px-4 pb-4">
<div className="mx-auto w-full min-w-0 max-w-3xl">
<AgentPromptBar
placeholder="Add a follow up"
compact
busy={isStreaming}
onSubmit={(content, images) =>
sendMessage.mutateAsync({
content,
images,
model_id: activeSelection?.modelId ?? null,
effort: activeSelection?.effort ?? null,
})
}
models={models}
selection={activeSelection}
onSelectionChange={setSelection}
/>
</div>
</div>
)}
</div>
) : isHydrating ? (
<div className="flex flex-1 flex-col items-center justify-center gap-4 px-6">
<p className="text-xs text-[var(--ui-text-dim)]">Loading conversation…</p>
</div>
) : (
<div className="flex flex-1 flex-col items-center justify-center gap-4 px-6">
<p className="text-xs text-[var(--ui-text-dim)]">
This thread has no messages yet.
</p>
{!isReadOnly && (
<div className="w-full max-w-3xl">
<AgentPromptBar
placeholder="Add a follow up"
placeholder="Send the first message"
compact
busy={isStreaming}
onSubmit={(content, images) =>
@ -96,35 +129,7 @@ export function AgentThreadView({ thread }: AgentThreadViewProps) {
onSelectionChange={setSelection}
/>
</div>
</div>
</div>
) : isHydrating ? (
<div className="flex flex-1 flex-col items-center justify-center gap-4 px-6">
<p className="text-xs text-[var(--ui-text-dim)]">Loading conversation…</p>
</div>
) : (
<div className="flex flex-1 flex-col items-center justify-center gap-4 px-6">
<p className="text-xs text-[var(--ui-text-dim)]">
This thread has no messages yet.
</p>
<div className="w-full max-w-3xl">
<AgentPromptBar
placeholder="Send the first message"
compact
busy={isStreaming}
onSubmit={(content, images) =>
sendMessage.mutateAsync({
content,
images,
model_id: activeSelection?.modelId ?? null,
effort: activeSelection?.effort ?? null,
})
}
models={models}
selection={activeSelection}
onSelectionChange={setSelection}
/>
</div>
)}
</div>
)}
</div>

View file

@ -315,6 +315,7 @@ function ThreadRow({
const deleteThread = useDeleteAgentThread()
const resolveThread = useResolveAgentThread()
const [deleteOpen, setDeleteOpen] = useState(false)
const isReadOnly = thread.isOwner === false
const badge =
thread.diffStats && thread.diffStats.additions > 0
? `+${thread.diffStats.additions}`
@ -422,29 +423,33 @@ function ThreadRow({
{badge}
</span>
)}
<button
type="button"
aria-label={isResolved ? "Unresolve thread" : "Resolve thread"}
title={isResolved ? "Unresolve thread" : "Resolve thread"}
onClick={onToggleResolved}
disabled={resolveThread.isPending}
className="hidden size-4 shrink-0 items-center justify-center rounded text-[var(--ui-text-dim)] group-hover:flex hover:bg-[var(--ui-panel-2)] hover:text-[var(--ui-text)]"
>
{isResolved ? (
<ArrowCounterClockwiseIcon className="size-3" weight="bold" />
) : (
<CheckCircleIcon className="size-3" weight="bold" />
)}
</button>
<button
type="button"
aria-label="Delete thread"
onClick={onDelete}
disabled={isDeleting}
className="hidden size-4 shrink-0 items-center justify-center rounded text-[var(--ui-text-dim)] group-hover:flex hover:bg-[var(--ui-panel-2)] hover:text-[var(--ui-text)]"
>
<XIcon className="size-3" weight="bold" />
</button>
{!isReadOnly && (
<button
type="button"
aria-label={isResolved ? "Unresolve thread" : "Resolve thread"}
title={isResolved ? "Unresolve thread" : "Resolve thread"}
onClick={onToggleResolved}
disabled={resolveThread.isPending}
className="hidden size-4 shrink-0 items-center justify-center rounded text-[var(--ui-text-dim)] group-hover:flex hover:bg-[var(--ui-panel-2)] hover:text-[var(--ui-text)]"
>
{isResolved ? (
<ArrowCounterClockwiseIcon className="size-3" weight="bold" />
) : (
<CheckCircleIcon className="size-3" weight="bold" />
)}
</button>
)}
{!isReadOnly && (
<button
type="button"
aria-label="Delete thread"
onClick={onDelete}
disabled={isDeleting}
className="hidden size-4 shrink-0 items-center justify-center rounded text-[var(--ui-text-dim)] group-hover:flex hover:bg-[var(--ui-panel-2)] hover:text-[var(--ui-text)]"
>
<XIcon className="size-3" weight="bold" />
</button>
)}
</ContextMenu.Trigger>
<ContextMenu.Portal>
<ContextMenu.Positioner className="z-50 outline-none">
@ -457,26 +462,30 @@ function ThreadRow({
<TreeStructureIcon className="size-3.5" />
Open trace
</ContextMenu.Item>
<ContextMenu.Item
onClick={() => onToggleResolved()}
disabled={resolveThread.isPending}
className="flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-xs outline-none select-none data-highlighted:bg-[var(--ui-sidebar-hover)] data-disabled:pointer-events-none data-disabled:opacity-50"
>
{isResolved ? (
<ArrowCounterClockwiseIcon className="size-3.5" />
) : (
<CheckCircleIcon className="size-3.5" />
)}
{isResolved ? "Unresolve thread" : "Resolve thread"}
</ContextMenu.Item>
<ContextMenu.Item
onClick={onDelete}
disabled={isDeleting}
className="flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-xs text-[var(--ui-danger)] outline-none select-none data-highlighted:bg-[var(--ui-sidebar-hover)] data-disabled:pointer-events-none data-disabled:opacity-50"
>
<TrashIcon className="size-3.5" />
Delete thread
</ContextMenu.Item>
{!isReadOnly && (
<ContextMenu.Item
onClick={() => onToggleResolved()}
disabled={resolveThread.isPending}
className="flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-xs outline-none select-none data-highlighted:bg-[var(--ui-sidebar-hover)] data-disabled:pointer-events-none data-disabled:opacity-50"
>
{isResolved ? (
<ArrowCounterClockwiseIcon className="size-3.5" />
) : (
<CheckCircleIcon className="size-3.5" />
)}
{isResolved ? "Unresolve thread" : "Resolve thread"}
</ContextMenu.Item>
)}
{!isReadOnly && (
<ContextMenu.Item
onClick={onDelete}
disabled={isDeleting}
className="flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-xs text-[var(--ui-danger)] outline-none select-none data-highlighted:bg-[var(--ui-sidebar-hover)] data-disabled:pointer-events-none data-disabled:opacity-50"
>
<TrashIcon className="size-3.5" />
Delete thread
</ContextMenu.Item>
)}
</ContextMenu.Popup>
</ContextMenu.Positioner>
</ContextMenu.Portal>

View file

@ -205,6 +205,7 @@ export function optimisticThread(
status: "running",
viewed: true,
viewedAt: now,
isOwner: true,
createdAt: now,
updatedAt: now,
traceUrl: null,

View file

@ -186,6 +186,7 @@ export interface AgentThread {
viewedAt?: number | null
resolved?: boolean
resolvedAt?: number | null
isOwner?: boolean
createdAt: number
updatedAt: number
traceUrl?: string | null