docs(agent-workflows): Plan Daytona secret delivery#5223
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🤖 The AI agent says: 🔸 Decision needed: Please review whether the first implementation scope should cover the Phase 0 security spike plus standard direct provider keys and exact-host custom-provider keys. The main points to challenge are the use of short-lived organization Secrets as per-sandbox leases, the exact host allowlist requirement, the fail-closed credential compatibility matrix, and the cleanup and janitor gates. Recommendation: approve the security spike first, then implement only credential classes whose values remain opaque and whose destination hosts are known before sandbox creation. |
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 08faae7d-2848-4432-8222-7629b2b64609
📒 Files selected for processing (7)
docs/design/agent-workflows/projects/daytona-secret-delivery/README.mddocs/design/agent-workflows/projects/daytona-secret-delivery/context.mddocs/design/agent-workflows/projects/daytona-secret-delivery/design.mddocs/design/agent-workflows/projects/daytona-secret-delivery/plan.mddocs/design/agent-workflows/projects/daytona-secret-delivery/qa.mddocs/design/agent-workflows/projects/daytona-secret-delivery/research.mddocs/design/agent-workflows/projects/daytona-secret-delivery/status.md
| (`services/runner/src/engines/sandbox_agent/sandbox-reconnect.ts:25-77`). A credential lease must | ||
| live as long as that sandbox, including stopped or archived periods. | ||
|
|
||
| The current `sandbox-agent` Daytona provider has no `pause` method. Its generic `pauseSandbox()` |
There was a problem hiding this comment.
we fixed this in a patch. see current state of worktree and update accordingly
There was a problem hiding this comment.
🤖 The AI agent says:
Updated from the current worktree. Native pause, reconnect, and delete now exist in the runner-owned Daytona wrapper, and Agenta auto-archive configuration is removed. The design now builds Secret lifecycle on that wrapper. It does not propose another pause fix or an upstream Secret patch.
| | `credentials[].value` | credential | Agenta vault, resolved per run and never logged or persisted by the runner | | ||
| | `credentials[].usage` | credential-use contract | resolver, stable for that credential kind | | ||
|
|
||
| Destination hosts are derived from the resolved consumer route, not transported in a sibling |
There was a problem hiding this comment.
🤖 The AI agent says:
🔎 Review note: Host ownership moved to the consumer route. Please check the choice to make the resolver emit effective endpoints for standard providers instead of maintaining a second provider-host registry in the runner.
| 2. Creates one organization Secret per isolated credential binding with that non-empty host list. | ||
| 3. For environment bindings, passes `{environmentName: daytonaSecretName}` in the sandbox create | ||
| `secrets` map instead of copying the value into `envVars`. | ||
| 4. For HTTP MCP header bindings, passes the Daytona placeholder as the header value in the ACP |
There was a problem hiding this comment.
🤖 The AI agent says:
🔎 Review note: HTTP MCP is now in scope. This line carries the unproven part: we need a live test that Daytona substitutes a placeholder placed directly in an ACP header. The fallback is an environment-backed MCP adapter or runner-side gateway, not plaintext.
| | Exact allowlisted HTTPS host | Host receives plaintext; sandbox sees placeholder. | | ||
| | Different host | Placeholder or rejected request; never plaintext. | | ||
| | Subdomain | No plaintext. | | ||
| | Any wildcard declaration | Rejected before Secret creation. | |
There was a problem hiding this comment.
🤖 The AI agent says:
🔎 Review note: The wildcard ambiguity is removed. The first version rejects every wildcard even though Daytona supports them.
|
|
||
| **Recommendation:** Option A. A separate API key alone does not narrow the permission. | ||
|
|
||
| ## 2. What happens to SigV4 and Vertex service-account connections? |
There was a problem hiding this comment.
🤖 The AI agent says:
🔸 Decision needed: My recommendation is to keep SigV4 and Vertex ADC usable only as an explicit non_isolated migration mode, while strict operators can reject them. Bedrock bearer and Azure API-key paths remain isolation candidates.
There was a problem hiding this comment.
let's go with option A for the moment.
|
|
||
| Daytona grants `manage:secrets` across an organization, not only Agenta-created Secrets. | ||
|
|
||
| - **Option A:** require a dedicated Daytona organization and dedicated runner credential. |
There was a problem hiding this comment.
🤖 The AI agent says:
🔸 Decision needed: Can production require a dedicated Daytona organization for Agenta sandboxes? A dedicated runner key alone still has organization-wide manage:secrets authority.
There was a problem hiding this comment.
No that is not reasonable. Use a shared organization. In any case we use Daytona only for this
| - Confirmed sandbox delete: delete every Secret in the lease, then mark cleanup complete. | ||
| - Secret cleanup failure: queue a durable retry and report cleanup as pending. | ||
|
|
||
| Archive is not part of the Agenta lifecycle. The current warm-Daytona create specification omits |
There was a problem hiding this comment.
🤖 The AI agent says:
🔎 Review note: This is aligned with the current warm-Daytona worktree: Agenta no longer configures archive. Reconciliation still treats a manually archived sandbox as existing so it cannot delete a valid lease.
|
|
||
| ### Credential changes | ||
|
|
||
| The first version does not rotate a live lease in place. The existing session compatibility path |
There was a problem hiding this comment.
🤖 The AI agent says:
🔎 Review note: Version one now avoids live Secret rotation. Credential epoch mismatch already invalidates reuse, so fresh sandbox plus fresh lease is simpler and avoids the 15-second propagation window. Later rotation must reassert and verify hosts.
|
🤖 The AI agent says: This is the second version. Here is what changed:
I answered every original comment inline and added fresh inline review notes on the revised lines. The most important decisions are the production Daytona organization boundary, the policy for SigV4 and Vertex ADC, and ownership of the effective provider endpoint. |
| Custom and Azure connections already carry a base URL. Standard providers and regional Bedrock | ||
| routes also need an exact effective endpoint so the host policy matches the client request. | ||
|
|
||
| - **Option A:** the Python connection resolver emits the effective endpoint for every deployment. |
| The runner can obtain the Secret placeholder and put it in ACP session configuration, but Daytona's | ||
| documentation demonstrates environment injection rather than this exact path. | ||
|
|
||
| **Recommendation:** make this a Phase 0 live gate. If direct placeholder substitution fails, either |
| Session state is already durable, but the janitor also needs organization-wide enumeration and | ||
| partial-provisioning recovery. | ||
|
|
||
| **Recommendation:** define a small runner-owned lease record in the control plane rather than |
| Users may need to know that a selected cloud credential remains readable inside Daytona. Exposing | ||
| mechanism-specific Daytona settings in the agent contract would be the wrong abstraction. | ||
|
|
||
| **Recommendation:** expose a portable credential-delivery requirement such as `isolated_required` |
There was a problem hiding this comment.
no will do it in the docs
| Google now documents API-key authentication for some Vertex Gemini paths, while Agenta's current | ||
| Vertex connection uses Application Default Credentials through `GOOGLE_APPLICATION_CREDENTIALS`. | ||
|
|
||
| **Recommendation:** keep Vertex API keys out of the first scope unless the provider-model contract |
| Secrets first shipped in SDK 0.192.0. The current package is 0.196.0, while the runner still uses | ||
| `@daytonaio/sdk` 0.187.0. Self-hosted control-plane compatibility is not yet proven. | ||
|
|
||
| **Recommendation:** select one exact `@daytona/sdk` version after the live spike and add startup |
There was a problem hiding this comment.
let's start by an upgrade to the right version and obviously test
mmabrouk
added
implementing
…connect Trust the stored sandbox pointer instead of comparing a create-spec fingerprint. A resumed Daytona turn reconnects the parked instance by id. Snapshot, image, and target drift is accepted as per-conversation version pinning. Network policy is converged at reconnect: read the live networkBlockAll/networkAllowList and call updateNetworkSettings only when they differ from the run's plan. Per-turn env sync is deferred to the daytona-secret-delivery direction (#5223). Removes the session_states.sandbox_fingerprint column and migration oss000000011, the createSpecFingerprint/buildResolvedDaytonaCreate helpers, and the fingerprint fields on the pointer read/write. Keeps the turn-index compare-and-set guard on sandbox_id, which uses only pre-existing columns. Claude-Session: https://claude.ai/code/session_018MaXPNpvzN22kngHno3VMj
…every turn (#5225) * feat(runner): warm daytona sessions slice 1 - provider pause/reconnect, pointer guard, typed teardown Slice 1 of the warm-daytona-sessions plan (PR #5214): the correctness base, with parking still inert (every teardown reason still deletes). - daytona provider wrapper adds pause/reconnect with a bounded state machine over Daytona transitional states; deleteSandbox helper; create-spec fingerprint (snapshot/image/target/env NAMES/network policy). - sandbox-agent package patch: a failed pause keeps the provider handles so the delete fallback works; a reconnected sandbox that fails later attach steps is paused, not leaked. - sessions API: nullable session_states.sandbox_fingerprint column; the sandbox pointer write is guarded by a compare-and-set on data.latest_turn_index (sandbox_turn_index token; tokenless writes keep today's behavior). - runner: pointer write is awaited, carries the fingerprint and guard token, and moved after continuity hydrate so a cold restart sends the right token; reconnect only on fingerprint match, best-effort delete on mismatch. - typed teardown reasons replace the keepWarm boolean; reason-to-disposition mapping with PARK_CLEAN_RESUMABLE_TURNS=false (Slice 2 flips it). Claude-Session: https://claude.ai/code/session_018MaXPNpvzN22kngHno3VMj * feat(runner): warm daytona sessions, slices 2-5 - park-to-stopped, provider pools, park-to-running, verified live Completes the warm-daytona-sessions plan (PR #5214) on top of the slice-1 correctness base. - Slice 2 (park-to-stopped active): clean resumable Daytona turns and idle shutdown stop the sandbox instead of deleting it; stop timer default 15 minutes; auto-archive removed entirely (create field, env override, compose and helm forwarding); terminal-vs-transient reconnect classification (only not-found/error/destroyed/unknown clear the stored pointer, guarded); clean remount before every remote mount so a reattached sandbox never serves a stale FUSE mount. - Slice 3 (provider-aware pools): one SessionPool per provider; local env vars and behavior unchanged; Daytona configured by AGENTA_RUNNER_DAYTONA_SESSION_IDLE_TTL_MS (0 disables, no separate flag) and AGENTA_RUNNER_DAYTONA_SESSION_MAX_WARM (default 20); pool evictions carry typed teardown reasons; unknown providers fail closed to cold. - Slice 4 (park-to-running warm slots): strict awaited capacity for the Daytona pool (a stopping sandbox keeps its warm slot until the stop confirms, so the billed count cannot overshoot); overflow parks to stopped; one-time activity refresh at live park, no recurring pings. - Slice 5: per-stage [timing] log lines in acquire; Daytona live-window default 120000 ms after the E3 gate passed; the TTL env parse accepts 0 as a valid off switch (a positive-only parse would silently re-enable the default). - Docs and config: the two new env vars forwarded in compose, env examples, and helm; self-host pages updated to the three-state lifecycle. - Live E3 verification: cold 12.5 s, live-warm 1.39 s, stopped-restart 7.7 s; one instance served all three turns; expiry observed as stop; SIGTERM drain observed as stop; zero sandbox leaks. Requires alembic migration core_oss oss000000011 (nullable session_states.sandbox_fingerprint). Carries shared-file hunks from the unmerged lanes feat/keepalive-project-scope (protocol.ts runContext scope and pool key preference) and fix/sessions-continuity-review (models.py replica_id validation, mdx edits) per the coordination board's shared-file rule. Claude-Session: https://claude.ai/code/session_018MaXPNpvzN22kngHno3VMj * feat(runner): drop sandbox fingerprint, converge network policy at reconnect Trust the stored sandbox pointer instead of comparing a create-spec fingerprint. A resumed Daytona turn reconnects the parked instance by id. Snapshot, image, and target drift is accepted as per-conversation version pinning. Network policy is converged at reconnect: read the live networkBlockAll/networkAllowList and call updateNetworkSettings only when they differ from the run's plan. Per-turn env sync is deferred to the daytona-secret-delivery direction (#5223). Removes the session_states.sandbox_fingerprint column and migration oss000000011, the createSpecFingerprint/buildResolvedDaytonaCreate helpers, and the fingerprint fields on the pointer read/write. Keeps the turn-index compare-and-set guard on sandbox_id, which uses only pre-existing columns. Claude-Session: https://claude.ai/code/session_018MaXPNpvzN22kngHno3VMj * fix(runner): service CodeRabbit review on warm daytona sessions - Gate the sandbox pointer write on the Daytona plan so a local run cannot overwrite a conversation's remote pointer. - Hydrate the continuity store before the guarded pointer clear so a post-restart clear carries the durable turn index. - pause() throws for the error sandbox state so teardown falls back to delete instead of reporting a parked sandbox with a stale pointer. - Catch live-park activity-refresh failures; the hook is best-effort. - Dedupe writeSandboxPointer/clearSandboxPointer into one guarded PUT. - Assert status_code in the tokenless pointer acceptance test. - ruff format on the warm_daytona_probe QA script (CI red). Claude-Session: https://claude.ai/code/session_018MaXPNpvzN22kngHno3VMj * fix(hosting): restore the MCP gate default to off The branch flipped AGENTA_AGENT_MCPS_ENABLED from false to true across the compose files, env examples, and the helm comment. The flip is unrelated to warm Daytona sessions and contradicts the SDK gate and the helm template, which both default off. Restore false everywhere. Claude-Session: https://claude.ai/code/session_018MaXPNpvzN22kngHno3VMj --------- Co-authored-by: Mahmoud Mabrouk <team@agenta.ai>
1 participant
Context
The runner currently resolves model-provider API keys and passes them to Daytona sandboxes as plaintext environment variables. That makes the values readable from the agent process and shell. Daytona Secrets can reduce direct exposure through proxy-side substitution, but their organization scope, resource lifecycle, host matching, and credential-format limits require explicit controls.
This PR is design-only. It does not update the Daytona SDK, create Daytona resources, or change runtime behavior.
Proposed direction
How to review
README.mdandcontext.mdfor scope and threat model.research.mdfor Daytona feasibility, SDK upgrade risk, and open evidence gaps.design.mdfor the delivery contract and lease lifecycle.plan.mdandqa.mdfor implementation slices and exit gates.Validation
docs/design/agent-workflows/projects/daytona-secret-delivery/.git diff --checkpasses.Decision requested
Please confirm whether the first implementation scope should include the Phase 0 security spike plus standard direct model-provider keys and exact-host custom-provider keys. Custom text secrets remain out of scope until each consumer and destination-host policy is explicit.