Skip to content

Commit 287bad1

Browse files
committed
feat(auth): validate provider-sandbox ownership alignment
When attaching a provider to a sandbox, verify the provider is either owned by the caller or shared (no owner label). This prevents user A from referencing user B's private provider in their sandbox. Adds two tests: one for the rejection case and one confirming shared providers remain attachable by any user. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
1 parent dfef92d commit 287bad1

1 file changed

Lines changed: 113 additions & 1 deletion

File tree

crates/openshell-server/src/grpc/sandbox.rs

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ pub(super) async fn handle_attach_sandbox_provider(
359359
)));
360360
}
361361

362-
get_provider_record(state.store.as_ref(), &request.provider_name)
362+
let provider = get_provider_record(state.store.as_ref(), &request.provider_name)
363363
.await
364364
.map_err(|err| {
365365
if err.code() == tonic::Code::NotFound {
@@ -372,6 +372,21 @@ pub(super) async fn handle_attach_sandbox_provider(
372372
}
373373
})?;
374374

375+
// Verify the caller can use this provider (must be own or shared).
376+
let empty = std::collections::HashMap::new();
377+
let provider_labels = provider.metadata.as_ref().map_or(&empty, |m| &m.labels);
378+
crate::auth::ownership::check_owner(
379+
provider_labels,
380+
principal.as_ref(),
381+
admin_role_name(state),
382+
)
383+
.map_err(|_| {
384+
Status::permission_denied(format!(
385+
"provider '{}' is owned by another user",
386+
request.provider_name
387+
))
388+
})?;
389+
375390
let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await;
376391
let sandbox = sandbox_by_name(state, &request.sandbox_name).await?;
377392
check_sandbox_owner(&sandbox, principal.as_ref(), state)?;
@@ -2536,6 +2551,103 @@ mod tests {
25362551
assert_eq!(err.code(), tonic::Code::FailedPrecondition);
25372552
}
25382553

2554+
#[tokio::test]
2555+
async fn attach_sandbox_provider_rejects_other_users_provider() {
2556+
use crate::auth::identity::{Identity, IdentityProvider};
2557+
use crate::auth::ownership::OWNER_LABEL;
2558+
use crate::auth::principal::{Principal, UserPrincipal};
2559+
2560+
let state = test_server_state().await;
2561+
2562+
// Create a provider owned by alice
2563+
let mut provider = test_provider("alice-llm", "generic");
2564+
provider
2565+
.metadata
2566+
.as_mut()
2567+
.unwrap()
2568+
.labels
2569+
.insert(OWNER_LABEL.to_string(), "alice-uuid".to_string());
2570+
state.store.put_message(&provider).await.unwrap();
2571+
2572+
// Create a sandbox owned by bob
2573+
let mut sandbox = test_sandbox("bob-work", Vec::new());
2574+
sandbox
2575+
.metadata
2576+
.as_mut()
2577+
.unwrap()
2578+
.labels
2579+
.insert(OWNER_LABEL.to_string(), "bob-uuid".to_string());
2580+
state.store.put_message(&sandbox).await.unwrap();
2581+
2582+
// Bob tries to attach alice's provider
2583+
let bob = Principal::User(UserPrincipal {
2584+
identity: Identity {
2585+
subject: "bob-uuid".to_string(),
2586+
display_name: None,
2587+
roles: vec!["openshell-user".to_string()],
2588+
scopes: vec![],
2589+
provider: IdentityProvider::Oidc,
2590+
},
2591+
});
2592+
let mut req = Request::new(AttachSandboxProviderRequest {
2593+
sandbox_name: "bob-work".to_string(),
2594+
provider_name: "alice-llm".to_string(),
2595+
expected_resource_version: 0,
2596+
});
2597+
req.extensions_mut().insert(bob);
2598+
2599+
let err = handle_attach_sandbox_provider(&state, req)
2600+
.await
2601+
.unwrap_err();
2602+
assert_eq!(err.code(), tonic::Code::PermissionDenied);
2603+
assert!(err.message().contains("owned by another user"));
2604+
}
2605+
2606+
#[tokio::test]
2607+
async fn attach_sandbox_provider_allows_shared_provider() {
2608+
use crate::auth::identity::{Identity, IdentityProvider};
2609+
use crate::auth::ownership::OWNER_LABEL;
2610+
use crate::auth::principal::{Principal, UserPrincipal};
2611+
2612+
let state = test_server_state().await;
2613+
2614+
// Shared provider (no owner label)
2615+
state
2616+
.store
2617+
.put_message(&test_provider("shared-llm", "generic"))
2618+
.await
2619+
.unwrap();
2620+
2621+
// Sandbox owned by bob
2622+
let mut sandbox = test_sandbox("bob-work", Vec::new());
2623+
sandbox
2624+
.metadata
2625+
.as_mut()
2626+
.unwrap()
2627+
.labels
2628+
.insert(OWNER_LABEL.to_string(), "bob-uuid".to_string());
2629+
state.store.put_message(&sandbox).await.unwrap();
2630+
2631+
let bob = Principal::User(UserPrincipal {
2632+
identity: Identity {
2633+
subject: "bob-uuid".to_string(),
2634+
display_name: None,
2635+
roles: vec!["openshell-user".to_string()],
2636+
scopes: vec![],
2637+
provider: IdentityProvider::Oidc,
2638+
},
2639+
});
2640+
let mut req = Request::new(AttachSandboxProviderRequest {
2641+
sandbox_name: "bob-work".to_string(),
2642+
provider_name: "shared-llm".to_string(),
2643+
expected_resource_version: 0,
2644+
});
2645+
req.extensions_mut().insert(bob);
2646+
2647+
let response = handle_attach_sandbox_provider(&state, req).await.unwrap();
2648+
assert!(response.into_inner().attached);
2649+
}
2650+
25392651
// ---- validate_interactive_exec_start ----
25402652

25412653
#[test]

0 commit comments

Comments
 (0)