Skip to content

feat(cloud-agent-next): contain managed SCM credentials#3665

Open
eshurakov wants to merge 2 commits into
mainfrom
feat/cloud-agent-managed-scm-containment
Open

feat(cloud-agent-next): contain managed SCM credentials#3665
eshurakov wants to merge 2 commits into
mainfrom
feat/cloud-agent-managed-scm-containment

Conversation

@eshurakov
Copy link
Copy Markdown
Contributor

Summary

  • Replace sandbox-visible managed GitHub and GitLab credentials with short-lived encrypted capabilities redeemed by git-token-service through sandbox outbound interception.
  • Bind capabilities to the repository, provider auth surface, selected credential identity, and outer Sandbox Durable Object container ID so replay from another sandbox fails closed before provider lookup.
  • Preserve staged rollout compatibility for legacy unbound capabilities during the one-hour lifetime window, and propagate Cloudflare runtime HTTPS-interception trust into nested DIND devcontainers.
  • Add standard and DIND outbound rewrite probes plus focused coverage for capability issuance, redemption, redirects, LFS paths, GitLab CLI surfaces, and replay rejection.

Verification

  • Manually exercised the real-Git outbound rewrite probes for standard sandbox and DIND containment paths during implementation.
  • Did not rerun manual probes during the final history cleanup; no UI or visual surface changed.

Visual Changes

N/A

Reviewer Notes

  • Deploy in order: provision the SCM capability secret, deploy rollout-compatible git-token-service, then deploy cloud-agent-next.
  • Remove temporary unbound kgh1.* / kgl1.* issuance and redemption support after the one-hour compatibility window.
  • GitLab intentionally allows broad authenticated /api/v4/** and /api/graphql access for glab; Git smart HTTP and LFS control paths remain repository-bound.

const repository = parseGitLabCloneUrl(params.gitUrl, instanceOrigin);
if (!repository.success) return repository;
const identity = this.getGitLabSessionIdentity(integration);
if (!identity) return { success: false, reason: 'no_token' };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Misleading error code for missing identity

When getGitLabSessionIdentity returns null (both accountId and accountLogin are null on the integration row), the caller returns { success: false, reason: 'no_token' }. This conflates a missing-identity problem with a missing-token problem, which will produce a confusing error in logs and downstream handling. A more accurate reason would be something like 'no_integration_found' or a dedicated code to indicate the identity couldn't be established.

If this combination (accountId: null && accountLogin: null) genuinely cannot occur for a valid active integration in practice, a comment or runtime assertion would document that invariant.

if (url.username || url.password) return 'invalid_upstream_url';
const method = requestMethod.toUpperCase();
if (url.hostname === 'api.github.com' && url.port === '') {
return ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD'].includes(method)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Installation-source capabilities allow unrestricted api.github.com access

validateGitHubCapabilityUpstream permits any GET/POST/PUT/PATCH/DELETE/HEAD to api.github.com without restricting to paths related to the bound owner/repo. For a user-source capability this is intentional (broad gh CLI compatibility). For an installation-source capability the issued token is scoped to a specific repository installation, but the redemption check does not enforce that api.github.com calls target that repository.

This means an attacker who obtains an installation-source capability for acme/repo can redeem it to call https://api.github.com/repos/acme/other-repo or enumerate all repositories for the installation. If installation tokens are intended to be repository-scoped at the API surface too, an additional path-prefix check against repos/${owner}/${repo} is needed for installation-source redemptions.

@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented Jun 2, 2026

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The core security design — encrypted short-lived capabilities bound to sandbox container ID, repository, and provider identity — is well-structured with comprehensive test coverage. Two issues warrant attention before merge.

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
services/git-token-service/src/index.ts 680 getGitLabSessionIdentity returns null when both accountId and accountLogin are null, but the caller maps this to reason: 'no_token', which is misleading.
services/git-token-service/src/index.ts 199 installation-source capabilities can redeem against any api.github.com path, not just endpoints scoped to the bound repository. If intentional, should be documented explicitly.
Other Observations (not in diff)
  • Workers DB client caching fix: The removal of the cached this.db field in GitLabLookupService and GitLabTokenService correctly addresses the AGENTS.md prohibition on caching DB clients in Workers/DO class fields. Good fix.
  • Staged rollout compatibility: The legacy kgh1.*/kgl1.* unbound capability path is preserved for the compatibility window as documented. The deploy ordering note in Reviewer Notes is critical to follow.
  • JSON.stringify identity comparison (matchesSessionIdentity): Safe here because both sides are produced by the same getSessionIdentity function after Zod .strict() parsing, so key ordering is stable. No issue.
  • api.github.com user-source access: The broad api.github.com access for user-source capabilities is explicitly tested and intentional for gh CLI compatibility.
Files Reviewed (14 files)
  • services/cloud-agent-next/src/index.ts
  • services/cloud-agent-next/src/kilo/devcontainer.ts
  • services/cloud-agent-next/src/kilo/devcontainer.test.ts
  • services/cloud-agent-next/src/persistence/types.ts
  • services/cloud-agent-next/src/sandbox-id.ts
  • services/cloud-agent-next/src/sandbox-id.test.ts
  • services/cloud-agent-next/src/sandbox-outbound.ts
  • services/cloud-agent-next/src/sandbox-outbound.test.ts
  • services/cloud-agent-next/src/services/git-token-service-client.ts
  • services/cloud-agent-next/src/session-service.ts
  • services/git-token-service/src/index.ts
  • services/git-token-service/src/github-session-capability.ts
  • services/git-token-service/src/gitlab-session-capability.ts
  • services/git-token-service/src/gitlab-lookup-service.ts
  • services/git-token-service/src/gitlab-token-service.ts
  • services/git-token-service/src/gitlab-runtime-token-resolver.ts

Fix these issues in Kilo Cloud


Reviewed by claude-4.6-sonnet-20260217 · 4,242,862 tokens

Review guidance: REVIEW.md from base branch main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant