trending/OpenShell
1
0
Fork 0
mirror of https://github.com/NVIDIA/OpenShell.git synced 2026-10-02 07:34:45 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): keep policy and provider diagnostics readable (#3444)

Browse Source 
Reuse character-safe truncation for policy history errors so a multibyte character cannot panic the table renderer.

Distinguish unavailable provider-profile YAML from an absent profile, display a bounded diagnostic, and preserve strict serialization and redacted object navigation.

Cover the actual CLI renderer and TUI display/navigation paths, including invalid and absent profiles, Unicode input, and redacted errors.

Signed-off-by: Shiju <shiju@nvidia.com>
This commit is contained in:
Shiju
2026-09-29 15:35:56 +00:00
committed by GitHub
parent cfcc3733bd
commit 12ef86c285
3 changed files with 268 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/openshell-cli/src/run.rs
+24 -1
View File
@@ -5940,7 +5940,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()
};
@@ -6605,6 +6605,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::<Vec<_>>();
super::print_policy_revision_table(&revisions);
}
#[test]
fn service_endpoint_json_has_raw_fields_and_normalized_url() {
let response = ServiceEndpointResponse {
crates/openshell-tui/src/app.rs
+223 -5
View File
@@ -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<String>,
pub raw_profile_yaml: ProviderProfileYaml,
pub raw_provider_yaml: String,
pub profile_name: Option<String>,
pub profile_category: Option<String>,
@@ -2933,7 +2953,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;
@@ -2946,7 +2966,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);
}
@@ -3441,9 +3461,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 {
@@ -3660,6 +3683,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<openshell_core::proto::ProviderProfile>) -> 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: <none> (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("<redacted>"));
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("<redacted>"));
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();
crates/openshell-tui/src/ui/create_provider.rs
+21 -8
View File
@@ -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),