Skip to content

Commit 42f6ff3

Browse files
committed
fix(server): validate draft chunk sandbox ownership
1 parent a21ff9e commit 42f6ff3

1 file changed

Lines changed: 156 additions & 1 deletion

File tree

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

Lines changed: 156 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,16 @@ fn summarize_cli_policy_merge_op(operation: &PolicyMergeOp) -> String {
167167
}
168168
}
169169

170+
fn ensure_chunk_belongs_to_sandbox(
171+
chunk: &DraftChunkRecord,
172+
sandbox_id: &str,
173+
) -> Result<(), Status> {
174+
if chunk.sandbox_id != sandbox_id {
175+
return Err(Status::not_found("chunk not found"));
176+
}
177+
Ok(())
178+
}
179+
170180
fn summarize_add_endpoint(rule_name: &str, rule: &NetworkPolicyRule) -> String {
171181
let endpoints = rule
172182
.endpoints
@@ -1336,6 +1346,7 @@ pub(super) async fn handle_approve_draft_chunk(
13361346
.await
13371347
.map_err(|e| Status::internal(format!("fetch chunk failed: {e}")))?
13381348
.ok_or_else(|| Status::not_found("chunk not found"))?;
1349+
ensure_chunk_belongs_to_sandbox(&chunk, &sandbox_id)?;
13391350

13401351
if chunk.status != "pending" && chunk.status != "rejected" {
13411352
return Err(Status::failed_precondition(format!(
@@ -1421,6 +1432,7 @@ pub(super) async fn handle_reject_draft_chunk(
14211432
.await
14221433
.map_err(|e| Status::internal(format!("fetch chunk failed: {e}")))?
14231434
.ok_or_else(|| Status::not_found("chunk not found"))?;
1435+
ensure_chunk_belongs_to_sandbox(&chunk, &sandbox_id)?;
14241436

14251437
if chunk.status != "pending" && chunk.status != "approved" {
14261438
return Err(Status::failed_precondition(format!(
@@ -1603,19 +1615,21 @@ pub(super) async fn handle_edit_draft_chunk(
16031615
.proposed_rule
16041616
.ok_or_else(|| Status::invalid_argument("proposed_rule is required"))?;
16051617

1606-
let _sandbox = state
1618+
let sandbox = state
16071619
.store
16081620
.get_message_by_name::<Sandbox>(&req.name)
16091621
.await
16101622
.map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))?
16111623
.ok_or_else(|| Status::not_found("sandbox not found"))?;
1624+
let sandbox_id = sandbox.object_id().to_string();
16121625

16131626
let chunk = state
16141627
.store
16151628
.get_draft_chunk(&req.chunk_id)
16161629
.await
16171630
.map_err(|e| Status::internal(format!("fetch chunk failed: {e}")))?
16181631
.ok_or_else(|| Status::not_found("chunk not found"))?;
1632+
ensure_chunk_belongs_to_sandbox(&chunk, &sandbox_id)?;
16191633

16201634
if chunk.status != "pending" {
16211635
return Err(Status::failed_precondition(format!(
@@ -1665,6 +1679,7 @@ pub(super) async fn handle_undo_draft_chunk(
16651679
.await
16661680
.map_err(|e| Status::internal(format!("fetch chunk failed: {e}")))?
16671681
.ok_or_else(|| Status::not_found("chunk not found"))?;
1682+
ensure_chunk_belongs_to_sandbox(&chunk, &sandbox_id)?;
16681683

16691684
if chunk.status != "approved" {
16701685
return Err(Status::failed_precondition(format!(
@@ -2893,6 +2908,146 @@ mod tests {
28932908
assert!(history_after_clear.entries.is_empty());
28942909
}
28952910

2911+
#[tokio::test]
2912+
async fn draft_chunk_handlers_reject_cross_sandbox_chunk_ids() {
2913+
use openshell_core::proto::{NetworkBinary, NetworkEndpoint, SandboxPhase, SandboxSpec};
2914+
2915+
let state = test_server_state().await;
2916+
let sandbox_a = Sandbox {
2917+
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
2918+
id: "sb-draft-owner".to_string(),
2919+
name: "draft-owner".to_string(),
2920+
created_at_ms: 1_000_000,
2921+
labels: std::collections::HashMap::new(),
2922+
}),
2923+
spec: Some(SandboxSpec {
2924+
policy: None,
2925+
..Default::default()
2926+
}),
2927+
phase: SandboxPhase::Ready as i32,
2928+
..Default::default()
2929+
};
2930+
let sandbox_b = Sandbox {
2931+
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
2932+
id: "sb-draft-other".to_string(),
2933+
name: "draft-other".to_string(),
2934+
created_at_ms: 1_000_001,
2935+
labels: std::collections::HashMap::new(),
2936+
}),
2937+
spec: Some(SandboxSpec {
2938+
policy: None,
2939+
..Default::default()
2940+
}),
2941+
phase: SandboxPhase::Ready as i32,
2942+
..Default::default()
2943+
};
2944+
state.store.put_message(&sandbox_a).await.unwrap();
2945+
state.store.put_message(&sandbox_b).await.unwrap();
2946+
2947+
let proposed_rule = NetworkPolicyRule {
2948+
name: "allow_example".to_string(),
2949+
endpoints: vec![NetworkEndpoint {
2950+
host: "api.example.com".to_string(),
2951+
port: 443,
2952+
..Default::default()
2953+
}],
2954+
binaries: vec![NetworkBinary {
2955+
path: "/usr/bin/curl".to_string(),
2956+
..Default::default()
2957+
}],
2958+
};
2959+
2960+
handle_submit_policy_analysis(
2961+
&state,
2962+
Request::new(SubmitPolicyAnalysisRequest {
2963+
name: sandbox_a.object_name().to_string(),
2964+
proposed_chunks: vec![PolicyChunk {
2965+
rule_name: "allow_example".to_string(),
2966+
proposed_rule: Some(proposed_rule.clone()),
2967+
rationale: "observed denied request".to_string(),
2968+
confidence: 0.85,
2969+
hit_count: 3,
2970+
first_seen_ms: 100,
2971+
last_seen_ms: 200,
2972+
binary: "/usr/bin/curl".to_string(),
2973+
..Default::default()
2974+
}],
2975+
..Default::default()
2976+
}),
2977+
)
2978+
.await
2979+
.unwrap();
2980+
2981+
let draft_policy = handle_get_draft_policy(
2982+
&state,
2983+
Request::new(GetDraftPolicyRequest {
2984+
name: sandbox_a.object_name().to_string(),
2985+
status_filter: String::new(),
2986+
}),
2987+
)
2988+
.await
2989+
.unwrap()
2990+
.into_inner();
2991+
let chunk_id = draft_policy.chunks[0].id.clone();
2992+
let other_name = sandbox_b.object_name().to_string();
2993+
2994+
let approve_err = handle_approve_draft_chunk(
2995+
&state,
2996+
Request::new(ApproveDraftChunkRequest {
2997+
name: other_name.clone(),
2998+
chunk_id: chunk_id.clone(),
2999+
}),
3000+
)
3001+
.await
3002+
.unwrap_err();
3003+
assert_eq!(approve_err.code(), Code::NotFound);
3004+
3005+
let reject_err = handle_reject_draft_chunk(
3006+
&state,
3007+
Request::new(RejectDraftChunkRequest {
3008+
name: other_name.clone(),
3009+
chunk_id: chunk_id.clone(),
3010+
reason: "wrong sandbox".to_string(),
3011+
}),
3012+
)
3013+
.await
3014+
.unwrap_err();
3015+
assert_eq!(reject_err.code(), Code::NotFound);
3016+
3017+
let edit_err = handle_edit_draft_chunk(
3018+
&state,
3019+
Request::new(EditDraftChunkRequest {
3020+
name: other_name.clone(),
3021+
chunk_id: chunk_id.clone(),
3022+
proposed_rule: Some(proposed_rule.clone()),
3023+
}),
3024+
)
3025+
.await
3026+
.unwrap_err();
3027+
assert_eq!(edit_err.code(), Code::NotFound);
3028+
3029+
handle_approve_draft_chunk(
3030+
&state,
3031+
Request::new(ApproveDraftChunkRequest {
3032+
name: sandbox_a.object_name().to_string(),
3033+
chunk_id: chunk_id.clone(),
3034+
}),
3035+
)
3036+
.await
3037+
.unwrap();
3038+
3039+
let undo_err = handle_undo_draft_chunk(
3040+
&state,
3041+
Request::new(UndoDraftChunkRequest {
3042+
name: other_name,
3043+
chunk_id,
3044+
}),
3045+
)
3046+
.await
3047+
.unwrap_err();
3048+
assert_eq!(undo_err.code(), Code::NotFound);
3049+
}
3050+
28963051
#[test]
28973052
fn build_gateway_policy_audit_message_formats_ocsf_config_line() {
28983053
let message = build_gateway_policy_audit_message(

0 commit comments

Comments
 (0)