Skip to content

Commit c2fbfe4

Browse files
committed
feat(tasks): rebind or log out sandbox GitHub identity on actor transitions
Follow-ups are gated per-actor for MCP; do the same for GitHub. On an actor transition, re-inject the new actor's GitHub token if they have usable access, otherwise log the sandbox out (strip the token from the git remote and env) so the previous actor's GitHub identity can't be used by a follow-up actor who lacks it. Reauthorization for that actor is surfaced by the existing credential-refresh path, unchanged. Fail closed only when the sandbox can be neither rebound nor cleared. Only USER-authored runs carry per-actor identity; BOT runs share one installation token.
1 parent 02268db commit c2fbfe4

4 files changed

Lines changed: 253 additions & 10 deletions

File tree

products/tasks/backend/temporal/process_task/activities/send_followup_to_sandbox.py

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from temporalio import activity
1010
from temporalio.exceptions import ApplicationError
1111

12+
from posthog.models.user_integration import ReauthorizationRequired
1213
from posthog.temporal.common.utils import close_db_connections
1314
from posthog.temporal.oauth import PosthogMcpScopes
1415

@@ -26,14 +27,23 @@
2627
from products.tasks.backend.models import TaskRun
2728
from products.tasks.backend.redis import get_tasks_stream_redis_sync, run_uses_dedicated_stream
2829
from products.tasks.backend.temporal.oauth import create_oauth_access_token_for_run
30+
from products.tasks.backend.temporal.process_task.sandbox_credentials import (
31+
apply_github_credentials_to_sandbox,
32+
clear_github_credentials_from_sandbox,
33+
)
2934
from products.tasks.backend.temporal.process_task.utils import (
35+
PrAuthorshipMode,
3036
get_actor_distinct_id,
3137
get_imported_mcp_server_configs,
38+
get_pr_authorship_mode,
39+
get_sandbox_github_identity_user,
40+
get_sandbox_github_token,
3241
get_sandbox_mcp_session_user,
3342
get_sandbox_ph_mcp_configs,
3443
get_task_run_credential_user,
3544
get_user_mcp_server_configs,
3645
is_slack_interaction_state,
46+
mark_sandbox_github_identity,
3747
mark_sandbox_mcp_session,
3848
record_message_actor,
3949
sandbox_identity_scope,
@@ -166,6 +176,13 @@ def _deliver_followup(input: SendFollowupToSandboxInput) -> None:
166176
if not _refresh_sandbox_mcp(task_run, input.posthog_mcp_scopes, auth_token, actor_user=actor_user, state=state):
167177
error_msg = "Could not rebind sandbox MCP credentials for the follow-up actor"
168178
raise RuntimeError(f"send_followup failed: {error_msg}")
179+
180+
# Bind the sandbox's GitHub credentials to this actor: rebind if they have
181+
# access, otherwise log out so the previous actor's identity can't be used.
182+
# Fail closed only if we can't even clear the prior credentials.
183+
if not _refresh_sandbox_github(task_run, actor_user, state):
184+
error_msg = "Could not rebind or clear sandbox GitHub credentials for the follow-up actor"
185+
raise RuntimeError(f"send_followup failed: {error_msg}")
169186
artifacts = None
170187
artifact_ids = input.artifact_ids or []
171188
if artifact_ids:
@@ -378,6 +395,96 @@ def _refresh_sandbox_mcp(
378395
return False # rebind never confirmed → fail closed (unknown binding may hide a live session)
379396

380397

398+
def _resolve_live_sandbox(state: dict[str, Any] | None) -> Any:
399+
"""The running Sandbox handle for a run's state, or None when unavailable.
400+
401+
GitHub credentials are written into the sandbox directly (git remote + env
402+
file), so the gate needs the handle. Absent/dead sandbox → None; the
403+
periodic credential-refresh loop reconciles identity in that case.
404+
"""
405+
sandbox_id = (state or {}).get("sandbox_id")
406+
if not sandbox_id:
407+
return None
408+
from products.tasks.backend.logic.services.sandbox import (
409+
Sandbox, # noqa: PLC0415 — keep the sandbox service off the import path
410+
)
411+
412+
try:
413+
sandbox = Sandbox.get_by_id(sandbox_id)
414+
return sandbox if sandbox.is_running() else None
415+
except Exception:
416+
return None
417+
418+
419+
def _refresh_sandbox_github(task_run: TaskRun, actor_user: Any, state: dict[str, Any] | None) -> bool:
420+
"""Bind the sandbox's in-place GitHub credentials to this message's actor.
421+
422+
On an actor transition: re-inject the new actor's token if they have usable
423+
access, otherwise log the sandbox out (strip the token from the git remote
424+
and env) so the previous actor's GitHub identity can never be used by a
425+
follow-up actor who lacks access. Reauthorization for that actor is surfaced
426+
by the existing credential-refresh path, unchanged.
427+
428+
Only USER-authored runs carry per-actor identity — BOT runs share one
429+
installation token, so every actor is already the same identity. This
430+
enforces the transition boundary; the periodic credential-refresh loop
431+
keeps a continuous actor's token rotated between transitions.
432+
433+
Returns ``True`` when the sandbox safely reflects this actor (rebound, logged
434+
out, or nothing to do) and ``False`` only when we could neither rebind nor
435+
even clear — the previous actor's credentials may still be live, so the
436+
caller fails the follow-up closed.
437+
"""
438+
if actor_user is None:
439+
return True
440+
441+
run_id = str(task_run.id)
442+
scope = sandbox_identity_scope(run_id, state)
443+
if get_sandbox_github_identity_user(scope) == actor_user.id:
444+
return True # sandbox already reflects this actor — cheapest check first
445+
446+
task = task_run.task
447+
if get_pr_authorship_mode(task, state) != PrAuthorshipMode.USER:
448+
return True
449+
450+
sandbox = _resolve_live_sandbox(state)
451+
if sandbox is None:
452+
return True # no live handle; the periodic refresh loop reconciles identity
453+
454+
repository = task.repository
455+
token: str | None = None
456+
try:
457+
token = get_sandbox_github_token(
458+
task.github_integration_id,
459+
run_id=run_id,
460+
state=state,
461+
task=task,
462+
actor_user=actor_user,
463+
repository=repository,
464+
)
465+
except ReauthorizationRequired:
466+
token = None # new actor lacks usable access → log out (reauth surfaced elsewhere)
467+
468+
if token:
469+
try:
470+
apply_github_credentials_to_sandbox(sandbox, repository, token)
471+
except Exception:
472+
logger.warning("refresh_github_apply_failed", run_id=run_id, exc_info=True)
473+
else:
474+
mark_sandbox_github_identity(scope, actor_user.id)
475+
logger.info("refresh_github_rebound", run_id=run_id, user_id=actor_user.id)
476+
return True
477+
478+
# No usable rebind: log the sandbox out. Fail closed only if even the clear
479+
# can't be confirmed — the previous actor's credentials might still be live.
480+
if clear_github_credentials_from_sandbox(sandbox, repository):
481+
mark_sandbox_github_identity(scope, actor_user.id)
482+
logger.info("refresh_github_logged_out", run_id=run_id, user_id=actor_user.id)
483+
return True
484+
logger.warning("refresh_github_logout_failed", run_id=run_id, user_id=actor_user.id)
485+
return False
486+
487+
381488
def _get_stop_reason(result_data: dict[str, Any] | None) -> str:
382489
if not isinstance(result_data, dict):
383490
return STOP_REASON_END_TURN

products/tasks/backend/temporal/process_task/sandbox_credentials.py

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,27 +51,29 @@ def github_refresh_interval_seconds(token: str) -> float:
5151
return DEFAULT_REFRESH_INTERVAL_SECONDS
5252

5353

54-
def set_git_remote_token(sandbox: "SandboxBase", repository: str, github_token: str) -> bool:
55-
"""Rewrite ``origin``'s remote URL with a fresh ``x-access-token``; git re-reads it on every op. No-ops pre-clone."""
54+
def _set_origin_remote(sandbox: "SandboxBase", repository: str, remote_url: str, failure_msg: str) -> bool:
55+
"""Point ``origin`` at ``remote_url`` inside the cloned repo; git re-reads it on every op. No-ops pre-clone."""
5656
org, repo = repository.lower().split("/")
5757
repo_path = f"/tmp/workspace/repos/{org}/{repo}"
58-
update_remote = (
58+
command = (
5959
f"if [ -d {shlex.quote(repo_path + '/.git')} ]; then "
6060
f"cd {shlex.quote(repo_path)} && "
61-
f"git remote set-url origin "
62-
f"https://x-access-token:{shlex.quote(github_token)}@github.com/{shlex.quote(repository)}.git; "
61+
f"git remote set-url origin {remote_url}; "
6362
f"fi"
6463
)
65-
result = sandbox.execute(update_remote, timeout_seconds=30)
64+
result = sandbox.execute(command, timeout_seconds=30)
6665
if result.exit_code != 0:
67-
logger.warning(
68-
"Failed to refresh git remote URL",
69-
extra={"sandbox_id": sandbox.id, "repository": repository, "stderr": result.stderr},
70-
)
66+
logger.warning(failure_msg, extra={"sandbox_id": sandbox.id, "repository": repository, "stderr": result.stderr})
7167
return False
7268
return True
7369

7470

71+
def set_git_remote_token(sandbox: "SandboxBase", repository: str, github_token: str) -> bool:
72+
"""Rewrite ``origin``'s remote URL with a fresh ``x-access-token``; git re-reads it on every op. No-ops pre-clone."""
73+
remote_url = f"https://x-access-token:{shlex.quote(github_token)}@github.com/{shlex.quote(repository)}.git"
74+
return _set_origin_remote(sandbox, repository, remote_url, "Failed to refresh git remote URL")
75+
76+
7577
def update_sandbox_env_file(sandbox: "SandboxBase", updates: dict[str, str]) -> bool:
7678
"""Replace specific keys in the NUL-delimited agentsh env file, preserving all other entries.
7779
@@ -124,6 +126,21 @@ def apply_github_credentials_to_sandbox(sandbox: "SandboxBase", repository: str
124126
update_sandbox_env_file(sandbox, dict.fromkeys(GITHUB_ENV_KEYS, github_token))
125127

126128

129+
def clear_git_remote_token(sandbox: "SandboxBase", repository: str) -> bool:
130+
"""Rewrite ``origin`` to a token-less URL so git can no longer authenticate. No-ops pre-clone."""
131+
remote_url = f"https://github.com/{shlex.quote(repository)}.git"
132+
return _set_origin_remote(sandbox, repository, remote_url, "Failed to clear git remote URL")
133+
134+
135+
def clear_github_credentials_from_sandbox(sandbox: "SandboxBase", repository: str | None) -> bool:
136+
"""Log the sandbox out of GitHub: strip the token from the git remote and blank the env
137+
keys, so the previous actor's identity can't be used by a follow-up actor who lacks access.
138+
Returns ``True`` only when both places were cleared."""
139+
remote_cleared = clear_git_remote_token(sandbox, repository) if repository else True
140+
env_cleared = update_sandbox_env_file(sandbox, dict.fromkeys(GITHUB_ENV_KEYS, ""))
141+
return remote_cleared and env_cleared
142+
143+
127144
USER_TOKEN_REFRESH_INTERVAL_SECONDS: float = _GITHUB_REFRESH_INTERVAL_BY_PREFIX["ghu_"]
128145
# TTL covers a slow mint + propagation; wait stays under the refresh activity's 2 min timeout.
129146
_ROTATION_LOCK_TTL_SECONDS = 120

products/tasks/backend/temporal/process_task/tests/test_send_followup_to_sandbox.py

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,24 @@
55

66
from temporalio.exceptions import ApplicationError
77

8+
from posthog.models.user_integration import ReauthorizationRequired
9+
810
from products.tasks.backend.logic.services.agent_command import CommandResult
911
from products.tasks.backend.temporal.process_task.activities.send_followup_to_sandbox import (
1012
REFRESH_RETRY_DELAY_SECONDS,
1113
SEND_FOLLOWUP_MAX_ATTEMPTS,
1214
SendFollowupToSandboxInput,
15+
_refresh_sandbox_github,
1316
_refresh_sandbox_mcp,
1417
send_followup_to_sandbox,
1518
)
1619
from products.tasks.backend.temporal.process_task.utils import (
1720
McpServerConfig,
21+
PrAuthorshipMode,
1822
_sandbox_mcp_session_cache_key,
23+
get_sandbox_github_identity_user,
1924
get_sandbox_mcp_session_user,
25+
mark_sandbox_github_identity,
2026
mark_sandbox_mcp_session,
2127
)
2228

@@ -331,6 +337,95 @@ def test_transition_with_no_configs_leaves_previous_binding(
331337
assert get_sandbox_mcp_session_user("run-1") == 99 # binding unchanged
332338

333339

340+
_GH_MODULE = "products.tasks.backend.temporal.process_task.activities.send_followup_to_sandbox"
341+
342+
343+
@patch(f"{_GH_MODULE}.clear_github_credentials_from_sandbox")
344+
@patch(f"{_GH_MODULE}.apply_github_credentials_to_sandbox")
345+
@patch(f"{_GH_MODULE}.get_sandbox_github_token")
346+
@patch(f"{_GH_MODULE}._resolve_live_sandbox")
347+
@patch(f"{_GH_MODULE}.get_pr_authorship_mode")
348+
class TestSandboxGithubIdentityGate:
349+
"""On an actor transition the sandbox's GitHub credentials rebind to the new
350+
actor when they have access, otherwise the sandbox is logged out so the
351+
previous actor's identity can't be used."""
352+
353+
def test_same_actor_skips(self, mock_authorship, mock_resolve, mock_get_token, mock_apply, mock_clear):
354+
mock_authorship.return_value = PrAuthorshipMode.USER
355+
mark_sandbox_github_identity("run-1", 42)
356+
357+
assert _refresh_sandbox_github(_make_task_run_mock(), MagicMock(id=42), None) is True
358+
mock_resolve.assert_not_called()
359+
mock_get_token.assert_not_called()
360+
mock_apply.assert_not_called()
361+
mock_clear.assert_not_called()
362+
363+
def test_bot_authorship_skips(self, mock_authorship, mock_resolve, mock_get_token, mock_apply, mock_clear):
364+
# BOT runs share a single installation token, so every actor is already
365+
# the same GitHub identity — nothing to rebind.
366+
mock_authorship.return_value = PrAuthorshipMode.BOT
367+
mark_sandbox_github_identity("run-1", 99)
368+
369+
assert _refresh_sandbox_github(_make_task_run_mock(), MagicMock(id=42), None) is True
370+
mock_get_token.assert_not_called()
371+
mock_apply.assert_not_called()
372+
mock_clear.assert_not_called()
373+
374+
def test_transition_with_access_rebinds(
375+
self, mock_authorship, mock_resolve, mock_get_token, mock_apply, mock_clear
376+
):
377+
mock_authorship.return_value = PrAuthorshipMode.USER
378+
mock_resolve.return_value = MagicMock()
379+
mock_get_token.return_value = "ghu_newtoken"
380+
mark_sandbox_github_identity("run-1", 99)
381+
382+
assert _refresh_sandbox_github(_make_task_run_mock(), MagicMock(id=42), None) is True
383+
mock_apply.assert_called_once()
384+
assert mock_apply.call_args.args[2] == "ghu_newtoken"
385+
mock_clear.assert_not_called()
386+
assert get_sandbox_github_identity_user("run-1") == 42
387+
388+
def test_transition_without_access_logs_out(
389+
self, mock_authorship, mock_resolve, mock_get_token, mock_apply, mock_clear
390+
):
391+
mock_authorship.return_value = PrAuthorshipMode.USER
392+
mock_resolve.return_value = MagicMock()
393+
mock_get_token.side_effect = ReauthorizationRequired("no repo access")
394+
mock_clear.return_value = True
395+
mark_sandbox_github_identity("run-1", 99)
396+
397+
assert _refresh_sandbox_github(_make_task_run_mock(), MagicMock(id=42), None) is True
398+
mock_apply.assert_not_called()
399+
mock_clear.assert_called_once()
400+
assert get_sandbox_github_identity_user("run-1") == 42
401+
402+
def test_apply_failure_falls_back_to_logout(
403+
self, mock_authorship, mock_resolve, mock_get_token, mock_apply, mock_clear
404+
):
405+
mock_authorship.return_value = PrAuthorshipMode.USER
406+
mock_resolve.return_value = MagicMock()
407+
mock_get_token.return_value = "ghu_newtoken"
408+
mock_apply.side_effect = RuntimeError("write failed")
409+
mock_clear.return_value = True
410+
mark_sandbox_github_identity("run-1", 99)
411+
412+
assert _refresh_sandbox_github(_make_task_run_mock(), MagicMock(id=42), None) is True
413+
mock_apply.assert_called_once()
414+
mock_clear.assert_called_once() # fell through to logout so no stale creds remain
415+
416+
def test_logout_failure_fails_closed(self, mock_authorship, mock_resolve, mock_get_token, mock_apply, mock_clear):
417+
# New actor has no access and the sandbox can't even be cleared — the
418+
# previous actor's creds may still be live, so fail closed.
419+
mock_authorship.return_value = PrAuthorshipMode.USER
420+
mock_resolve.return_value = MagicMock()
421+
mock_get_token.side_effect = ReauthorizationRequired("no repo access")
422+
mock_clear.return_value = False
423+
mark_sandbox_github_identity("run-1", 99)
424+
425+
assert _refresh_sandbox_github(_make_task_run_mock(), MagicMock(id=42), None) is False
426+
assert get_sandbox_github_identity_user("run-1") == 99 # binding unchanged
427+
428+
334429
class TestSendFollowupActivityRefreshOrdering:
335430
"""Refresh call must precede user_message, and the activity must succeed
336431
when refresh fails (non-fatal) as long as user_message succeeds."""

products/tasks/backend/temporal/process_task/utils.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,30 @@ def get_sandbox_mcp_session_user(scope: str) -> int | None:
390390
return get_tasks_cache().get(_sandbox_mcp_session_cache_key(scope))
391391

392392

393+
def _sandbox_github_identity_cache_key(scope: str) -> str:
394+
return f"tasks:sandbox-github-identity:{scope}"
395+
396+
397+
def mark_sandbox_github_identity(scope: str, user_id: int) -> None:
398+
"""Record which actor the sandbox's in-place GitHub credentials reflect.
399+
400+
The value is the actor whose token was applied, or who was logged out (no
401+
usable access) — either way the sandbox no longer carries a *different*
402+
actor's identity. Self-expires after MCP_TOKEN_REFRESH_INTERVAL_SECONDS; an
403+
absent entry reads as "must re-establish", which is always safe because
404+
re-establishing re-applies or clears rather than trusting stale creds.
405+
"""
406+
get_tasks_cache().set(
407+
_sandbox_github_identity_cache_key(scope), user_id, timeout=MCP_TOKEN_REFRESH_INTERVAL_SECONDS
408+
)
409+
410+
411+
def get_sandbox_github_identity_user(scope: str) -> int | None:
412+
"""Actor id the sandbox's GitHub credentials were last bound to (or logged
413+
out for) within the freshness window, or None when unknown."""
414+
return get_tasks_cache().get(_sandbox_github_identity_cache_key(scope))
415+
416+
393417
@dataclass(frozen=True)
394418
class McpServerConfig:
395419
"""Configuration for a remote MCP server matching the ACP McpServer schema.

0 commit comments

Comments
 (0)