Skip to content

Commit d2a3508

Browse files
authored
fix(server): reject reserved annotation keys (#4626)
Signed-off-by: Eric Curtin <eric.curtin@docker.com>
1 parent 2798108 commit d2a3508

5 files changed

Lines changed: 119 additions & 14 deletions

File tree

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

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ use tonic::{Code, Request, Response, Status};
112112
use tracing::{debug, info, warn};
113113

114114
use super::validation::{
115-
level_matches, source_matches, validate_and_canonicalize_policy, validate_annotations,
115+
level_matches, source_matches, validate_and_canonicalize_policy, validate_caller_annotations,
116116
validate_no_reserved_provider_policy_keys, validate_policy_safety,
117117
validate_static_fields_unchanged,
118118
};
@@ -4167,7 +4167,7 @@ async fn handle_update_config_inner(
41674167
) -> Result<Response<UpdateConfigResponse>, Status> {
41684168
let replay_facts = super::mutation_replay::ordinary::Facts::from_request(&request);
41694169
let req = request.into_inner();
4170-
validate_annotations(&req.annotations, "annotations")?;
4170+
validate_caller_annotations(&req.annotations, "annotations")?;
41714171
let sandbox = if req.global {
41724172
if !req.sandbox.is_empty() || req.workspace_scope.is_some() {
41734173
return Err(Status::invalid_argument(
@@ -24751,6 +24751,49 @@ mod tests {
2475124751
assert!(err.message().contains("label key"));
2475224752
}
2475324753

24754+
#[tokio::test]
24755+
async fn update_config_rejects_reserved_annotations() {
24756+
let state = test_server_state().await;
24757+
state
24758+
.store
24759+
.put_message(&test_sandbox(
24760+
"sb-reserved-annotation",
24761+
"reserved-annotation",
24762+
ProtoSandboxPolicy::default(),
24763+
Vec::new(),
24764+
))
24765+
.await
24766+
.unwrap();
24767+
24768+
let err = handle_update_config(
24769+
&state,
24770+
with_user(Request::new(UpdateConfigRequest {
24771+
sandbox: "reserved-annotation".to_string(),
24772+
workspace_scope: Some(openshell_core::proto::workspace_selector(
24773+
"default".to_string(),
24774+
)),
24775+
policy: Some(ProtoSandboxPolicy::default()),
24776+
annotations: HashMap::from([(
24777+
"internal.openshell.ai/auth-epoch".to_string(),
24778+
"1".to_string(),
24779+
)]),
24780+
..Default::default()
24781+
})),
24782+
)
24783+
.await
24784+
.unwrap_err();
24785+
24786+
assert_eq!(err.code(), Code::InvalidArgument);
24787+
assert!(err.message().contains("internal.openshell.ai/"));
24788+
let stored = state
24789+
.store
24790+
.get_message_by_name::<Sandbox>("default", "reserved-annotation")
24791+
.await
24792+
.unwrap()
24793+
.unwrap();
24794+
assert!(stored.metadata.unwrap().annotations.is_empty());
24795+
}
24796+
2475424797
#[tokio::test]
2475524798
async fn update_config_user_policy_rejects_reserved_provider_key() {
2475624799
let state = test_server_state().await;

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

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -462,13 +462,6 @@ async fn handle_create_sandbox_inner(
462462

463463
validate_create_sandbox_request_pre_io(&request, &workload_template_name)?;
464464

465-
// Validate labels (keys and values must meet Kubernetes requirements).
466-
for (key, value) in &request.labels {
467-
crate::grpc::validation::validate_label_key(key)?;
468-
crate::grpc::validation::validate_label_value(value)?;
469-
}
470-
crate::grpc::validation::validate_annotations(&request.annotations, "annotations")?;
471-
472465
let authz = authorize_workspace(
473466
&state.store,
474467
&state.admin_role,
@@ -791,7 +784,7 @@ fn validate_create_sandbox_request_pre_io(
791784
crate::grpc::validation::validate_label_key(key)?;
792785
crate::grpc::validation::validate_label_value(value)?;
793786
}
794-
crate::grpc::validation::validate_annotations(&request.annotations, "annotations")?;
787+
crate::grpc::validation::validate_caller_annotations(&request.annotations, "annotations")?;
795788

796789
if request.service_exposures.len() > MAX_CREATE_SERVICE_EXPOSURES {
797790
return Err(Status::invalid_argument(format!(
@@ -7236,6 +7229,44 @@ mod tests {
72367229
);
72377230
}
72387231

7232+
#[tokio::test]
7233+
async fn create_sandbox_rejects_reserved_annotations() {
7234+
let state = test_server_state().await;
7235+
7236+
let err = handle_create_sandbox(
7237+
&state,
7238+
authed_request(CreateSandboxRequest {
7239+
request_id: String::new(),
7240+
name: "reserved".to_string(),
7241+
spec: Some(SandboxSpec::default()),
7242+
labels: HashMap::new(),
7243+
annotations: HashMap::from([(
7244+
"internal.openshell.ai/compute-driver".to_string(),
7245+
"x".to_string(),
7246+
)]),
7247+
workspace_scope: Some(openshell_core::proto::workspace_selector(
7248+
"default".to_string(),
7249+
)),
7250+
await_main_process_attachment: false,
7251+
workload_template: String::new(),
7252+
service_exposures: Vec::new(),
7253+
}),
7254+
)
7255+
.await
7256+
.unwrap_err();
7257+
7258+
assert_eq!(err.code(), tonic::Code::InvalidArgument);
7259+
assert!(err.message().contains("internal.openshell.ai/"));
7260+
assert!(
7261+
state
7262+
.store
7263+
.get_message_by_name::<Sandbox>("default", "reserved")
7264+
.await
7265+
.unwrap()
7266+
.is_none()
7267+
);
7268+
}
7269+
72397270
#[tokio::test]
72407271
async fn single_replica_get_sandbox_sends_no_owner_hint() {
72417272
let state = test_server_state().await;

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,33 @@ pub(super) fn validate_annotations(
481481
Ok(())
482482
}
483483

484+
/// Annotation key prefix owned by the gateway.
485+
const RESERVED_ANNOTATION_PREFIX: &str = "internal.openshell.ai/";
486+
487+
/// Validate annotations supplied by an API caller.
488+
///
489+
/// Same as [`validate_annotations`], but also rejects keys under the
490+
/// gateway-owned `internal.openshell.ai/` prefix. The gateway keeps session
491+
/// and runtime state there and trusts it on read.
492+
pub(super) fn validate_caller_annotations(
493+
annotations: &std::collections::HashMap<String, String>,
494+
field_name: &str,
495+
) -> Result<(), Status> {
496+
validate_annotations(annotations, field_name)?;
497+
if let Some(key) = annotations
498+
.keys()
499+
.find(|key| key.starts_with(RESERVED_ANNOTATION_PREFIX))
500+
{
501+
return Err(invalid_argument(
502+
field_name,
503+
format!(
504+
"{field_name} key '{key}' uses the reserved '{RESERVED_ANNOTATION_PREFIX}' prefix"
505+
),
506+
));
507+
}
508+
Ok(())
509+
}
510+
484511
/// OPENSHELL_* keys that are allowed in exec environment. The Python SDK's
485512
/// `exec_python()` sends a serialized callable via this key.
486513
const EXEC_ALLOWED_OPENSHELL_KEYS: &[&str] = &["OPENSHELL_PYFUNC_B64"];

‎proto/openshell.proto‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,7 +1287,8 @@ message CreateSandboxRequest {
12871287
string name = 2;
12881288
// Optional labels for the sandbox (key-value metadata).
12891289
map<string, string> labels = 3;
1290-
// Optional annotations for the sandbox (non-selector metadata).
1290+
// Optional annotations for the sandbox (non-selector metadata). Keys under
1291+
// the `internal.openshell.ai/` prefix are reserved for the gateway.
12911292
map<string, string> annotations = 4;
12921293
// One-shot launch hint indicating that the creating client will attach to
12931294
// the canonical main process. The supervisor keeps the terminal transport
@@ -2844,7 +2845,8 @@ message UpdateConfigRequest {
28442845
// not interpret or verify their semantics. For policy updates, the gateway
28452846
// stores the annotations immutably with the revision and merges them into
28462847
// sandbox metadata as a convenience projection. For setting-only updates, it
2847-
// only merges them into sandbox metadata.
2848+
// only merges them into sandbox metadata. Keys under the
2849+
// `internal.openshell.ai/` prefix are reserved for the gateway.
28482850
map<string, string> annotations = 9;
28492851
// Required for sandbox-scoped updates and empty for global updates.
28502852
string sandbox = 1;

‎sdk/go/proto/openshellv1/openshell.pb.go‎

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)