Skip to content

feat(cloud-agent): commit as yourself instead of the Kilo bot#3638

Merged
eshurakov merged 1 commit into
mainfrom
mercurial-script
Jun 2, 2026
Merged

feat(cloud-agent): commit as yourself instead of the Kilo bot#3638
eshurakov merged 1 commit into
mainfrom
mercurial-script

Conversation

@eshurakov
Copy link
Copy Markdown
Contributor

@eshurakov eshurakov commented Jun 1, 2026

Summary

  • Add GitHub user-to-server authorization through the existing Kilo GitHub App so Cloud Agent sessions can perform Git transport and GitHub API actions as the signed-in user instead of the installation bot.
  • Keep persisted GitHub user credentials in git-token-service: Web encrypts new authorizations with a public-key envelope, while the Worker owns private-key decryption, refresh, disconnect, revocation, and serialized token mutation.

Verification

  • Tested the GitHub personal-authorization flow locally through the web UI across different scenarios.
  • Verified the Cloud Agent identity setup tip locally in the browser and refined its placement, centering, and dismissal behavior.
  • Verified local connect and disconnect behavior against the GitHub user-authorization flow.

Visual Changes

Tip when you pick a repo and personal account is not connected

Screenshot 2026-06-01 at 22 29 05

Integration

Screenshot 2026-06-01 at 22 29 16

Bot is a co-author

Screenshot 2026-06-01 at 22 02 29

Comment thread services/cloud-agent-next/src/workspace.ts Outdated
Comment thread services/git-token-service/src/github-user-authorization-service.ts
@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented Jun 1, 2026

Code Review Summary

Status: 2 Observations (Non-blocking) | Recommendation: Merge

Executive Summary

This is a large feature PR adding user-attributed Cloud Agent actions (personal GitHub OAuth, commit co-authorship, identity hints). The security and correctness posture is strong. Two minor issues found — neither blocks merge.

Overview

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

WARNING

File Line Issue
services/git-token-service/worker-configuration.d.ts 27 Duplicate NEXTAUTH_SECRET in DevEnv — declared as SecretsStoreSecret on line 11 and again as string on line 27. TypeScript interfaces resolve duplicate keys to the last declaration, so NEXTAUTH_SECRET effectively becomes string in DevEnv, shadowing the SecretsStoreSecret binding type. This is a wrangler types generation artifact from having both a Secrets Store binding and a .dev.vars string override in the same environment. Runtime behavior is unaffected (the resolveJwtSecret helper correctly handles both union types via CloudflareEnv.Env), but the DevEnv type is technically wrong for tools relying on it. Cannot post as inline comment — GitHub 422s on this file's large diff.

SUGGESTION

File Line Issue
services/git-token-service/src/github-user-authorization-service.ts 220 Narrow permanent-failure edge case in second-transaction revocation path: if revokeAuthorizationOnGitHub returns 'token_invalid' (GitHub 401/403/422) for a freshly refreshed access token, the function throws 'GitHub authorization grant could not be revoked' without deleting the row, leaving the authorization permanently un-revocable. Low probability (requires a valid refresh token that mints an invalid access token), but structurally similar to the line-194 concern addressed in earlier review rounds. Consider treating token_invalid on the second-transaction path as a terminal revocation (proceed with deletion) the same way the expired-access-token + terminal-refresh path does at line 189–191.
Carried-forward issue (deferred)
File Line Issue
services/git-token-service/src/github-user-authorization-service.ts 448 SUGGESTION (deferred): refreshAuthorizationForDisconnect and refreshAuthorization remain near-identical. PR author has acknowledged and deferred.
New files reviewed in this pass
  • apps/web/src/app/api/integrations/github/user-connect/callback/route.ts — PKCE state consumed correctly, OAuthcode validated, Sentry context sanitized ✓
  • apps/web/src/lib/integrations/platforms/github/user-authorization.ts — advisory lock, upsert-with-setWhere guard, GDPR soft-delete coverage added ✓
  • apps/web/src/lib/integrations/platforms/github/user-authorization-state.ts — PKCE verifier bound to userId, single-use via redisGetDel
  • apps/web/src/lib/integrations/platforms/github/user-authorization-client.ts — short-lived JWT, no body identity, no URL-injected secrets ✓
  • apps/web/src/routers/github-apps-router.tsconnectUserAuthorization blocks re-connect while connected; revoked path correctly allows reconnect ✓
  • services/git-token-service/src/github-user-authorization-service.ts — RSA+AES-GCM envelope encryption, credential AAD binding, access check via GitHub API ✓
  • services/git-token-service/src/index.tsgetCloudAgentAuthForRepo RPC added, JWT-auth disconnect endpoint ✓
  • services/git-token-service/src/installation-lookup-service.tsfindManagedInstallationForRepo cross-references repo list correctly with case-insensitive name match; selected repository_access guard ✓
  • services/cloud-agent-next/src/session-service.tsgetInstallationAuthor throws if slug/userId unconfigured; co-author threaded through prompt/command ✓
  • services/cloud-agent-next/wrapper/src/auto-commit.tssanitizeGitOutput redacts auth tokens from error messages ✓
  • services/cloud-agent-next/wrapper/src/commit-co-author-hook.tsvalidCommitCoAuthor guards against injection in hook script, shellQuote used throughout ✓
  • services/cloud-agent-next/wrapper/src/main.tsupdateRuntimeEnvironment restarts Kilo runtime on env change; kiloClient = undefined properly cleared before restart ✓
  • packages/encryption/src/keyed-envelope.tsencryptKeyedEnvelope / decryptKeyedEnvelope correctly validate keyId, scheme, and version ✓
  • apps/web/src/lib/user/index.ts — GDPR softDeleteUser deletes user_github_app_tokens rows ✓
  • Migration 0154_familiar_franklin_richards.sql — unique indexes on (kilo_user_id, github_app_type) and (github_user_id, github_app_type) enforce both user ownership and GitHub identity uniqueness ✓

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-4.6 · 4,897,645 tokens

Review guidance: REVIEW.md from base branch main

@eshurakov eshurakov force-pushed the mercurial-script branch 2 times, most recently from 4c663ac to 0d905fb Compare June 1, 2026 20:43
@eshurakov eshurakov changed the title feat(github): add user-attributed cloud agent actions feat(cloud-agent): commit as yourself instead of the Kilo bot Jun 2, 2026
Copy link
Copy Markdown
Contributor

@jeanduplessis jeanduplessis left a comment

Choose a reason for hiding this comment

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

Reviewed the final combined diff. Solid security posture overall: the webhook signature is verified before the new github_app_authorization revocation branch, timingSafeEqual guards the length/empty-secret bypass, the AAD-bound RSA-AES envelope matches exactly between Web encrypt and Worker decrypt/refresh, both unique indexes back the upsert, the co-author hook + auto-commit trailer are idempotent, and GDPR soft-delete now covers user_github_app_tokens with a test.

One concrete edge-case bug (inline): disconnecting an authorization with expired/dead tokens returns 502 permanently and never removes the local row, leaving the user unable to disconnect or reconnect.

Two non-blocking suggestions:

  • refreshAuthorizationForDisconnect duplicates ~50 lines of refreshAuthorization (already acknowledged/deferred — fine to leave).
  • sanitizeGitOutput is duplicated across the src and wrapper bundles and can silently drift if a new credential-username scheme is added to only one regex; consider sharing it.

Comment thread services/git-token-service/src/github-user-authorization-service.ts
@eshurakov eshurakov requested a review from jeanduplessis June 2, 2026 08:43
@eshurakov eshurakov merged commit 6b28b9d into main Jun 2, 2026
54 checks passed
@eshurakov eshurakov deleted the mercurial-script branch June 2, 2026 13:09
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.

2 participants