This is an automated email from the ASF dual-hosted git repository.
jialiang pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push:
new 99630e03e0 AMBARI-26240: Fix alter dispatcher (#3896)
99630e03e0 is described below
commit 99630e03e0d9315bafbc0f81c0bcd2a94b34058d
Author: jialiang <[email protected]>
AuthorDate: Tue Nov 26 13:45:08 2024 +0800
AMBARI-26240: Fix alter dispatcher (#3896)
---
.../dispatchers/AlertScriptDispatcher.java | 33 ++++
.../dispatchers/AlertScriptDispatcherTest.java | 166 ++++++++++++++++++++-
2 files changed, 195 insertions(+), 4 deletions(-)
diff --git
a/ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
b/ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
index 2dfcaa58f7..4766aeb171 100644
---
a/ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
+++
b/ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
@@ -18,6 +18,9 @@
package org.apache.ambari.server.notifications.dispatchers;
import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.Map;
import java.util.concurrent.Executor;
import java.util.concurrent.LinkedBlockingQueue;
@@ -279,6 +282,36 @@ public class AlertScriptDispatcher implements
NotificationDispatcher {
* @return
*/
ProcessBuilder getProcessBuilder(String script, AlertNotification
notification) {
+ if (script == null || script.isEmpty()) {
+ throw new IllegalArgumentException("Script path cannot be null or
empty");
+ }
+
+ String unixPathRegex = "^(/[a-zA-Z0-9._-]+)+$";
+ String windowsPathRegex = "^[a-zA-Z]:\\\\([a-zA-Z0-9._-]+\\\\?)+$";
+
+ boolean isValidPath = SystemUtils.IS_OS_WINDOWS
+ ? script.matches(windowsPathRegex)
+ : script.matches(unixPathRegex);
+
+ if (!isValidPath) {
+ throw new IllegalArgumentException("Invalid script path format: " +
script);
+ }
+
+ Path scriptPath = Paths.get(script);
+
+ if (!scriptPath.isAbsolute()) {
+ throw new IllegalArgumentException("Script path must be an absolute
path: " + script);
+ }
+
+ if (!Files.exists(scriptPath)) {
+ throw new IllegalArgumentException("Script does not exist: " + script);
+ }
+
+ if (!Files.isExecutable(scriptPath)) {
+ throw new IllegalArgumentException("Script is not executable: " +
script);
+ }
+
+
final String shellCommand;
final String shellCommandOption;
if (SystemUtils.IS_OS_WINDOWS) {
diff --git
a/ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
b/ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
index 4b1480ca81..f3fb7e22c8 100644
---
a/ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
+++
b/ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
@@ -17,6 +17,8 @@
*/
package org.apache.ambari.server.notifications.dispatchers;
+import java.io.File;
+import java.io.IOException;
import java.lang.reflect.Field;
import java.util.Collections;
import java.util.HashMap;
@@ -38,7 +40,9 @@ import
org.apache.ambari.server.state.services.AlertNoticeDispatchService.AlertI
import org.apache.ambari.server.state.stack.OsFamily;
import org.easymock.EasyMock;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.powermock.api.easymock.PowerMock;
import org.powermock.core.classloader.annotations.PrepareForTest;
@@ -70,12 +74,162 @@ public class AlertScriptDispatcherTest {
@Inject
private Configuration m_configuration;
+ @Rule
+ public TemporaryFolder tempFolder = new TemporaryFolder();
+
+ private final AlertNotification mockNotification =
EasyMock.createNiceMock(AlertNotification.class);
+ private final AlertScriptDispatcher dispatcher = new AlertScriptDispatcher();
+
+
@Before
public void before() throws Exception {
m_injector = Guice.createInjector(new MockModule());
m_injector.injectMembers(this);
}
+
+ @Test
+ public void testNullScriptPath() {
+ try {
+ dispatcher.getProcessBuilder(null, mockNotification);
+ Assert.fail("Expected IllegalArgumentException was not thrown");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Script path cannot be null or empty",
e.getMessage());
+ }
+ }
+
+ @Test
+ public void testEmptyScriptPath() {
+ try {
+ dispatcher.getProcessBuilder("", mockNotification);
+ Assert.fail("Expected IllegalArgumentException was not thrown");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Script path cannot be null or empty",
e.getMessage());
+ }
+ }
+
+ @Test
+ public void testRelativeScriptPath() {
+ try {
+ dispatcher.getProcessBuilder("relative/path/to/script.sh",
mockNotification);
+ Assert.fail("Expected IllegalArgumentException was not thrown");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Invalid script path format:
relative/path/to/script.sh", e.getMessage());
+ }
+ }
+
+ @Test
+ public void testNonExistentScriptPath() {
+ try {
+ dispatcher.getProcessBuilder("/non/existent/path/to/script.sh",
mockNotification);
+ Assert.fail("Expected IllegalArgumentException was not thrown");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Script does not exist:
/non/existent/path/to/script.sh", e.getMessage());
+ }
+ }
+
+ @Test
+ public void testNonExecutableScriptPath() throws IOException {
+ // Create a temp file that is not executable
+ File nonExecutableFile = tempFolder.newFile("non_executable_script.sh");
+
+ try {
+ dispatcher.getProcessBuilder(nonExecutableFile.getAbsolutePath(),
mockNotification);
+ Assert.fail("Expected IllegalArgumentException was not thrown");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Script is not executable: " +
nonExecutableFile.getAbsolutePath(), e.getMessage());
+ }
+ }
+
+ @Test
+ public void testValidScriptPath() throws IOException {
+ File validScript = tempFolder.newFile("valid_script.sh");
+ validScript.setExecutable(true);
+ String scriptAbsolutePath = validScript.getAbsolutePath();
+
+ final String ALERT_DEFINITION_NAME = "mock_alert_with_quotes";
+ final String ALERT_DEFINITION_LABEL = "Mock alert with Quotes";
+ final String ALERT_LABEL = "Alert Label";
+ final String ALERT_SERVICE_NAME = "FOO_SERVICE";
+ final String ALERT_TEXT = "Did you know, \"Quotes are hard!!!\"";
+ final String ALERT_TEXT_ESCAPED = "Did you know, \\\"Quotes are
hard\\!\\!\\!\\\"";
+ final String ALERT_HOST = "mock_host";
+ final long ALERT_TIMESTAMP = 1111111l;
+
+ DispatchCallback callback =
EasyMock.createNiceMock(DispatchCallback.class);
+ AlertNotification notification = new AlertNotification();
+ notification.Callback = callback;
+ notification.CallbackIds =
Collections.singletonList(UUID.randomUUID().toString());
+
+ AlertDefinitionEntity definition = new AlertDefinitionEntity();
+ definition.setDefinitionName(ALERT_DEFINITION_NAME);
+ definition.setLabel(ALERT_DEFINITION_LABEL);
+
+ AlertHistoryEntity history = new AlertHistoryEntity();
+ history.setAlertDefinition(definition);
+ history.setAlertLabel(ALERT_LABEL);
+ history.setAlertText(ALERT_TEXT);
+ history.setAlertState(AlertState.OK);
+ history.setServiceName(ALERT_SERVICE_NAME);
+ history.setHostName(ALERT_HOST);
+ history.setAlertTimestamp(ALERT_TIMESTAMP);
+
+
+ AlertInfo alertInfo = new AlertInfo(history);
+ notification.setAlertInfo(alertInfo);
+
+ AlertScriptDispatcher dispatcher = new AlertScriptDispatcher();
+ m_injector.injectMembers(dispatcher);
+
+ ProcessBuilder processBuilder =
dispatcher.getProcessBuilder(scriptAbsolutePath, notification);
+
+ Assert.assertNotNull(processBuilder);
+ Assert.assertEquals("sh", processBuilder.command().get(0));
+ Assert.assertEquals("-c", processBuilder.command().get(1));
+
Assert.assertTrue(processBuilder.command().get(2).contains(scriptAbsolutePath));
+
+ }
+
+ @Test
+ public void testInjectionAttemptWithSemicolon() {
+ try {
+ dispatcher.getProcessBuilder("/path/to/script.sh; rm -rf /",
mockNotification);
+ Assert.fail("Expected IllegalArgumentException was not thrown");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Invalid script path format: /path/to/script.sh; rm
-rf /", e.getMessage());
+ }
+ }
+
+ @Test
+ public void testInjectionAttemptWithAndOperator() {
+ try {
+ dispatcher.getProcessBuilder("/path/to/script.sh && touch /tmp/hacked",
mockNotification);
+ Assert.fail("Expected IllegalArgumentException was not thrown");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Invalid script path format: /path/to/script.sh &&
touch /tmp/hacked", e.getMessage());
+ }
+ }
+
+ @Test
+ public void testInjectionAttemptWithPipeOperator() {
+ try {
+ dispatcher.getProcessBuilder("/path/to/script.sh | ls",
mockNotification);
+ Assert.fail("Expected IllegalArgumentException was not thrown");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Invalid script path format: /path/to/script.sh |
ls", e.getMessage());
+ }
+ }
+
+ @Test
+ public void testInjectionAttemptWithBackticks() {
+ try {
+ dispatcher.getProcessBuilder("/path/to/script.sh `rm -rf /`",
mockNotification);
+ Assert.fail("Expected IllegalArgumentException was not thrown");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Invalid script path format: /path/to/script.sh `rm
-rf /`", e.getMessage());
+ }
+ }
+
/**
* Tests that a callback error happens when the notification is not an
* {@link AlertNotification}.
@@ -307,6 +461,10 @@ public class AlertScriptDispatcherTest {
final String ALERT_HOST = "mock_host";
final long ALERT_TIMESTAMP = 1111111l;
+ File validScript = tempFolder.newFile("valid_script.sh");
+ validScript.setExecutable(true);
+ String scriptAbsolutePath = validScript.getAbsolutePath();
+
DispatchCallback callback =
EasyMock.createNiceMock(DispatchCallback.class);
AlertNotification notification = new AlertNotification();
notification.Callback = callback;
@@ -332,14 +490,14 @@ public class AlertScriptDispatcherTest {
AlertScriptDispatcher dispatcher = new AlertScriptDispatcher();
m_injector.injectMembers(dispatcher);
- ProcessBuilder processBuilder =
dispatcher.getProcessBuilder(SCRIPT_CONFIG_VALUE, notification);
+ ProcessBuilder processBuilder =
dispatcher.getProcessBuilder(scriptAbsolutePath, notification);
List<String> commands = processBuilder.command();
Assert.assertEquals(3, commands.size());
Assert.assertEquals("sh", commands.get(0));
Assert.assertEquals("-c", commands.get(1));
StringBuilder buffer = new StringBuilder();
- buffer.append(SCRIPT_CONFIG_VALUE).append(" ");
+ buffer.append(scriptAbsolutePath).append(" ");
buffer.append(ALERT_DEFINITION_NAME).append(" ");
buffer.append("\"").append(ALERT_DEFINITION_LABEL).append("\"").append("
");
buffer.append(ALERT_SERVICE_NAME).append(" ");
@@ -355,10 +513,10 @@ public class AlertScriptDispatcherTest {
alertInfo = new AlertInfo(history);
notification.setAlertInfo(alertInfo);
- processBuilder = dispatcher.getProcessBuilder(SCRIPT_CONFIG_VALUE,
notification);
+ processBuilder = dispatcher.getProcessBuilder(scriptAbsolutePath,
notification);
commands = processBuilder.command();
buffer = new StringBuilder();
- buffer.append(SCRIPT_CONFIG_VALUE).append(" ");
+ buffer.append(scriptAbsolutePath).append(" ");
buffer.append(ALERT_DEFINITION_NAME).append(" ");
buffer.append("\"").append(ALERT_DEFINITION_LABEL).append("\"").append("
");
buffer.append(ALERT_SERVICE_NAME).append(" ");
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
