Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/sandboxes/secrets.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ In the CLI form, `ENV@HOST[,HOST...]` records a host-side source reference: the
<Tooltip tip="modify is not yet available on microsandbox cloud; recreate the sandbox to change its secrets."><span className="msb-badge-local">Local-only <Icon icon="circle-info" size={11} /></span></Tooltip>
Rotate or remove existing secrets without a restart. Adding a secret or changing its guest-visible placeholder requires a restart so the new environment reaches the guest. Later rotations keep that placeholder stable and only change the value injected at the network boundary.

Adding a secret to a sandbox that has TLS interception off turns it on, the same way `secret` does at create time — secrets are substituted by the TLS proxy, so interception is what makes them work at all. Interception cannot start on a running sandbox, so that change is restart-backed too and appears in the plan as `tls`. Removing every secret leaves interception on, since it may have been enabled for reasons of its own.

<CodeGroup>
```rust Rust
let plan = sb.modify()
Expand Down
3 changes: 3 additions & 0 deletions docs/sandboxes/tuning.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ The plan labels each change as `live`, `next start`, `requires restart`, or `uns
| `labels` | Live | Host-side metadata; no guest process changes |
| `env`, `workdir` | Future execs | Running processes keep what they already have |
| `secrets` | Live for rotation | Placeholder changes need a restart |
| `tls` | Restart or next start | Configuring a secret turns interception on, and interception cannot start on a running sandbox |
| Root disk size | Restart or next start | Managed and flat OCI disks grow only; tmpfs changes on next boot |
| Other storage | Create or mount time | Named volumes, mount tmpfs, and user disk images are sized outside `modify` |

Expand Down Expand Up @@ -332,6 +333,8 @@ msb modify worker --secret-rm OLD_TOKEN
```
</CodeGroup>

Configuring a secret on a sandbox with TLS interception off also turns it on, since the proxy is what substitutes the value. That shows up in the plan as a `tls` change and needs a restart or the next start. Removing every secret leaves interception on.

For the credential model and host allow lists, see [Secrets](/sandboxes/secrets).

## Storage
Expand Down
4 changes: 1 addition & 3 deletions sdk/rust/lib/sandbox/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,9 +861,7 @@ impl SandboxBuilder {
match self.config.local_network_config() {
Ok(mut network) => {
network.secrets.secrets.push(entry);
if !network.tls.enabled {
network.tls.enabled = true;
}
super::config::ensure_tls_for_secrets(&mut network);
if let Err(err) = self.config.set_local_network_config(network)
&& self.build_error.is_none()
{
Expand Down
55 changes: 55 additions & 0 deletions sdk/rust/lib/sandbox/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,25 @@ pub(crate) fn network_config_from_spec(
Ok(serde_json::from_value(serde_json::to_value(spec)?)?)
}

/// Enforce the invariant that a non-empty secret set requires TLS
/// interception, returning whether `tls.enabled` had to be flipped. The
/// proxy is what substitutes secrets, so without interception the
/// placeholder reaches the upstream unchanged.
///
/// One-way: emptying the secret set leaves interception on, since
/// `TlsConfig::enabled` records no provenance and callers enable it
/// independently of secrets.
#[cfg(feature = "net")]
pub(crate) fn ensure_tls_for_secrets(
network: &mut microsandbox_network::config::NetworkConfig,
) -> bool {
if network.secrets.secrets.is_empty() || network.tls.enabled {
return false;
}
network.tls.enabled = true;
true
}

#[cfg(feature = "net")]
impl SandboxConfig {
pub(crate) fn local_network_config(
Expand Down Expand Up @@ -1739,6 +1758,42 @@ mod tests {
//----------------------------------------------------------------------------------------------
// Tests: Secret source references (create path + spawn resolution)
//----------------------------------------------------------------------------------------------
#[cfg(feature = "net")]
#[test]
fn ensure_tls_for_secrets_enables_interception_for_a_non_empty_set() {
use microsandbox_network::secrets::config::{HostPattern, SecretEntry};

let mut network = microsandbox_network::config::NetworkConfig::default();
assert!(!network.tls.enabled);

network.secrets.secrets.push(SecretEntry {
env_var: "API_KEY".into(),
value: zeroize::Zeroizing::new(String::new()),
source: None,
placeholder: "$MSB_API_KEY".into(),
allowed_hosts: vec![HostPattern::Exact("api.example.com".into())],
injection: Default::default(),
on_violation: None,
require_tls_identity: true,
});
assert!(super::ensure_tls_for_secrets(&mut network));
assert!(network.tls.enabled);
assert!(!super::ensure_tls_for_secrets(&mut network));
}

/// One-way: an empty set never enables interception, and never disables
/// interception enabled for other reasons.
#[cfg(feature = "net")]
#[test]
fn ensure_tls_for_secrets_leaves_an_empty_set_alone() {
let mut network = microsandbox_network::config::NetworkConfig::default();
assert!(!super::ensure_tls_for_secrets(&mut network));
assert!(!network.tls.enabled);

network.tls.enabled = true;
assert!(!super::ensure_tls_for_secrets(&mut network));
assert!(network.tls.enabled);
}

#[cfg(feature = "net")]
const SECRET_SENTINEL: &str = "sentinel-secret-value";
Expand Down
228 changes: 228 additions & 0 deletions sdk/rust/lib/sandbox/modify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ const FUTURE_EXECS_ONLY: &str =
const SECRETS_UNAVAILABLE_WITHOUT_NET: &str =
"secret modification requires a build with the net feature";
const SECRET_FIELD: &str = "secret";
const TLS_FIELD: &str = "tls";
const TLS_INTERCEPTION_REQUIRES_RESTART: &str =
"secrets require TLS interception, which cannot be enabled on a running sandbox";
const ROOT_DISK_FIELD: &str = "root_disk_size";
const ENV_FIELD: &str = "env";
const LABEL_FIELD: &str = "label";
Expand Down Expand Up @@ -294,6 +297,12 @@ impl SandboxModificationBuilder {
/// socket; the durable config records host-side source references for
/// source-based specs and persists the value for value-based specs (the
/// same at-rest property as create's `secret_env`).
///
/// Configuring a secret on a sandbox with TLS interception off also
/// turns interception on, matching create's `secret` — secrets are
/// substituted by the TLS proxy, so they do nothing without it. That is
/// planned as a `tls` change and, like every other restart-backed change,
/// needs `restart` or `next_start` on a running sandbox.
pub async fn apply(self) -> MicrosandboxResult<SandboxModificationPlan> {
let handle = self
.backend
Expand Down Expand Up @@ -967,6 +976,10 @@ fn apply_secret_patch_to_config(
.secrets
.secrets
.retain(|entry| !patch.secrets_remove.contains(&entry.env_var));
// Same invariant create upholds in `SandboxBuilder::secret_entry`. The
// planner emits a matching `tls` change under the same condition, so the
// plan cannot disagree with what is persisted.
super::config::ensure_tls_for_secrets(&mut network);
// Enforce env-var and placeholder shape rules before anything persists;
// validation errors carry entry indexes and sizes, never values.
network.secrets.validate().map_err(|err| {
Expand Down Expand Up @@ -1657,6 +1670,22 @@ fn push_secret_changes(
reason,
}));
}

// Surface the implied TLS enable as its own change rather than flipping
// a config field invisibly: it keeps the dry-run honest and routes the
// patch through the restart path, the only way interception can start.
// Removal-only patches enable nothing, so they emit nothing.
if !patch.secrets.is_empty() && !tls_interception_enabled(config) {
changes.push(spec_change(
TLS_FIELD,
ChangeKind::Updated,
Some("interception disabled".to_string()),
Some("interception enabled".to_string()),
status,
policy,
TLS_INTERCEPTION_REQUIRES_RESTART,
));
}
}

/// Infer what a declarative secret spec changes by diffing it against the
Expand Down Expand Up @@ -2237,6 +2266,23 @@ fn existing_secret_from_network_config(
None
}

/// Whether the config already has TLS interception on. An unreadable network
/// config counts as disabled: a redundant `tls` change beats a secret landing
/// without interception.
#[cfg(feature = "net")]
fn tls_interception_enabled(config: &SandboxConfig) -> bool {
config
.local_network_config()
.map(|network| network.tls.enabled)
.unwrap_or(false)
}

/// Without `net`, secret changes already plan as unsupported; no `tls` on top.
#[cfg(not(feature = "net"))]
fn tls_interception_enabled(_config: &SandboxConfig) -> bool {
true
}

#[cfg(feature = "net")]
fn format_host_pattern(host: microsandbox_network::secrets::config::HostPattern) -> String {
match host {
Expand Down Expand Up @@ -3438,6 +3484,18 @@ mod tests {
on_violation: None,
require_tls_identity: true,
});
// Mirror the invariant every real entry point upholds.
crate::sandbox::config::ensure_tls_for_secrets(&mut network);
config.set_local_network_config(network).unwrap();
config
}

/// The pre-fix shape this bug used to persist: a secret with TLS off.
#[cfg(feature = "net")]
fn config_with_secret_and_tls_disabled(name: &str, value: &str) -> SandboxConfig {
let mut config = config_with_secret(name, value);
let mut network = config.local_network_config().unwrap();
network.tls.enabled = false;
config.set_local_network_config(network).unwrap();
config
}
Expand Down Expand Up @@ -3474,6 +3532,15 @@ mod tests {
}
}

/// The `tls` change a secret patch emits when it must turn interception on.
#[cfg(feature = "net")]
fn tls_plan_change(plan: &SandboxModificationPlan) -> Option<&ConfigPlannedChange> {
plan.changes.iter().find_map(|change| match change {
PlannedChange::Config(change) if change.field == TLS_FIELD => Some(change),
_ => None,
})
}

#[cfg(feature = "net")]
fn secret_plan_dispositions(plan: &SandboxModificationPlan) -> Vec<ModificationDisposition> {
plan.changes
Expand Down Expand Up @@ -3851,6 +3918,167 @@ mod tests {
assert!(conflicts[0].message.contains("needs a name"));
}

/// Regression for #1422: the first secret left `tls.enabled` false, so the
/// placeholder reached the upstream unsubstituted.
#[cfg(feature = "net")]
#[test]
fn adding_first_secret_enables_tls_in_durable_config() {
let mut config = config(2, 1024);
assert!(!config.local_network_config().unwrap().tls.enabled);

let patch = patch_with_specs(vec![source_spec("API_KEY", &["api.example.com"])]);
apply_secret_patch_to_config(&mut config, &patch).unwrap();

let network = config.local_network_config().unwrap();
assert_eq!(network.secrets.secrets.len(), 1);
assert!(network.tls.enabled);
}

/// Interception is restart-backed, so the first secret must show up in the
/// plan and drive the restart rather than flip silently under a running VM.
#[cfg(feature = "net")]
#[test]
fn first_secret_plans_tls_change_and_forces_restart() {
let config = config(2, 1024);
let patch = patch_with_specs(vec![source_spec("API_KEY", &["api.example.com"])]);

// Stopped: lands on the next start.
let plan = build_plan(
"api".to_string(),
SandboxStatus::Stopped,
&config,
None,
LiveControl::default(),
patch.clone(),
ModificationPolicy::NoRestart,
);
let tls = tls_plan_change(&plan).expect("expected a tls change");
assert_eq!(tls.change, ChangeKind::Updated);
assert_eq!(tls.disposition, ModificationDisposition::NextStart);
assert!(validate_apply_supported(&plan).is_ok());

// Running under the default policy: an explicit error.
let plan = build_plan(
"api".to_string(),
SandboxStatus::Running,
&config,
None,
LiveControl {
resize: false,
secrets: true,
},
patch.clone(),
ModificationPolicy::NoRestart,
);
assert_eq!(
tls_plan_change(&plan).unwrap().disposition,
ModificationDisposition::RequiresRestart
);
assert!(validate_apply_supported(&plan).is_err());

// Restart opted in: the restart rebuilds active from the durable config.
let plan = build_plan(
"api".to_string(),
SandboxStatus::Running,
&config,
None,
LiveControl {
resize: false,
secrets: true,
},
patch,
ModificationPolicy::Restart,
);
assert!(validate_apply_supported(&plan).is_ok());
assert!(plan_requires_restart(&plan));
}

/// Rotating a secret on a legacy TLS-off config classifies as live, and
/// mirroring it would claim TLS the running proxy does not have.
#[cfg(feature = "net")]
#[test]
fn live_secret_change_on_tls_disabled_config_requires_restart() {
let config = config_with_secret_and_tls_disabled("API_KEY", SECRET_SENTINEL);
let patch = patch_with_specs(vec![source_spec("API_KEY", &[])]);

let plan = build_plan(
"api".to_string(),
SandboxStatus::Running,
&config,
None,
LiveControl {
resize: false,
secrets: true,
},
patch,
ModificationPolicy::NoRestart,
);

// The rotate itself is live-applicable, so tls is the only blocker.
let secret_dispositions: Vec<_> = plan
.changes
.iter()
.filter_map(|change| match change {
PlannedChange::Secret(change) => Some(change.disposition),
PlannedChange::Config(_) => None,
})
.collect();
assert_eq!(secret_dispositions, vec![ModificationDisposition::Live]);
assert_eq!(
tls_plan_change(&plan).unwrap().disposition,
ModificationDisposition::RequiresRestart
);
let err = validate_apply_supported(&plan).unwrap_err().to_string();
assert_eq!(err, "cannot apply modification: tls requires restart");
}

/// One-way: emptying the secret set must not turn interception off.
#[cfg(feature = "net")]
#[test]
fn removing_last_secret_keeps_tls_enabled() {
let mut config = config_with_secret("API_KEY", SECRET_SENTINEL);
let patch = SandboxModificationPatch {
secrets_remove: vec!["API_KEY".to_string()],
..SandboxModificationPatch::default()
};

let plan = build_plan(
"api".to_string(),
SandboxStatus::Stopped,
&config,
None,
LiveControl::default(),
patch.clone(),
ModificationPolicy::NoRestart,
);
assert!(tls_plan_change(&plan).is_none());

apply_secret_patch_to_config(&mut config, &patch).unwrap();

let network = config.local_network_config().unwrap();
assert!(network.secrets.secrets.is_empty());
assert!(network.tls.enabled);
}

/// Interception already on: nothing to enable, so no extra plan noise.
#[cfg(feature = "net")]
#[test]
fn secret_change_with_tls_already_enabled_plans_no_tls_change() {
let config = config_with_secret("API_KEY", SECRET_SENTINEL);
let patch = patch_with_specs(vec![source_spec("API_KEY", &[])]);

let plan = build_plan(
"api".to_string(),
SandboxStatus::Stopped,
&config,
None,
LiveControl::default(),
patch,
ModificationPolicy::NoRestart,
);

assert!(tls_plan_change(&plan).is_none());
}
#[cfg(feature = "net")]
#[test]
fn applying_new_source_spec_uses_create_placeholder_default() {
Expand Down