feat(cloud-agent-next): consume Kilo opaque session capabilities#4469
feat(cloud-agent-next): consume Kilo opaque session capabilities#4469eshurakov wants to merge 2 commits into
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThis incremental pass reviewed the latest commit (775c382), which correctly and completely fixes all 3 outstanding must-fix/warning items from the prior human review round (session-service.ts capability issuance retryability, Files Reviewed (4 files)
Previous Review Summaries (2 snapshots, latest commit 033002e)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 033002e)Status: No Issues Found | Recommendation: Merge Executive SummaryAll 6 previously flagged issues (the CRITICAL JWT-audience bypass and 5 WARNING/SUGGESTION findings across auth, server, session-service, sandbox-outbound, and the wrapper log uploader) are correctly and completely fixed in this incremental diff, with no new issues found in the changed code. Files Reviewed (10 files)
Previous review (commit 71a01cf)Status: 6 Issues Found | Recommendation: Address before merge Executive SummaryThe new legacy-JWT fallback in Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (27 files)
Reviewed by claude-sonnet-5 · Input: 78 · Output: 18.4K · Cached: 2.6M Review guidance: REVIEW.md from base branch |
pandemicsyn
left a comment
There was a problem hiding this comment.
lgtm, couple of feedback items but pre-approved
| KILOCODE_TOKEN: kilocodeToken, | ||
| KILO_AUTH_CONTENT: JSON.stringify({ kilo: { type: 'api', key: originalToken } }), | ||
| // Opaque Kilo capability — redeemed for the real credential at the outbound interceptor | ||
| KILOCODE_TOKEN: kiloCapability, |
There was a problem hiding this comment.
Must-fix: This capability is exposed to sandbox code, but redemption currently treats almost every path under backendBaseUrl as backend_api and substitutes the full user bearer token. The sandbox can therefore call a credential-minting route such as POST /api/organizations/:id/user-tokens and receive an exportable organization JWT, defeating credential containment. Replace the backend catch-all with an explicit method-and-path allowlist for the Kilo runtime operations that are actually required.
…sumption - Materialize the derived sessionIngestBaseUrl into normal and devcontainer runtime env so contained restore/import/ingest calls match the interceptor origin baked into the capability, not the raw localhost value - Accept the legacy log-upload URL without kiloSessionId for legacy_kilo_token claims; resolve the authoritative kiloSessionId from current session access, keeping the query/claim requirement for dispatch tickets - Map transient Kilo capability issuance failures (rpc_error, service_not_configured, temporarily_unavailable) to retryable workspaceSetupFailed instead of terminal invalidRequest
Summary
Why
Cloud Agent sandboxes currently receive the raw, long-lived Kilo user token in plaintext across many carriers —
KILOCODE_TOKEN,KILO_AUTH_CONTENT,auth.json, providerapiKey/kilocodeToken, and the wrapper-to-WorkerworkerAuthToken. Any compromise or accidental logging inside the untrusted sandbox exposes a reusable user credential. This PR contains that token by issuing a short-lived opaquekka1.*Kilo capability per dispatch and redeeming it per-request at the existing managed-SCM outbound interceptor, so contained sandboxes never see the raw token.What was done
kka1.*Kilo capability per wrapper dispatch (viagit-token-service) when the session uses managed SCM containment, and replace every sandbox-visible carrier of the raw token (KILOCODE_TOKEN,KILO_AUTH_CONTENT,auth.json, providerapiKey/kilocodeToken/baseURL) with that capability. The raw token now lives only in trusted Worker/DO/broker metadata.sandbox-outbound.ts) to classify and redeemkka1.*capabilities through a new route-awareredeemKiloSessionCapabilitybroker policy, reusing the same per-container egress interception already used for GitHub/GitLab capabilities. The capability carries normalized targets so runtime policy cannot drift.workerAuthTokenwith a fencedwrapper_dispatch_ticketJWT (4h lifetime) carrying the dispatch fence (cloud-agent session +kiloSessionId+wrapperRunId/wrapperGeneration/wrapperConnectionId). Wrapper-owned routes (/ingest,/kilo-global-ingest,/logs) validate the ticket and reject fence mismatches; a legacy raw-Kilo-JWT path keeps already-bound wrappers working until their next dispatch.POST /api/sessionextracts the bootstrapsessionIdand permits it only when it matches the capability-bound Kilo session. Existing exact-path ingest routes (/api/session/<id>/{export,import,ingest}) continue to cover historical full-sync and incremental batches.auth.jsonis now written from the materializedKILOCODE_TOKENcapability and refreshed on every ready request; the log uploader reads the current dispatch ticket lazily (it outlives any single ticket) and aborts active uploads before swapping uploaders.MessageBubblerenders the Worker-published sanitized string assistant errors instead of throwing when the error is a plain string.High-level architecture
sequenceDiagram participant DO as SessionService (DO, trusted) participant GTS as git-token-service (trusted broker) participant SB as Sandbox wrapper/SDK (untrusted) participant INT as Outbound interceptor (egress) participant UP as Kilo upstream (backend/provider/ingest) rect rgb(220,240,220) Note over DO,GTS: Trusted: raw user token never leaves here DO->>GTS: issueKiloSessionCapability(userToken, targets, containerId) GTS-->>DO: opaque kka1.* capability end DO->>SB: KILOCODE_TOKEN / KILO_AUTH_CONTENT / auth.json = capability<br/>workerAuthToken = wrapper_dispatch_ticket (fenced) SB->>INT: Kilo SDK outbound (Bearer kka1.*) INT->>GTS: redeemKiloSessionCapability(capability, containerId, method, url, bootstrapSessionId?) GTS-->>INT: real authorization + routeClass INT->>UP: forward with real Authorization UP-->>INT: response INT-->>SB: response alt bootstrap POST /api/session INT->>INT: parse body.sessionId, permit only if == capability-bound session end alt uncontained / DIND sandbox DO->>SB: KILOCODE_TOKEN = raw userToken (direct egress, no interception) endTwo distinct credentials are introduced, on purpose:
kka1.*Kilo capability — sandbox → Kilo upstream egress, redeemed per-request at the outbound interceptor (the headline containment change).wrapper_dispatch_ticketJWT — wrapper → Worker routes only (/ingest,/kilo-global-ingest,/logs), minted withNEXTAUTH_SECRETand carrying the dispatch fence so a stolen ticket cannot be replayed against a different session/connection.Architecture decision
Decision: Use one opaque
kka1.*Kilo capability everywhere the sandbox previously received the raw Kilo token, issued once per dispatch and redeemed per-request through the existing route-aware managed-SCM broker.Context: The sandbox is an untrusted execution environment that must present a Kilo credential to call backend API, provider-model, and session-ingest routes. The raw user token is long-lived and crossed many plaintext carriers inside the sandbox. The repo already has a managed-SCM containment + per-container outbound-interception mechanism for GitHub/GitLab capabilities, gated by
MANAGED_SCM_CONTAINMENT_ORG_IDS.Rationale: Reusing the existing outbound interception and the
git-token-servicebroker (withSCM_SESSION_CAPABILITY_ENCRYPTION_KEY) keeps the raw token in trusted metadata only, leaves the Kilo CLI unchanged, and adds no new secret. A per-dispatch capability with a 4h lifetime (intentionally no background refresh — a new trusted dispatch mints a fresh one) bounds exposure, and the route-aware allowlist narrows what a leaked capability can reach.Alternatives considered:
Consequences: Contained sessions get strong token containment but depend on the outbound interceptor being able to intercept egress (ports 80/443); DIND sandboxes and uncontained orgs fall back to the raw token. The wrapper-to-Worker auth surface changes from a raw Kilo JWT to a fenced dispatch ticket, so already-running wrappers rely on a legacy-JWT path until their next dispatch. Route classification is strict: the provider
baseURLbaked into the capability targets must match what the sandbox actually calls, or valid capabilities are rejected asupstream_not_allowed.Verification
kgsession reportedOPAQUE_CAPABILITY_PRESENT, and Logpush confirmedkka1redemption by the containment container with 200 responses on backend API, organization-model discovery, and provider-model routes.Session-history persistence through the ingest bootstrap path was discovered broken during that smoke (Kilo's import bootstrap emits
POST /api/session, which the capability policy rejected asupstream_not_allowed) and fixed in this commit. It has unit/integration coverage but has not been re-verified end-to-end against a deployed Worker — that is the main open verification item.Visual Changes
Minor:
MessageBubblenow renders the Worker-published sanitized string assistant errors instead of throwing when the error field is a plain string. No screenshot; this is a fallback rendering path for error strings already produced by the Worker sanitization boundary.Reviewer Notes
git-token-servicedeployment that implementsissueKiloSessionCapability/redeemKiloSessionCapability. Containment is gated byMANAGED_SCM_CONTAINMENT_ORG_IDS; thewrangler.jsoncchange setting it to*is the localenv.devblock only (for branch testing), not production.workerAuthTokenpath keeps already-bound wrappers working until their next dispatch; DIND sandboxes and uncontained orgs fall back to the raw token and authenticate directly.baseURLmust match the capability's baked-in targets, or valid capabilities are rejected asupstream_not_allowed.LOG_REJECTED_KILO_URLS=1is local-debug-only and logs full rejected URLs (which may contain tokens); it must never be set on a deployed Worker.