bug(driver-vm): lifecycle requests fall back to sandbox_name when the supplied sandbox_id is absent #3254

Activity

1. github-actions

   added

   state:triage-neededOpened without agent diagnostics and needs triageOpened without agent diagnostics and needs triage

   on Sep 10, 2026
2. letv1nnn commented on Sep 11, 2026

   

   letv1nnn

   on Sep 11, 2026

   ContributorAuthor

   More actions

   📋 triage-agent

   Triage Assessment

   Classification: validated-bug

   Summary

   The defect is real and confirmed by code inspection at 289481f5. All three lifecycle methods fall back to name matching when the supplied sandbox_id is absent from the registry, and the registry can hold same-named records from different workspaces. Two corrections to the report are needed: the stated primary trigger (a repeated client delete) is not reachable through the public API, and a stronger, deterministic trigger inside the gateway was missed. Not reproduced against a running VM-driver gateway.

   Investigation

   Fallback confirmed (line numbers shifted from the reported a0814443):

   Function	Definition	id check	Name fallback
   stop_sandbox	driver.rs:1386	contains_key :1392	:1395-1398
   start_sandbox	driver.rs:1460	get_key_value :1466	:1469-1472
   delete_sandbox	driver.rs:1519	get_key_value :1531	:1534-1538

   The registry is a HashMap<String, SandboxRecord> (driver.rs:601), so .iter().find(...) is genuinely order-nondeterministic. create_sandbox rejects duplicates by id only (driver.rs:843-845) — no name or workspace check — so colliding records are admissible. get_sandbox is indeed unaffected: it gates on sandbox_id.is_empty() (driver.rs:1609-1623) and its handler post-checks the resolved id (driver.rs:3903-3907), while the three lifecycle handlers (:3923, :3933, :3943) forward both fields with no verification. All structural claims in the report verified accurate.

   Correction 1 — the repeated-delete trigger does not exist. A second client DeleteSandbox for the same (workspace, name) never reaches the driver. handle_delete_sandbox_inner (grpc/sandbox.rs:1348) calls compute.delete_sandbox(&workspace, &name), which resolves the name within the caller's workspace at compute/mod.rs:1535-1540 and returns Status::not_found("sandbox not found") when the row is gone. The caller receives NOT_FOUND, not deleted: true. Reproduction step 4 and the corresponding sentence in Impact are both incorrect as written.

   Correction 2 — the real trigger is the gateway's own prune sweep, and it is deterministic. prune_missing_sandbox (compute/mod.rs:3615) calls get_driver_sandbox(&sandbox_id, &sandbox_name) at :3651. The VM driver's id-only get_sandbox returns None, the sweep concludes the resource is gone, and it calls spawn_driver_sandbox_cleanup(&sandbox_id, &sandbox_name) at :3732 → call_driver_delete_sandbox (:3401) → DeleteSandboxRequest { sandbox_id, sandbox_name } (:3412) → VM driver id-miss → name fallback → deletes a same-named sandbox in another workspace. This path is entered precisely because the id was already found absent, so it selects for the failing condition rather than merely tolerating it. Driver failures here are logged with warn! and never propagated (:3422-3428), so the loss is silent — but because no client is on the other end, not because a client gets a false success.

   Two secondary paths: apply_deleted_locked (:3290, cleanup spawn at :3303) re-issues DeleteSandbox with the same id+name after the driver's own Deleted watch event, escaping only when the try_lock_owned lifecycle gate at :3376 is contended — a timing race, not a guarantee. And a partial restore after gateway restart (driver.rs:1786-1806 returns false on a missing image ref or unusable TLS paths) leaves a store row without a registry entry, so "gateway restart is not a reliable trigger" understates the case.

   Correction 3 — workspace-scoped matching is not an available fix. The report notes snapshot.workspace is populated (driver.rs:6405) but "not consulted when matching." StopSandboxRequest, StartSandboxRequest, and DeleteSandboxRequest carry no workspace field (proto/compute_driver.proto:368-391), so the driver cannot disambiguate by workspace regardless. The acceptance criteria land on the right fix anyway — id-authoritative plus deterministic rejection of ambiguous name-only requests.

   Precedent. The Docker fix did land: #3240 merged upstream as c6c85734 on 2026-09-10, introducing resolve_pending_id (driver-docker/src/lib.rs:2269), which treats a non-empty id as authoritative and rejects multi-match names with failed_precondition("sandbox_name matches multiple pending sandboxes; specify sandbox_id"). The pattern transfers directly to a single resolve_registry_id helper replacing all three inline blocks. One shape difference: the Docker defect lived in the pending map while find_managed_container_summary already required an exact id match, so the VM instance is strictly broader — it is the only lookup there is.

   Sibling drivers checked. openshell-driver-podman (grpc.rs:180-222, driver.rs:1112/:1175/:1217) and openshell-driver-kubernetes (grpc.rs:182-227, driver.rs:1632/:1702/:1808) both require a non-empty sandbox_id and are clean. openshell-driver-mxc resolves by name only (driver.rs:345, :441) — that is #3253, a distinct defect. No instances missed; VM is the last id→name fallback.

   Duplicates: none. #3234/#3240 is openshell-driver-docker, #3253 is openshell-driver-mxc. Three crates, three changesets.

   Impact Signals

   • Affected users/scope: VM-driver gateways only (libkrun/Linux, opt-in standalone subprocess), and only where the same sandbox name exists in two or more workspaces.
   • Regression: No. Latent since introduction — delete_sandbox fallback traces to e4d6f92d feat(vm): add standalone libkrun compute driver (2026-04-17), stop/start to 0f8fad23 (feat(sandbox): add stop and start operations #2653, 2026-08-13).
   • Workaround: Unavailable, as claimed — the offending call originates inside the gateway, not the client, so no caller-side change avoids it. Operators can only refrain from reusing names across workspaces.
   • Evidence quality: Medium-high. Structural claims verified against source and accurate; the reachability analysis in the report is wrong in its headline case, but a stronger deterministic trigger replaces it. Not reproduced live.
   • Note for maintainers: the confirmed outcome is destruction of a running sandbox in a workspace the requester has no authority over — a cross-tenant isolation break. Adversarial reachability was not established, and the Docker analog was handled as an ordinary public bug (bug(driver-docker): deleting a sandbox can destroy a same-named sandbox in another workspace #3234 → fix(driver-docker): scope pending sandbox matching by id and workspace #3240), so this is triaged the same way. Apply topic:security if you judge the isolation dimension warrants the specialized review path instead.

   Suggested additions to the acceptance criteria

   • Add the workspace-scoped duplicate-name check to create_sandbox (driver.rs:843), mirroring what reserve_pending_sandbox received in the Docker fix — otherwise the driver keeps admitting the colliding state the fix defends against.
   • Add the get_sandbox-style id post-check to the three handlers (driver.rs:3923, :3933, :3943) as defense in depth, mirroring :3903.
   • Worth resolving during planning: no gateway call site sends an empty sandbox_id (all source it from object_id()). If conformance and VM e2e confirm no direct driver client relies on name-only lifecycle requests, the fallback can be deleted outright rather than made ambiguity-rejecting — a smaller change than the Docker fix.

   Human Decision Required

   Decide whether OpenShell should address this issue. If yes, apply state:accepted, associate it with a roadmap item, or do both, and decide whether the work remains human-owned. Either action records acceptance; roadmap placement additionally records sequencing.
   To queue investigation or planning for an unattended agent, also apply agent:plan-requested. You can instead directly ask an agent to use create-spike or build-from-issue on this issue; the agent will warn about missing expected workflow labels and continue without changing them. If no, close it as not planned and record the rationale.
3. johntmyers

   closed this as completedin #3305on Sep 22, 2026
4. purp

   removed

   state:triage-neededOpened without agent diagnostics and needs triageOpened without agent diagnostics and needs triage

   on Oct 1, 2026