Skip to content

Commit 0096f0d

Browse files
pboydclaude
andauthored
fix(catalog): populate securityIndicators from YAML MCP server configs (kubeflow#2461)
The YAML provider lacked a securityIndicators field, so demo and rh-mcp manifests stored verifiedSource, secureEndpoint, sast, and readOnlyTools as custom properties. This left securityIndicators null in API responses and broke named queries on these fields, since the converter reads them from standard properties. Add yamlMCPSecurityIndicator struct and SecurityIndicators field to yamlMCPServer, convert the values to standard bool properties in ToMCPServerProviderRecord(), and update all affected YAML manifests to use the new top-level securityIndicators field. Signed-off-by: Paul Boyd <paul@pboyd.io> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 50e1b84 commit 0096f0d

7 files changed

Lines changed: 221 additions & 222 deletions

File tree

catalog/clients/python/tests/mcp_servers/test_named_queries.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class TestMCPServerNamedQueries:
1717
"""Test suite for MCP server named query functionality."""
1818

1919
@pytest.mark.parametrize(
20-
"named_query, expected_custom_properties",
20+
"named_query, expected_security_indicators",
2121
[
2222
pytest.param(
2323
"production_ready",
@@ -36,18 +36,18 @@ def test_named_query_execution(
3636
api_client: CatalogAPIClient,
3737
suppress_ssl_warnings: None,
3838
named_query: str,
39-
expected_custom_properties: dict[str, bool],
39+
expected_security_indicators: dict[str, bool],
4040
):
41-
"""TC-API-011: Test executing a named query filters servers by custom properties."""
41+
"""TC-API-011: Test executing a named query filters servers by security indicators."""
4242
response = api_client.get_mcp_servers(named_query=named_query)
4343
items = response.get("items", [])
4444
assert len(items) == 1, f"Expected 1 server matching '{named_query}', got {len(items)}"
4545
assert items[0]["name"] == "calculator"
4646

47-
custom_props = items[0].get("customProperties", {})
48-
for prop_name, expected_value in expected_custom_properties.items():
49-
assert custom_props.get(prop_name, {}).get("bool_value") is expected_value, (
50-
f"Expected {prop_name}={expected_value}, got {custom_props.get(prop_name)}"
47+
security_indicators = items[0].get("securityIndicators", {})
48+
for prop_name, expected_value in expected_security_indicators.items():
49+
assert security_indicators.get(prop_name) is expected_value, (
50+
f"Expected {prop_name}={expected_value}, got {security_indicators.get(prop_name)}"
5151
)
5252

5353
@pytest.mark.parametrize(

catalog/internal/catalog/mcpcatalog/providers.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,14 @@ func GetMCPProvider(typeName string) (MCPServerProviderFunc, bool) {
9393
return providerFunc, exists
9494
}
9595

96+
// yamlMCPSecurityIndicator represents the security indicators for an MCP server
97+
type yamlMCPSecurityIndicator struct {
98+
VerifiedSource *bool `yaml:"verifiedSource,omitempty"`
99+
SecureEndpoint *bool `yaml:"secureEndpoint,omitempty"`
100+
Sast *bool `yaml:"sast,omitempty"`
101+
ReadOnlyTools *bool `yaml:"readOnlyTools,omitempty"`
102+
}
103+
96104
// yamlMCPServer represents an MCP server definition in YAML
97105
type yamlMCPServer struct {
98106
Name string `yaml:"name"`
@@ -115,6 +123,7 @@ type yamlMCPServer struct {
115123
Endpoints *yamlMCPEndpoints `yaml:"endpoints,omitempty"`
116124
RuntimeMetadata *apimodels.MCPRuntimeMetadata `yaml:"runtimeMetadata,omitempty"`
117125
Tags []string `yaml:"tags,omitempty"`
126+
SecurityIndicators *yamlMCPSecurityIndicator `yaml:"securityIndicators,omitempty"`
118127
CustomProperties *map[string]apimodels.MetadataValue `yaml:"customProperties,omitempty"`
119128
CreateTimeSinceEpoch *string `yaml:"createTimeSinceEpoch,omitempty"`
120129
LastUpdateTimeSinceEpoch *string `yaml:"lastUpdateTimeSinceEpoch,omitempty"`
@@ -381,6 +390,23 @@ func (ys *yamlMCPServer) ToMCPServerProviderRecord() MCPServerProviderRecord {
381390
}
382391
}
383392

393+
// Convert security indicators to standard bool properties
394+
if ys.SecurityIndicators != nil {
395+
si := ys.SecurityIndicators
396+
if si.VerifiedSource != nil {
397+
properties = append(properties, mrmodels.NewBoolProperty("verifiedSource", *si.VerifiedSource, false))
398+
}
399+
if si.SecureEndpoint != nil {
400+
properties = append(properties, mrmodels.NewBoolProperty("secureEndpoint", *si.SecureEndpoint, false))
401+
}
402+
if si.Sast != nil {
403+
properties = append(properties, mrmodels.NewBoolProperty("sast", *si.Sast, false))
404+
}
405+
if si.ReadOnlyTools != nil {
406+
properties = append(properties, mrmodels.NewBoolProperty("readOnlyTools", *si.ReadOnlyTools, false))
407+
}
408+
}
409+
384410
server.Properties = &properties
385411

386412
// Convert custom properties

catalog/internal/catalog/mcpcatalog/providers_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,105 @@ func TestYamlMCPProviderEmitWithRuntimeMetadata(t *testing.T) {
817817
assert.Equal(t, "API_KEY", *parsed.Prerequisites.Secrets[0].Keys[0].EnvVarName)
818818
}
819819

820+
func TestYamlMCPServerSecurityIndicatorsToStandardProperties(t *testing.T) {
821+
trueVal := true
822+
falseVal := false
823+
yamlServer := &yamlMCPServer{
824+
Name: "test-server",
825+
SecurityIndicators: &yamlMCPSecurityIndicator{
826+
VerifiedSource: &trueVal,
827+
SecureEndpoint: &falseVal,
828+
Sast: &trueVal,
829+
ReadOnlyTools: &falseVal,
830+
},
831+
}
832+
833+
record := yamlServer.ToMCPServerProviderRecord()
834+
835+
require.NotNil(t, record.Server)
836+
require.Nil(t, record.Error)
837+
838+
// Security indicators must land in standard properties, not custom properties
839+
props := record.Server.GetProperties()
840+
require.NotNil(t, props)
841+
842+
propMap := make(map[string]bool)
843+
for _, p := range *props {
844+
if p.BoolValue != nil {
845+
propMap[p.Name] = *p.BoolValue
846+
}
847+
}
848+
849+
assert.True(t, propMap["verifiedSource"], "verifiedSource should be true in standard properties")
850+
assert.False(t, propMap["secureEndpoint"], "secureEndpoint should be false in standard properties")
851+
assert.True(t, propMap["sast"], "sast should be true in standard properties")
852+
assert.False(t, propMap["readOnlyTools"], "readOnlyTools should be false in standard properties")
853+
854+
// Security indicators must NOT appear in custom properties
855+
customProps := record.Server.GetCustomProperties()
856+
if customProps != nil {
857+
for _, p := range *customProps {
858+
assert.NotEqual(t, "verifiedSource", p.Name)
859+
assert.NotEqual(t, "secureEndpoint", p.Name)
860+
assert.NotEqual(t, "sast", p.Name)
861+
assert.NotEqual(t, "readOnlyTools", p.Name)
862+
}
863+
}
864+
}
865+
866+
func TestYamlMCPServerSecurityIndicatorsNil(t *testing.T) {
867+
yamlServer := &yamlMCPServer{
868+
Name: "test-server",
869+
SecurityIndicators: nil,
870+
}
871+
872+
record := yamlServer.ToMCPServerProviderRecord()
873+
874+
require.NotNil(t, record.Server)
875+
require.Nil(t, record.Error)
876+
877+
props := record.Server.GetProperties()
878+
if props != nil {
879+
for _, p := range *props {
880+
assert.Nil(t, p.BoolValue, "no bool properties should exist when SecurityIndicators is nil (prop: %s)", p.Name)
881+
}
882+
}
883+
}
884+
885+
func TestYamlMCPServerSecurityIndicatorsPartial(t *testing.T) {
886+
trueVal := true
887+
yamlServer := &yamlMCPServer{
888+
Name: "test-server",
889+
SecurityIndicators: &yamlMCPSecurityIndicator{
890+
VerifiedSource: &trueVal,
891+
// SecureEndpoint, Sast, ReadOnlyTools intentionally unset
892+
},
893+
}
894+
895+
record := yamlServer.ToMCPServerProviderRecord()
896+
897+
require.NotNil(t, record.Server)
898+
require.Nil(t, record.Error)
899+
900+
props := record.Server.GetProperties()
901+
require.NotNil(t, props)
902+
903+
boolProps := make(map[string]bool)
904+
for _, p := range *props {
905+
if p.BoolValue != nil {
906+
boolProps[p.Name] = *p.BoolValue
907+
}
908+
}
909+
910+
assert.True(t, boolProps["verifiedSource"], "verifiedSource should be true")
911+
_, hasSecureEndpoint := boolProps["secureEndpoint"]
912+
assert.False(t, hasSecureEndpoint, "secureEndpoint should not be present when unset")
913+
_, hasSast := boolProps["sast"]
914+
assert.False(t, hasSast, "sast should not be present when unset")
915+
_, hasReadOnlyTools := boolProps["readOnlyTools"]
916+
assert.False(t, hasReadOnlyTools, "readOnlyTools should not be present when unset")
917+
}
918+
820919
// Helper function
821920
func strPtr(s string) *string {
822921
return &s

0 commit comments

Comments
 (0)