Skip to content

Commit d941488

Browse files
jpgough-msjpgough-ms
authored
fix(calm-hub): CalmHub UI Colour Improvements Refactor MCP Sever Tools (#2424)
* fix(calm-hub-ui): unify tab colours to DaisyUI primary token in ControlDetailSection Replace hardcoded accent token with primary so all active tabs in ControlDetailSection use a single DaisyUI semantic token, ensuring uniform recolouring when the theme changes. Closes #2420 (item 4) * fix(calm-hub): ControlTools hardening and integration tests for #2420 - Refactor createControlRequirement and createControlConfiguration onto McpValidationHelper.firstError for consistency with read methods - Add max-length validation on name (200), description (1024), and JSON payloads (100k bytes) in both create methods - Add unit tests covering all new validation paths in TestControlToolsShould - Add create->list->versions->get->configure integration tests for controls in MongoMcpIntegration and NitriteMcpIntegration Closes #2420 (items 1-3) * refactor(calm-hub): extract MAX_NAME_LENGTH constant in McpValidationHelper Replace inline literal 200 in ControlTools and ArchitectureTools with a named constant McpValidationHelper.MAX_NAME_LENGTH, consistent with the existing MAX_DESCRIPTION_LENGTH and MAX_JSON_PAYLOAD_LENGTH constants. Update the matching unit test to reference the constant instead of the literal.
1 parent 23c9cec commit d941488

7 files changed

Lines changed: 252 additions & 54 deletions

File tree

‎calm-hub-ui/src/hub/components/control-detail-section/ControlDetailSection.tsx‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ export function ControlDetailSection({ controlData }: ControlDetailSectionProps)
9999
<div className="flex-1 min-h-0 bg-base-100 rounded-2xl overflow-hidden flex flex-col shadow-xl">
100100
{/* Requirement breadcrumb header */}
101101
<div className="bg-base-200 px-6 py-2 flex items-center justify-between border-b border-base-300">
102-
<h2 className="text-sm font-bold flex items-center gap-2 text-blue-700">
103-
<IoShieldCheckmarkOutline className="text-blue-700" />
102+
<h2 className="text-sm font-bold flex items-center gap-2 text-primary">
103+
<IoShieldCheckmarkOutline className="text-primary" />
104104
<span>{controlData.controlName}</span>
105105
<span className="text-gray-400">/</span>
106106
<span>Requirement</span>
@@ -115,14 +115,14 @@ export function ControlDetailSection({ controlData }: ControlDetailSectionProps)
115115
<div role="tablist" className="tabs tabs-boxed tabs-xs bg-base-100">
116116
<button
117117
role="tab"
118-
className={`tab ${reqViewMode === 'readable' ? 'tab-active !bg-blue-700 !text-white' : ''}`}
118+
className={`tab ${reqViewMode === 'readable' ? 'tab-active !bg-primary !text-primary-content' : ''}`}
119119
onClick={() => setReqViewMode('readable')}
120120
>
121121
Readable
122122
</button>
123123
<button
124124
role="tab"
125-
className={`tab ${reqViewMode === 'raw' ? 'tab-active !bg-blue-700 !text-white' : ''}`}
125+
className={`tab ${reqViewMode === 'raw' ? 'tab-active !bg-primary !text-primary-content' : ''}`}
126126
onClick={() => setReqViewMode('raw')}
127127
>
128128
Raw JSON
@@ -138,7 +138,7 @@ export function ControlDetailSection({ controlData }: ControlDetailSectionProps)
138138
<button
139139
key={v}
140140
role="tab"
141-
className={`tab gap-1 rounded-lg ${selectedReqVersion === v ? 'tab-active !bg-blue-700 !text-white' : ''}`}
141+
className={`tab gap-1 rounded-lg ${selectedReqVersion === v ? 'tab-active !bg-primary !text-primary-content' : ''}`}
142142
onClick={() => handleReqVersionClick(v)}
143143
>
144144
{v}
@@ -185,14 +185,14 @@ export function ControlDetailSection({ controlData }: ControlDetailSectionProps)
185185
<div role="tablist" className="tabs tabs-boxed tabs-xs bg-base-100">
186186
<button
187187
role="tab"
188-
className={`tab ${cfgViewMode === 'readable' ? 'tab-active !bg-accent !text-white' : ''}`}
188+
className={`tab ${cfgViewMode === 'readable' ? 'tab-active !bg-primary !text-primary-content' : ''}`}
189189
onClick={() => setCfgViewMode('readable')}
190190
>
191191
Readable
192192
</button>
193193
<button
194194
role="tab"
195-
className={`tab ${cfgViewMode === 'raw' ? 'tab-active !bg-accent !text-white' : ''}`}
195+
className={`tab ${cfgViewMode === 'raw' ? 'tab-active !bg-primary !text-primary-content' : ''}`}
196196
onClick={() => setCfgViewMode('raw')}
197197
>
198198
Raw JSON
@@ -208,7 +208,7 @@ export function ControlDetailSection({ controlData }: ControlDetailSectionProps)
208208
<button
209209
key={cid}
210210
role="tab"
211-
className={`tab gap-1 rounded-lg ${selectedConfigId === cid ? 'tab-active !bg-accent !text-white' : ''}`}
211+
className={`tab gap-1 rounded-lg ${selectedConfigId === cid ? 'tab-active !bg-primary !text-primary-content' : ''}`}
212212
onClick={() => handleConfigClick(cid)}
213213
>
214214
Config {cid}
@@ -225,7 +225,7 @@ export function ControlDetailSection({ controlData }: ControlDetailSectionProps)
225225
<button
226226
key={v}
227227
role="tab"
228-
className={`tab gap-1 rounded-lg ${selectedConfigVersion === v ? 'tab-active !bg-accent !text-white' : ''}`}
228+
className={`tab gap-1 rounded-lg ${selectedConfigVersion === v ? 'tab-active !bg-primary !text-primary-content' : ''}`}
229229
onClick={() => handleConfigVersionClick(v)}
230230
>
231231
{v}

‎calm-hub/src/integration-test/java/integration/MongoMcpIntegration.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,25 @@ public class MongoMcpIntegration {
6666
}
6767
""";
6868

69+
private static final String CONTROL_REQUIREMENT_JSON = """
70+
{
71+
"control-id": "mcp-test-control",
72+
"name": "MCP Test Control",
73+
"description": "Integration test control requirement"
74+
}
75+
""";
76+
77+
private static final String CONTROL_CONFIGURATION_JSON = """
78+
{
79+
"control-id": "mcp-test-control",
80+
"value": "enforced",
81+
"environment": "integration-test"
82+
}
83+
""";
84+
6985
private static int createdArchitectureId;
7086
private static int createdDecoratorId;
87+
private static int createdControlId;
7188

7289
@Inject
7390
ArchitectureTools architectureTools;
@@ -350,4 +367,70 @@ void mcp_validation_rejects_decorator_type_filter_with_newline() {
350367
assertThat(result.isError(), is(true));
351368
assertThat(text(result), containsString("Type filter"));
352369
}
370+
371+
// --- Control Tools (create paths) ---
372+
373+
@Test
374+
@Order(26)
375+
void mcp_create_control_requirement() {
376+
ToolResponse result = controlTools.createControlRequirement(
377+
"security", "MCP Test Control", "Integration test control requirement", CONTROL_REQUIREMENT_JSON);
378+
assertThat(result.isError(), is(false));
379+
assertThat(text(result), containsString("created successfully"));
380+
381+
Matcher matcher = ID_PATTERN.matcher(text(result));
382+
assertThat("Response should contain control ID", matcher.find());
383+
createdControlId = Integer.parseInt(matcher.group(1));
384+
logger.info("Created control with ID: {}", createdControlId);
385+
}
386+
387+
@Test
388+
@Order(27)
389+
void mcp_list_controls_contains_created() {
390+
ToolResponse result = controlTools.listControls("security");
391+
assertThat(result.isError(), is(false));
392+
assertThat(text(result), containsString("MCP Test Control"));
393+
}
394+
395+
@Test
396+
@Order(28)
397+
void mcp_list_control_versions_after_create() {
398+
ToolResponse result = controlTools.listControlVersions("security", createdControlId);
399+
assertThat(result.isError(), is(false));
400+
assertThat(text(result), containsString("1.0.0"));
401+
}
402+
403+
@Test
404+
@Order(29)
405+
void mcp_get_control_after_create() {
406+
ToolResponse result = controlTools.getControl("security", createdControlId, "1.0.0");
407+
assertThat(result.isError(), is(false));
408+
assertThat(text(result), containsString("mcp-test-control"));
409+
}
410+
411+
@Test
412+
@Order(30)
413+
void mcp_create_control_configuration() {
414+
ToolResponse result = controlTools.createControlConfiguration(
415+
"security", createdControlId, CONTROL_CONFIGURATION_JSON);
416+
assertThat(result.isError(), is(false));
417+
assertThat(text(result), containsString("created successfully"));
418+
}
419+
420+
@Test
421+
@Order(31)
422+
void mcp_create_control_configuration_for_missing_control_returns_error() {
423+
ToolResponse result = controlTools.createControlConfiguration(
424+
"security", 99999, CONTROL_CONFIGURATION_JSON);
425+
assertThat(result.isError(), is(true));
426+
assertThat(text(result), containsString("not found"));
427+
}
428+
429+
@Test
430+
@Order(32)
431+
void mcp_create_control_requirement_rejects_invalid_json() {
432+
ToolResponse result = controlTools.createControlRequirement(
433+
"security", "Bad", "desc", "not-json");
434+
assertThat(result.isError(), is(true));
435+
}
353436
}

‎calm-hub/src/integration-test/java/integration/NitriteMcpIntegration.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,25 @@ public class NitriteMcpIntegration {
6060
}
6161
""";
6262

63+
private static final String CONTROL_REQUIREMENT_JSON = """
64+
{
65+
"control-id": "mcp-nitrite-control",
66+
"name": "MCP Nitrite Control",
67+
"description": "Nitrite integration test control requirement"
68+
}
69+
""";
70+
71+
private static final String CONTROL_CONFIGURATION_JSON = """
72+
{
73+
"control-id": "mcp-nitrite-control",
74+
"value": "enforced",
75+
"environment": "nitrite-test"
76+
}
77+
""";
78+
6379
private static int createdArchitectureId;
6480
private static int createdDecoratorId;
81+
private static int createdControlId;
6582

6683
@Inject
6784
ArchitectureTools architectureTools;
@@ -310,4 +327,70 @@ void mcp_validation_rejects_decorator_type_filter_with_newline() {
310327
assertThat(result.isError(), is(true));
311328
assertThat(text(result), containsString("Type filter"));
312329
}
330+
331+
// --- Control Tools (create paths) ---
332+
333+
@Test
334+
@Order(26)
335+
void mcp_create_control_requirement() {
336+
ToolResponse result = controlTools.createControlRequirement(
337+
"security", "MCP Nitrite Control", "Nitrite integration test control requirement", CONTROL_REQUIREMENT_JSON);
338+
assertThat(result.isError(), is(false));
339+
assertThat(text(result), containsString("created successfully"));
340+
341+
Matcher matcher = ID_PATTERN.matcher(text(result));
342+
assertThat("Response should contain control ID", matcher.find());
343+
createdControlId = Integer.parseInt(matcher.group(1));
344+
logger.info("Created control with ID: {}", createdControlId);
345+
}
346+
347+
@Test
348+
@Order(27)
349+
void mcp_list_controls_contains_created() {
350+
ToolResponse result = controlTools.listControls("security");
351+
assertThat(result.isError(), is(false));
352+
assertThat(text(result), containsString("MCP Nitrite Control"));
353+
}
354+
355+
@Test
356+
@Order(28)
357+
void mcp_list_control_versions_after_create() {
358+
ToolResponse result = controlTools.listControlVersions("security", createdControlId);
359+
assertThat(result.isError(), is(false));
360+
assertThat(text(result), containsString("1.0.0"));
361+
}
362+
363+
@Test
364+
@Order(29)
365+
void mcp_get_control_after_create() {
366+
ToolResponse result = controlTools.getControl("security", createdControlId, "1.0.0");
367+
assertThat(result.isError(), is(false));
368+
assertThat(text(result), containsString("mcp-nitrite-control"));
369+
}
370+
371+
@Test
372+
@Order(30)
373+
void mcp_create_control_configuration() {
374+
ToolResponse result = controlTools.createControlConfiguration(
375+
"security", createdControlId, CONTROL_CONFIGURATION_JSON);
376+
assertThat(result.isError(), is(false));
377+
assertThat(text(result), containsString("created successfully"));
378+
}
379+
380+
@Test
381+
@Order(31)
382+
void mcp_create_control_configuration_for_missing_control_returns_error() {
383+
ToolResponse result = controlTools.createControlConfiguration(
384+
"security", 99999, CONTROL_CONFIGURATION_JSON);
385+
assertThat(result.isError(), is(true));
386+
assertThat(text(result), containsString("not found"));
387+
}
388+
389+
@Test
390+
@Order(32)
391+
void mcp_create_control_requirement_rejects_invalid_json() {
392+
ToolResponse result = controlTools.createControlRequirement(
393+
"security", "Bad", "desc", "not-json");
394+
assertThat(result.isError(), is(true));
395+
}
313396
}

‎calm-hub/src/main/java/org/finos/calm/mcp/tools/ArchitectureTools.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ public ToolResponse createArchitecture(
138138
Optional<ToolResponse> err = McpValidationHelper.firstError(
139139
() -> McpValidationHelper.checkEnabled(mcpEnabled),
140140
() -> McpValidationHelper.validateNamespace(namespace),
141-
() -> McpValidationHelper.validateMaxLength(name, 200, "Architecture name"),
141+
() -> McpValidationHelper.validateMaxLength(name, McpValidationHelper.MAX_NAME_LENGTH, "Architecture name"),
142142
() -> McpValidationHelper.validateDescriptionLength(description, "Architecture description"),
143143
() -> McpValidationHelper.validateNotBlank(architectureJson, "Architecture JSON"),
144144
() -> McpValidationHelper.validateJson(architectureJson, "Architecture JSON"));

‎calm-hub/src/main/java/org/finos/calm/mcp/tools/ControlTools.java‎

Lines changed: 19 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -127,30 +127,17 @@ public ToolResponse createControlRequirement(
127127
@ToolArg(description = "The name of the control requirement") String name,
128128
@ToolArg(description = "A description of the control requirement") String description,
129129
@ToolArg(description = "The full control requirement JSON content") String requirementJson) {
130-
String error = McpValidationHelper.checkEnabled(mcpEnabled);
131-
if (error != null) {
132-
return ToolResponse.error(error);
133-
}
134-
error = McpValidationHelper.validateDomain(domain);
135-
if (error != null) {
136-
return ToolResponse.error(error);
137-
}
138-
error = McpValidationHelper.validateNotBlank(name, "Name");
139-
if (error != null) {
140-
return ToolResponse.error(error);
141-
}
142-
error = McpValidationHelper.validateNotBlank(description, "Description");
143-
if (error != null) {
144-
return ToolResponse.error(error);
145-
}
146-
error = McpValidationHelper.validateNotBlank(requirementJson, "Requirement JSON");
147-
if (error != null) {
148-
return ToolResponse.error(error);
149-
}
150-
error = McpValidationHelper.validateJson(requirementJson, "Requirement JSON");
151-
if (error != null) {
152-
return ToolResponse.error(error);
153-
}
130+
Optional<ToolResponse> err = McpValidationHelper.firstError(
131+
() -> McpValidationHelper.checkEnabled(mcpEnabled),
132+
() -> McpValidationHelper.validateDomain(domain),
133+
() -> McpValidationHelper.validateNotBlank(name, "Name"),
134+
() -> McpValidationHelper.validateMaxLength(name, McpValidationHelper.MAX_NAME_LENGTH, "Name"),
135+
() -> McpValidationHelper.validateNotBlank(description, "Description"),
136+
() -> McpValidationHelper.validateDescriptionLength(description, "Description"),
137+
() -> McpValidationHelper.validateNotBlank(requirementJson, "Requirement JSON"),
138+
() -> McpValidationHelper.validateMaxLength(requirementJson, McpValidationHelper.MAX_JSON_PAYLOAD_LENGTH, "Requirement JSON"),
139+
() -> McpValidationHelper.validateJson(requirementJson, "Requirement JSON"));
140+
if (err.isPresent()) return err.get();
154141

155142
try {
156143
CreateControlRequirement request = new CreateControlRequirement(name, description, requirementJson);
@@ -168,26 +155,14 @@ public ToolResponse createControlConfiguration(
168155
@ToolArg(description = "The domain containing the control (e.g. 'security')") String domain,
169156
@ToolArg(description = "The control ID (positive integer) to create a configuration for") int controlId,
170157
@ToolArg(description = "The full control configuration JSON content") String configurationJson) {
171-
String error = McpValidationHelper.checkEnabled(mcpEnabled);
172-
if (error != null) {
173-
return ToolResponse.error(error);
174-
}
175-
error = McpValidationHelper.validateDomain(domain);
176-
if (error != null) {
177-
return ToolResponse.error(error);
178-
}
179-
error = McpValidationHelper.validatePositiveId(controlId, "Control ID");
180-
if (error != null) {
181-
return ToolResponse.error(error);
182-
}
183-
error = McpValidationHelper.validateNotBlank(configurationJson, "Configuration JSON");
184-
if (error != null) {
185-
return ToolResponse.error(error);
186-
}
187-
error = McpValidationHelper.validateJson(configurationJson, "Configuration JSON");
188-
if (error != null) {
189-
return ToolResponse.error(error);
190-
}
158+
Optional<ToolResponse> err = McpValidationHelper.firstError(
159+
() -> McpValidationHelper.checkEnabled(mcpEnabled),
160+
() -> McpValidationHelper.validateDomain(domain),
161+
() -> McpValidationHelper.validatePositiveId(controlId, "Control ID"),
162+
() -> McpValidationHelper.validateNotBlank(configurationJson, "Configuration JSON"),
163+
() -> McpValidationHelper.validateMaxLength(configurationJson, McpValidationHelper.MAX_JSON_PAYLOAD_LENGTH, "Configuration JSON"),
164+
() -> McpValidationHelper.validateJson(configurationJson, "Configuration JSON"));
165+
if (err.isPresent()) return err.get();
191166

192167
try {
193168
CreateControlConfiguration request = new CreateControlConfiguration(configurationJson);

‎calm-hub/src/main/java/org/finos/calm/mcp/tools/McpValidationHelper.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,17 @@ final class McpValidationHelper {
2020
static final String MCP_DISABLED_MESSAGE =
2121
"Error: MCP tools are currently disabled. Set the environment variable CALM_MCP_ENABLED=true to enable.";
2222

23+
static final int MAX_NAME_LENGTH = 200;
24+
2325
private static final int MAX_DESCRIPTION_LENGTH = 1024;
26+
27+
/**
28+
* Default upper bound for JSON payloads accepted by MCP create tools.
29+
* Sized for typical CALM control / decorator / architecture documents
30+
* while preventing an authenticated client from storing unbounded blobs.
31+
*/
32+
static final int MAX_JSON_PAYLOAD_LENGTH = 100_000;
33+
2434
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
2535

2636
private McpValidationHelper() {

0 commit comments

Comments
 (0)
⚡