mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-07 16:19:09 +00:00
Adam Moussa
e4bee74c53
A PR head-branch name is attacker-controllable and get_thread_id_from_branch derives a thread_id from its first UUID with no repo binding (TID-COLLIDE-01). The in-memory sandbox cache and the per-thread GitHub-token cache were keyed on thread_id alone, and a cached sandbox was reused after only an echo-ping, so a different repo's webhook could bind to another thread's sandbox or token. Without changing the persistent thread-id scheme: - Persist the bound repo (owner/name) in thread metadata on sandbox creation and refuse to reuse a sandbox whose bound repo does not match the current event (SandboxRepoMismatchError); the in-memory proxy also carries the binding. - Bind the GitHub-token cache entries to their repo and evict on a cross-repo read so a colliding thread_id cannot be served another repo's token. - Thread repo through the reviewer and the webhook token resolvers.
124 lines
4.6 KiB
Python
124 lines
4.6 KiB
Python
"""Repo-binding isolation for the sandbox and GitHub-token caches.
|
|
|
|
Covers TID-COLLIDE-01: a second repo presenting a colliding thread_id must not
|
|
reuse the first repo's in-process sandbox or cached GitHub token.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from datetime import UTC, datetime, timedelta
|
|
from typing import Any
|
|
|
|
import pytest
|
|
|
|
from agent import server
|
|
from agent.utils import github_token
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _clear_caches() -> None:
|
|
github_token._GITHUB_TOKEN_CACHE.clear()
|
|
server.SANDBOX_BACKENDS.clear()
|
|
|
|
|
|
# --- token cache repo binding ------------------------------------------------
|
|
|
|
|
|
def test_cached_token_not_reused_across_repos() -> None:
|
|
"""A token bound to repo A is refused for a colliding thread_id from repo B."""
|
|
future = (datetime.now(UTC) + timedelta(hours=1)).isoformat()
|
|
github_token.cache_github_token_for_thread(
|
|
"tid", "ghp_repoA", expires_at=future, repo={"owner": "acme", "name": "alpha"}
|
|
)
|
|
|
|
# Same thread_id, different repo → must NOT serve repo A's token.
|
|
cfg_b = {"configurable": {"thread_id": "tid", "repo": {"owner": "evil", "name": "beta"}}}
|
|
assert github_token.get_github_token(cfg_b) is None
|
|
# And the poisoned entry is evicted.
|
|
assert "tid" not in github_token._GITHUB_TOKEN_CACHE
|
|
|
|
|
|
def test_cached_token_reused_for_same_repo() -> None:
|
|
future = (datetime.now(UTC) + timedelta(hours=1)).isoformat()
|
|
github_token.cache_github_token_for_thread(
|
|
"tid", "ghp_repoA", expires_at=future, repo={"owner": "acme", "name": "alpha"}
|
|
)
|
|
cfg_a = {"configurable": {"thread_id": "tid", "repo": {"owner": "acme", "name": "alpha"}}}
|
|
assert github_token.get_github_token(cfg_a) == "ghp_repoA"
|
|
|
|
|
|
def test_unbound_token_served_when_repo_unknown() -> None:
|
|
"""Legacy entries with no bound repo still resolve when no repo is supplied."""
|
|
github_token.cache_github_token_for_thread("tid", "ghp_legacy")
|
|
assert github_token.get_github_token({"configurable": {"thread_id": "tid"}}) == "ghp_legacy"
|
|
|
|
|
|
def test_repo_cache_key_normalizes() -> None:
|
|
assert github_token.repo_cache_key({"owner": "o", "name": "r"}) == "o/r"
|
|
assert github_token.repo_cache_key("o/r") == "o/r"
|
|
assert github_token.repo_cache_key({"owner": "o"}) is None
|
|
assert github_token.repo_cache_key(None) is None
|
|
|
|
|
|
# --- sandbox repo binding ----------------------------------------------------
|
|
|
|
|
|
class _FakeBackend:
|
|
def __init__(self, sandbox_id: str) -> None:
|
|
self.id = sandbox_id
|
|
self.bound_repo: str | None = None
|
|
|
|
def execute(self, *_a: Any, **_k: Any) -> Any: # pragma: no cover - must not run
|
|
raise AssertionError("colliding-repo sandbox must not be pinged/reused")
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_ensure_sandbox_refuses_colliding_repo(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
async def fake_sandbox_id(_tid: str) -> str:
|
|
return "sb-A"
|
|
|
|
async def fake_bound_repo(_tid: str) -> str:
|
|
return "acme/alpha"
|
|
|
|
monkeypatch.setattr(server, "get_sandbox_id_from_metadata", fake_sandbox_id)
|
|
monkeypatch.setattr(server, "get_bound_repo_from_metadata", fake_bound_repo)
|
|
backend = _FakeBackend("sb-A")
|
|
backend.bound_repo = "acme/alpha"
|
|
server.SANDBOX_BACKENDS["tid"] = backend # type: ignore[assignment]
|
|
|
|
with pytest.raises(server.SandboxRepoMismatchError):
|
|
await server.ensure_sandbox_for_thread("tid", repo={"owner": "evil", "name": "beta"})
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_ensure_sandbox_allows_matching_repo(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
async def fake_sandbox_id(_tid: str) -> str:
|
|
return "sb-A"
|
|
|
|
async def fake_bound_repo(_tid: str) -> str:
|
|
return "acme/alpha"
|
|
|
|
calls: dict[str, int] = {"git": 0}
|
|
backend = _FakeBackend("sb-A")
|
|
backend.bound_repo = "acme/alpha"
|
|
|
|
async def fake_check(b: Any, *_a: Any, **_k: Any) -> Any:
|
|
return b
|
|
|
|
async def fake_refresh(b: Any, *_a: Any, **_k: Any) -> Any:
|
|
return b
|
|
|
|
async def fake_git(_b: Any) -> None:
|
|
calls["git"] += 1
|
|
|
|
monkeypatch.setattr(server, "get_sandbox_id_from_metadata", fake_sandbox_id)
|
|
monkeypatch.setattr(server, "get_bound_repo_from_metadata", fake_bound_repo)
|
|
monkeypatch.setattr(server, "check_or_recreate_sandbox", fake_check)
|
|
monkeypatch.setattr(server, "_refresh_github_proxy_or_recreate", fake_refresh)
|
|
monkeypatch.setattr(server, "set_sandbox_backend", lambda _tid, b, **_k: b)
|
|
monkeypatch.setattr(server, "_configure_git_identity", fake_git)
|
|
server.SANDBOX_BACKENDS["tid"] = backend # type: ignore[assignment]
|
|
|
|
result = await server.ensure_sandbox_for_thread("tid", repo={"owner": "acme", "name": "alpha"})
|
|
assert result is backend
|
|
assert calls["git"] == 1
|