diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 817b7e7eaf..0cc43bb6f4 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -5587,7 +5587,7 @@ fn print_policy_revision_table(revisions: &[openshell_core::proto::SandboxPolicy &rev.policy_hash }; let error_short = if rev.load_error.len() > 40 { - format!("{}...", &rev.load_error[..40]) + truncate_status_field(&rev.load_error, 40) } else { rev.load_error.clone() }; @@ -6239,6 +6239,29 @@ mod tests { assert!(unknown[0].get("sandbox").is_none()); } + #[test] + fn policy_revision_table_handles_unicode_load_errors() { + // Stored diagnostics can contain Unicode paths. Byte 40 splits the + // character in the 40-scalar case; longer errors must also remain safe. + let revisions = [ + String::new(), + "a".repeat(40), + "a".repeat(41), + format!("{}é", "a".repeat(39)), + format!("{}éz", "a".repeat(39)), + ] + .into_iter() + .map(|load_error| SandboxPolicyRevision { + version: 1, + status: PolicyStatus::Failed as i32, + load_error, + ..Default::default() + }) + .collect::>(); + + super::print_policy_revision_table(&revisions); + } + #[test] fn service_endpoint_json_has_raw_fields_and_normalized_url() { let response = ServiceEndpointResponse { diff --git a/crates/openshell-tui/src/app.rs b/crates/openshell-tui/src/app.rs index ee86b5f4f6..9f044a7893 100644 --- a/crates/openshell-tui/src/app.rs +++ b/crates/openshell-tui/src/app.rs @@ -420,6 +420,26 @@ fn apply_discovered_provider(form: &mut CreateProviderForm, discovered: Discover // Provider detail view (Get) // --------------------------------------------------------------------------- +/// Availability of strictly serialized provider profile YAML in the detail view. +pub enum ProviderProfileYaml { + /// The provider has no associated profile. + Absent, + /// The profile serialized successfully. + Valid(String), + /// A profile exists, but its YAML serialization failed. + Invalid, +} + +impl ProviderProfileYaml { + /// Return YAML only when strict profile serialization succeeded. + pub fn yaml(&self) -> Option<&str> { + match self { + Self::Valid(yaml) => Some(yaml), + Self::Absent | Self::Invalid => None, + } + } +} + pub struct ProviderDetailView { pub name: String, pub provider_id: String, @@ -430,7 +450,7 @@ pub struct ProviderDetailView { pub show_raw_provider: bool, pub raw_profile_scroll: usize, pub raw_provider_scroll: usize, - pub raw_profile_yaml: Option, + pub raw_profile_yaml: ProviderProfileYaml, pub raw_provider_yaml: String, pub profile_name: Option, pub profile_category: Option, @@ -2923,7 +2943,7 @@ impl App { KeyCode::Esc | KeyCode::Enter => { self.provider_detail = None; } - KeyCode::Char('y') if detail.raw_profile_yaml.is_some() => { + KeyCode::Char('y') if detail.raw_profile_yaml.yaml().is_some() => { detail.show_raw_profile = !detail.show_raw_profile; detail.show_raw_provider = false; detail.raw_profile_scroll = 0; @@ -2936,7 +2956,7 @@ impl App { KeyCode::Char('j') | KeyCode::Down if detail.show_raw_profile => { let max_scroll = detail .raw_profile_yaml - .as_ref() + .yaml() .map_or(0, |raw| raw.lines().count().saturating_sub(1)); detail.raw_profile_scroll = (detail.raw_profile_scroll + 1).min(max_scroll); } @@ -3437,9 +3457,12 @@ impl App { }, ); - let raw_profile_yaml = profile.and_then(|profile| { + let raw_profile_yaml = profile.map_or(ProviderProfileYaml::Absent, |profile| { let dto = ProviderTypeProfile::from_proto(profile); - openshell_providers::profile_to_yaml(&dto).ok() + // Serializer errors may echo authored values. Keep the failure + // distinct from absence without retaining potentially secret text. + openshell_providers::profile_to_yaml(&dto) + .map_or(ProviderProfileYaml::Invalid, ProviderProfileYaml::Valid) }); ProviderDetailView { @@ -3651,6 +3674,201 @@ mod tests { KeyEvent::new(code, KeyModifiers::NONE) } + fn detail_mcp_profile(versions: &[&str]) -> openshell_core::proto::ProviderProfile { + openshell_core::proto::ProviderProfile { + id: "mcp-example".to_string(), + display_name: "MCP Example".to_string(), + endpoints: vec![openshell_core::proto::NetworkEndpoint { + host: "mcp.example.com".to_string(), + port: 443, + protocol: "mcp".to_string(), + mcp: Some(openshell_core::proto::McpOptions { + versions: versions.iter().map(ToString::to_string).collect(), + ..Default::default() + }), + ..Default::default() + }], + ..Default::default() + } + } + + fn detail_app(profile: Option) -> App { + let mut app = test_app(); + let provider = openshell_core::proto::Provider { + metadata: Some(openshell_core::proto::ObjectMeta { + id: "provider-id".to_string(), + name: "example-provider".to_string(), + ..Default::default() + }), + r#type: "mcp-example".to_string(), + credentials: HashMap::from([("API_KEY".to_string(), "test-only-secret".to_string())]), + ..Default::default() + }; + app.provider_entries.push(ProviderListEntry { + provider: provider.clone(), + profile, + }); + app.provider_detail = Some(app.provider_detail_from_provider(&provider)); + app + } + + fn render_provider_detail(app: &App) -> String { + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(100, 36)) + .expect("test terminal"); + terminal + .draw(|frame| crate::ui::create_provider::draw_detail(frame, app, frame.size())) + .expect("provider detail renders"); + terminal + .backend() + .buffer() + .content() + .iter() + .map(ratatui::buffer::Cell::symbol) + .collect() + } + + #[tokio::test] + async fn provider_detail_absent_profile_keeps_object_yaml_available() { + let mut app = detail_app(None); + let detail = app.provider_detail.as_ref().expect("detail view"); + assert!(matches!( + detail.raw_profile_yaml, + ProviderProfileYaml::Absent + )); + let text = render_provider_detail(&app); + assert!(text.contains("Profile: (legacy/unprofiled provider)")); + assert!(!text.contains("Profile YAML unavailable")); + assert!(!text.contains("[y]")); + + app.handle_provider_detail_key(key(KeyCode::Char('y'))); + assert!( + !app.provider_detail + .as_ref() + .expect("detail view") + .show_raw_profile + ); + app.handle_provider_detail_key(key(KeyCode::Char('o'))); + let text = render_provider_detail(&app); + assert!(text.contains("Provider Object YAML")); + assert!(text.contains("")); + assert!(!text.contains("test-only-secret")); + } + + #[tokio::test] + async fn provider_detail_valid_profile_preserves_yaml_navigation() { + let profile = detail_mcp_profile(&["2025-11-25"]); + let expected_yaml = + openshell_providers::profile_to_yaml(&ProviderTypeProfile::from_proto(&profile)) + .expect("valid profile YAML"); + let mut app = detail_app(Some(profile)); + assert_eq!( + app.provider_detail + .as_ref() + .expect("detail view") + .raw_profile_yaml + .yaml(), + Some(expected_yaml.as_str()) + ); + assert!(render_provider_detail(&app).contains("[y] Profile YAML")); + app.handle_provider_detail_key(key(KeyCode::Char('y'))); + let text = render_provider_detail(&app); + assert!(text.contains("Provider Profile YAML")); + assert!(text.contains("mcp-example")); + assert!(!text.contains("test-only-secret")); + app.handle_provider_detail_key(key(KeyCode::Char('j'))); + assert_eq!( + app.provider_detail + .as_ref() + .expect("detail view") + .raw_profile_scroll, + 1 + ); + app.handle_provider_detail_key(key(KeyCode::Char('k'))); + assert_eq!( + app.provider_detail + .as_ref() + .expect("detail view") + .raw_profile_scroll, + 0 + ); + app.handle_provider_detail_key(key(KeyCode::Char('o'))); + assert!( + app.provider_detail + .as_ref() + .expect("detail view") + .show_raw_provider + ); + app.handle_provider_detail_key(key(KeyCode::Char('y'))); + let detail = app.provider_detail.as_ref().expect("detail view"); + assert!(detail.show_raw_profile); + assert!(!detail.show_raw_provider); + app.handle_provider_detail_key(key(KeyCode::Esc)); + assert!( + !app.provider_detail + .as_ref() + .expect("detail view") + .show_raw_profile + ); + app.handle_provider_detail_key(key(KeyCode::Esc)); + assert!(app.provider_detail.is_none()); + } + + #[tokio::test] + async fn provider_detail_invalid_profile_remains_visible_without_yaml_action() { + for versions in [vec!["2025-11-25", "2025-11-25"], vec!["latest"]] { + let mut app = detail_app(Some(detail_mcp_profile(&versions))); + let detail = app.provider_detail.as_ref().expect("detail view"); + assert!(matches!( + detail.raw_profile_yaml, + ProviderProfileYaml::Invalid + )); + assert_eq!(detail.profile_name.as_deref(), Some("MCP Example")); + let text = render_provider_detail(&app); + assert!(text.contains("example-provider")); + assert!(text.contains("Profile YAML unavailable: serialization failed.")); + assert!(text.contains("Correct the profile at its source, then reopen this view.")); + assert!(!text.contains("legacy/unprofiled")); + assert!(!text.contains("[y]")); + app.handle_provider_detail_key(key(KeyCode::Char('y'))); + assert!( + !app.provider_detail + .as_ref() + .expect("detail view") + .show_raw_profile + ); + app.handle_provider_detail_key(key(KeyCode::Char('o'))); + let text = render_provider_detail(&app); + assert!(text.contains("Provider Object YAML")); + assert!(text.contains("")); + assert!(!text.contains("test-only-secret")); + } + } + + #[tokio::test] + async fn provider_detail_multibyte_profile_error_is_bounded_and_redacted() { + let malformed = "秘密é🚧\nprivate-profile-value".repeat(2048); + let profile = detail_mcp_profile(&[malformed.as_str()]); + let error = + openshell_providers::profile_to_yaml(&ProviderTypeProfile::from_proto(&profile)) + .expect_err("malformed revision must fail strict serialization"); + assert!(error.to_string().contains("秘密")); + let app = detail_app(Some(profile)); + assert!(matches!( + app.provider_detail + .as_ref() + .expect("detail view") + .raw_profile_yaml, + ProviderProfileYaml::Invalid + )); + let text = render_provider_detail(&app); + assert!(text.contains("Profile YAML unavailable: serialization failed.")); + assert!(!text.contains("秘密")); + assert!(!text.contains("private-profile-value")); + assert!(!text.contains("test-only-secret")); + let ordinary_failure = detail_app(Some(detail_mcp_profile(&["latest"]))); + assert_eq!(text, render_provider_detail(&ordinary_failure)); + } + #[tokio::test] async fn create_provider_enter_with_empty_profile_catalog_is_recoverable() { let mut app = test_app(); diff --git a/crates/openshell-tui/src/ui/create_provider.rs b/crates/openshell-tui/src/ui/create_provider.rs index b68ddf5ea9..fcd7cdcc1a 100644 --- a/crates/openshell-tui/src/ui/create_provider.rs +++ b/crates/openshell-tui/src/ui/create_provider.rs @@ -6,7 +6,9 @@ use ratatui::layout::{Constraint, Direction, Layout, Rect}; use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, Borders, Clear, Padding, Paragraph}; -use crate::app::{App, CreateProviderPhase, ProviderKeyField, UpdateProviderField}; +use crate::app::{ + App, CreateProviderPhase, ProviderKeyField, ProviderProfileYaml, UpdateProviderField, +}; use indexmap::IndexMap; use std::ops::Range; @@ -602,7 +604,7 @@ pub fn draw_detail(frame: &mut Frame<'_>, app: &App, area: Rect) { let title = if detail.show_raw_provider { " Provider Object YAML " - } else if detail.show_raw_profile { + } else if detail.show_raw_profile && detail.raw_profile_yaml.yaml().is_some() { " Provider Profile YAML " } else { " Provider Detail " @@ -629,13 +631,12 @@ pub fn draw_detail(frame: &mut Frame<'_>, app: &App, area: Rect) { return; } - if detail.show_raw_profile { + if detail.show_raw_profile + && let Some(raw) = detail.raw_profile_yaml.yaml() + { draw_raw_yaml( frame, - detail - .raw_profile_yaml - .as_deref() - .unwrap_or("No provider profile is available for this provider."), + raw, detail.raw_profile_scroll, "Summary", "y", @@ -676,6 +677,18 @@ pub fn draw_detail(frame: &mut Frame<'_>, app: &App, area: Rect) { t.status_warn, ))); } + if matches!(detail.raw_profile_yaml, ProviderProfileYaml::Invalid) { + // Fixed diagnostics bound the display and never expose values echoed by + // the serializer, even when malformed profile fields contain secrets. + lines.push(Line::from(Span::styled( + "Profile YAML unavailable: serialization failed.", + t.status_warn, + ))); + lines.push(Line::from(Span::styled( + "Correct the profile at its source, then reopen this view.", + t.status_warn, + ))); + } if let Some(description) = &detail.profile_description { lines.push(Line::from(vec![ Span::styled("Description: ", t.muted), @@ -704,7 +717,7 @@ pub fn draw_detail(frame: &mut Frame<'_>, app: &App, area: Rect) { Span::styled("[o]", t.key_hint), Span::styled(" Object YAML ", t.muted), ]; - if detail.raw_profile_yaml.is_some() { + if detail.raw_profile_yaml.yaml().is_some() { hint_spans.extend([ Span::styled("[y]", t.key_hint), Span::styled(" Profile YAML ", t.muted),