fix(agent): treat an emptied sandbox env file as GitHub logout#3611
Open
VojtechBartos wants to merge 3 commits into
Open
fix(agent): treat an emptied sandbox env file as GitHub logout#3611VojtechBartos wants to merge 3 commits into
VojtechBartos wants to merge 3 commits into
Conversation
resolveGithubToken prefers the live /tmp/agent-env file (which the backend rewrites when the sandbox's actor changes mid-session) but fell back to the process env whenever the file yielded no token. On an actor transition the backend logs the sandbox out by emptying the token vars in that file, so the fallback resurrected the previous actor's token — frozen in the agent-server's launch-time process env — letting a follow-up actor push as the prior actor. Distinguish a file that carries the token vars but empties them (an explicit logout: return "") from a file that never had them (unmanaged: defer to the process env). Only the latter falls back.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Contributor
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
fetchPrAttribution and fetchGhLogin called gh with no env, inheriting the process env — the actor's token frozen at launch. After a mid-session actor transition that reports the wrong identity, and once the backend stops baking the token into the process env these calls would run unauthenticated. Resolve the live sandbox token (same file-first path as the signed-commit tools) and pass it explicitly; fall back to the process env only when unmanaged.
Contributor
|
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Address review findings on the emptied-env-file logout: - list_repos built its gh env with a truthiness check, so an empty (logged-out) token fell back to the raw process env and could enumerate the previous actor's private repos. Clear both token vars on a managed logout; only an undefined (unmanaged) token inherits the process env. - fetchGhLogin memoized the login across actor transitions, so after a rebind attribution used the previous actor's login. Key the cache on the live token. - readGithubTokenFromSandboxEnvFile treated every read error as an unmanaged sandbox; a present-but-unreadable file during a transition then resurrected the frozen process token. Only ENOENT falls back; other errors fail closed.
Contributor
🦔 ReviewHog reviewed this pull requestNothing worth raising this time, so here's a calming gif instead:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On a GitHub actor transition (multiplayer follow-ups on the PostHog side), the backend logs the sandbox out by emptying the token vars in
/tmp/agent-env. This agent-server was resurrecting the previous actor's token anyway.flowchart TD R([resolveGithubToken]) --> F{ /tmp/agent-env present? } F -- "no (local / desktop)" --> P[use process.env] F -- yes --> T{ token var non-empty? } T -- yes --> U[use the file token] T -- "no (emptied = logout)" --> E["return '' (no token)"]Changes
resolveGithubToken: a present-but-empty file is an explicit logout (return""), not a fallback toprocess.env. Only an absent/unmanaged file defers to the process env. ("" ?? processEnv === "", so an emptied file no longer resurrects the frozen launch-time token.)ghattribution/whoami (fetchPrAttribution,fetchGhLogin): run as the current actor via the live file token instead of inheritingprocess.env.Companion PR (required together)
PostHog/posthog#71941 (backend): removes the frozen process-env token and adds the rebind/logout gate.
How did you test this code?
github-token.test.ts(11 pass), including the assertion that an emptied file does not resurrect the process-env token. Validated live in a running sandbox (old resolver returns the previous actor's token; new returns""), and end-to-end on a dedicated GitHub org where both users have access to the same repo: no-access follow-up blocked, with-access follow-up rebinds.Tested locally together with posthog/posthog changes.
Agent context
Model: Claude Opus 4.8.