bug: make stable sandbox ID authoritative across compute-driver operations
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 8.7k
- Forks
- 1.3k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 253
Description
Agent Diagnostic
- Skills loaded:
create-github-issue - OpenShell version tested: source at PR #2340, commit
ab966edbd868a0a5fd04adbb7c8c67a0580f8f5d - Latest release checked: v0.0.86
- Known fixes reviewed: searched merged compute-driver identity and lifecycle PRs; no fix defining or consistently enforcing stable-ID semantics was found
- Possible duplicates reviewed: #2183 proposes a driver conformance suite but does not define identifier precedence; #2326 and PR #2340 exposed this existing inconsistency while changing delete synchronization
- Findings:
GetSandboxRequest,StopSandboxRequest, andDeleteSandboxRequesthave carried bothsandbox_idandsandbox_namesince the compute-driver API was introduced. Drivers inconsistently treat the stable ID as authoritative. Kubernetes delete ignores it, Podman stop targets only by name, VM delete falls back to name after an ID miss, and Docker pending-operation matching accepts an ID or name match. Read paths generally validate the returned ID after resolving by name. - Remaining reason for filing: a stale request for sandbox A can mutate a replacement sandbox B that reused A's name. The protocol contract and all first-party drivers need one consistent rule.
Description
Actual behavior: Compute-driver mutation requests carry both a stable gateway sandbox ID and a runtime name, but drivers do not consistently verify that the runtime resource selected by name has the requested ID before stopping or deleting it. In an out-of-band deletion, delayed watcher, restart, or multi-gateway sequence, a name can be reused while an operation for the old ID remains active. That operation can then stop or delete the replacement sandbox.
Known inconsistencies include:
- Kubernetes delete selects and deletes the Sandbox resource by name without checking
sandbox_id. - Podman stop ignores
sandbox_id; delete inspects and removes the container by name even if its ID label disagrees, while using the request ID for associated cleanup. - VM delete looks up by ID first but falls back to a matching name when the requested ID is absent.
- Docker active-container lookup verifies ID and name, but pending sandbox matching currently accepts either an ID match or a name match.
- Kubernetes and VM stop are currently unimplemented, but should follow the same contract when implemented.
Expected behavior: When a request supplies sandbox_id, that ID is authoritative. sandbox_name may be used as a runtime locator, but the driver must verify the located resource belongs to the requested ID before any mutation. An ID/name mismatch or an absent requested ID must never fall back to mutating a resource that merely has the same name.
Reproduction Steps
- Create sandbox A with ID
Aand runtime namefoo. - Retain or delay a stop/delete request containing
{sandbox_id: A, sandbox_name: foo}. - Remove A outside that request, then recreate
fooas sandbox B with IDB. - Allow the stale operation to reach a driver that selects resources by name.
- Observe that the operation can stop or delete B even though the request identifies A.
The exact mechanism for delaying step 2 differs by driver. Unit tests can reproduce the contract violation directly by seeding a runtime resource with name foo and ID B, then sending a request for ID A and name foo.
Proposed Design
- Define the identifier contract for
GetSandbox,StopSandbox, andDeleteSandbox:- If
sandbox_idis present, it is the authoritative identity. sandbox_nameis an additional runtime locator or diagnostic field, not an alternative identity.- A located resource whose stable ID differs must produce
FAILED_PRECONDITION(or a documented equivalent) before mutation. - If the requested ID is absent, do not fall back to another resource with the same name.
- If
- Audit Docker, Kubernetes, Podman, and VM implementations of get, stop, and delete.
- After resolving a name, mutate through an immutable backend identifier where available (for example, a container ID or Kubernetes UID precondition).
- Preserve name-only behavior only for explicitly supported legacy calls where
sandbox_idis empty; gateway-originated mutation requests should provide the ID. - Add the mismatch and name-reuse cases to the driver conformance work tracked by #2183.
Definition of Done
- The compute-driver identifier precedence and mismatch behavior are documented in the protobuf contract.
- All first-party
GetSandbox,StopSandbox, andDeleteSandboximplementations follow that contract. - No driver mutates a same-name resource after a supplied stable ID fails to match.
- Driver-level tests cover
{requested ID: A, selected resource ID: B, shared name: foo}for every implemented stop/delete path. - Name-reuse coverage exercises delayed or stale operations against replacement resources.
- The driver conformance suite issue #2183 includes an identifier-mismatch scenario or explicitly links to the resulting shared test coverage.
Environment
- OS: Linux development workspace
- Docker: not required; finding confirmed through source inspection and existing unit-test infrastructure
- OpenShell: PR #2340 at
ab966edbd868a0a5fd04adbb7c8c67a0580f8f5d - Latest release checked: v0.0.86
- Possible duplicates checked: yes; #2183 is related but not a duplicate
Logs
No runtime error log is required. This issue records an inconsistent compute-driver identity contract found during review of PR #2340.
Related
- #2326
- #2183
- #2340
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the protobuf contract for GetSandbox, StopSandbox, and DeleteSandbox, then trace the corresponding Docker, Kubernetes, Podman, and VM driver implementations. Use the existing unit-test infrastructure to cover a requested ID A, selected resource ID B, and shared name foo; done means the contract, implemented paths, and conformance work in #2183 all reject mismatches before mutation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- docker, kubernetes, rust
- Domain
- backend-api-design, infrastructure, testing-qa
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100