feat(sandbox): broker SCM credentials for git operations#679
Merged
ColeMurray merged 5 commits intoMay 28, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors git auth to a credential-helper flow: control plane exposes POST /sessions/:id/scm-credentials; providers implement generateCredentialHelperAuth(); sandboxes obtain short-lived per-request credentials via a system git credential helper and a GH CLI wrapper; clone-token embedding is removed (legacy fallbacks preserved). ChangesGit Credential Helper Brokering
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py`:
- Around line 62-65: The current entrypoint wrapper condition gates all token
checks behind OI_GITHUB_TOKEN_IS_FALLBACK != "1", which prevents
GITHUB_APP_TOKEN from being treated as a real override when fallback is set;
update the shell conditional around the exec of REAL_GH so that GITHUB_APP_TOKEN
is accepted regardless of OI_GITHUB_TOKEN_IS_FALLBACK (i.e., execute REAL_GH if
GITHUB_APP_TOKEN is set OR if OI_GITHUB_TOKEN_IS_FALLBACK != "1" and either
GH_TOKEN or GITHUB_TOKEN is set), leaving the exec "$REAL_GH" "$@" call intact.
- Around line 230-245: In _ensure_credential_helper_configured, the code sets
git's "credential.helper" even when writing the shim (shim_path.write_text with
shim_body) fails; change the flow so the credential.helper is only configured
when the shim actually exists/wrote successfully: after attempting to write and
chmod shim_path (inside the try/except around shim_path.write_text/chmod) set a
local success flag (e.g., shim_written) or re-check shim_path.exists() and only
proceed to set ("credential.helper", str(shim_path)) when that flag/check is
true; ensure the except block does not fall through to configure
credential.helper pointing at a non-existent shim and keep logging the original
warning (self.log.warn("credential_helper.shim_write_failed", error=str(e))).
In `@packages/sandbox-runtime/tests/test_git_credential_helper.py`:
- Around line 85-590: Tests repeat raw timing literals (3600, 60, 0.1); add
module-level named constants (e.g. ONE_HOUR_SECONDS = 3600, ONE_MINUTE_SECONDS =
60, SHORT_SLEEP_SECONDS = 0.1) and replace the numeric literals used in tests
such as test_get_returns_credentials_on_success,
test_refreshes_when_cache_within_expiry_buffer, test_uses_cache_within_buffer,
test_concurrent_invocations_share_one_refresh (slow_handler sleep),
test_control_plane_response_invalid_expiry_is_fatal,
test_token_action_prints_bare_token, and any other occurrences so all
timeouts/TTLs use the new constants to encode units consistently.
- Around line 551-568: The problem is concurrent per-thread patching of
module-global stdio in run_one(), which can clobber patches when two threads
run; remove the patch.object(helper.sys, "stdin"/"stdout"/"stderr") calls from
inside run_one() and instead apply a single patch around the threaded section
(wrap the t1/t2 start/join block) so the global sys streams are patched once, or
alternatively provide thread-local I/O to helper.main without modifying
helper.sys; update the test_concurrent_invocations_share_one_refresh test to
call run_one() without patching helper.sys and perform any required
sys.stdin/stdout/stderr setup outside the threads (or via thread-local
redirect_context) using the unique symbols run_one(), helper.sys.stdin,
helper.sys.stdout, helper.sys.stderr, and helper.main.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 054d978c-199b-42a9-ae48-0d5e505331e1
📒 Files selected for processing (44)
README.mddocs/HOW_IT_WORKS.mddocs/adr/0001-single-provider-scm-boundaries.mddocs/provider-contribution-checklist.mdpackages/control-plane/README.mdpackages/control-plane/src/auth/github-app.test.tspackages/control-plane/src/auth/github-app.tspackages/control-plane/src/router.scm-credentials.test.tspackages/control-plane/src/router.tspackages/control-plane/src/routes/shared.tspackages/control-plane/src/sandbox/providers/daytona-provider.test.tspackages/control-plane/src/sandbox/providers/daytona-provider.tspackages/control-plane/src/session/contracts.tspackages/control-plane/src/session/durable-object.tspackages/control-plane/src/session/http/handlers/sandbox.handler.test.tspackages/control-plane/src/session/http/handlers/sandbox.handler.tspackages/control-plane/src/session/http/routes.test.tspackages/control-plane/src/session/http/routes.tspackages/control-plane/src/session/scm-credentials-service.test.tspackages/control-plane/src/session/scm-credentials-service.tspackages/control-plane/src/source-control/errors.tspackages/control-plane/src/source-control/index.tspackages/control-plane/src/source-control/provider-from-env.test.tspackages/control-plane/src/source-control/provider-from-env.tspackages/control-plane/src/source-control/providers/github-provider.test.tspackages/control-plane/src/source-control/providers/github-provider.tspackages/control-plane/src/source-control/providers/gitlab-provider.test.tspackages/control-plane/src/source-control/providers/gitlab-provider.tspackages/control-plane/src/source-control/types.tspackages/control-plane/test/integration/scm-credentials.test.tspackages/daytona-infra/src/toolchain.pypackages/github-bot/README.mdpackages/modal-infra/src/images/base.pypackages/modal-infra/src/sandbox/manager.pypackages/modal-infra/src/web_api.pypackages/modal-infra/tests/test_sandbox_env_vars.pypackages/modal-infra/tests/test_web_api_create_sandbox.pypackages/sandbox-runtime/src/sandbox_runtime/credentials/__init__.pypackages/sandbox-runtime/src/sandbox_runtime/credentials/git_credential_helper.pypackages/sandbox-runtime/src/sandbox_runtime/entrypoint.pypackages/sandbox-runtime/tests/test_entrypoint_build_mode.pypackages/sandbox-runtime/tests/test_entrypoint_urls.pypackages/sandbox-runtime/tests/test_gh_wrapper.pypackages/sandbox-runtime/tests/test_git_credential_helper.py
ColeMurray
reviewed
May 26, 2026
ColeMurray
reviewed
May 26, 2026
| env would silently fail once it expires (or immediately, for | ||
| providers with short-lived tokens like GitHub Apps). | ||
|
|
||
| For image-build sandboxes (one-shot, no control-plane access) |
Owner
There was a problem hiding this comment.
not required in this PR, but it likely makes sense as a follow-up to have all token auth run through the credentials helper to avoid two different credential approaches
Contributor
Author
There was a problem hiding this comment.
Agreed, that direction makes sense as a follow-up so we do not keep two token paths around longer than necessary. I left it out of this PR per your note and kept this pass focused on preserving same-host auxiliary repo compatibility.
Owner
|
Small correction on the restore motivation: for Modal snapshot restores, we already mint and inject a fresh auth token. Daytona is different: it does not use Modal-style snapshot restore. It resumes the same persistent sandbox via One extra Daytona note after tracing this more closely: existing Daytona sandboxes created before this helper lands will not automatically get the new helper on resume, since they keep the old filesystem/env. They likely need a fresh spawn or an explicit resume-time migration/patch. |
ColeMurray
approved these changes
May 28, 2026
Owner
|
thanks! |
5 tasks
5 tasks
hreiten
added a commit
to watchdog-no/background-agents
that referenced
this pull request
May 28, 2026
chore: sync main with upstream (SCM credential broker ColeMurray#679, dashboard link glyph)
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
2 participants
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.
Why
We hit this in our own Open-Inspect deployment: long-running or revisited sessions could no longer run git commands after the GitHub App installation token expired. The common workflow was to kick off an agent, come back more than an hour later, and continue the same session. At that point follow-up fetch/push/ls-remote operations were still using the token captured when the sandbox was created, so git failed even though the session itself was otherwise healthy.
What changed
POST /sessions/:id/scm-credentialsbroker endpoint that mints fresh SCM credentials through the configured provider.originremotes to plain HTTPS URLs before fetch so older snapshots route through the helper.ghwrapper for GitHub CLI calls, sinceghreads env tokens rather than git's credential protocol.Security and compatibility
The helper fails closed by scoping credential responses to HTTPS and the configured SCM host. It intentionally preserves the existing installation-wide repository access model for that host, so setup/start hooks can still clone auxiliary private repositories the installation can access. It caches credentials locally with a short refresh buffer and
0600permissions, and it refuses stale-cache fallback on broker failures. Live sessions also refuse to fall back to staticVCS_CLONE_TOKENwhen control-plane context is present but incomplete.Modal snapshot restores already mint a fresh fallback token during restore; the stale-token motivation here applies primarily to continuously running sessions and Daytona persistent resumes.
Validation
npm test -w @open-inspect/control-planenpm run test:integration -w @open-inspect/control-plane -- test/integration/scm-credentials.test.tsnpm run typecheck -w @open-inspect/control-planenpm run lint -w @open-inspect/control-planenpm run build -w @open-inspect/control-planecd packages/sandbox-runtime && uv run --extra dev pytest tests/test_git_credential_helper.py tests/test_gh_wrapper.py tests/test_entrypoint_build_mode.py tests/test_entrypoint_urls.py -qcd packages/sandbox-runtime && uv run --extra dev pytest tests -qcd packages/sandbox-runtime && uv run --extra dev ruff check src tests && uv run --extra dev ruff format --check src testsnpx prettier --check README.md docs/HOW_IT_WORKS.md packages/control-plane/README.md packages/github-bot/README.mdcd packages/modal-infra && uv run --extra dev pytest tests/test_sandbox_env_vars.py tests/test_web_api_create_sandbox.py -qcd packages/modal-infra && uv run --extra dev ruff check src tests && uv run --extra dev ruff format --check src testspython3 -m compileall packages/daytona-infra/srcSummary by CodeRabbit
New Features
Documentation
Infrastructure / Runtime
Tests