Skip to content

Commit fd1a801

Browse files
committed
fix(vm): restore inactive sandbox workload identity
Recover the persisted overlay owner before publishing stopped and terminal sandboxes. Keep resources manageable when identity metadata is invalid. Clarify fixed MicroVM ownership in policy-generation guidance. Signed-off-by: Shiju <shiju@nvidia.com>
1 parent 042bc23 commit fd1a801

2 files changed

Lines changed: 231 additions & 27 deletions

File tree

‎crates/openshell-driver-vm/src/driver.rs‎

Lines changed: 229 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2170,38 +2170,41 @@ impl VmDriver {
21702170
continue;
21712171
}
21722172

2173-
if tokio::fs::metadata(state_dir.join(SANDBOX_STOPPED_FILE))
2173+
let inactive_condition = if tokio::fs::metadata(state_dir.join(SANDBOX_STOPPED_FILE))
21742174
.await
21752175
.is_ok()
21762176
{
2177-
let snapshot = sandbox_snapshot(&sandbox, stopped_condition(), false);
2178-
let mut registry = self.registry.lock().await;
2179-
registry.entry(sandbox.id.clone()).or_insert(SandboxRecord {
2180-
snapshot: snapshot.clone(),
2181-
state_dir: state_dir.clone(),
2182-
process: None,
2183-
provisioning_task: None,
2184-
gpu_bdf: None,
2185-
deleting: false,
2186-
});
2187-
drop(registry);
2188-
self.publish_snapshot(snapshot);
2189-
info!(sandbox_id = %sandbox.id, "vm driver: restored stopped sandbox without launching compute");
2190-
continue;
2191-
}
2192-
2193-
if tokio::fs::try_exists(state_dir.join(MAIN_PROCESS_EXITED_FILE))
2177+
Some(stopped_condition())
2178+
} else if tokio::fs::try_exists(state_dir.join(MAIN_PROCESS_EXITED_FILE))
21942179
.await
21952180
.unwrap_or(false)
21962181
{
2197-
let snapshot = sandbox_snapshot(
2198-
&sandbox,
2199-
error_condition(
2200-
"ProcessExited",
2201-
"Canonical main process exited before VM driver restart",
2202-
),
2203-
false,
2204-
);
2182+
Some(error_condition(
2183+
"ProcessExited",
2184+
"Canonical main process exited before VM driver restart",
2185+
))
2186+
} else {
2187+
None
2188+
};
2189+
if let Some(condition) = inactive_condition {
2190+
// These sandboxes do not run the launch path that publishes
2191+
// identity. Recover it from the same validated owner marker
2192+
// before either GetSandbox or WatchSandboxes can observe them.
2193+
let identity = match read_persisted_workload_identity(&state_dir).await {
2194+
Ok(identity) => identity,
2195+
Err(error) => {
2196+
warn!(sandbox_id = %sandbox.id, %error, "vm driver: ignoring invalid persisted workload identity");
2197+
// Keep inactive resources manageable even when their
2198+
// identity cannot be trusted. Explicit deletion still
2199+
// needs this registry entry to remove persisted state.
2200+
None
2201+
}
2202+
};
2203+
let mut snapshot = sandbox_snapshot(&sandbox, condition, false);
2204+
snapshot
2205+
.status
2206+
.get_or_insert_with(SandboxStatus::default)
2207+
.resolved_identity = identity;
22052208
let mut registry = self.registry.lock().await;
22062209
registry.entry(sandbox.id.clone()).or_insert(SandboxRecord {
22072210
snapshot: snapshot.clone(),
@@ -2215,7 +2218,7 @@ impl VmDriver {
22152218
self.publish_snapshot(snapshot);
22162219
info!(
22172220
sandbox_id = %sandbox.id,
2218-
"vm driver: preserved terminal sandbox without restarting canonical process"
2221+
"vm driver: restored inactive sandbox without launching compute"
22192222
);
22202223
continue;
22212224
}
@@ -5790,6 +5793,35 @@ async fn persisted_sandbox_owner_identity(
57905793
Ok(None)
57915794
}
57925795

5796+
/// Reconstruct status from the owner chosen during preparation, never from a
5797+
/// new driver default or a status embedded in the persisted create request.
5798+
/// Missing or v1 markers do not contain an identity to report. Invalid owner
5799+
/// data must not be published as a resolved identity.
5800+
async fn read_persisted_workload_identity(
5801+
state_dir: &Path,
5802+
) -> Result<Option<ResolvedWorkloadIdentity>, String> {
5803+
let contents = match tokio::fs::read_to_string(state_dir.join(SANDBOX_OWNER_STATE_FILE)).await {
5804+
Ok(contents) if contents.trim() == SANDBOX_OWNER_STATE_V1 => return Ok(None),
5805+
Ok(contents) => contents,
5806+
Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None),
5807+
Err(error) => return Err(format!("read sandbox owner state: {error}")),
5808+
};
5809+
let owner = parse_sandbox_owner_state(&contents)
5810+
.map_err(|error| format!("invalid sandbox owner state: {error}"))?;
5811+
let resource_digest = match read_persisted_image_identity(state_dir).await {
5812+
Ok(identity) => identity,
5813+
Err(error) if error.kind() == std::io::ErrorKind::NotFound => String::new(),
5814+
Err(error) => return Err(format!("read persisted VM image identity: {error}")),
5815+
};
5816+
Ok(Some(ResolvedWorkloadIdentity {
5817+
uid: owner.uid,
5818+
gid: owner.gid,
5819+
supplementary_gids: Vec::new(),
5820+
source: "vm-config".into(),
5821+
resource_digest,
5822+
}))
5823+
}
5824+
57935825
async fn sandbox_owner_identity_from_image(
57945826
image_path: &Path,
57955827
) -> Result<SandboxOwnerIdentity, String> {
@@ -7504,6 +7536,172 @@ mod tests {
75047536
}
75057537
}
75067538

7539+
async fn assert_inactive_restore_retains_identity(marker: &str, reason: &str) {
7540+
let temp = tempfile::tempdir().unwrap();
7541+
let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new());
7542+
driver.config.state_dir = temp.path().to_path_buf();
7543+
// Current driver defaults and serialized status are not evidence of
7544+
// the identity that owns an already prepared overlay.
7545+
driver.config.sandbox_uid = Some(9000);
7546+
driver.config.sandbox_gid = Some(9001);
7547+
let sandbox = Sandbox {
7548+
id: "sb-inactive-identity".into(),
7549+
name: "inactive-identity".into(),
7550+
status: Some(SandboxStatus {
7551+
resolved_identity: Some(ResolvedWorkloadIdentity {
7552+
uid: 8000,
7553+
gid: 8001,
7554+
..Default::default()
7555+
}),
7556+
..Default::default()
7557+
}),
7558+
..Default::default()
7559+
};
7560+
let state_dir = sandboxes_root_dir(temp.path()).join(&sandbox.id);
7561+
tokio::fs::create_dir_all(&state_dir).await.unwrap();
7562+
write_sandbox_request(&state_dir, &sandbox).await.unwrap();
7563+
write_sandbox_owner_state(
7564+
&state_dir,
7565+
SandboxOwnerIdentity {
7566+
uid: 4242,
7567+
gid: 4343,
7568+
},
7569+
)
7570+
.await
7571+
.unwrap();
7572+
write_sandbox_image_metadata(&state_dir, "unused-image", "sha256:original-image")
7573+
.await
7574+
.unwrap();
7575+
tokio::fs::write(state_dir.join(marker), b"inactive\n")
7576+
.await
7577+
.unwrap();
7578+
let mut events = driver.events.subscribe();
7579+
7580+
driver.restore_persisted_sandboxes().await;
7581+
7582+
let restored = driver
7583+
.get_sandbox(&sandbox.id, &sandbox.name)
7584+
.await
7585+
.unwrap()
7586+
.unwrap();
7587+
let status = restored.status.as_ref().unwrap();
7588+
assert_eq!(status.conditions[0].reason, reason);
7589+
let identity = status
7590+
.resolved_identity
7591+
.as_ref()
7592+
.expect("restored overlay owner");
7593+
assert_eq!((identity.uid, identity.gid), (4242, 4343));
7594+
assert!(identity.supplementary_gids.is_empty());
7595+
assert_eq!(identity.source, "vm-config");
7596+
assert_eq!(identity.resource_digest, "sha256:original-image");
7597+
let registry = driver.registry.lock().await;
7598+
let record = registry.get(&sandbox.id).unwrap();
7599+
assert!(record.process.is_none());
7600+
assert!(record.provisioning_task.is_none());
7601+
drop(registry);
7602+
let event = events.try_recv().expect("restored snapshot event");
7603+
let Some(watch_sandboxes_event::Payload::Sandbox(event)) = event.payload else {
7604+
panic!("expected sandbox snapshot event");
7605+
};
7606+
assert_eq!(event.sandbox.as_ref(), Some(&restored));
7607+
assert_eq!(
7608+
tokio::fs::read_to_string(state_dir.join(SANDBOX_OWNER_STATE_FILE))
7609+
.await
7610+
.unwrap(),
7611+
"sandbox-owner-v2:4242:4343\n"
7612+
);
7613+
assert!(state_dir.join(marker).exists());
7614+
}
7615+
7616+
#[tokio::test]
7617+
async fn stopped_restore_retains_persisted_workload_identity() {
7618+
assert_inactive_restore_retains_identity(SANDBOX_STOPPED_FILE, "ComputeStopped").await;
7619+
}
7620+
7621+
#[tokio::test]
7622+
async fn terminal_restore_retains_persisted_workload_identity() {
7623+
assert_inactive_restore_retains_identity(MAIN_PROCESS_EXITED_FILE, "ProcessExited").await;
7624+
}
7625+
7626+
#[tokio::test]
7627+
async fn inactive_restore_keeps_invalid_metadata_manageable() {
7628+
for (marker, reason) in [
7629+
(SANDBOX_STOPPED_FILE, "ComputeStopped"),
7630+
(MAIN_PROCESS_EXITED_FILE, "ProcessExited"),
7631+
] {
7632+
for invalid_owner in [true, false] {
7633+
let temp = tempfile::tempdir().unwrap();
7634+
let mut driver = test_driver_with_extensions(LifecycleExtensionRegistry::new());
7635+
driver.config.state_dir = temp.path().to_path_buf();
7636+
let sandbox = Sandbox {
7637+
id: "sb-invalid-owner".into(),
7638+
name: "invalid-owner".into(),
7639+
..Default::default()
7640+
};
7641+
let state_dir = sandboxes_root_dir(temp.path()).join(&sandbox.id);
7642+
tokio::fs::create_dir_all(&state_dir).await.unwrap();
7643+
write_sandbox_request(&state_dir, &sandbox).await.unwrap();
7644+
tokio::fs::write(state_dir.join(marker), b"inactive\n")
7645+
.await
7646+
.unwrap();
7647+
let owner = if invalid_owner {
7648+
"sandbox-owner-v2:0:4343\n"
7649+
} else {
7650+
"sandbox-owner-v2:4242:4343\n"
7651+
};
7652+
tokio::fs::write(state_dir.join(SANDBOX_OWNER_STATE_FILE), owner)
7653+
.await
7654+
.unwrap();
7655+
if !invalid_owner {
7656+
// A directory deterministically fails read_to_string on every test host.
7657+
tokio::fs::create_dir(state_dir.join(IMAGE_IDENTITY_FILE))
7658+
.await
7659+
.unwrap();
7660+
}
7661+
let mut events = driver.events.subscribe();
7662+
7663+
driver.restore_persisted_sandboxes().await;
7664+
7665+
let restored = driver
7666+
.get_sandbox(&sandbox.id, &sandbox.name)
7667+
.await
7668+
.unwrap()
7669+
.expect("inactive sandbox remains manageable");
7670+
let status = restored.status.as_ref().unwrap();
7671+
assert_eq!(status.conditions[0].reason, reason);
7672+
assert!(status.resolved_identity.is_none());
7673+
let registry = driver.registry.lock().await;
7674+
let record = registry.get(&sandbox.id).unwrap();
7675+
assert!(record.process.is_none());
7676+
assert!(record.provisioning_task.is_none());
7677+
drop(registry);
7678+
let event = events.try_recv().expect("inactive snapshot event");
7679+
let Some(watch_sandboxes_event::Payload::Sandbox(event)) = event.payload else {
7680+
panic!("expected sandbox snapshot");
7681+
};
7682+
assert_eq!(event.sandbox.as_ref(), Some(&restored));
7683+
assert!(state_dir.join(marker).exists());
7684+
assert_eq!(
7685+
tokio::fs::read_to_string(state_dir.join(SANDBOX_OWNER_STATE_FILE))
7686+
.await
7687+
.unwrap(),
7688+
owner
7689+
);
7690+
assert!(
7691+
driver
7692+
.delete_sandbox(&sandbox.id, &sandbox.name)
7693+
.await
7694+
.unwrap()
7695+
.deleted
7696+
);
7697+
assert!(
7698+
!state_dir.exists(),
7699+
"explicit delete removes retained state"
7700+
);
7701+
}
7702+
}
7703+
}
7704+
75077705
#[tokio::test]
75087706
async fn startup_does_not_restore_terminal_canonical_process() {
75097707
let temp = tempfile::tempdir().unwrap();
@@ -7538,6 +7736,10 @@ mod tests {
75387736
assert!(record.process.is_none());
75397737
assert!(record.provisioning_task.is_none());
75407738
let status = record.snapshot.status.as_ref().expect("terminal status");
7739+
assert!(
7740+
status.resolved_identity.is_none(),
7741+
"an absent owner marker must not invent an identity"
7742+
);
75417743
assert!(status.conditions.iter().any(|condition| {
75427744
condition.reason == "ProcessExited" && condition.status == "False"
75437745
}));

‎skills/generate-sandbox-policy/SKILL.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,8 @@ When the user explicitly requests `process.run_as_user` or
514514
(`4294967295`). Warn that a low numeric identity inherits permissions granted
515515
to the same ID on image files, mounted volumes, or devices.
516516

517+
For MicroVM, numeric selectors must match the resolved owner of the sandbox's writable overlay. User and group are checked independently; either mismatch prevents startup. If the owner UID:GID is unknown, omit the selectors or use `sandbox` so the driver retains that identity. Do not choose another numeric identity or suggest that the policy can change an existing overlay's owner. For example, with an owner of `1000:1000`, a request for UID `10000` must be rejected with an explanation and the omission/`sandbox` alternatives, rather than generating a policy that the VM cannot start.
518+
517519
If the user provides a file path, write to it. Otherwise, ask where to place it. A common convention is a project-local policy file (e.g., `sandbox-policy.yaml`) passed to `openshell sandbox create --policy <path>` or set via the `OPENSHELL_SANDBOX_POLICY` env var.
518520

519521
### Mode C: Present Only (no file write)

0 commit comments

Comments
 (0)