Skip to content

Commit e77cbb2

Browse files
committed
Sanitize error messages to avoid reflecting user input
1 parent 709ba4b commit e77cbb2

File tree

10 files changed

+45
-24
lines changed

10 files changed

+45
-24
lines changed

common/src/main/java/org/opensearch/ml/common/agent/MLAgent.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,7 @@
1212

1313
import java.io.IOException;
1414
import java.time.Instant;
15-
import java.util.ArrayList;
16-
import java.util.HashSet;
17-
import java.util.List;
18-
import java.util.Locale;
19-
import java.util.Map;
20-
import java.util.Optional;
21-
import java.util.Set;
15+
import java.util.*;
2216

2317
import org.opensearch.Version;
2418
import org.opensearch.core.common.io.stream.StreamInput;
@@ -122,7 +116,7 @@ private void validate() {
122116
for (MLToolSpec toolSpec : tools) {
123117
String toolName = Optional.ofNullable(toolSpec.getName()).orElse(toolSpec.getType());
124118
if (toolNames.contains(toolName)) {
125-
throw new IllegalArgumentException("Duplicate tool defined: " + toolName);
119+
throw new IllegalArgumentException("Duplicate tool defined in agent configuration");
126120
} else {
127121
toolNames.add(toolName);
128122
}
@@ -138,7 +132,7 @@ private void validateMLAgentType(String agentType) {
138132
MLAgentType.valueOf(agentType.toUpperCase(Locale.ROOT)); // Use toUpperCase() to allow case-insensitive matching
139133
} catch (IllegalArgumentException e) {
140134
// The typeStr does not match any MLAgentType, so throw a new exception with a clearer message.
141-
throw new IllegalArgumentException(agentType + " is not a valid Agent Type");
135+
throw new IllegalArgumentException("Invalid Agent Type, Please use one of " + Arrays.toString(MLAgentType.values()));
142136
}
143137
}
144138
}

common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInput.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import lombok.Builder;
3636
import lombok.Data;
3737
import lombok.Setter;
38+
import org.opensearch.ml.common.connector.ConnectorProtocols;
3839

3940
@Data
4041
public class MLCreateConnectorInput implements ToXContentObject, Writeable {
@@ -121,6 +122,7 @@ public MLCreateConnectorInput(
121122
if ((url == null || url.isBlank()) && isMcpConnector) {
122123
throw new IllegalArgumentException("MCP Connector url is null or blank");
123124
}
125+
ConnectorProtocols.validateProtocol(protocol);
124126
}
125127
this.name = name;
126128
this.description = description;

common/src/test/java/org/opensearch/ml/common/agent/MLAgentTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import java.io.IOException;
1111
import java.time.Instant;
12+
import java.util.Arrays;
1213
import java.util.Collections;
1314
import java.util.List;
1415
import java.util.Map;
@@ -114,7 +115,7 @@ public void constructor_NullLLMSpec() {
114115
@Test
115116
public void constructor_DuplicateTool() {
116117
exceptionRule.expect(IllegalArgumentException.class);
117-
exceptionRule.expectMessage("Duplicate tool defined: test");
118+
exceptionRule.expectMessage("Duplicate tool defined in agent configuration");
118119

119120
MLAgent agent = new MLAgent(
120121
"test_name",
@@ -353,7 +354,7 @@ public void fromStream() throws IOException {
353354
@Test
354355
public void constructor_InvalidAgentType() {
355356
exceptionRule.expect(IllegalArgumentException.class);
356-
exceptionRule.expectMessage(" is not a valid Agent Type");
357+
exceptionRule.expectMessage("Invalid Agent Type, Please use one of "+ Arrays.toString(MLAgentType.values()));
357358

358359
new MLAgent(
359360
"test_name",

common/src/test/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInputTests.java

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@
1010
import static org.junit.Assert.assertNull;
1111
import static org.junit.Assert.assertThrows;
1212
import static org.junit.Assert.assertTrue;
13-
import static org.opensearch.ml.common.connector.ConnectorProtocols.MCP_SSE;
14-
import static org.opensearch.ml.common.connector.ConnectorProtocols.MCP_STREAMABLE_HTTP;
13+
import static org.opensearch.ml.common.connector.ConnectorProtocols.*;
1514

1615
import java.io.IOException;
1716
import java.util.Arrays;
@@ -169,6 +168,26 @@ public void constructorMLCreateConnectorInput_NullProtocol() {
169168
assertEquals("Connector protocol is null", exception.getMessage());
170169
}
171170

171+
@Test
172+
public void constructorMLCreateConnectorInput_InvalidProtocol() {
173+
Throwable exception = assertThrows(IllegalArgumentException.class, () -> {
174+
MLCreateConnectorInput
175+
.builder()
176+
.name(TEST_CONNECTOR_NAME)
177+
.description(TEST_CONNECTOR_DESCRIPTION)
178+
.version(TEST_CONNECTOR_VERSION)
179+
.protocol("dummy")
180+
.parameters(Map.of(TEST_PARAM_KEY, TEST_PARAM_VALUE))
181+
.credential(Map.of(TEST_CREDENTIAL_KEY, TEST_CREDENTIAL_VALUE))
182+
.actions(List.of())
183+
.access(AccessMode.PUBLIC)
184+
.backendRoles(Arrays.asList(TEST_ROLE1, TEST_ROLE2))
185+
.addAllBackendRoles(false)
186+
.build();
187+
});
188+
assertEquals( "Unsupported connector protocol. Please use one of " + Arrays.toString(VALID_PROTOCOLS.toArray(new String[0])), exception.getMessage());
189+
}
190+
172191
@Test
173192
public void constructorMLCreateConnectorInput_NullCredential() {
174193
Throwable exception = assertThrows(IllegalArgumentException.class, () -> {

ml-algorithms/src/main/java/org/opensearch/ml/engine/algorithms/agent/AgentUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,7 @@ public static List<String> getToolNames(Map<String, Tool> tools) {
993993

994994
public static Tool createTool(Map<String, Tool.Factory> toolFactories, Map<String, String> executeParams, MLToolSpec toolSpec) {
995995
if (!toolFactories.containsKey(toolSpec.getType())) {
996-
throw new IllegalArgumentException("Tool not found: " + toolSpec.getType());
996+
throw new IllegalArgumentException("Tool type not found" );
997997
}
998998
Map<String, Object> toolParams = new HashMap<>();
999999
toolParams.putAll(executeParams);

ml-algorithms/src/main/java/org/opensearch/ml/engine/algorithms/agent/MLAgentExecutor.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,7 @@
2121
import java.security.PrivilegedActionException;
2222
import java.security.PrivilegedExceptionAction;
2323
import java.time.Instant;
24-
import java.util.ArrayList;
25-
import java.util.HashMap;
26-
import java.util.List;
27-
import java.util.Locale;
28-
import java.util.Map;
29-
import java.util.Objects;
24+
import java.util.*;
3025

3126
import org.opensearch.ExceptionsHelper;
3227
import org.opensearch.OpenSearchException;
@@ -654,7 +649,8 @@ protected MLAgentRunner getAgentRunner(MLAgent mlAgent) {
654649
encryptor
655650
);
656651
default:
657-
throw new IllegalArgumentException("Unsupported agent type: " + mlAgent.getType());
652+
throw new IllegalArgumentException("Unsupported agent type. Please use one of " +
653+
Arrays.toString(MLAgentType.values()));
658654
}
659655
}
660656

ml-algorithms/src/main/java/org/opensearch/ml/engine/function_calling/FunctionCallingFactory.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@ public static FunctionCalling create(String llmInterface) {
2727
case LLM_INTERFACE_BEDROCK_CONVERSE_DEEPSEEK_R1:
2828
return new BedrockConverseDeepseekR1FunctionCalling();
2929
default:
30-
throw new IllegalArgumentException(String.format("Invalid _llm_interface: %s", llmInterface));
30+
throw new IllegalArgumentException(
31+
String.format("Invalid _llm_interface. Supported values are %s,%s,%s",
32+
LLM_INTERFACE_BEDROCK_CONVERSE_CLAUDE,
33+
LLM_INTERFACE_OPENAI_V1_CHAT_COMPLETIONS,
34+
LLM_INTERFACE_BEDROCK_CONVERSE_DEEPSEEK_R1));
3135
}
3236
}
3337
}

ml-algorithms/src/test/java/org/opensearch/ml/engine/algorithms/agent/MLAgentExecutorTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@
3232

3333
import org.junit.Assert;
3434
import org.junit.Before;
35+
import org.junit.Rule;
3536
import org.junit.Test;
37+
import org.junit.rules.ExpectedException;
3638
import org.mockito.ArgumentCaptor;
3739
import org.mockito.Captor;
3840
import org.mockito.Mock;
@@ -140,6 +142,9 @@ public class MLAgentExecutorTest {
140142
@Captor
141143
private ArgumentCaptor<Exception> exceptionCaptor;
142144

145+
@Rule
146+
public ExpectedException exceptionRule = ExpectedException.none();
147+
143148
private DiscoveryNode localNode = new DiscoveryNode(
144149
"mockClusterManagerNodeId",
145150
"mockClusterManagerNodeId",

plugin/src/main/java/org/opensearch/ml/action/agents/TransportRegisterAgentAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ private void registerAgent(MLAgent agent, ActionListener<MLRegisterAgentResponse
104104
try {
105105
FunctionCallingFactory.create(llmInterface);
106106
} catch (Exception e) {
107-
listener.onFailure(new IllegalArgumentException("Invalid _llm_interface: " + llmInterface));
107+
listener.onFailure(new IllegalArgumentException("Invalid _llm_interface"));
108108
return;
109109
}
110110
}

plugin/src/main/java/org/opensearch/ml/rest/mcpserver/RestMLMcpToolsRegisterAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
109109
.filter(type -> !buildInToolNames.contains(type))
110110
.collect(Collectors.toSet());
111111
if (!unrecognizedTools.isEmpty()) {
112-
exception.addValidationError(String.format(Locale.ROOT, "Unrecognized tool in request: %s", unrecognizedTools));
112+
exception.addValidationError("Unrecognized tool in request");
113113
throw exception;
114114
}
115115
return channel -> client.execute(MLMcpToolsRegisterAction.INSTANCE, registerNodesRequest, new RestToXContentListener<>(channel));

0 commit comments

Comments
 (0)