Skip to content

Commit 00f7ba7

Browse files
authored
fix(tui): show MCP tool selectors in the policy viewer (#5345)
* fix(tui): show MCP tool selectors in policy viewer Signed-off-by: Shiju <shiju@nvidia.com> * chore(tui): keep policy display fix scoped to code Signed-off-by: Shiju <shiju@nvidia.com> --------- Signed-off-by: Shiju <shiju@nvidia.com>
1 parent 53bf840 commit 00f7ba7

1 file changed

Lines changed: 223 additions & 0 deletions

File tree

  • crates/openshell-tui/src

‎crates/openshell-tui/src/lib.rs‎

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,6 +1315,16 @@ fn render_policy_lines(
13151315
// L7 allow rules.
13161316
for l7 in &ep.rules {
13171317
if let Some(allow) = &l7.allow {
1318+
if ep.protocol.eq_ignore_ascii_case("mcp") {
1319+
render_mcp_rule(
1320+
&mut lines,
1321+
"Allow",
1322+
&allow.method,
1323+
allow.params.get("name"),
1324+
t,
1325+
);
1326+
continue;
1327+
}
13181328
let method = if allow.method.is_empty() {
13191329
"*"
13201330
} else {
@@ -1334,6 +1344,18 @@ fn render_policy_lines(
13341344
}
13351345
}
13361346

1347+
if ep.protocol.eq_ignore_ascii_case("mcp") {
1348+
for deny in &ep.deny_rules {
1349+
render_mcp_rule(
1350+
&mut lines,
1351+
"Deny",
1352+
&deny.method,
1353+
deny.params.get("name"),
1354+
t,
1355+
);
1356+
}
1357+
}
1358+
13371359
// Access preset (if set instead of explicit rules).
13381360
if ep.access != 0 && ep.rules.is_empty() {
13391361
lines.push(Line::from(vec![
@@ -1371,6 +1393,37 @@ fn render_policy_lines(
13711393
lines
13721394
}
13731395

1396+
fn render_mcp_rule(
1397+
lines: &mut Vec<ratatui::text::Line<'static>>,
1398+
action: &str,
1399+
method: &str,
1400+
tool: Option<&openshell_core::proto::L7QueryMatcher>,
1401+
theme: &theme::Theme,
1402+
) {
1403+
use ratatui::text::{Line, Span};
1404+
1405+
let method = if method.is_empty() {
1406+
if tool.is_some() { "tools/call" } else { "*" }
1407+
} else {
1408+
method
1409+
};
1410+
lines.push(Line::from(vec![
1411+
Span::styled(format!(" {action}: "), theme.muted),
1412+
Span::styled(method.to_string(), theme.text),
1413+
]));
1414+
if let Some(tool) = tool {
1415+
for name in std::iter::once(&tool.glob)
1416+
.filter(|name| !name.is_empty())
1417+
.chain(&tool.any)
1418+
{
1419+
lines.push(Line::from(vec![
1420+
Span::styled(" Tool: ", theme.muted),
1421+
Span::styled(name.clone(), theme.text),
1422+
]));
1423+
}
1424+
}
1425+
}
1426+
13741427
// ---------------------------------------------------------------------------
13751428
// Animation helper
13761429
// ---------------------------------------------------------------------------
@@ -3444,3 +3497,173 @@ mod sandbox_notes_tests {
34443497
assert_eq!(sandbox_notes(&sandbox, String::new()), "");
34453498
}
34463499
}
3500+
3501+
#[cfg(test)]
3502+
mod policy_render_tests {
3503+
use super::*;
3504+
3505+
fn policy_lines(yaml: &str) -> Vec<ratatui::text::Line<'static>> {
3506+
let policy = openshell_policy::parse_sandbox_policy(yaml).expect("valid policy");
3507+
render_policy_lines(&policy, &theme::Theme::dark())
3508+
}
3509+
3510+
#[test]
3511+
fn mcp_tool_allowlist_is_visible_one_tool_per_line() {
3512+
let lines = policy_lines(
3513+
r"
3514+
version: 1
3515+
network_policies:
3516+
review:
3517+
endpoints:
3518+
- host: host.openshell.internal
3519+
port: 32443
3520+
protocol: mcp
3521+
enforcement: enforce
3522+
rules:
3523+
- allow:
3524+
method: tools/call
3525+
tool:
3526+
any:
3527+
- local_gh_issue_view
3528+
- local_gh_pr_checks
3529+
- local_gh_pr_details
3530+
- local_gh_pr_diff
3531+
- local_gh_pr_list
3532+
- local_gh_pr_view
3533+
- local_gh_repo_view
3534+
- local_gh_run_failed_logs
3535+
- local_gh_run_view
3536+
- local_gh_source_blob
3537+
- local_gh_source_tree
3538+
- local_gh_version
3539+
",
3540+
);
3541+
let tools: Vec<_> = lines
3542+
.iter()
3543+
.map(ToString::to_string)
3544+
.filter_map(|line| line.strip_prefix(" Tool: ").map(str::to_string))
3545+
.collect();
3546+
assert_eq!(
3547+
tools,
3548+
[
3549+
"local_gh_issue_view",
3550+
"local_gh_pr_checks",
3551+
"local_gh_pr_details",
3552+
"local_gh_pr_diff",
3553+
"local_gh_pr_list",
3554+
"local_gh_pr_view",
3555+
"local_gh_repo_view",
3556+
"local_gh_run_failed_logs",
3557+
"local_gh_run_view",
3558+
"local_gh_source_blob",
3559+
"local_gh_source_tree",
3560+
"local_gh_version",
3561+
]
3562+
);
3563+
assert!(
3564+
lines
3565+
.iter()
3566+
.any(|line| line.to_string() == " Allow: tools/call")
3567+
);
3568+
assert!(
3569+
!lines
3570+
.iter()
3571+
.any(|line| line.to_string().contains("tools/call *"))
3572+
);
3573+
3574+
let mut buffer = ratatui::buffer::Buffer::empty(ratatui::layout::Rect::new(0, 0, 60, 30));
3575+
ratatui::widgets::Widget::render(
3576+
ratatui::widgets::Paragraph::new(lines),
3577+
buffer.area,
3578+
&mut buffer,
3579+
);
3580+
for tool in tools {
3581+
assert!(buffer.content.chunks(60).any(|row| {
3582+
row.iter()
3583+
.map(ratatui::buffer::Cell::symbol)
3584+
.collect::<String>()
3585+
.contains(&tool)
3586+
}));
3587+
}
3588+
}
3589+
3590+
#[test]
3591+
fn mcp_methods_and_allow_deny_selectors_preserve_their_scope() {
3592+
let lines = policy_lines(
3593+
r"
3594+
version: 1
3595+
network_policies:
3596+
review:
3597+
endpoints:
3598+
- host: mcp.example.com
3599+
port: 443
3600+
protocol: mcp
3601+
mcp:
3602+
allow_all_known_mcp_methods: true
3603+
rules:
3604+
- allow:
3605+
method: tools/list
3606+
- allow:
3607+
tool: read_source
3608+
- allow:
3609+
method: tools/call
3610+
params:
3611+
name:
3612+
any: [read_*, list_*]
3613+
deny_rules:
3614+
- tool: read_private
3615+
- method: tools/call
3616+
params:
3617+
name:
3618+
any: [read_secret_*, list_private]
3619+
- method: resources/read
3620+
profile:
3621+
endpoints:
3622+
- host: profile.example.com
3623+
port: 443
3624+
protocol: mcp
3625+
mcp:
3626+
allow_all_known_mcp_methods: true
3627+
rules:
3628+
- allow: {}
3629+
deny_rules:
3630+
- {}
3631+
rest:
3632+
endpoints:
3633+
- host: api.example.com
3634+
port: 443
3635+
protocol: rest
3636+
rules:
3637+
- allow: {method: GET, path: /repos/**}
3638+
",
3639+
);
3640+
let text: Vec<_> = lines.iter().map(ToString::to_string).collect();
3641+
assert!(text.iter().any(|line| line == " Allow: *"));
3642+
assert!(text.iter().any(|line| line == " Deny: *"));
3643+
let mcp_start = text
3644+
.iter()
3645+
.position(|line| line == " Allow: tools/list")
3646+
.unwrap();
3647+
assert_eq!(
3648+
&text[mcp_start..mcp_start + 12],
3649+
[
3650+
" Allow: tools/list",
3651+
" Allow: tools/call",
3652+
" Tool: read_source",
3653+
" Allow: tools/call",
3654+
" Tool: read_*",
3655+
" Tool: list_*",
3656+
" Deny: tools/call",
3657+
" Tool: read_private",
3658+
" Deny: tools/call",
3659+
" Tool: read_secret_*",
3660+
" Tool: list_private",
3661+
" Deny: resources/read",
3662+
]
3663+
);
3664+
assert!(
3665+
text.iter()
3666+
.any(|line| line == " Allow: GET /repos/**")
3667+
);
3668+
}
3669+
}

0 commit comments

Comments
 (0)