Skip to content

bug(driver-mxc): StopSandbox discards sandbox_id and can stop a same-named sandbox in another workspace #3253

Open
#3278
Open
bug(driver-mxc): StopSandbox discards sandbox_id and can stop a same-named sandbox in another workspace#3253
#3278
Labels
state:triage-neededOpened without agent diagnostics and needs triage

Description

@letv1nnn
letv1nnn
opened on Sep 10, 2026

User Story

As an operator running sandboxes across multiple workspaces on the MXC driver, I want a stop request to act on exactly the sandbox the gateway identified, so that stopping a sandbox in one workspace never interrupts an unrelated sandbox that happens to share its name.

Problem Statement

StopSandbox on the MXC driver discards the sandbox_id it receives and resolves the target by name alone.

crates/openshell-driver-mxc/src/grpc.rs:124-128:

if req.sandbox_name.is_empty() {
    return Err(Status::invalid_argument("sandbox_name is required"));
}
self.backend.stop_sandbox(&req.sandbox_name).await?;

req.sandbox_id is never read. The backend then resolves by name (crates/openshell-driver-mxc/src/driver.rs:441):

let entry = registry
    .values()
    .find(|entry| entry.sandbox.name == sandbox_name)
    .ok_or_else(|| tonic::Status::not_found(format!("sandbox {sandbox_name} not found")))?;

Nothing verifies afterward that the resolved entry is the one the request named.

Sandbox names are unique per workspace, not globally (crates/openshell-server/src/persistence/tests.rs:684, sqlite_name_unique_scoped_by_workspace), so two sandboxes in different workspaces can legitimately share a name. The MXC registry permits this: create_sandbox rejects duplicates by id only (driver.rs:389), never by name. With two same-named entries present, HashMap::values().find(...) picks one by iteration order.

The driver already carries the information needed to disambiguate — SandboxEntry.sandbox.workspace is populated (driver.rs:877) — and the same file already demonstrates the missing guard. GetSandbox performs a post-resolution id check (grpc.rs:90):

if !req.sandbox_id.is_empty() && req.sandbox_id != sandbox.id {
    return Err(Status::failed_precondition("sandbox_id did not match the fetched sandbox"));
}

StopSandbox has no equivalent.

Impact / Why This Matters

The gateway always sends a valid sandbox_id on StopSandbox, so this misfires on ordinary traffic — no restart, retry, or unusual state is required. An operator stopping demo in one workspace can silently stop demo in another, terminating a live workload with no error surfaced to either caller. Which sandbox is hit depends on hash iteration order, so the failure is not reproducible run to run.

There is no workaround available to the caller: the request already carries the correct id and the driver ignores it. Avoiding the bug requires never reusing a sandbox name across workspaces on an MXC gateway, which contradicts the workspace-scoped naming the rest of the product guarantees.

Related: #3234 and #3240 address the same class of defect in the Docker driver.

Acceptance Criteria

  • StopSandbox resolves the target using sandbox_id when the request supplies one, and does not fall back to the name in that case.
  • A stop request carrying a valid sandbox_id never acts on a different sandbox that shares the sandbox_name.
  • A name-only stop request that matches more than one sandbox fails with a deterministic error instead of selecting one by iteration order.
  • Regression tests cover: id-scoped stop with two same-named sandboxes in different workspaces stops only the requested one; ambiguous name-only stop is rejected.
  • DeleteSandbox on the same driver is audited for the same weakness (grpc.rs:145-155 forwards both fields; its backend path was not traced during this investigation).

Reproduction Steps

Code inspection; not reproduced against a running MXC gateway (Windows-only driver).

  1. Create workspaces alpha and beta on a gateway using the MXC compute driver.
  2. Create a sandbox named demo in alpha, and another named demo in beta. Both are accepted — names are unique per workspace, and the MXC registry rejects duplicates by id only.
  3. Issue StopSandbox for the demo in beta. The gateway sends both that sandbox's id and the name demo.
  4. Observe that the driver ignores the id and resolves demo by name, so the sandbox stopped may be the one in alpha.

Environment

  • OpenShell: main at a0814443
  • OS: Windows (MXC driver is Windows-only)
  • Runtime, deployment, or integration: MXC compute driver, gateway with more than one workspace

Activity

  1. github-actions
    added
    state:triage-neededOpened without agent diagnostics and needs triage
    on Sep 10, 2026

  2. letv1nnn commented on Sep 11, 2026

    @letv1nnn
    letv1nnn
    on Sep 11, 2026
    ContributorAuthor

    📋 triage-agent

    Triage Assessment

    Classification: validated-bug

    Summary

    Confirmed. StopSandbox on the MXC driver ignores the sandbox_id the gateway supplies and resolves the target by name alone, over a registry that is shared across workspaces. With two same-named sandboxes in different workspaces, the driver stops one chosen by HashMap iteration order. Every code citation in the report reproduces against main. Evidence is code-level only — the driver is Windows-gated and was not exercised at runtime — but the defect does not depend on timing, restart, or unusual state, so confidence is high.

    Investigation

    The reported code paths are accurate. grpc.rs:120-129 validates only sandbox_name and calls self.backend.stop_sandbox(&req.sandbox_name); req.sandbox_id is never read, though StopSandboxRequest carries it (proto/compute_driver.proto:368-373). driver.rs:441-449 then resolves via registry.values().find(|entry| entry.sandbox.name == sandbox_name), with no post-resolution id check.

    The workspace-scoped naming premise holds. migrations/sqlite/006_add_workspace_column.sql:12-15 drops the global objects_name_uq and recreates it as (object_type, workspace, name). Sandbox create uses put_if(..., sandbox.object_workspace(), ..., MustCreate) (compute/mod.rs:967-990) — no global name reservation. One citation drifted: sqlite_name_unique_scoped_by_workspace is at persistence/tests.rs:734, not :684 (:684 is sqlite_name_unique_per_object_type). Substance unaffected.

    The driver also receives the raw user-supplied name, not a workspace-qualified one — driver_sandbox_from_public sets name: sandbox.object_name() (compute/mod.rs:3900), and object_name() returns metadata.name verbatim (openshell-core/src/metadata.rs:54-57). Unlike Docker, MXC never derives a collision-proof runtime name.

    The gateway always supplies the id. All three call sites populate sandbox_id from object_id(): compute/mod.rs:1166-1169, :2191-2194, :2428-2431. No gateway path issues a name-only stop.

    The bug is reachable — the registry is not partitioned by workspace. This was the load-bearing question, and it does not clear the report. The registry is a single flat HashMap<String, SandboxEntry> keyed by sandbox id (driver.rs:177, insert at :406), and there is one driver instance per gateway process (ServerState.compute, server/src/lib.rs:263, built once at :598/:1364). MXC explicitly disclaims workspace partitioning: ensure_workspace and delete_workspace are unconditional no-ops (grpc.rs:171-186), asserted by workspace_lifecycle_is_an_idempotent_no_op (grpc.rs:206-218). Workspace creation is not driver-gated (grpc/workspace.rs:142-190), and with_local_singleplayer() (openshell-gateway/src/lib.rs:41-49) affects only mTLS-auth defaults and a JWT warning — it does not restrict workspace count. So two demo sandboxes in alpha and beta both persist, both reach create_sandbox with distinct ids and identical names, and both land in the same map.

    Impact is somewhat worse than reported. start_sandbox is Unimplemented on MXC (grpc.rs:132-139), so a wrongly-stopped sandbox cannot be restarted — recovery requires delete-and-recreate. State also diverges in both directions: the driver mutates and broadcasts a watch event for the victim (driver.rs:483-501), flipping the gateway's view of B to Stopped, while the gateway independently records A as Stopped even though A's isolation session was never touched and is still running.

    Acceptance criterion 5 resolves now: DeleteSandbox is already sound. driver.rs:504-517 looks up registry.get(sandbox_id) id-first and returns failed_precondition("sandbox_id did not match sandbox_name") on mismatch; subsequent operations (:525, :551, :561) all key on sandbox_id. GetSandbox (grpc.rs:90-94) resolves by name but fails closed on id mismatch — it can return a wrong-workspace error but never mutates the wrong sandbox. StopSandbox is the only handler with no id gate at all, which makes the omission look unintentional rather than designed. No delete-side fix appears needed.

    MXC is the outlier among compute drivers. Kubernetes (driver-kubernetes/src/grpc.rs:183-188) and Podman (driver-podman/src/grpc.rs:181-187) require sandbox_id and resolve by id only. Docker's find_managed_container_summary (driver-docker/src/lib.rs:1538-1580) filters on the id label and requires id_matches && name_matches.

    Duplicates and relationships. Not a duplicate of anything open. #3234 is the same root cause in Docker's pending map and is closed as completed by merged PR #3240 — direct precedent that this defect class has already been accepted and fixed in a sibling driver. #2347 (state:stale, area:compute) is the umbrella contract issue for exactly this rule; it audits Docker, Kubernetes, Podman, and VM but predates the MXC driver (introduced in #2721, 2026-08-27), so MXC is the un-swept corner. #3254 is the sibling report for the VM driver. A maintainer may prefer to fold this into #2347 rather than track it separately.

    Not a regression. The name-only stop is present in the MXC driver's introducing commit bcd517bb (#2721, 2026-08-27); workspace-scoped naming landed earlier in 5952a5a2 (#2243, 2026-07-20). It shipped broken against an invariant that already existed. Reported commit a0814443 is current main, post-v0.0.116; no released or merged change addresses it.

    Impact Signals

    • Affected users/scope: Operators on a Windows MXC gateway with more than one workspace and a reused sandbox name. Cross-tenant blast radius when it triggers, but MXC is a Windows-only, single-host, in-process driver registered with single-player defaults, so the population of affected deployments today is likely small. No evidence of a field report.
    • Regression: No. Present since the driver was introduced (feat(driver-mxc): native Windows MXC compute driver + server wiring #2721).
    • Workaround: Unavailable to the caller — the request already carries the correct id and the driver discards it. Operators can only avoid reusing sandbox names across workspaces, which contradicts the workspace-scoped naming the rest of the product guarantees. Recovery from a misfire is worse than usual because MXC cannot restart a stopped sandbox.
    • Evidence quality: High for the defect; medium for real-world exposure. Every citation was verified against main and the reachability question was checked specifically. Not reproduced at runtime — the crate is #[cfg(target_os = "windows")]-gated end to end (driver-mxc/src/lib.rs:18-32) and real MXC requires wxc-exec.

    Notes for whoever picks this up

    • A regression test is expressible on the existing windows:test:x64 lane against MxcComputeBackend::new_mocked, but the current helper driver_sandbox_with_command sets name: id.to_string() (driver.rs:911-920), so no existing test can express a name collision. A helper variant taking the name separately is a prerequisite. The existing sb-stop test (driver.rs:1197-1218) passes only because name == id there.
    • Acceptance criterion 3 (ambiguous name-only stop must fail deterministically) guards a path no supported caller uses, since the gateway always sends an id. Removing name-only resolution outright may be simpler than making it deterministic, and would match Podman and Kubernetes.
    • Open question worth a maintainer answer: is a multi-workspace MXC gateway a supported deployment, or an unstated non-goal? Nothing in the code forbids it, and ensure_workspace being a no-op is ambiguous — it could mean "workspaces need no driver resource" or "workspaces are out of scope here." If it is a non-goal, this is low-priority consistency cleanup; if it is supported, it is the same cross-tenant defect already fixed in fix(driver-docker): scope pending sandbox matching by id and workspace #3240.

    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. Also worth deciding whether this is tracked here or folded into #2347.

    To queue investigation or planning for an unattended agent, also apply agent:plan-requested — note that this label does not currently exist in the repository and would need to be created. 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 commented on Sep 15, 2026

    @johntmyers
    johntmyers
    on Sep 15, 2026
    Collaborator

    we should be using (workspace, name) as the ID for a sandbox. please adjust or close

  4. letv1nnn commented on Sep 15, 2026

    @letv1nnn
    letv1nnn
    on Sep 15, 2026
    ContributorAuthor

    hey @johntmyers, as I mentioned in #3305, I can refactor both issues, re-scope #3254 to use workspace + name for sandbox management, reapply the fix in #3305 on top of a compute_driver.proto change, and then if that shape suits the architecture requirements, reapply the fix for this issue. Does that sound good?

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    state:triage-neededOpened without agent diagnostics and needs triage

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions