Skip to content

Commit 01c9af4

Browse files
Add tool security-related tests
Signed-off-by: Nathalie Jonathan <[email protected]>
1 parent 7d25d56 commit 01c9af4

File tree

4 files changed

+104
-0
lines changed

4 files changed

+104
-0
lines changed

ml-algorithms/src/test/java/org/opensearch/ml/engine/algorithms/tool/MLToolExecutorTest.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,4 +202,26 @@ public void test_ImmutableEmptyParametersMap() {
202202
Output output = outputCaptor.getValue();
203203
Assert.assertTrue(output instanceof ModelTensorOutput);
204204
}
205+
206+
@Test
207+
public void test_ToolExecutionFailsWithoutProperPermission() {
208+
when(toolMLInput.getToolName()).thenReturn("TestTool");
209+
when(toolMLInput.getInputDataset()).thenReturn(inputDataSet);
210+
when(inputDataSet.getParameters()).thenReturn(parameters);
211+
when(toolFactory.create(any())).thenReturn(tool);
212+
when(tool.validate(parameters)).thenReturn(true);
213+
214+
Mockito.doAnswer(invocation -> {
215+
ActionListener<Object> listener = invocation.getArgument(1);
216+
listener.onFailure(new SecurityException("no permissions for [indices:data/read/search] and User [name=test_user]"));
217+
return null;
218+
}).when(tool).run(Mockito.eq(parameters), any());
219+
220+
mlToolExecutor.execute(toolMLInput, actionListener);
221+
222+
Mockito.verify(actionListener).onFailure(exceptionCaptor.capture());
223+
Exception exception = exceptionCaptor.getValue();
224+
Assert.assertTrue(exception instanceof SecurityException);
225+
Assert.assertTrue(exception.getMessage().contains("no permissions"));
226+
}
205227
}

plugin/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ integTest {
237237
filter {
238238
excludeTestsMatching "org.opensearch.ml.rest.SecureMLRestIT"
239239
excludeTestsMatching "org.opensearch.ml.rest.MLModelGroupRestIT"
240+
excludeTestsMatching "org.opensearch.ml.tools.ListIndexToolIT"
240241
}
241242
}
242243

plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,13 @@
1414
import java.util.Objects;
1515

1616
import org.apache.commons.lang3.StringUtils;
17+
import org.apache.hc.core5.http.HttpHost;
1718
import org.junit.Before;
1819
import org.opensearch.client.Response;
20+
import org.opensearch.client.ResponseException;
21+
import org.opensearch.client.RestClient;
1922
import org.opensearch.common.settings.Settings;
23+
import org.opensearch.commons.rest.SecureRestClientBuilder;
2024
import org.opensearch.ml.engine.tools.ListIndexTool;
2125
import org.opensearch.ml.rest.RestBaseAgentToolsIT;
2226
import org.opensearch.ml.utils.TestHelper;
@@ -37,6 +41,44 @@ public void setUpCluster() throws Exception {
3741
registerListIndexFlowAgent();
3842
}
3943

44+
public void testListIndexWithNoPermissions() throws Exception {
45+
if (!isHttps()) {
46+
log.info("Skipping permission test as security is not enabled");
47+
return;
48+
}
49+
50+
String noPermissionUser = "no_permission_user";
51+
String password = "TestPassword123!";
52+
53+
try {
54+
createUser(noPermissionUser, password, new ArrayList<>());
55+
56+
final RestClient noPermissionClient = new SecureRestClientBuilder(
57+
getClusterHosts().toArray(new HttpHost[0]),
58+
isHttps(),
59+
noPermissionUser,
60+
password
61+
).setSocketTimeout(60000).build();
62+
63+
try {
64+
ResponseException exception = expectThrows(ResponseException.class, () -> {
65+
TestHelper
66+
.makeRequest(noPermissionClient, "POST", "/_plugins/_ml/agents/" + agentId + "/_execute", null, question, null);
67+
});
68+
69+
String errorMessage = exception.getMessage().toLowerCase();
70+
assertTrue(
71+
"Expected permission error, got: " + errorMessage,
72+
errorMessage.contains("no permissions") || errorMessage.contains("forbidden") || errorMessage.contains("unauthorized")
73+
);
74+
} finally {
75+
noPermissionClient.close();
76+
}
77+
} finally {
78+
deleteUser(noPermissionUser);
79+
}
80+
}
81+
4082
private List<String> createIndices(int count) throws IOException {
4183
List<String> indices = new ArrayList<>();
4284
for (int i = 0; i < count; i++) {
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.ml.tools;
7+
8+
import static org.opensearch.ml.utils.TestHelper.makeRequest;
9+
10+
import org.junit.Before;
11+
import org.opensearch.ml.rest.MLCommonsRestTestCase;
12+
import org.opensearch.test.OpenSearchIntegTestCase;
13+
14+
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, numDataNodes = 3)
15+
public class ScratchPadToolIT extends MLCommonsRestTestCase {
16+
17+
@Before
18+
public void setUp() throws Exception {
19+
super.setUp();
20+
}
21+
22+
public void testScratchpadSizeLimit() throws Exception {
23+
String largeContent = "A".repeat(100 * 1024 * 1024);
24+
String requestBody = String.format("{\"parameters\":{\"notes\":\"%s\"}}", largeContent);
25+
26+
Exception exception = expectThrows(Exception.class, () -> {
27+
makeRequest(client(), "POST", "/_plugins/_ml/tools/_execute/WriteToScratchPadTool", null, requestBody, null);
28+
});
29+
30+
String errorMessage = exception.getMessage().toLowerCase();
31+
assertTrue(
32+
"Expected HTTP content length error, got: " + errorMessage,
33+
errorMessage.contains("content length")
34+
|| errorMessage.contains("too large")
35+
|| errorMessage.contains("entity too large")
36+
|| errorMessage.contains("413")
37+
);
38+
}
39+
}

0 commit comments

Comments
 (0)