Skip to content

Commit 5aba844

Browse files
committed
fix(api): scope workspace request IDs by target
Include the requested workspace name in create/delete admission keys while leaving workspace UUID guards unset. Cover cross-target UUID reuse, replay, and missing targets with server and live gateway regressions. Merge the latest phase-two SDK fixes and preserve the approved interceptor replay changes. Refs #3051. Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
1 parent 6713da8 commit 5aba844

5 files changed

Lines changed: 220 additions & 6 deletions

File tree

‎architecture/gateway.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,9 @@ Mutation admission uses a private, version-tagged JSON envelope in the same
478478
object store. Its identity namespace stays stable across format changes, and an
479479
unknown format fails closed. It contains explicit typed receipts, not arbitrary
480480
public response payloads, and is not part of the protobuf storage closure.
481+
Workspace create/delete admissions include the requested workspace name in the
482+
key, but omit a workspace UUID guard. Different names have independent request-ID
483+
namespaces; deletion receipts remain replayable after the target disappears.
481484

482485
Each sandbox policy revision stores the complete provenance annotation map
483486
supplied with that update. The revision payload is the authoritative immutable

‎crates/openshell-server/src/grpc/mutation_replay.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -646,17 +646,21 @@ resource_mutation!(
646646
"CreateWorkspace",
647647
workspace::handle_create_workspace,
648648
workspace,
649-
async |_req: &CreateWorkspaceRequest, state: &ServerState, principal: &Principal| {
650-
global_scope(state, principal)
649+
async |req: &CreateWorkspaceRequest, state: &ServerState, principal: &Principal| {
650+
let mut scope = global_scope(state, principal)?;
651+
scope.name.clone_from(&req.name);
652+
Ok(scope)
651653
}
652654
);
653655
deletion_mutation!(
654656
DeleteWorkspaceRequest,
655657
DeleteWorkspaceResponse,
656658
"DeleteWorkspace",
657659
workspace::handle_delete_workspace,
658-
async |_req: &DeleteWorkspaceRequest, state: &ServerState, principal: &Principal| {
659-
global_scope(state, principal)
660+
async |req: &DeleteWorkspaceRequest, state: &ServerState, principal: &Principal| {
661+
let mut scope = global_scope(state, principal)?;
662+
scope.name.clone_from(&req.name);
663+
Ok(scope)
660664
}
661665
);
662666
resource_mutation!(

‎crates/openshell-server/src/grpc/mutation_replay/tests.rs‎

Lines changed: 140 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,139 @@ async fn sqlite_concurrency_restart_name_reuse_expiry_and_uncertainty() {
183183
.await;
184184
}
185185

186+
#[tokio::test]
187+
async fn workspace_create_request_id_is_scoped_to_each_target() {
188+
let state = test_server_state().await;
189+
let service = crate::grpc::OpenShellService::new(state.clone());
190+
let first = create("scope-a");
191+
let second = CreateWorkspaceRequest {
192+
name: "scope-b".into(),
193+
..first.clone()
194+
};
195+
let mut originals = Vec::new();
196+
for req in [&first, &second] {
197+
let response = service
198+
.create_workspace(authed_request(req.clone()))
199+
.await
200+
.unwrap();
201+
assert!(response.metadata().get("openshell-replayed").is_none());
202+
let original = response.into_inner();
203+
assert_eq!(
204+
original
205+
.workspace
206+
.as_ref()
207+
.unwrap()
208+
.metadata
209+
.as_ref()
210+
.unwrap()
211+
.name,
212+
req.name
213+
);
214+
assert_eq!(
215+
state
216+
.store
217+
.get_message_by_name::<Workspace>("", &req.name)
218+
.await
219+
.unwrap(),
220+
original.workspace
221+
);
222+
originals.push(original);
223+
}
224+
assert_ne!(
225+
originals[0].workspace.as_ref().unwrap().object_id(),
226+
originals[1].workspace.as_ref().unwrap().object_id()
227+
);
228+
for (req, original) in [&first, &second].into_iter().zip(originals) {
229+
let replay = service
230+
.create_workspace(authed_request(req.clone()))
231+
.await
232+
.unwrap();
233+
assert_eq!(replay.metadata().get("openshell-replayed").unwrap(), "true");
234+
assert_eq!(replay.into_inner(), original);
235+
let mut mismatch = req.clone();
236+
mismatch.labels.insert("changed".into(), "payload".into());
237+
assert_eq!(
238+
reason(
239+
&service
240+
.create_workspace(authed_request(mismatch))
241+
.await
242+
.unwrap_err()
243+
),
244+
"REQUEST_ID_PAYLOAD_MISMATCH"
245+
);
246+
}
247+
let rows = state
248+
.store
249+
.list_by_type_after(OBJECT_TYPE, None, 100)
250+
.await
251+
.unwrap();
252+
assert_eq!(rows.len(), 2);
253+
for row in rows {
254+
let admission: Admission = serde_json::from_slice(&row.payload).unwrap();
255+
assert!(admission.workspace_id.is_none());
256+
}
257+
}
258+
259+
#[tokio::test]
260+
async fn workspace_delete_request_id_is_scoped_to_each_target() {
261+
let state = test_server_state().await;
262+
let service = crate::grpc::OpenShellService::new(state.clone());
263+
let request_id = uuid::Uuid::new_v4().to_string();
264+
let mut requests = Vec::new();
265+
for name in ["scope-a", "scope-b"] {
266+
service
267+
.create_workspace(authed_request(CreateWorkspaceRequest {
268+
name: name.into(),
269+
..Default::default()
270+
}))
271+
.await
272+
.unwrap();
273+
requests.push(DeleteWorkspaceRequest {
274+
name: name.into(),
275+
request_id: request_id.clone(),
276+
..Default::default()
277+
});
278+
}
279+
for req in &requests {
280+
let response = service
281+
.delete_workspace(authed_request(req.clone()))
282+
.await
283+
.unwrap();
284+
assert!(response.metadata().get("openshell-replayed").is_none());
285+
assert_eq!(
286+
response.get_ref().outcome,
287+
openshell_core::proto::DeletionOutcome::Completed as i32
288+
);
289+
assert!(
290+
state
291+
.store
292+
.get_message_by_name::<Workspace>("", &req.name)
293+
.await
294+
.unwrap()
295+
.is_none()
296+
);
297+
}
298+
for req in requests {
299+
// The target no longer exists; deletion replay must not require its UUID.
300+
let replay = service.delete_workspace(authed_request(req)).await.unwrap();
301+
assert_eq!(replay.metadata().get("openshell-replayed").unwrap(), "true");
302+
assert_eq!(
303+
replay.get_ref().outcome,
304+
openshell_core::proto::DeletionOutcome::Completed as i32
305+
);
306+
}
307+
let rows = state
308+
.store
309+
.list_by_type_after(OBJECT_TYPE, None, 100)
310+
.await
311+
.unwrap();
312+
assert_eq!(rows.len(), 2);
313+
for row in rows {
314+
let admission: Admission = serde_json::from_slice(&row.payload).unwrap();
315+
assert!(admission.workspace_id.is_none());
316+
}
317+
}
318+
186319
#[tokio::test]
187320
#[ignore = "requires a disposable PostgreSQL database in OPENSHELL_REPLAY_TEST_DATABASE_URL"]
188321
async fn postgres_concurrency_restart_name_reuse_expiry_and_uncertainty() {
@@ -463,7 +596,13 @@ impl Mutation for ControlledCreate {
463596
&self.request_id
464597
}
465598
async fn authorize(&self, state: &ServerState, principal: &Principal) -> Result<Scope, Status> {
466-
global_scope(state, principal)
599+
// Fault-injected execution must use the real adapter's admission key.
600+
CreateWorkspaceRequest {
601+
name: self.name.clone(),
602+
..Default::default()
603+
}
604+
.authorize(state, principal)
605+
.await
467606
}
468607
async fn execute(
469608
state: &Arc<ServerState>,

‎docs/reference/api-errors.mdx‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,17 @@ subject, RPC method, and requested workspace. Use the same identity, scope, ID,
111111
and payload when checking an admitted attempt. Map ordering does not affect the
112112
payload fingerprint; changed values or protobuf message presence do.
113113

114+
For `CreateWorkspace` and `DeleteWorkspace`, the requested workspace is the
115+
request's `name`. The same caller can reuse an ID for the same method on different
116+
workspace names. Within one workspace name, a changed payload still returns
117+
`REQUEST_ID_PAYLOAD_MISMATCH`. Workspace deletion replay does not require the
118+
deleted workspace to exist and does not delete a same-name replacement.
119+
120+
Pre-release gateways that used unscoped workspace admission require reconciliation
121+
before upgrading. Their workspace create/delete receipts do not replay under the
122+
corrected name-scoped namespace. Do not retry those IDs after upgrading without
123+
reconciling the original operations' effects.
124+
114125
The gateway durably admits one owner before executing the mutation. Concurrent
115126
duplicates either replay its confirmed success or return
116127
`REQUEST_OUTCOME_UNCERTAIN`. Disconnecting or cancelling the client does not stop

‎e2e/python/test_workspace_api.py‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,61 @@ def test_workspace_get_nonexistent_raises_not_found(
7878
assert exc_info.value.code() == grpc.StatusCode.NOT_FOUND
7979

8080

81+
def test_workspace_request_ids_are_scoped_to_each_target(
82+
workspace_client: WorkspaceClient,
83+
) -> None:
84+
names = [f"ws-scope-{uuid.uuid4().hex[:8]}" for _ in range(2)]
85+
create_id = str(uuid.uuid4())
86+
delete_id = str(uuid.uuid4())
87+
stub = workspace_client._stub
88+
originals = []
89+
try:
90+
for name in names:
91+
request = openshell_pb2.CreateWorkspaceRequest(
92+
name=name, request_id=create_id
93+
)
94+
response, call = stub.CreateWorkspace.with_call(request, timeout=20)
95+
assert "openshell-replayed" not in dict(call.initial_metadata())
96+
assert response.workspace.metadata.name == name
97+
fetched = stub.GetWorkspace(
98+
openshell_pb2.GetWorkspaceRequest(name=name), timeout=20
99+
)
100+
assert fetched.workspace.metadata.id == response.workspace.metadata.id
101+
originals.append(response)
102+
assert originals[0].workspace.metadata.id != originals[1].workspace.metadata.id
103+
104+
for name, original in zip(names, originals, strict=True):
105+
replay, call = stub.CreateWorkspace.with_call(
106+
openshell_pb2.CreateWorkspaceRequest(name=name, request_id=create_id),
107+
timeout=20,
108+
)
109+
assert replay == original
110+
assert dict(call.initial_metadata())["openshell-replayed"] == "true"
111+
112+
for name in names:
113+
request = openshell_pb2.DeleteWorkspaceRequest(
114+
name=name, request_id=delete_id
115+
)
116+
response, call = stub.DeleteWorkspace.with_call(request, timeout=20)
117+
assert "openshell-replayed" not in dict(call.initial_metadata())
118+
assert response.outcome == openshell_pb2.DELETION_OUTCOME_COMPLETED
119+
with pytest.raises(grpc.RpcError) as exc_info:
120+
workspace_client.get(name)
121+
assert exc_info.value.code() == grpc.StatusCode.NOT_FOUND
122+
123+
for name in names:
124+
replay, call = stub.DeleteWorkspace.with_call(
125+
openshell_pb2.DeleteWorkspaceRequest(name=name, request_id=delete_id),
126+
timeout=20,
127+
)
128+
assert replay.outcome == openshell_pb2.DELETION_OUTCOME_COMPLETED
129+
assert dict(call.initial_metadata())["openshell-replayed"] == "true"
130+
finally:
131+
for name in names:
132+
with contextlib.suppress(Exception):
133+
workspace_client.delete(name, allow_missing=True)
134+
135+
81136
def test_workspace_request_id_replays_without_deleting_replacement(
82137
workspace_client: WorkspaceClient,
83138
) -> None:
@@ -109,7 +164,9 @@ def test_workspace_request_id_replays_without_deleting_replacement(
109164
)
110165
assert replacement.workspace.metadata.id != original.workspace.metadata.id
111166
assert stub.DeleteWorkspace(delete, timeout=20) == removed
112-
fetched = stub.GetWorkspace(openshell_pb2.GetWorkspaceRequest(name=name), timeout=20)
167+
fetched = stub.GetWorkspace(
168+
openshell_pb2.GetWorkspaceRequest(name=name), timeout=20
169+
)
113170
assert fetched.workspace.metadata.id == replacement.workspace.metadata.id
114171
finally:
115172
with contextlib.suppress(Exception):

0 commit comments

Comments
 (0)