Skip to content

Commit 4a7fbb2

Browse files
committed
fix(tasks): stop the sandbox retaining a prior actor's GitHub token
Two runtime gaps let a follow-up actor keep the previous actor's GitHub identity after a per-message logout: - The agent-server was launched with GITHUB_TOKEN/GH_TOKEN in its process env, frozen for the process lifetime. Clearing the live /tmp/agent-env file could not revoke that copy, so in-process tools resurrected it. Add both vars to SANDBOX_AGENT_LAUNCH_UNSET_ENV_VARS; the token is still delivered per command via the file (re-sourced by BASH_ENV, seeded before the unset). - The periodic user-token refresh loop re-applied the run owner's token to every live sandbox keyed on the owner's integration, overwriting a transition. Skip runs whose sandbox-github-identity marker is bound to a different actor. Pairs with the agent-server fix (PostHog/code) that treats an emptied env file as an explicit logout instead of falling back to the process env.
1 parent 88fab4e commit 4a7fbb2

3 files changed

Lines changed: 67 additions & 1 deletion

File tree

products/tasks/backend/constants.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,14 @@ def vm_sandbox_allowed_origins(*, distinct_id: str, organization_id: str) -> set
290290
}
291291
)
292292

293+
# Stripped from the agent-server's process environment at launch (env -u).
294+
# Two categories:
295+
# - code-injection vectors a resume snapshot could smuggle in (NODE_*, LD_*, DYLD_*);
296+
# - the GitHub token, so the agent-server holds no frozen copy of the acting user's
297+
# credentials. The token is delivered per command via the live /tmp/agent-env file
298+
# (re-sourced by BASH_ENV, seeded before this unset), so git/gh still authenticate;
299+
# removing the static process-env copy is what lets a mid-session logout or rebind
300+
# actually take effect instead of being resurrected from os.environ.
293301
SANDBOX_AGENT_LAUNCH_UNSET_ENV_VARS: tuple[str, ...] = (
294302
"NODE_OPTIONS",
295303
"NODE_REPL_EXTERNAL_MODULE",
@@ -298,6 +306,8 @@ def vm_sandbox_allowed_origins(*, distinct_id: str, organization_id: str) -> set
298306
"LD_AUDIT",
299307
"DYLD_INSERT_LIBRARIES",
300308
"DYLD_LIBRARY_PATH",
309+
"GITHUB_TOKEN",
310+
"GH_TOKEN",
301311
)
302312

303313

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@
2020
get_github_token,
2121
get_pr_authorship_mode,
2222
get_readonly_github_token,
23+
get_sandbox_github_identity_user,
2324
get_sandbox_github_token,
2425
get_task_run_credential_user,
2526
is_caller_token_run,
2627
is_slack_interaction_state,
2728
resolve_user_github_integration_for_task,
29+
sandbox_identity_scope,
2830
)
2931

3032
if TYPE_CHECKING:
@@ -160,7 +162,14 @@ def _live_sandboxes_for_user_integration(user_integration_id: int) -> list[tuple
160162
task__github_user_integration_id=user_integration_id,
161163
)
162164
.select_related("task")
163-
.only("id", "state", "task__repository", "task__github_user_integration_id", "task__origin_product")
165+
.only(
166+
"id",
167+
"state",
168+
"task__repository",
169+
"task__github_user_integration_id",
170+
"task__origin_product",
171+
"task__created_by_id",
172+
)
164173
)
165174
for run in runs:
166175
sandbox_id = (run.state or {}).get("sandbox_id")
@@ -173,6 +182,13 @@ def _live_sandboxes_for_user_integration(user_integration_id: int) -> list[tuple
173182
continue
174183
if is_caller_token_run(str(run.id), run.state):
175184
continue
185+
# A per-message actor transition may have rebound (or logged out) this sandbox's GitHub
186+
# identity to someone other than the run owner. This loop carries the owner's token, so
187+
# re-applying it would undo that transition and resurrect the owner's identity for the
188+
# current actor. Skip when the sandbox is bound to a different actor.
189+
bound_actor = get_sandbox_github_identity_user(sandbox_identity_scope(str(run.id), run.state))
190+
if bound_actor is not None and bound_actor != run.task.created_by_id:
191+
continue
176192
rows.append((str(run.id), sandbox_id, run.task.repository))
177193
return rows
178194

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,3 +462,43 @@ def _task(repo):
462462
result = _live_sandboxes_for_user_integration(integration.id)
463463

464464
assert result == [(str(live_run.id), "sb-live", "org/live")]
465+
466+
@pytest.mark.parametrize("marker,included", [("none", True), ("owner", True), ("other", False)])
467+
def test_actor_transition_gates_owner_token_propagation(self, marker, included):
468+
from posthog.models import Organization, Team
469+
from posthog.models.user import User
470+
from posthog.models.user_integration import UserIntegration
471+
472+
from products.tasks.backend.models import Task, TaskRun
473+
from products.tasks.backend.temporal.process_task.sandbox_credentials import (
474+
_live_sandboxes_for_user_integration,
475+
)
476+
from products.tasks.backend.temporal.process_task.utils import mark_sandbox_github_identity
477+
478+
org = Organization.objects.create(name="o")
479+
team = Team.objects.create(organization=org, name="t")
480+
owner = User.objects.create(email="owner@test.com")
481+
other = User.objects.create(email="other@test.com")
482+
integration = UserIntegration.objects.create(
483+
user=owner, kind=UserIntegration.IntegrationKind.GITHUB, integration_id="i1", config={}, sensitive_config={}
484+
)
485+
task = Task.objects.create(
486+
team=team, created_by=owner, repository="org/repo", github_user_integration=integration
487+
)
488+
run = TaskRun.objects.create(
489+
task=task,
490+
team=team,
491+
status=TaskRun.Status.IN_PROGRESS,
492+
state={"sandbox_id": "sb-x", "pr_authorship_mode": "user"},
493+
)
494+
# An unset marker (no transition yet) and one bound to the owner both propagate; a marker
495+
# bound to a different per-message actor means the sandbox was logged out / rebound, so the
496+
# owner's rotating token must not overwrite it.
497+
if marker == "owner":
498+
mark_sandbox_github_identity("sb-x", owner.id)
499+
elif marker == "other":
500+
mark_sandbox_github_identity("sb-x", other.id)
501+
502+
result = _live_sandboxes_for_user_integration(integration.id)
503+
504+
assert (result == [(str(run.id), "sb-x", "org/repo")]) is included

0 commit comments

Comments
 (0)