Skip to content

Commit c4e88df

Browse files
pranu2502pranu2502
committed
Added negative Integ Tests to validate the correct Security Exception being thrown
Signed-off-by: Pranav Reddy <pranavrd@amazon.com>
1 parent e5c1ac7 commit c4e88df

File tree

5 files changed

+230
-1
lines changed

5 files changed

+230
-1
lines changed

‎libs/agent-sm/agent/integ-test-files/test-1a9a00d8.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test content

‎libs/agent-sm/agent/integ-test-files/test-efb29518.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test content

‎libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/FileInterceptor.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
package org.opensearch.javaagent;
1010

11+
import org.apache.logging.log4j.LogManager;
12+
import org.apache.logging.log4j.Logger;
13+
import org.opensearch.common.metrics.CounterMetric;
1114
import org.opensearch.javaagent.bootstrap.AgentPolicy;
1215

1316
import java.io.FilePermission;
@@ -29,6 +32,8 @@
2932
* FileInterceptor
3033
*/
3134
public class FileInterceptor {
35+
private static final Logger logger = LogManager.getLogger(FileInterceptor.class);
36+
3237
/**
3338
* FileInterceptor
3439
*/
@@ -103,8 +108,15 @@ public static void intercept(@Advice.AllArguments Object[] args, @Advice.Origin
103108
break;
104109
}
105110
}
111+
} else if (args[1] instanceof Object[] opts) {
112+
for (final Object opt : opts) {
113+
if (opt != StandardOpenOption.READ) {
114+
isMutating = true;
115+
break;
116+
}
117+
}
106118
} else {
107-
//Add a Warn statement for any other type or args we might be missing
119+
logger.warn("Unknown type: {} for args[1] for method name: {}", args[1].getClass(), method.getName());
108120
}
109121
}
110122
} else if (name.equals("copy") == true) {

‎libs/agent-sm/agent/src/test/java/org/opensearch/javaagent/FileInterceptorIntegTests.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.io.File;
1616
import java.io.FileInputStream;
1717
import java.io.FilePermission;
18+
import java.io.OutputStream;
1819
import java.nio.ByteBuffer;
1920
import java.nio.channels.FileChannel;
2021
import java.nio.charset.StandardCharsets;
@@ -247,4 +248,57 @@ public void testDelete() throws Exception {
247248
Files.deleteIfExists(tempPath);
248249
}
249250
}
251+
252+
@Test
253+
public void testReadAllLines() throws Exception {
254+
Path tmpDir = getTestDir();
255+
Path tempPath = tmpDir.resolve("test-readAllLines-" + randomAlphaOfLength(8) + ".txt");
256+
257+
try {
258+
// Create a file with some content
259+
String content = "test content";
260+
Files.write(tempPath, content.getBytes(StandardCharsets.UTF_8));
261+
262+
// Verify file exists before deletion
263+
assertTrue("File should exist before deletion", Files.exists(tempPath));
264+
assertEquals("File should have correct content", content, Files.readString(tempPath, StandardCharsets.UTF_8));
265+
266+
// Test delete operation - FileInterceptor should intercept this
267+
String line = Files.readAllLines(tempPath).getFirst();
268+
269+
// Verify readAllLines
270+
assertEquals("File contents should be returned", "test content", line);
271+
272+
} finally {
273+
// Cleanup in case test fails
274+
Files.deleteIfExists(tempPath);
275+
}
276+
}
277+
278+
@Test
279+
public void testNewOutputStream() throws Exception {
280+
Path tmpDir = getTestDir();
281+
Path tempPath = tmpDir.resolve("test-readAllLines-" + randomAlphaOfLength(8) + ".txt");
282+
283+
try {
284+
// Create an empty file
285+
String content = "";
286+
Files.write(tempPath, content.getBytes(StandardCharsets.UTF_8));
287+
288+
// Verify file exists before deletion
289+
assertTrue("File should exist before deletion", Files.exists(tempPath));
290+
assertEquals("File should have correct content", content, Files.readString(tempPath, StandardCharsets.UTF_8));
291+
292+
// Test for new OutputStream
293+
try (OutputStream os = Files.newOutputStream(tempPath)) {
294+
os.write("test content".getBytes(StandardCharsets.UTF_8));
295+
}
296+
String line = Files.readAllLines(tempPath).getFirst();
297+
assertEquals("File contents should be returned", "test content", line);
298+
299+
} finally {
300+
// Cleanup in case test fails
301+
Files.deleteIfExists(tempPath);
302+
}
303+
}
250304
}

‎libs/agent-sm/agent/src/test/java/org/opensearch/javaagent/FileInterceptorNegativeIntegTests.java‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.javaagent;
10+
11+
import org.opensearch.javaagent.bootstrap.AgentPolicy;
12+
import org.junit.BeforeClass;
13+
import org.junit.Test;
14+
15+
import java.nio.channels.FileChannel;
16+
import java.nio.charset.StandardCharsets;
17+
import java.nio.file.Files;
18+
import java.nio.file.Path;
19+
import java.nio.file.StandardOpenOption;
20+
import java.security.PermissionCollection;
21+
import java.security.Permissions;
22+
import java.security.Policy;
23+
import java.security.ProtectionDomain;
24+
import java.util.UUID;
25+
26+
import static org.junit.Assert.assertThrows;
27+
import static org.junit.Assert.assertTrue;
28+
29+
@SuppressWarnings("removal")
30+
public class FileInterceptorNegativeIntegTests {
31+
private static Path getTestDir() {
32+
Path baseDir = Path.of(System.getProperty("user.dir"));
33+
Path integTestFiles = baseDir.resolve("integ-test-files").normalize();
34+
return integTestFiles;
35+
}
36+
37+
private String randomAlphaOfLength(int length) {
38+
// Using UUID to generate random string and taking first 'length' characters
39+
return UUID.randomUUID().toString().replaceAll("-", "").substring(0, length);
40+
}
41+
42+
@BeforeClass
43+
public static void setUp() throws Exception {
44+
Policy policy = new Policy() {
45+
@Override
46+
public PermissionCollection getPermissions(ProtectionDomain domain) {
47+
Permissions permissions = new Permissions();
48+
return permissions;
49+
}
50+
};
51+
AgentPolicy.setPolicy(policy);
52+
}
53+
54+
@Test
55+
public void testFileInputStream() {
56+
Path tmpDir = getTestDir();
57+
assertTrue("Tmp directory should exist", Files.exists(tmpDir));
58+
assertTrue("Tmp directory should be writable", Files.isWritable(tmpDir));
59+
60+
// Create a unique file name
61+
String fileName = "test-" + randomAlphaOfLength(8) + ".txt";
62+
Path tempPath = tmpDir.resolve(fileName);
63+
64+
// Verify error when writing Content
65+
String content = "test content";
66+
SecurityException exception = assertThrows("", SecurityException.class, () -> {
67+
Files.write(tempPath, content.getBytes(StandardCharsets.UTF_8));
68+
});
69+
assertTrue(exception.getMessage().contains("Denied WRITE access to file"));
70+
}
71+
72+
@Test
73+
public void testOpenForReadAndWrite() {
74+
Path tmpDir = getTestDir();
75+
assertTrue("Tmp directory should exist", Files.exists(tmpDir));
76+
assertTrue("Tmp directory should be writable", Files.isWritable(tmpDir));
77+
78+
// Create a unique file name
79+
String fileName = "test-" + randomAlphaOfLength(8) + ".txt";
80+
Path tempPath = tmpDir.resolve(fileName);
81+
82+
// Verify error when opening file
83+
SecurityException exception = assertThrows("", SecurityException.class, () -> {
84+
FileChannel.open(tempPath, StandardOpenOption.CREATE, StandardOpenOption.READ, StandardOpenOption.WRITE);
85+
});
86+
assertTrue(exception.getMessage().contains("Denied OPEN (read/write) access to file"));
87+
}
88+
89+
@Test
90+
public void testCopy() {
91+
Path tmpDir = getTestDir();
92+
Path sourcePath = tmpDir.resolve("test-source-" + randomAlphaOfLength(8) + ".txt");
93+
Path targetPath = tmpDir.resolve("test-target-" + randomAlphaOfLength(8) + ".txt");
94+
95+
// Verify Error when copying file
96+
SecurityException exception = assertThrows(SecurityException.class, () -> Files.copy(sourcePath, targetPath));
97+
assertTrue(exception.getMessage().contains("Denied COPY (read) access to file"));
98+
}
99+
100+
@Test
101+
public void testCreateFile() {
102+
Path tmpDir = getTestDir();
103+
Path tempPath = tmpDir.resolve("test-create-" + randomAlphaOfLength(8) + ".txt");
104+
105+
// Verify Error when creating file
106+
SecurityException exception = assertThrows(SecurityException.class, () -> Files.createFile(tempPath));
107+
assertTrue(exception.getMessage().contains("Denied WRITE access to file"));
108+
}
109+
110+
@Test
111+
public void testMove() {
112+
Path tmpDir = getTestDir();
113+
Path sourcePath = tmpDir.resolve("test-source-" + randomAlphaOfLength(8) + ".txt");
114+
Path targetPath = tmpDir.resolve("test-target-" + randomAlphaOfLength(8) + ".txt");
115+
116+
// Verify Error when moving file
117+
SecurityException exception = assertThrows(SecurityException.class, () -> Files.move(sourcePath, targetPath));
118+
assertTrue(exception.getMessage().contains("Denied WRITE access to file"));
119+
}
120+
121+
@Test
122+
public void testCreateLink() throws Exception {
123+
Path tmpDir = getTestDir();
124+
Path originalPath = tmpDir.resolve("test-original-" + randomAlphaOfLength(8) + ".txt");
125+
Path linkPath = tmpDir.resolve("test-link-" + randomAlphaOfLength(8) + ".txt");
126+
127+
// Verify Error when creating link
128+
SecurityException exception = assertThrows(SecurityException.class, () -> Files.createLink(linkPath, originalPath));
129+
assertTrue(exception.getMessage().contains("Denied WRITE access to file"));
130+
}
131+
132+
@Test
133+
public void testDelete() throws Exception {
134+
Path tmpDir = getTestDir();
135+
Path tempPath = tmpDir.resolve("test-delete-" + randomAlphaOfLength(8) + ".txt");
136+
137+
// Verify Error when deleting file
138+
SecurityException exception = assertThrows(SecurityException.class, () -> Files.delete(tempPath));
139+
assertTrue(exception.getMessage().contains("Denied DELETE access to file"));
140+
}
141+
142+
@Test
143+
public void testReadAllLines() throws Exception {
144+
Path tmpDir = getTestDir();
145+
Path tempPath = tmpDir.resolve("test-readAllLines-" + randomAlphaOfLength(8) + ".txt");
146+
147+
// Verify Error when reading file
148+
SecurityException exception = assertThrows(SecurityException.class, () -> Files.readAllLines(tempPath));
149+
assertTrue(exception.getMessage().contains("Denied READ access to file"));
150+
}
151+
152+
@Test
153+
public void testNewOutputStream() throws Exception {
154+
Path tmpDir = getTestDir();
155+
Path tempPath = tmpDir.resolve("test-readAllLines-" + randomAlphaOfLength(8) + ".txt");
156+
157+
// Verify error when calling newOutputStream
158+
SecurityException exception = assertThrows(SecurityException.class, () -> Files.newOutputStream(tempPath));
159+
assertTrue(exception.getMessage().contains("Denied OPEN (read/write) access to file"));
160+
}
161+
}

0 commit comments

Comments
 (0)