Skip to content

Commit 5601c2c

Browse files
committed
fix(upgrade): preserve pre.4 supervisor compatibility
Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent cbfb9a1 commit 5601c2c

9 files changed

Lines changed: 205 additions & 46 deletions

File tree

‎crates/openshell-policy/src/lib.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ fn yaml_mcp_method(
472472
method.to_string()
473473
}
474474

475+
#[allow(deprecated)]
475476
fn to_proto(raw: PolicyFile) -> Result<SandboxPolicy> {
476477
for (policy_name, rule) in &raw.network_policies {
477478
for (endpoint_index, endpoint) in rule.endpoints.iter().enumerate() {
@@ -518,6 +519,9 @@ fn to_proto(raw: PolicyFile) -> Result<SandboxPolicy> {
518519
};
519520
NetworkEndpoint {
520521
host: e.host,
522+
legacy_tls: String::new(),
523+
legacy_enforcement: String::new(),
524+
legacy_access: String::new(),
521525
path: e.path,
522526
port: normalized_ports.first().copied().unwrap_or(0),
523527
ports: normalized_ports,

‎crates/openshell-providers/src/profiles.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1630,9 +1630,13 @@ fn discovery_to_proto(discovery: &DiscoveryProfile) -> ProviderProfileDiscovery
16301630
}
16311631
}
16321632

1633+
#[allow(deprecated)]
16331634
fn endpoint_to_proto(endpoint: &EndpointProfile) -> NetworkEndpoint {
16341635
NetworkEndpoint {
16351636
host: endpoint.host.clone(),
1637+
legacy_tls: String::new(),
1638+
legacy_enforcement: String::new(),
1639+
legacy_access: String::new(),
16361640
port: endpoint.port,
16371641
protocol: endpoint.protocol.clone(),
16381642
tls: network_tls_mode_from_str(&endpoint.tls).map_or(-1, |value| value as i32),

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

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2576,6 +2576,10 @@ async fn handle_get_sandbox_config_inner(
25762576
) -> Result<Response<GetSandboxConfigResponse>, Status> {
25772577
let principal = super::extract_principal(&request)?;
25782578
let req = request.into_inner();
2579+
let legacy_supervisor = matches!(
2580+
&principal,
2581+
Principal::Sandbox(sandbox) if req.workspace_scope.is_none() && req.name == sandbox.sandbox_id
2582+
);
25792583
let workspace = match &principal {
25802584
Principal::Sandbox(_) if req.workspace_scope.is_none() => "",
25812585
_ => crate::auth::workspace_authz::selected_workspace_name(req.workspace_scope.as_ref())?,
@@ -2588,7 +2592,35 @@ async fn handle_get_sandbox_config_inner(
25882592
MinWorkspaceRole::User,
25892593
)
25902594
.await?;
2591-
Ok(Response::new(load_sandbox_config(state, &sandbox).await?))
2595+
let mut config = load_sandbox_config(state, &sandbox).await?;
2596+
if legacy_supervisor && let Some(policy) = config.policy.as_mut() {
2597+
populate_legacy_endpoint_modes(policy)?;
2598+
}
2599+
Ok(Response::new(config))
2600+
}
2601+
2602+
#[allow(deprecated)]
2603+
fn populate_legacy_endpoint_modes(policy: &mut ProtoSandboxPolicy) -> Result<(), Status> {
2604+
for rule in policy.network_policies.values_mut() {
2605+
for endpoint in &mut rule.endpoints {
2606+
endpoint.legacy_tls = openshell_policy::network_tls_mode_to_str(endpoint.tls)
2607+
.ok_or_else(|| Status::internal("effective policy contains an unknown TLS mode"))?
2608+
.to_string();
2609+
endpoint.legacy_enforcement =
2610+
openshell_policy::network_enforcement_mode_to_str(endpoint.enforcement)
2611+
.ok_or_else(|| {
2612+
Status::internal("effective policy contains an unknown enforcement mode")
2613+
})?
2614+
.to_string();
2615+
endpoint.legacy_access =
2616+
openshell_policy::network_access_preset_to_str(endpoint.access)
2617+
.ok_or_else(|| {
2618+
Status::internal("effective policy contains an unknown access preset")
2619+
})?
2620+
.to_string();
2621+
}
2622+
}
2623+
Ok(())
25922624
}
25932625

25942626
/// Resolve the same effective configuration for authenticated RPCs and trusted
@@ -7503,6 +7535,45 @@ mod tests {
75037535
use std::sync::atomic::{AtomicUsize, Ordering};
75047536
use tonic::Code;
75057537

7538+
#[derive(Clone, PartialEq, prost::Message)]
7539+
struct LegacyNetworkEndpoint {
7540+
#[prost(string, tag = "4")]
7541+
tls: String,
7542+
#[prost(string, tag = "5")]
7543+
enforcement: String,
7544+
#[prost(string, tag = "6")]
7545+
access: String,
7546+
}
7547+
7548+
#[test]
7549+
fn legacy_endpoint_modes_remain_decodable_by_pre4_supervisors() {
7550+
use openshell_core::proto::{NetworkAccessPreset, NetworkEnforcementMode, NetworkTlsMode};
7551+
7552+
let mut policy = ProtoSandboxPolicy {
7553+
network_policies: HashMap::from([(
7554+
"api".to_string(),
7555+
NetworkPolicyRule {
7556+
endpoints: vec![NetworkEndpoint {
7557+
tls: NetworkTlsMode::Skip.into(),
7558+
enforcement: NetworkEnforcementMode::Enforce.into(),
7559+
access: NetworkAccessPreset::ReadWrite.into(),
7560+
..Default::default()
7561+
}],
7562+
..Default::default()
7563+
},
7564+
)]),
7565+
..Default::default()
7566+
};
7567+
7568+
populate_legacy_endpoint_modes(&mut policy).unwrap();
7569+
let encoded = policy.network_policies["api"].endpoints[0].encode_to_vec();
7570+
let legacy = LegacyNetworkEndpoint::decode(encoded.as_slice()).unwrap();
7571+
7572+
assert_eq!(legacy.tls, "skip");
7573+
assert_eq!(legacy.enforcement, "enforce");
7574+
assert_eq!(legacy.access, "read-write");
7575+
}
7576+
75067577
/// Wrap a request with a user `Principal` so handler scope guards treat
75077578
/// the test caller as a CLI user. Most handler tests exercise
75087579
/// user-facing behavior and should not trip sandbox equality checks.

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,11 @@ pub(super) async fn resolve_and_authorize_sandbox_name(
203203
.map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))?
204204
.filter(|sandbox| {
205205
sandbox.metadata.as_ref().is_some_and(|metadata| {
206-
metadata.name == sandbox_name
206+
// Supervisors released before sandbox names became the
207+
// canonical RPC reference send their authenticated sandbox
208+
// UUID in the same protobuf field. The principal is already
209+
// bound to that UUID, so accepting it cannot widen scope.
210+
(metadata.name == sandbox_name || sandbox_principal.sandbox_id == sandbox_name)
207211
&& (workspace.is_empty() || workspace == sandbox.object_workspace())
208212
})
209213
});
@@ -3920,6 +3924,7 @@ async fn run_exec_with_russh(
39203924
#[cfg(test)]
39213925
mod tests {
39223926
use super::*;
3927+
use crate::auth::principal::{Principal, SandboxIdentitySource, SandboxPrincipal};
39233928
use crate::compute::NoopTestDriver;
39243929

39253930
#[test]
@@ -4391,6 +4396,33 @@ mod tests {
43914396
sandbox
43924397
}
43934398

4399+
#[tokio::test]
4400+
async fn sandbox_principal_accepts_legacy_id_reference_for_its_own_sandbox() {
4401+
let state = test_server_state().await;
4402+
let sandbox = test_sandbox("legacy-reference", Vec::new());
4403+
let sandbox_id = sandbox.object_id().to_string();
4404+
state.store.put_message(&sandbox).await.unwrap();
4405+
let principal = Principal::Sandbox(SandboxPrincipal {
4406+
sandbox_id: sandbox_id.clone(),
4407+
source: SandboxIdentitySource::BootstrapJwt {
4408+
issuer: "openshell-gateway:test".to_string(),
4409+
},
4410+
trust_domain: Some("openshell".to_string()),
4411+
});
4412+
4413+
let resolved = resolve_and_authorize_sandbox_name(
4414+
&state,
4415+
&principal,
4416+
&sandbox_id,
4417+
"",
4418+
MinWorkspaceRole::User,
4419+
)
4420+
.await
4421+
.unwrap();
4422+
4423+
assert_eq!(resolved.object_id(), sandbox_id);
4424+
}
4425+
43944426
fn test_workload_template(name: &str) -> SandboxWorkloadTemplate {
43954427
SandboxWorkloadTemplate {
43964428
metadata: Some(ObjectMeta {

‎crates/openshell-server/src/persistence/legacy_time_wire.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -332,13 +332,13 @@ fn conversion(message: &str, field: u32) -> Option<Conversion> {
332332
Some(D { new_tag: 116 })
333333
}
334334
("openshell.sandbox.v1.MiddlewareBinding", 4) => Some(DS { new_tag: 104 }),
335-
("openshell.sandbox.v1.NetworkEndpoint", 4) => Some(E { tag: 4, kind: Tls }),
335+
("openshell.sandbox.v1.NetworkEndpoint", 4) => Some(E { tag: 27, kind: Tls }),
336336
("openshell.sandbox.v1.NetworkEndpoint", 5) => Some(E {
337-
tag: 5,
337+
tag: 28,
338338
kind: Enforcement,
339339
}),
340340
("openshell.sandbox.v1.NetworkEndpoint", 6) => Some(E {
341-
tag: 6,
341+
tag: 29,
342342
kind: Access,
343343
}),
344344
_ => None,

‎crates/openshell-server/src/storage_proto.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,12 @@ mod tests {
128128
// the additive file map while retaining the rest of the response.
129129
// Service authorization also extends both schemas additively. Legacy
130130
// payloads retain the safe Strip default.
131+
// NetworkEndpoint retains the pre.4 string fields while current enum fields
132+
// use new tags, changing both schema fingerprints without changing ownership.
131133
const PUBLIC_RPC_SCHEMA_SHA256: &str =
132-
"2e156c6ad3c8eb51bcd30dc13b173fe339b38207a1b1f98f7be2e0cad8e3bd45";
134+
"013858ac39a29d738626115f9bdc70f30c5bcf21072dd42cddb2e77a14d721d1";
133135
const DURABLE_SCHEMA_SHA256: &str =
134-
"38165d9d76f49fcfe98a12f241e032838a2376c1d1a87ea2796fd33b9b1a3541";
136+
"6f53e9371c30c7d8586f588363f618aaccbe7bba36e8c27f2a5f829f7535bd55";
135137
const PUBLIC_DURABLE_OVERLAP_SHA256: &str =
136138
"761dea31a521b0650840fe2a823ad6e36a265ed323ba4506889781d630df0ee3";
137139
// A persisted Sandbox without endpoint status retains its lifecycle fields;

‎crates/openshell-supervisor-network/src/policy_local.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,7 @@ fn network_rule_from_json(
11201120
})
11211121
}
11221122

1123+
#[allow(deprecated)]
11231124
fn network_endpoint_from_json(
11241125
endpoint: NetworkEndpointJson,
11251126
) -> std::result::Result<NetworkEndpoint, String> {
@@ -1192,6 +1193,9 @@ fn network_endpoint_from_json(
11921193

11931194
Ok(NetworkEndpoint {
11941195
host: endpoint.host,
1196+
legacy_tls: String::new(),
1197+
legacy_enforcement: String::new(),
1198+
legacy_access: String::new(),
11951199
port,
11961200
protocol: endpoint.protocol,
11971201
tls: openshell_policy::network_tls_mode_from_str(&endpoint.tls)

‎proto/sandbox.proto‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -148,13 +148,11 @@ message NetworkEndpoint {
148148
// Endpoint protocol. "tcp" and "" select L4-only handling; "rest",
149149
// "websocket", "graphql", "sql", "json-rpc", and "mcp" select L7 inspection.
150150
string protocol = 3;
151-
// TLS handling. Unspecified enables automatic detection and termination.
152-
NetworkTlsMode tls = 4;
153-
// Enforcement mode. Unspecified preserves the audit default.
154-
NetworkEnforcementMode enforcement = 5;
155-
// Access preset shorthand. Unspecified means no preset.
156-
// Mutually exclusive with rules.
157-
NetworkAccessPreset access = 6;
151+
// Deprecated string representation retained for supervisors released before
152+
// endpoint security modes became typed enums.
153+
string legacy_tls = 4 [deprecated = true];
154+
string legacy_enforcement = 5 [deprecated = true];
155+
string legacy_access = 6 [deprecated = true];
158156
// Explicit L7 rules (mutually exclusive with access).
159157
repeated L7Rule rules = 7;
160158
// Allowed resolved IP addresses or CIDR ranges for this endpoint.
@@ -230,6 +228,13 @@ message NetworkEndpoint {
230228
// Internal gateway-derived marker indicating that this endpoint belongs to
231229
// an attached credentialed provider. User-authored values are ignored.
232230
bool provider_credentialed = 26;
231+
// TLS handling. Unspecified enables automatic detection and termination.
232+
NetworkTlsMode tls = 27;
233+
// Enforcement mode. Unspecified preserves the audit default.
234+
NetworkEnforcementMode enforcement = 28;
235+
// Access preset shorthand. Unspecified means no preset.
236+
// Mutually exclusive with rules.
237+
NetworkAccessPreset access = 29;
233238
}
234239

235240
// MCP options are grouped so MCP-specific policy can grow without adding more

0 commit comments

Comments
 (0)