fix(providers): reserve credential placeholder revisions (#2049)

* fix(providers): reserve credential placeholder revisions

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* fix(providers): share placeholder namespace parser

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* test(providers): cover non-revision env key

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

---------

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>
Co-authored-by: John Myers <johntmyers@users.noreply.github.com>

Author: johntmyers

architecture/sandbox.md

@@ -95,7 +95,9 @@ Credential placeholders in proxied HTTP requests can be resolved by the proxy
 when policy allows the target endpoint. For GCP providers, a loopback metadata
 server inside the network namespace serves placeholders to SDKs that bypass the
 proxy (e.g. Go's `cloud.google.com/go/compute/metadata`). Secrets must not be
-logged in OCSF or plain tracing output.
+logged in OCSF or plain tracing output. The supervisor uses revision-scoped
+placeholders for rotating provider credentials; provider environment keys
+beginning with `v<digits>_` are reserved for that placeholder namespace.
 
 Provider profiles can also declare dynamic token grants. For matching HTTP
 endpoints, the supervisor obtains a SPIFFE JWT-SVID from the local Workload API,

crates/openshell-core/src/provider_credentials.rs

@@ -593,4 +593,54 @@ mod tests {
             "suppressed key must not reappear after install_environment"
         );
     }
+
+    #[test]
+    fn stale_generation_falls_back_to_current_credential_after_retention_window() {
+        let state = ProviderCredentialState::from_environment(
+            10,
+            HashMap::from([("GITHUB_TOKEN".to_string(), "old".to_string())]),
+            HashMap::new(),
+            HashMap::new(),
+        );
+
+        for revision in 11..20 {
+            state.install_environment(
+                revision,
+                HashMap::from([("GITHUB_TOKEN".to_string(), format!("new-{revision}"))]),
+                HashMap::new(),
+                HashMap::new(),
+            );
+        }
+
+        let resolver = state.resolver().expect("resolver");
+        assert_eq!(
+            resolver.resolve_placeholder("openshell:resolve:env:v10_GITHUB_TOKEN"),
+            Some("new-19")
+        );
+    }
+
+    #[test]
+    fn stale_removed_generation_fails_closed_after_retention_window() {
+        let state = ProviderCredentialState::from_environment(
+            10,
+            HashMap::from([("GITHUB_TOKEN".to_string(), "old".to_string())]),
+            HashMap::new(),
+            HashMap::new(),
+        );
+
+        for revision in 11..20 {
+            state.install_environment(
+                revision,
+                HashMap::from([("OTHER_TOKEN".to_string(), format!("other-{revision}"))]),
+                HashMap::new(),
+                HashMap::new(),
+            );
+        }
+
+        let resolver = state.resolver().expect("retained resolver");
+        assert_eq!(
+            resolver.resolve_placeholder("openshell:resolve:env:v10_GITHUB_TOKEN"),
+            None
+        );
+    }
 }

crates/openshell-core/src/secrets.rs

@@ -177,6 +177,13 @@ impl SecretResolver {
         let mut by_placeholder = HashMap::with_capacity(provider_env.len());
 
         for (key, value) in provider_env {
+            if uses_reserved_revision_namespace(&key) {
+                tracing::warn!(
+                    provider_env_key = %key,
+                    "skipping provider credential env var in reserved placeholder namespace"
+                );
+                continue;
+            }
             let placeholder = placeholder_for_env_key_for_revision(&key, revision);
             let secret = SecretValue {
                 value,
@@ -192,7 +199,11 @@ impl SecretResolver {
             }
         }
 
-        (child_env, Some(Self { by_placeholder }))
+        if by_placeholder.is_empty() {
+            (child_env, None)
+        } else {
+            (child_env, Some(Self { by_placeholder }))
+        }
     }
 
     pub fn merge<'a>(resolvers: impl IntoIterator<Item = &'a Self>) -> Option<Self> {
@@ -215,7 +226,10 @@ impl SecretResolver {
         let secret = if let Some(secret) = self.by_placeholder.get(value) {
             secret
         } else {
-            let key = alias_env_key(value)?;
+            // Once an old generation ages out, the revision number is only a
+            // namespace marker. Fall back by key to the current credential so
+            // long-running child processes survive provider credential refresh.
+            let key = revisioned_placeholder_env_key(value).or_else(|| alias_env_key(value))?;
             let canonical = placeholder_for_env_key(key);
             self.by_placeholder.get(&canonical)?
         };
@@ -475,6 +489,29 @@ fn alias_env_key(token: &str) -> Option<&str> {
     (key_end == token.len() && key_end > key_start).then_some(&token[key_start..key_end])
 }
 
+fn revisioned_placeholder_env_key(token: &str) -> Option<&str> {
+    let suffix = token.strip_prefix(PLACEHOLDER_PREFIX)?;
+    let (_, key) = split_revisioned_env_key(suffix)?;
+    Some(key)
+}
+
+pub fn uses_reserved_revision_namespace(key: &str) -> bool {
+    split_revisioned_env_key(key).is_some()
+}
+
+fn split_revisioned_env_key(key: &str) -> Option<(&str, &str)> {
+    let suffix = key.strip_prefix('v')?;
+    let (revision, env_key) = suffix.split_once('_')?;
+    if revision.is_empty()
+        || !revision.bytes().all(|b| b.is_ascii_digit())
+        || env_key.is_empty()
+        || !env_key.bytes().all(is_env_key_char)
+    {
+        return None;
+    }
+    Some((revision, env_key))
+}
+
 fn token_boundary_ok(text: &str, abs_start: usize, token_end: usize, token: &str) -> bool {
     if token.starts_with(PLACEHOLDER_PREFIX) {
         return token_end == text.len()
@@ -1050,6 +1087,28 @@ mod tests {
         );
     }
 
+    #[test]
+    fn provider_env_rejects_revision_namespace_keys() {
+        let (child_env, resolver) = SecretResolver::from_provider_env(
+            [("v10_GITHUB_TOKEN".to_string(), "ambiguous".to_string())]
+                .into_iter()
+                .collect(),
+        );
+
+        assert!(child_env.is_empty());
+        assert!(resolver.is_none());
+    }
+
+    #[test]
+    fn reserved_revision_namespace_requires_version_and_key() {
+        assert!(uses_reserved_revision_namespace("v10_GITHUB_TOKEN"));
+        assert!(uses_reserved_revision_namespace("v999999_very_unlikely"));
+        assert!(!uses_reserved_revision_namespace("v_GITHUB_TOKEN"));
+        assert!(!uses_reserved_revision_namespace("v10_"));
+        assert!(!uses_reserved_revision_namespace("very_unlikely"));
+        assert!(!uses_reserved_revision_namespace("GITHUB_TOKEN"));
+    }
+
     #[test]
     fn rewrites_exact_placeholder_header_values() {
         let (_, resolver) = SecretResolver::from_provider_env(
@@ -1083,6 +1142,49 @@ mod tests {
         );
     }
 
+    #[test]
+    fn rewrites_stale_revisioned_bearer_placeholder_to_current_alias() {
+        let (_, resolver) = SecretResolver::from_provider_env_for_revision_with_current_aliases(
+            [("GITHUB_TOKEN".to_string(), "ghp-current".to_string())]
+                .into_iter()
+                .collect(),
+            HashMap::new(),
+            42,
+            true,
+        );
+        let resolver = resolver.expect("resolver");
+
+        assert_eq!(
+            rewrite_header_line_checked(
+                "Authorization: Bearer openshell:resolve:env:v10_GITHUB_TOKEN",
+                &resolver,
+            )
+            .expect("stale revision should fall back to current alias"),
+            "Authorization: Bearer ghp-current"
+        );
+    }
+
+    #[test]
+    fn stale_revisioned_placeholder_fails_when_key_is_unknown() {
+        let (_, resolver) = SecretResolver::from_provider_env_for_revision_with_current_aliases(
+            [("GITHUB_TOKEN".to_string(), "ghp-current".to_string())]
+                .into_iter()
+                .collect(),
+            HashMap::new(),
+            42,
+            true,
+        );
+        let resolver = resolver.expect("resolver");
+
+        assert!(
+            rewrite_header_line_checked(
+                "Authorization: Bearer openshell:resolve:env:v999999_very_unlikely",
+                &resolver,
+            )
+            .is_err()
+        );
+    }
+
     #[test]
     fn rewrites_provider_shaped_alias_header_values() {
         let (_, resolver) = SecretResolver::from_provider_env(

crates/openshell-providers/src/profiles.rs

@@ -11,6 +11,7 @@ use openshell_core::proto::{
     ProviderCredentialRefreshMaterial, ProviderCredentialRefreshStrategy, ProviderProfile,
     ProviderProfileCategory, ProviderProfileCredential, ProviderProfileDiscovery,
 };
+use openshell_core::secrets::uses_reserved_revision_namespace;
 use serde::ser::SerializeStruct;
 use serde::{Deserialize, Deserializer, Serialize, Serializer, de};
 use std::collections::{HashMap, HashSet};
@@ -1167,6 +1168,15 @@ pub fn validate_profile_set(
                         "credentials.env_vars",
                         "credential env var must not be empty",
                     ));
+                } else if uses_reserved_revision_namespace(env_var.trim()) {
+                    diagnostics.push(ProfileValidationDiagnostic::error(
+                        source,
+                        profile_id,
+                        "credentials.env_vars",
+                        format!(
+                            "credential env var '{env_var}' uses reserved OpenShell placeholder revision namespace"
+                        ),
+                    ));
                 } else if !env_vars.insert(env_var.trim().to_string()) {
                     diagnostics.push(ProfileValidationDiagnostic::error(
                         source,
@@ -2526,7 +2536,7 @@ credentials:
     env_vars: [BROKEN_TOKEN]
     auth_style: query
   - name: api_key
-    env_vars: [BROKEN_TOKEN, ""]
+    env_vars: [BROKEN_TOKEN, "", v10_GITHUB_TOKEN]
     auth_style: unknown
   - name: path_key
     env_vars: [PATH_TOKEN]
@@ -2554,6 +2564,11 @@ binaries: ["", /usr/bin/broken]
         assert!(messages.contains(&"duplicate credential name: api_key"));
         assert!(messages.contains(&"duplicate credential env var 'BROKEN_TOKEN'"));
         assert!(messages.contains(&"credential env var must not be empty"));
+        assert!(
+            messages.iter().any(
+                |message| message.contains("reserved OpenShell placeholder revision namespace")
+            )
+        );
         assert!(messages.contains(&"query_param is required for query auth"));
         assert!(messages.contains(&"path_template is required for path auth"));
         assert!(messages.iter().any(|message| {

docs/sandboxes/providers-v2.mdx

@@ -273,7 +273,7 @@ binaries:
 
 `category` groups profiles in `openshell provider list-profiles`. Use one of the values in the category enum.
 
-`credentials` declares the credential names, environment variables, auth metadata, optional refresh metadata, and optional dynamic token grant metadata for the provider type. The `auth_style` field accepts `basic`, `bearer`, `header`, `query`, or `path`. When `auth_style` is `path`, set `path_template` to a URL path containing the `{credential}` placeholder exactly once (for example, `/v1/{credential}/resources`). Static credentials are exposed as placeholder environment variables and resolved in outbound HTTP requests. Dynamic token grants are resolved by the sandbox proxy on demand for matching profile endpoints and support `bearer` or `header` placement.
+`credentials` declares the credential names, environment variables, auth metadata, optional refresh metadata, and optional dynamic token grant metadata for the provider type. The `auth_style` field accepts `basic`, `bearer`, `header`, `query`, or `path`. When `auth_style` is `path`, set `path_template` to a URL path containing the `{credential}` placeholder exactly once (for example, `/v1/{credential}/resources`). Static credentials are exposed as placeholder environment variables and resolved in outbound HTTP requests. Dynamic token grants are resolved by the sandbox proxy on demand for matching profile endpoints and support `bearer` or `header` placement. Credential environment variable names must not use the reserved `v<digits>_` prefix, such as `v10_GITHUB_TOKEN`, because OpenShell uses that namespace for revision-scoped placeholders.
 
 `discovery` controls what `--from-existing` scans when
 `providers_v2_enabled=true`. Each entry in `discovery.credentials` must name a