Merge branch 'master' into fix/005-greptime-compose-port-binding

hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java

@@ -190,13 +190,28 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine
      *                                  be resolved, and saving would persist the placeholder
      */
     private void resolveMaskedSecrets(NoticeReceiver noticeReceiver) {
+        resolveMaskedSecrets(noticeReceiver, false);
+    }
+
+    /**
+     * Bind stored secrets to their original notification type and destination for test messages.
+     */
+    private void resolveMaskedSecretsForTest(NoticeReceiver noticeReceiver) {
+        resolveMaskedSecrets(noticeReceiver, true);
+    }
+
+    private void resolveMaskedSecrets(NoticeReceiver noticeReceiver, boolean testMessage) {
         if (noticeReceiver == null || noticeReceiver.getId() == null) {
             return;
         }
         NoticeReceiver existing = noticeReceiverDao.findById(noticeReceiver.getId())
                 .orElseThrow(() -> new IllegalArgumentException(
                         "The receiver with id " + noticeReceiver.getId() + " does not exist."));
-        NoticeReceiverMaskUtil.resolveMask(noticeReceiver, existing);
+        if (testMessage) {
+            NoticeReceiverMaskUtil.resolveMaskForTest(noticeReceiver, existing);
+        } else {
+            NoticeReceiverMaskUtil.resolveMask(noticeReceiver, existing);
+        }
     }
 
     @Override
@@ -339,7 +354,7 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine
 
     @Override
     public boolean sendTestMsg(NoticeReceiver noticeReceiver) {
-        resolveMaskedSecrets(noticeReceiver);
+        resolveMaskedSecretsForTest(noticeReceiver);
         Map<String, String> labels = new HashMap<>(8);
         labels.put(CommonConstants.LABEL_INSTANCE, "127.0.0.1");
         labels.put(CommonConstants.LABEL_ALERT_NAME, "CPU Usage Alert");

hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtil.java

@@ -18,6 +18,7 @@
 package org.apache.hertzbeat.alert.util;
 
 import java.util.List;
+import java.util.Objects;
 import java.util.function.BiConsumer;
 import java.util.function.Function;
 import org.apache.commons.lang3.StringUtils;
@@ -92,6 +93,7 @@ public final class NoticeReceiverMaskUtil {
      * A re-entered secret or a cleared field is left untouched.
      * @param incoming receiver submitted by the ui, modified in place
      * @param existing receiver currently stored in the database
+     * @throws IllegalArgumentException if a submitted mask does not match the stored secret
      */
     public static void resolveMask(NoticeReceiver incoming, NoticeReceiver existing) {
         if (incoming == null || existing == null) {
@@ -102,15 +104,72 @@ public final class NoticeReceiverMaskUtil {
             String stored = field.getter().apply(existing);
             if (isMaskOf(submitted, stored)) {
                 field.setter().accept(incoming, stored);
+            } else if (isMaskValue(submitted)) {
+                throw new IllegalArgumentException(
+                        "The submitted secret mask does not match the stored secret.");
             }
         }
     }
 
+    /**
+     * Resolve masked secrets for a test message while binding them to their persisted destination.
+     * A caller that changes the notification type or a URL receiving authentication data
+     * must submit the new secret explicitly instead of reusing a stored secret mask.
+     * @param incoming receiver submitted for a test message, modified in place
+     * @param existing receiver currently stored in the database
+     * @throws IllegalArgumentException if a masked secret is combined with a changed destination
+     */
+    public static void resolveMaskForTest(NoticeReceiver incoming, NoticeReceiver existing) {
+        if (incoming == null || existing == null) {
+            return;
+        }
+
+        boolean containsStoredSecretMask = SECRET_FIELDS.stream()
+                .anyMatch(field -> isMaskOf(field.getter().apply(incoming), field.getter().apply(existing)));
+        if (containsStoredSecretMask && !Objects.equals(incoming.getType(), existing.getType())) {
+            throw new IllegalArgumentException(
+                    "The notification type cannot be changed when reusing a masked secret.");
+        }
+        rejectChangedSecretDestination(
+                incoming.getHookAuthToken(),
+                existing.getHookAuthToken(),
+                incoming.getHookUrl(),
+                existing.getHookUrl(),
+                "webhook URL");
+        rejectChangedSecretDestination(
+                incoming.getNtfyToken(),
+                existing.getNtfyToken(),
+                incoming.getNtfyServerUrl(),
+                existing.getNtfyServerUrl(),
+                "ntfy server URL");
+        resolveMask(incoming, existing);
+    }
+
+    private static void rejectChangedSecretDestination(
+            String submittedSecret,
+            String storedSecret,
+            String submittedDestination,
+            String storedDestination,
+            String destinationName) {
+        if (isMaskOf(submittedSecret, storedSecret)
+                && !Objects.equals(submittedDestination, storedDestination)) {
+            throw new IllegalArgumentException(
+                    "The " + destinationName + " cannot be changed when reusing a masked secret.");
+        }
+    }
+
     private static boolean isMaskOf(String submitted, String stored) {
         if (submitted == null || StringUtils.isBlank(stored)) {
             return false;
         }
-        return submitted.equals(SECRET_MASK) || submitted.equals(maskValue(stored));
+        return submitted.equals(maskValue(stored));
+    }
+
+    private static boolean isMaskValue(String value) {
+        return value != null
+                && value.startsWith(SECRET_MASK)
+                && (value.length() == SECRET_MASK.length()
+                    || value.length() == SECRET_MASK.length() + VISIBLE_SUFFIX_LENGTH);
     }
 
     private static String maskValue(String value) {

hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java

@@ -279,6 +279,41 @@ class NoticeConfigServiceTest {
         verify(dispatcherAlarm, never()).sendNoticeMsg(any(), any(), any());
     }
 
+    @Test
+    void sendTestMsgRejectsMaskedSecretReplayToChangedWebhookUrl() {
+        final NoticeReceiver stored = new NoticeReceiver();
+        stored.setId(5L);
+        stored.setType((byte) 2);
+        stored.setHookUrl("https://trusted.example/hook");
+        stored.setHookAuthToken("hook-auth-token-abcd");
+        when(noticeReceiverDao.findById(5L)).thenReturn(Optional.of(stored));
+
+        final NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(stored);
+        incoming.setHookUrl("https://attacker.example/collect");
+
+        assertThrows(IllegalArgumentException.class, () -> noticeConfigService.sendTestMsg(incoming));
+        verify(dispatcherAlarm, never()).sendNoticeMsg(any(), any(), any());
+    }
+
+    @Test
+    void sendTestMsgRejectsBareMaskReplayToChangedWebhookUrl() {
+        final NoticeReceiver stored = new NoticeReceiver();
+        stored.setId(5L);
+        stored.setType((byte) 2);
+        stored.setHookUrl("https://trusted.example/hook");
+        stored.setHookAuthToken("hook-auth-token-abcd");
+        when(noticeReceiverDao.findById(5L)).thenReturn(Optional.of(stored));
+
+        final NoticeReceiver incoming = new NoticeReceiver();
+        incoming.setId(5L);
+        incoming.setType((byte) 2);
+        incoming.setHookUrl("https://attacker.example/collect");
+        incoming.setHookAuthToken(NoticeReceiverMaskUtil.SECRET_MASK);
+
+        assertThrows(IllegalArgumentException.class, () -> noticeConfigService.sendTestMsg(incoming));
+        verify(dispatcherAlarm, never()).sendNoticeMsg(any(), any(), any());
+    }
+
     @Test
     void deleteReceiver() {
         final Long receiverId = 23342525L;

hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtilTest.java

@@ -23,6 +23,7 @@ import org.junit.jupiter.api.Test;
 import static org.apache.hertzbeat.alert.util.NoticeReceiverMaskUtil.SECRET_MASK;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Test case for {@link NoticeReceiverMaskUtil}
@@ -104,7 +105,6 @@ class NoticeReceiverMaskUtilTest {
         NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
         incoming.setAccessToken("new-access-token-1234");
         incoming.setGotifyToken(null);
-        incoming.setNtfyToken(SECRET_MASK);
 
         NoticeReceiverMaskUtil.resolveMask(incoming, existing);
 
@@ -122,13 +122,73 @@ class NoticeReceiverMaskUtilTest {
     }
 
     @Test
-    void resolveMaskIgnoresMaskWhenNothingIsStored() {
+    void resolveMaskRejectsMaskWhenNothingIsStored() {
         NoticeReceiver existing = new NoticeReceiver();
         NoticeReceiver incoming = new NoticeReceiver();
         incoming.setAccessToken(SECRET_MASK);
 
+        assertThrows(
+                IllegalArgumentException.class,
+                () -> NoticeReceiverMaskUtil.resolveMask(incoming, existing));
+    }
+
+    @Test
+    void resolveMaskRejectsBareMaskAsWildcard() {
+        NoticeReceiver existing = buildReceiverWithSecrets();
+        NoticeReceiver incoming = new NoticeReceiver();
+        incoming.setAccessToken(SECRET_MASK);
+
+        assertThrows(
+                IllegalArgumentException.class,
+                () -> NoticeReceiverMaskUtil.resolveMask(incoming, existing));
+    }
+
+    @Test
+    void resolveMaskRestoresBareMaskForShortStoredSecret() {
+        NoticeReceiver existing = new NoticeReceiver();
+        existing.setAccessToken("short-token");
+        NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
+
         NoticeReceiverMaskUtil.resolveMask(incoming, existing);
 
-        assertEquals(SECRET_MASK, incoming.getAccessToken());
+        assertEquals("short-token", incoming.getAccessToken());
+    }
+
+    @Test
+    void resolveMaskForTestRejectsMaskedWebhookSecretForChangedUrl() {
+        NoticeReceiver existing = buildReceiverWithSecrets();
+        existing.setType((byte) 2);
+        NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
+        incoming.setHookUrl("https://attacker.example/collect");
+
+        assertThrows(
+                IllegalArgumentException.class,
+                () -> NoticeReceiverMaskUtil.resolveMaskForTest(incoming, existing));
+        assertEquals(SECRET_MASK + "abcd", incoming.getHookAuthToken());
+    }
+
+    @Test
+    void resolveMaskForTestRejectsMaskedNtfySecretForChangedServer() {
+        NoticeReceiver existing = buildReceiverWithSecrets();
+        existing.setType((byte) 15);
+        existing.setNtfyServerUrl("https://ntfy.example");
+        NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
+        incoming.setNtfyServerUrl("https://attacker.example");
+
+        assertThrows(
+                IllegalArgumentException.class,
+                () -> NoticeReceiverMaskUtil.resolveMaskForTest(incoming, existing));
+        assertEquals(SECRET_MASK + "NIz2", incoming.getNtfyToken());
+    }
+
+    @Test
+    void resolveMaskForTestRestoresSecretForUnchangedDestination() {
+        NoticeReceiver existing = buildReceiverWithSecrets();
+        existing.setType((byte) 2);
+        NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
+
+        NoticeReceiverMaskUtil.resolveMaskForTest(incoming, existing);
+
+        assertEquals("hook-auth-token-abcd", incoming.getHookAuthToken());
     }
 }