fix(grpc): allow credential rotation when legacy provider.type exceeds current limit (#1350)

Closes #1347. Reported by @KodeDaemon in the NVIDIA Developer Discord after
upgrading NemoClaw 0.0.38 -> 0.0.39 left the inference provider stranded
with `provider.type exceeds maximum length (79 > 64)` on every credential
update.

`update_provider_record` rebuilt the merged Provider from `existing.r#type`
and ran `validate_provider_fields` over it. The CLI's `provider update`
sends `r#type: ""`, so the existing value is preserved without modification,
but the validator still measured it. Any record whose stored type predates
current MAX_PROVIDER_TYPE_LEN (or was written by a path that bypassed
validation, e.g. the TUI form which forwards r#type to the request without
going through normalize_provider_type) became unupdateable: the only escape
was `provider delete` + recreate, which loses any provider.config entries
the caller never sees.

Split the validator into create-time (validate_provider_fields, full check
including immutable name/type) and update-time (validate_provider_mutable_fields,
checks only credentials/config maps). The update path validates only what
the caller is mutating; immutable fields carried forward from existing are
trusted because they passed validation when stored.

Tests: new regression in `update_provider_allows_credential_rotation_on_legacy_oversized_type`
inserts a Provider with a 79-char type directly via `store.put_message`
(bypassing gRPC validation), updates a credential, asserts the update
succeeds and the original type is preserved. Existing
`update_provider_validates_merged_result` still covers the case where
incoming credential entries push the merged map past limits.

Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
Co-authored-by: latenighthackathon <latenighthackathon@users.noreply.github.com>

Author: latenighthackathon

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

@@ -16,7 +16,7 @@ use prost::Message;
 use tonic::Status;
 use tracing::warn;
 
-use super::validation::validate_provider_fields;
+use super::validation::{validate_provider_fields, validate_provider_mutable_fields};
 use super::{
     MAX_MAP_KEY_LEN, MAX_MAP_VALUE_LEN, MAX_PAGE_SIZE, MAX_PROVIDER_CONFIG_ENTRIES, clamp_limit,
 };
@@ -218,9 +218,14 @@ pub(super) async fn update_provider_record(
         provider.credential_expires_at_ms,
     );
 
-    // Validate BEFORE writing to prevent persisting invalid state
+    // Validate BEFORE writing to prevent persisting invalid state.
+    // Validate only the mutable fields (credentials/config) plus metadata and
+    // attached-sandbox invariants. The immutable name/type are carried forward
+    // from `existing` and re-running full `validate_provider_fields` here would
+    // strand legacy records whose stored type predates current limits. See
+    // #1347.
     super::validation::validate_object_metadata(candidate.metadata.as_ref(), "provider")?;
-    validate_provider_fields(&candidate)?;
+    validate_provider_mutable_fields(&candidate)?;
     validate_provider_update_against_attached_sandboxes(store, &candidate).await?;
 
     // Serialize labels for storage
@@ -1602,8 +1607,8 @@ fn telemetry_provider_profile(provider_type: &str) -> TelemetryProviderProfile {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::grpc::MAX_MAP_KEY_LEN;
     use crate::grpc::test_support::test_server_state;
+    use crate::grpc::{MAX_MAP_KEY_LEN, MAX_PROVIDER_TYPE_LEN};
     use crate::persistence::test_store;
     use openshell_core::proto::{
         DeleteProviderProfileRequest, GetProviderProfileRequest, ImportProviderProfilesRequest,
@@ -3557,6 +3562,52 @@ mod tests {
         assert_eq!(err.code(), Code::InvalidArgument);
     }
 
+    /// Regression for #1347: a credential rotation must not fail just because the
+    /// existing record's `type` field exceeds the current `MAX_PROVIDER_TYPE_LEN`.
+    /// The caller never touches `type`; re-validating it strands legacy records.
+    #[tokio::test]
+    async fn update_provider_allows_credential_rotation_on_legacy_oversized_type() {
+        let store = test_store().await;
+
+        let oversized_type = "x".repeat(MAX_PROVIDER_TYPE_LEN + 15);
+        let legacy = Provider {
+            metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
+                id: uuid::Uuid::new_v4().to_string(),
+                name: "legacy-oversized-type".to_string(),
+                created_at_ms: 0,
+                labels: HashMap::new(),
+                resource_version: 0,
+            }),
+            r#type: oversized_type.clone(),
+            credentials: std::iter::once(("API_TOKEN".to_string(), "old".to_string())).collect(),
+            config: HashMap::new(),
+            credential_expires_at_ms: HashMap::new(),
+        };
+        store.put_message(&legacy).await.unwrap();
+
+        let updated = update_provider_record(
+            &store,
+            Provider {
+                metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
+                    id: String::new(),
+                    name: "legacy-oversized-type".to_string(),
+                    created_at_ms: 0,
+                    labels: HashMap::new(),
+                    resource_version: 0,
+                }),
+                r#type: String::new(),
+                credentials: std::iter::once(("API_TOKEN".to_string(), "new".to_string()))
+                    .collect(),
+                config: HashMap::new(),
+                credential_expires_at_ms: HashMap::new(),
+            },
+        )
+        .await
+        .unwrap();
+
+        assert_eq!(updated.r#type, oversized_type);
+    }
+
     #[tokio::test]
     async fn resolve_provider_env_empty_list_returns_empty() {
         let store = test_store().await;

crates/openshell-server/src/grpc/validation.rs

@@ -239,7 +239,7 @@ pub(super) fn validate_string_map(
 // Provider field validation
 // ---------------------------------------------------------------------------
 
-/// Validate field sizes on a `Provider` before persisting.
+/// Validate field sizes on a `Provider` before persisting a new record.
 pub(super) fn validate_provider_fields(provider: &Provider) -> Result<(), Status> {
     let name_len = provider.metadata.as_ref().map_or(0, |m| m.name.len());
     if name_len > MAX_NAME_LEN {
@@ -253,6 +253,17 @@ pub(super) fn validate_provider_fields(provider: &Provider) -> Result<(), Status
             provider.r#type.len()
         )));
     }
+    validate_provider_mutable_fields(provider)
+}
+
+/// Validate field sizes on a `Provider` before persisting an update.
+///
+/// Skips the immutable `name` and `type` fields, which are carried forward from
+/// the existing record. Re-checking them would block credential rotation on any
+/// legacy record whose stored `name`/`type` predates current limits (or was
+/// written by a path that bypassed validation), even though the caller never
+/// touches those fields. See #1347.
+pub(super) fn validate_provider_mutable_fields(provider: &Provider) -> Result<(), Status> {
     validate_string_map(
         &provider.credentials,
         MAX_PROVIDER_CREDENTIALS_ENTRIES,