Skip to content

Commit df02e79

Browse files
committed
fix(providers): retain caller overrides in metadata snapshots
Keep effective caller values for profile-declared non-secret environment destinations so supervisor metadata sees the same project ID as workloads. Apply sandbox environment, template environment, then provider default precedence, preserving explicit empty values and credential ownership. Include effective overrides in the provider environment revision to prevent stale snapshots, and document precedence and independent project aliases. Add regression coverage for missing defaults, credential classification, and revision consistency, plus Google SDK discovery cases for overrides. Validation: 73 Rust tests and all 6 focused Podman SDK E2E cases passed. Gateway Clippy, Rust/Python formatting, documentation, Markdown, and git diff --check passed. Python lint and type-check diagnostics match the committed baseline. No Go SDK changes. Signed-off-by: Philippe Martin <phmartin@redhat.com>
1 parent 24efcc8 commit df02e79

5 files changed

Lines changed: 302 additions & 31 deletions

File tree

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

Lines changed: 190 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3075,13 +3075,19 @@ pub async fn build_sandbox_config_snapshot_from_inputs(
30753075
&policy_credential_bindings,
30763076
)?;
30773077
}
3078+
let environment_overrides = super::provider::non_secret_provider_environment_overrides(
3079+
provider_profile_catalog,
3080+
provider_records,
3081+
sandbox,
3082+
);
30783083
let provider_env_revision = compute_provider_env_revision_from_records_and_policy_bindings(
30793084
provider_profile_catalog,
30803085
provider_records,
30813086
&policy_credential_bindings,
30823087
policy
30833088
.as_ref()
30843089
.filter(|_| matches!(policy_source, PolicySource::Global)),
3090+
&environment_overrides,
30853091
)?;
30863092

30873093
let service_routes = super::service::configured_app_routes(state, sandbox).await?;
@@ -3381,6 +3387,7 @@ fn compute_provider_env_revision_from_records(
33813387
records,
33823388
&HashMap::new(),
33833389
None,
3390+
&BTreeMap::new(),
33843391
)
33853392
}
33863393

@@ -3389,6 +3396,7 @@ fn compute_provider_env_revision_from_records_and_policy_bindings(
33893396
records: &[super::provider::ProviderEnvironmentRecord],
33903397
policy_bindings: &HashMap<String, Vec<StaticCredentialEndpointBinding>>,
33913398
global_policy: Option<&ProtoSandboxPolicy>,
3399+
environment_overrides: &BTreeMap<String, String>,
33923400
) -> Result<u64, Status> {
33933401
let mut hasher = Sha256::new();
33943402
hasher.update(b"openshell-provider-env-revision-v4");
@@ -3432,6 +3440,18 @@ fn compute_provider_env_revision_from_records_and_policy_bindings(
34323440
hasher.update(deterministic_policy_hash(policy).as_bytes());
34333441
}
34343442

3443+
if !environment_overrides.is_empty() {
3444+
// Hash the same effective values that the environment snapshot installs.
3445+
// Length prefixes preserve boundaries, including explicitly empty values.
3446+
hasher.update(b"caller-non-secret-environment");
3447+
for (key, value) in environment_overrides {
3448+
hasher.update((key.len() as u64).to_le_bytes());
3449+
hasher.update(key.as_bytes());
3450+
hasher.update((value.len() as u64).to_le_bytes());
3451+
hasher.update(value.as_bytes());
3452+
}
3453+
}
3454+
34353455
let digest = hasher.finalize();
34363456
Ok(u64::from_le_bytes(digest[..8].try_into().map_err(
34373457
|_| Status::internal("provider env revision digest too short"),
@@ -3898,6 +3918,7 @@ pub async fn build_provider_environment_snapshot(
38983918
/// is resolved.
38993919
struct ProviderEnvironmentTarget<'a> {
39003920
spec: &'a openshell_core::proto::SandboxSpec,
3921+
environment_overrides: BTreeMap<String, String>,
39013922
effective_policy: ProtoSandboxPolicy,
39023923
policy_source: PolicySource,
39033924
policy_credential_bindings: HashMap<String, Vec<StaticCredentialEndpointBinding>>,
@@ -3929,14 +3950,21 @@ fn provider_environment_target(
39293950
&effective_policy,
39303951
&policy_credential_bindings,
39313952
)?;
3953+
let environment_overrides = super::provider::non_secret_provider_environment_overrides(
3954+
provider_profile_catalog,
3955+
provider_records,
3956+
sandbox,
3957+
);
39323958
let provider_env_revision = compute_provider_env_revision_from_records_and_policy_bindings(
39333959
provider_profile_catalog,
39343960
provider_records,
39353961
&policy_credential_bindings,
39363962
matches!(policy_source, PolicySource::Global).then_some(&effective_policy),
3963+
&environment_overrides,
39373964
)?;
39383965
Ok(ProviderEnvironmentTarget {
39393966
spec,
3967+
environment_overrides,
39403968
effective_policy,
39413969
policy_source,
39423970
policy_credential_bindings,
@@ -3955,6 +3983,7 @@ pub async fn build_provider_environment_snapshot_from_inputs(
39553983
let sandbox_id = inputs.sandbox.object_id().to_string();
39563984
let ProviderEnvironmentTarget {
39573985
spec,
3986+
environment_overrides,
39583987
effective_policy,
39593988
policy_source,
39603989
policy_credential_bindings,
@@ -3992,20 +4021,12 @@ pub async fn build_provider_environment_snapshot_from_inputs(
39924021
credential.token_grant_owners.clone_from(&owners);
39934022
}
39944023
}
3995-
// Sandbox template and spec values are caller-owned. Drop only non-secret
3996-
// profile defaults for those keys before sending the snapshot to either
3997-
// the canonical process or later execs. Credential keys keep their
3998-
// existing binding and placeholder precedence.
3999-
let caller_has_key = |key: &str| {
4000-
spec.environment.contains_key(key)
4001-
|| spec
4002-
.template
4003-
.as_ref()
4004-
.is_some_and(|template| template.environment.contains_key(key))
4005-
};
4006-
provider_environment.environment.retain(|key, _| {
4007-
provider_environment.static_credential_keys.contains(key) || !caller_has_key(key)
4008-
});
4024+
// Retain effective non-secret values for both process environments and
4025+
// supervisor consumers such as metadata. Credential keys are excluded
4026+
// from overrides and keep their binding and placeholder precedence.
4027+
provider_environment
4028+
.environment
4029+
.extend(environment_overrides);
40094030

40104031
let mut readiness_reason = provider_environment.readiness_reason;
40114032

@@ -14460,6 +14481,10 @@ mod tests {
1446014481
.environment
1446114482
.config
1446214483
.insert("CUSTOM_PROJECT".to_string(), "project".to_string());
14484+
profile
14485+
.environment
14486+
.config
14487+
.insert("CUSTOM_MISSING".to_string(), "missing".to_string());
1446314488
profile
1446414489
.environment
1446514490
.fixed
@@ -14492,17 +14517,39 @@ mod tests {
1449214517
.insert("CUSTOM_PROJECT".to_string(), "caller-project".to_string());
1449314518
spec.environment
1449414519
.insert("GITHUB_TOKEN".to_string(), "caller-token".to_string());
14495-
spec.template
14496-
.get_or_insert_with(Default::default)
14497-
.environment
14498-
.insert("CUSTOM_MODE".to_string(), String::new());
14520+
spec.environment
14521+
.insert("CUSTOM_MISSING".to_string(), " caller value ".to_string());
14522+
spec.environment
14523+
.insert("UNRELATED".to_string(), "caller-only".to_string());
14524+
let template = spec.template.get_or_insert_with(Default::default);
14525+
template.environment = HashMap::from([
14526+
("CUSTOM_PROJECT".to_string(), "template-project".to_string()),
14527+
("CUSTOM_MODE".to_string(), String::new()),
14528+
]);
1449914529
state.store.put_message(&sandbox).await.unwrap();
1450014530

1450114531
let snapshot = load_sandbox_provider_environment(&state, &sandbox, true, true)
1450214532
.await
1450314533
.unwrap();
14504-
assert!(!snapshot.environment.contains_key("CUSTOM_PROJECT"));
14505-
assert!(!snapshot.environment.contains_key("CUSTOM_MODE"));
14534+
assert_eq!(
14535+
snapshot
14536+
.environment
14537+
.get("CUSTOM_PROJECT")
14538+
.map(String::as_str),
14539+
Some("caller-project")
14540+
);
14541+
assert_eq!(
14542+
snapshot.environment.get("CUSTOM_MODE").map(String::as_str),
14543+
Some("")
14544+
);
14545+
assert_eq!(
14546+
snapshot
14547+
.environment
14548+
.get("CUSTOM_MISSING")
14549+
.map(String::as_str),
14550+
Some(" caller value ")
14551+
);
14552+
assert!(!snapshot.environment.contains_key("UNRELATED"));
1450614553
assert_eq!(
1450714554
snapshot.environment.get("CUSTOM_UNSET").map(String::as_str),
1450814555
Some("default")
@@ -14516,7 +14563,129 @@ mod tests {
1451614563
.static_credential_bindings
1451714564
.contains_key("GITHUB_TOKEN")
1451814565
);
14519-
assert_eq!(snapshot.non_secret_environment_keys, ["CUSTOM_UNSET"]);
14566+
assert_eq!(
14567+
snapshot.non_secret_environment_keys,
14568+
[
14569+
"CUSTOM_MISSING",
14570+
"CUSTOM_MODE",
14571+
"CUSTOM_PROJECT",
14572+
"CUSTOM_UNSET"
14573+
]
14574+
);
14575+
}
14576+
14577+
#[tokio::test]
14578+
async fn caller_environment_revision_tracks_effective_non_secret_values() {
14579+
let state = test_server_state().await;
14580+
let mut provider = test_provider("work-gcp", "google-cloud");
14581+
provider.credentials.clear();
14582+
state.store.put_message(&provider).await.unwrap();
14583+
let sandbox = test_sandbox(
14584+
"sb-caller-revision",
14585+
"caller-revision",
14586+
ProtoSandboxPolicy::default(),
14587+
vec!["work-gcp".to_string()],
14588+
);
14589+
let mut inputs = load_sandbox_config_inputs(&state, sandbox).await.unwrap();
14590+
let mut revisions = Vec::new();
14591+
for (spec_value, template_value, unrelated, expected) in [
14592+
(None, None, "one", None),
14593+
(
14594+
None,
14595+
Some("template-project"),
14596+
"one",
14597+
Some("template-project"),
14598+
),
14599+
(
14600+
Some("caller-project"),
14601+
Some("template-project"),
14602+
"one",
14603+
Some("caller-project"),
14604+
),
14605+
(
14606+
Some("caller-project"),
14607+
Some("shadowed-project"),
14608+
"two",
14609+
Some("caller-project"),
14610+
),
14611+
(Some(""), Some("template-project"), "one", Some("")),
14612+
] {
14613+
let spec = inputs.sandbox.spec.as_mut().unwrap();
14614+
spec.environment = HashMap::from([("UNRELATED".to_string(), unrelated.to_string())]);
14615+
if let Some(value) = spec_value {
14616+
spec.environment
14617+
.insert("GCP_PROJECT_ID".to_string(), value.to_string());
14618+
}
14619+
spec.template
14620+
.get_or_insert_with(Default::default)
14621+
.environment = template_value
14622+
.map(|value| HashMap::from([("GCP_PROJECT_ID".to_string(), value.to_string())]))
14623+
.unwrap_or_default();
14624+
let config = build_sandbox_config_snapshot_from_inputs(&state, &inputs)
14625+
.await
14626+
.unwrap();
14627+
let snapshot =
14628+
build_provider_environment_snapshot_from_inputs(&state, &inputs, true, true)
14629+
.await
14630+
.unwrap();
14631+
assert_eq!(config.provider_env_revision, snapshot.provider_env_revision);
14632+
let value = snapshot
14633+
.values
14634+
.iter()
14635+
.find(|value| value.name == "GCP_PROJECT_ID");
14636+
assert_eq!(value.map(|value| value.value.as_str()), expected);
14637+
if let Some(value) = value {
14638+
assert_eq!(
14639+
value.classification,
14640+
ProviderEnvironmentValueClassification::NonSecret as i32
14641+
);
14642+
}
14643+
revisions.push(snapshot.provider_env_revision);
14644+
}
14645+
assert_ne!(revisions[0], revisions[1]);
14646+
assert_ne!(revisions[1], revisions[2]);
14647+
assert_eq!(revisions[2], revisions[3]);
14648+
assert_ne!(revisions[2], revisions[4]);
14649+
assert_ne!(revisions[0], revisions[4]);
14650+
}
14651+
14652+
#[tokio::test]
14653+
async fn caller_environment_does_not_reclassify_expired_credentials_as_non_secret() {
14654+
let state = test_server_state().await;
14655+
let mut provider = test_provider("work-gcp", "google-cloud");
14656+
provider.credentials.clear();
14657+
state.store.put_message(&provider).await.unwrap();
14658+
let mut legacy_provider = test_provider("legacy-project", "legacy-custom");
14659+
legacy_provider.credentials =
14660+
HashMap::from([("GCP_PROJECT_ID".to_string(), "expired-secret".to_string())]);
14661+
legacy_provider.credential_expiration_times.insert(
14662+
"GCP_PROJECT_ID".to_string(),
14663+
openshell_core::time::timestamp_from_millis(1).unwrap(),
14664+
);
14665+
state.store.put_message(&legacy_provider).await.unwrap();
14666+
let mut sandbox = test_sandbox(
14667+
"sb-caller-expired",
14668+
"caller-expired",
14669+
ProtoSandboxPolicy::default(),
14670+
vec!["work-gcp".to_string(), "legacy-project".to_string()],
14671+
);
14672+
sandbox
14673+
.spec
14674+
.as_mut()
14675+
.unwrap()
14676+
.environment
14677+
.insert("GCP_PROJECT_ID".to_string(), "caller-project".to_string());
14678+
14679+
let snapshot = load_sandbox_provider_environment(&state, &sandbox, true, true)
14680+
.await
14681+
.unwrap();
14682+
assert!(!snapshot.environment.contains_key("GCP_PROJECT_ID"));
14683+
assert!(
14684+
!snapshot
14685+
.non_secret_environment_keys
14686+
.iter()
14687+
.any(|key| key == "GCP_PROJECT_ID")
14688+
);
1452014689
}
1452114690

1452214691
#[tokio::test]

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

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use openshell_policy::ProviderPolicyLayer;
3838
use openshell_providers::LEGACY_VERTEX_PRIVATE_KEY_ENV;
3939
use prost::Message;
4040
use sha2::{Digest, Sha256};
41-
use std::collections::{HashMap, HashSet};
41+
use std::collections::{BTreeMap, HashMap, HashSet};
4242
use tonic::Status;
4343
use tracing::warn;
4444

@@ -2253,6 +2253,57 @@ fn validate_provider_environment_records_unique_at(
22532253
Ok(())
22542254
}
22552255

2256+
/// Effective caller values for declared non-secret destinations only. Reserve
2257+
/// credential keys even when their values are missing, expired, or withheld.
2258+
pub(super) fn non_secret_provider_environment_overrides(
2259+
catalog: &EffectiveProviderProfileCatalog,
2260+
records: &[ProviderEnvironmentRecord],
2261+
sandbox: &Sandbox,
2262+
) -> BTreeMap<String, String> {
2263+
let Some(spec) = sandbox.spec.as_ref() else {
2264+
return BTreeMap::new();
2265+
};
2266+
let mut destinations = HashSet::new();
2267+
let mut credential_keys = reserved_bootstrap_credential_keys();
2268+
for record in records {
2269+
let provider = &record.provider;
2270+
credential_keys.extend(provider.credentials.keys().cloned());
2271+
credential_keys.extend(provider.credential_handles.keys().cloned());
2272+
for state in &record.refresh_states {
2273+
credential_keys.insert(state.credential_key.clone());
2274+
credential_keys.extend(state.additional_output_keys.values().cloned());
2275+
}
2276+
if let Some(profile) = get_provider_type_profile_for_scope(
2277+
catalog,
2278+
&provider.r#type,
2279+
&provider.profile_workspace,
2280+
) {
2281+
destinations.extend(profile.environment.config.keys().cloned());
2282+
destinations.extend(profile.environment.fixed.keys().cloned());
2283+
credential_keys.extend(
2284+
profile
2285+
.credentials
2286+
.iter()
2287+
.flat_map(|credential| credential.accepted_stored_keys())
2288+
.map(str::to_string),
2289+
);
2290+
credential_keys.extend(broker_only_provider_credential_keys(&profile.to_proto()));
2291+
}
2292+
}
2293+
destinations
2294+
.into_iter()
2295+
.filter(|key| is_valid_env_key(key) && !credential_keys.contains(key))
2296+
.filter_map(|key| {
2297+
let value = spec.environment.get(&key).or_else(|| {
2298+
spec.template
2299+
.as_ref()
2300+
.and_then(|template| template.environment.get(&key))
2301+
})?;
2302+
Some((key, value.clone()))
2303+
})
2304+
.collect()
2305+
}
2306+
22562307
fn provider_profile_environment_values(
22572308
catalog: &EffectiveProviderProfileCatalog,
22582309
provider: &Provider,

‎docs/how-it-works/providers/google.mdx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,13 @@ as placeholders for proxy-time resolution.
172172
| `CLOUD_ML_REGION` | from `region` config | GCP region |
173173
| `GCP_LOCATION` | from `region` config | Alternative region var |
174174

175+
For each non-secret variable, a sandbox create-time value overrides its template
176+
value, which overrides the provider default. The metadata project-ID endpoint
177+
uses the effective `GCP_PROJECT_ID`, including an explicitly empty value. A caller
178+
can supply it even when the provider has no `project_id` config. OpenShell resolves
179+
aliases independently; set both `GCP_PROJECT_ID` and `GOOGLE_CLOUD_PROJECT` when
180+
overriding the project so SDKs that read different variables agree.
181+
175182
## Using with GCP APIs
176183

177184
The metadata emulator serves tokens with the `cloud-platform` OAuth2 scope,

0 commit comments

Comments
 (0)