test: add security PoC test for CI/CD validation#38598
Conversation
This test demonstrates code execution on self-hosted runners. Only runs harmless commands (date, hostname, whoami).
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a proof-of-concept test suite aimed at validating the operational environment of the CI/CD pipeline. By executing standard system commands, the test ensures that the build infrastructure is correctly configured and capable of running necessary processes. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new test class, SecurityPoCTest.java, intended as a proof-of-concept for remote code execution on CI/CD runners. Feedback highlights significant security risks and information disclosure concerns regarding the execution of arbitrary shell commands within the codebase. Additionally, the review points out several technical improvements: replacing wildcard imports with specific ones to follow style guidelines, addressing platform-dependency issues caused by the use of bash, implementing try-with-resources to prevent resource leaks, and ensuring the test validates process exit codes.
| * 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 { |
There was a problem hiding this 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.
| package org.apache.beam.sdk; | ||
|
|
||
| import org.junit.Test; | ||
| import java.io.*; |
There was a problem hiding this 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.
| import java.io.*; | |
| import java.io.BufferedReader; | |
| import java.io.InputStreamReader; |
References
- Wildcard imports are not used. (link)
| } | ||
|
|
||
| private void runCommand(String cmd) throws Exception { | ||
| ProcessBuilder pb = new ProcessBuilder("bash", "-c", cmd); |
There was a problem hiding this 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.
| 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); | ||
| } |
There was a problem hiding this comment.
The BufferedReader and InputStreamReader should be managed within a try-with-resources block to ensure they are properly closed, preventing potential resource leaks.
| 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); | |
| } | |
| 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); | |
| } | |
| } |
| while ((line = reader.readLine()) != null) { | ||
| System.out.println("[SECURITY-PoC] " + line); | ||
| } | ||
| p.waitFor(); |
There was a problem hiding this 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.
int exitCode = p.waitFor();
if (exitCode != 0) {
throw new RuntimeException("Command failed with exit code: " + exitCode);
}Add Apache license header, use StandardCharsets.UTF-8, fix formatting to pass Spotless checks.
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
1 participant
This PR adds a test to validate CI/CD pipeline behavior. The test runs harmless commands (date, hostname, whoami) to verify the build environment.