This is an automated email from the ASF dual-hosted git repository.
Duansg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hertzbeat.git
The following commit(s) were added to refs/heads/master by this push:
new 1a7b45ec26 [fix] restrict monitoring template writes to admin (#4278)
1a7b45ec26 is described below
commit 1a7b45ec263a8593b3ea27ebfb8da7d9d4e3140d
Author: Duansg <[email protected]>
AuthorDate: Sun Aug 9 20:46:58 2026 +0800
[fix] restrict monitoring template writes to admin (#4278)
---
.../src/test/resources/sureness.yml | 6 ++
hertzbeat-manager/src/test/resources/sureness.yml | 6 ++
hertzbeat-startup/src/main/resources/sureness.yml | 6 ++
.../startup/security/SurenessResourceRuleTest.java | 104 ++++++++++++++++-----
.../hertzbeat-mysql-iotdb/conf/sureness.yml | 6 ++
.../hertzbeat-mysql-tdengine/conf/sureness.yml | 6 ++
.../conf/sureness.yml | 6 ++
.../conf/sureness.yml | 6 ++
.../conf/sureness.yml | 6 ++
script/sureness.yml | 6 ++
10 files changed, 137 insertions(+), 21 deletions(-)
diff --git a/hertzbeat-e2e/hertzbeat-log-e2e/src/test/resources/sureness.yml
b/hertzbeat-e2e/hertzbeat-log-e2e/src/test/resources/sureness.yml
index 6f2a7dbb36..1763868f1e 100644
--- a/hertzbeat-e2e/hertzbeat-log-e2e/src/test/resources/sureness.yml
+++ b/hertzbeat-e2e/hertzbeat-log-e2e/src/test/resources/sureness.yml
@@ -22,6 +22,12 @@
resourceRole:
- /api/account/auth/refresh===post===[admin,user,guest]
- /api/apps/**===get===[admin,user,guest]
+ # the define yml routes persist the global collection templates
+ # post can only add a new type, put overwrites an existing one and
immediately
+ # redispatches its collect job to every monitor already using it - hence
admin only
+ - /api/apps/**===post===[admin,user]
+ - /api/apps/**===put===[admin]
+ - /api/apps/**===delete===[admin]
- /api/monitor/**===get===[admin,user,guest]
- /api/monitor/**===post===[admin,user]
- /api/monitor/**===put===[admin,user]
diff --git a/hertzbeat-manager/src/test/resources/sureness.yml
b/hertzbeat-manager/src/test/resources/sureness.yml
index 6f2a7dbb36..1763868f1e 100644
--- a/hertzbeat-manager/src/test/resources/sureness.yml
+++ b/hertzbeat-manager/src/test/resources/sureness.yml
@@ -22,6 +22,12 @@
resourceRole:
- /api/account/auth/refresh===post===[admin,user,guest]
- /api/apps/**===get===[admin,user,guest]
+ # the define yml routes persist the global collection templates
+ # post can only add a new type, put overwrites an existing one and
immediately
+ # redispatches its collect job to every monitor already using it - hence
admin only
+ - /api/apps/**===post===[admin,user]
+ - /api/apps/**===put===[admin]
+ - /api/apps/**===delete===[admin]
- /api/monitor/**===get===[admin,user,guest]
- /api/monitor/**===post===[admin,user]
- /api/monitor/**===put===[admin,user]
diff --git a/hertzbeat-startup/src/main/resources/sureness.yml
b/hertzbeat-startup/src/main/resources/sureness.yml
index 04c9b3b739..da66fe4cea 100644
--- a/hertzbeat-startup/src/main/resources/sureness.yml
+++ b/hertzbeat-startup/src/main/resources/sureness.yml
@@ -22,6 +22,12 @@
resourceRole:
- /api/account/auth/refresh===post===[admin,user,guest]
- /api/apps/**===get===[admin,user,guest]
+ # the define yml routes persist the global collection templates
+ # post can only add a new type, put overwrites an existing one and
immediately
+ # redispatches its collect job to every monitor already using it - hence
admin only
+ - /api/apps/**===post===[admin,user]
+ - /api/apps/**===put===[admin]
+ - /api/apps/**===delete===[admin]
- /api/monitor/**===get===[admin,user,guest]
- /api/monitor/**===post===[admin,user]
- /api/monitor/**===put===[admin,user]
diff --git
a/hertzbeat-startup/src/test/java/org/apache/hertzbeat/startup/security/SurenessResourceRuleTest.java
b/hertzbeat-startup/src/test/java/org/apache/hertzbeat/startup/security/SurenessResourceRuleTest.java
index 40fba97784..17301b30b7 100644
---
a/hertzbeat-startup/src/test/java/org/apache/hertzbeat/startup/security/SurenessResourceRuleTest.java
+++
b/hertzbeat-startup/src/test/java/org/apache/hertzbeat/startup/security/SurenessResourceRuleTest.java
@@ -20,6 +20,7 @@ package org.apache.hertzbeat.startup.security;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
import com.usthe.sureness.matcher.util.TirePathTree;
import java.io.IOException;
import java.io.InputStream;
@@ -53,6 +54,8 @@ class SurenessResourceRuleTest {
private static final int EXCLUDED_RESOURCE_SEGMENTS = 2;
+ private static final Path SCRIPT_DIR = Path.of("..", "script");
+
private static List<String> resourceRole;
private static List<String> excludedResource;
@@ -114,6 +117,38 @@ class SurenessResourceRuleTest {
assertEquals("[admin,user,guest]",
roleTree.searchPathFilterRoles("/api/monitors" + SEPARATOR + "get"));
}
+ /**
+ * Only the {@code get} verb under {@code /api/apps} used to be listed, so
the monitoring
+ * template writes behind {@code AppController} reached any authenticated
caller, down to
+ * a {@code guest}.
+ *
+ * <p>The write verbs are not equally dangerous, and the rules
deliberately differ.
+ * {@code post} refuses to overwrite an existing template, so it only adds
a type that
+ * nobody is obliged to use, which puts it on the same footing as creating
a monitor.
+ * {@code put} overwrites any template including the built-in ones and
then hands the new
+ * definition to {@code updateAppCollectJob}, retroactively changing what
every existing
+ * monitor of that type collects for every account, so it stays admin only.
+ */
+ @Test
+ void addingMonitorTemplatesIsOpenToUsers() {
+ assertEquals("[admin,user]",
roleTree.searchPathFilterRoles("/api/apps/define/yml" + SEPARATOR + "post"));
+ }
+
+ @Test
+ void overwritingMonitorTemplatesIsRestrictedToAdmin() {
+ assertEquals("[admin]",
roleTree.searchPathFilterRoles("/api/apps/define/yml" + SEPARATOR + "put"));
+ }
+
+ @Test
+ void deletingMonitorTemplatesIsRestrictedToAdmin() {
+ assertEquals("[admin]",
roleTree.searchPathFilterRoles("/api/apps/linux/define/yml" + SEPARATOR +
"delete"));
+ }
+
+ @Test
+ void readingMonitorTemplatesStaysOpenToEveryRole() {
+ assertEquals("[admin,user,guest]",
roleTree.searchPathFilterRoles("/api/apps/linux/define/yml" + SEPARATOR +
"get"));
+ }
+
/**
* The deployment scripts ship their own copies of {@code sureness.yml}; a
rule fixed only
* in the packaged file would still leave every container deployment
exposed.
@@ -127,28 +162,10 @@ class SurenessResourceRuleTest {
* wave through.
*/
@Test
- @SuppressWarnings("unchecked")
void deploymentCopiesCarryWellFormedRules() throws IOException {
- Path scriptDir = Path.of("..", "script");
- if (!Files.isDirectory(scriptDir)) {
- // running outside the source tree, the packaged file asserted
above is all we can see
- return;
- }
- Set<Path> copies;
- try (Stream<Path> paths = Files.walk(scriptDir)) {
- copies = paths.filter(path ->
path.getFileName().toString().equals("sureness.yml"))
- .collect(Collectors.toCollection(LinkedHashSet::new));
- }
- assertFalse(copies.isEmpty(), "expected the deployment scripts to ship
sureness.yml copies");
- for (Path copy : copies) {
- Map<String, Object> document;
- try (InputStream in = Files.newInputStream(copy)) {
- document = new Yaml().load(in);
- }
- List<String> copyResourceRole = (List<String>)
document.get("resourceRole");
- List<String> copyExcludedResource = (List<String>)
document.get("excludedResource");
- assertNotNull(copyResourceRole, "resourceRole must be present in "
+ copy);
- assertNotNull(copyExcludedResource, "excludedResource must be
present in " + copy);
+ for (Path copy : deploymentCopies()) {
+ List<String> copyResourceRole = sectionOf(copy, "resourceRole");
+ List<String> copyExcludedResource = sectionOf(copy,
"excludedResource");
for (String rule : copyResourceRole) {
assertEquals(RESOURCE_ROLE_SEGMENTS, rule.split(SEPARATOR,
-1).length,
@@ -162,4 +179,49 @@ class SurenessResourceRuleTest {
}
}
}
+
+ /**
+ * Well formed is not the same as correct: a copy that simply never listed
the write verbs
+ * passes every shape check above while leaving the monitoring template
writes unruled. The
+ * copies are edited by hand, one per deployment flavour, so the roles
themselves are pinned
+ * here too and a missed copy fails instead of shipping.
+ */
+ @Test
+ void deploymentCopiesRestrictMonitorTemplateWrites() throws IOException {
+ for (Path copy : deploymentCopies()) {
+ TirePathTree copyTree = new TirePathTree();
+ copyTree.buildTree(new LinkedHashSet<>(sectionOf(copy,
"resourceRole")));
+ assertEquals("[admin,user]",
copyTree.searchPathFilterRoles("/api/apps/define/yml" + SEPARATOR + "post"),
+ "adding a monitoring template is unruled or over-granted
in " + copy);
+ assertEquals("[admin]",
copyTree.searchPathFilterRoles("/api/apps/define/yml" + SEPARATOR + "put"),
+ "overwriting a monitoring template is unruled or
over-granted in " + copy);
+ assertEquals("[admin]",
copyTree.searchPathFilterRoles("/api/apps/linux/define/yml" + SEPARATOR +
"delete"),
+ "deleting a monitoring template is unruled or over-granted
in " + copy);
+ }
+ }
+
+ /**
+ * @return the {@code sureness.yml} copies shipped by the deployment
scripts
+ */
+ private static Set<Path> deploymentCopies() throws IOException {
+ assumeTrue(Files.isDirectory(SCRIPT_DIR),
+ "running outside the source tree, the packaged file asserted
above is all we can see");
+ try (Stream<Path> paths = Files.walk(SCRIPT_DIR)) {
+ Set<Path> copies = paths.filter(path ->
path.getFileName().toString().equals("sureness.yml"))
+ .collect(Collectors.toCollection(LinkedHashSet::new));
+ assertFalse(copies.isEmpty(), "expected the deployment scripts to
ship sureness.yml copies");
+ return copies;
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private static List<String> sectionOf(Path copy, String section) throws
IOException {
+ Map<String, Object> document;
+ try (InputStream in = Files.newInputStream(copy)) {
+ document = new Yaml().load(in);
+ }
+ List<String> rules = (List<String>) document.get(section);
+ assertNotNull(rules, section + " must be present in " + copy);
+ return rules;
+ }
}
diff --git a/script/docker-compose/hertzbeat-mysql-iotdb/conf/sureness.yml
b/script/docker-compose/hertzbeat-mysql-iotdb/conf/sureness.yml
index fe6f8938f9..f19414fb5b 100644
--- a/script/docker-compose/hertzbeat-mysql-iotdb/conf/sureness.yml
+++ b/script/docker-compose/hertzbeat-mysql-iotdb/conf/sureness.yml
@@ -22,6 +22,12 @@
resourceRole:
- /api/account/auth/refresh===post===[admin,user,guest]
- /api/apps/**===get===[admin,user,guest]
+ # the define yml routes persist the global collection templates
+ # post can only add a new type, put overwrites an existing one and
immediately
+ # redispatches its collect job to every monitor already using it - hence
admin only
+ - /api/apps/**===post===[admin,user]
+ - /api/apps/**===put===[admin]
+ - /api/apps/**===delete===[admin]
- /api/monitor/**===get===[admin,user,guest]
- /api/monitor/**===post===[admin,user]
- /api/monitor/**===put===[admin,user]
diff --git a/script/docker-compose/hertzbeat-mysql-tdengine/conf/sureness.yml
b/script/docker-compose/hertzbeat-mysql-tdengine/conf/sureness.yml
index fe6f8938f9..f19414fb5b 100644
--- a/script/docker-compose/hertzbeat-mysql-tdengine/conf/sureness.yml
+++ b/script/docker-compose/hertzbeat-mysql-tdengine/conf/sureness.yml
@@ -22,6 +22,12 @@
resourceRole:
- /api/account/auth/refresh===post===[admin,user,guest]
- /api/apps/**===get===[admin,user,guest]
+ # the define yml routes persist the global collection templates
+ # post can only add a new type, put overwrites an existing one and
immediately
+ # redispatches its collect job to every monitor already using it - hence
admin only
+ - /api/apps/**===post===[admin,user]
+ - /api/apps/**===put===[admin]
+ - /api/apps/**===delete===[admin]
- /api/monitor/**===get===[admin,user,guest]
- /api/monitor/**===post===[admin,user]
- /api/monitor/**===put===[admin,user]
diff --git
a/script/docker-compose/hertzbeat-mysql-victoria-metrics/conf/sureness.yml
b/script/docker-compose/hertzbeat-mysql-victoria-metrics/conf/sureness.yml
index fe6f8938f9..f19414fb5b 100644
--- a/script/docker-compose/hertzbeat-mysql-victoria-metrics/conf/sureness.yml
+++ b/script/docker-compose/hertzbeat-mysql-victoria-metrics/conf/sureness.yml
@@ -22,6 +22,12 @@
resourceRole:
- /api/account/auth/refresh===post===[admin,user,guest]
- /api/apps/**===get===[admin,user,guest]
+ # the define yml routes persist the global collection templates
+ # post can only add a new type, put overwrites an existing one and
immediately
+ # redispatches its collect job to every monitor already using it - hence
admin only
+ - /api/apps/**===post===[admin,user]
+ - /api/apps/**===put===[admin]
+ - /api/apps/**===delete===[admin]
- /api/monitor/**===get===[admin,user,guest]
- /api/monitor/**===post===[admin,user]
- /api/monitor/**===put===[admin,user]
diff --git
a/script/docker-compose/hertzbeat-postgresql-greptimedb/conf/sureness.yml
b/script/docker-compose/hertzbeat-postgresql-greptimedb/conf/sureness.yml
index 2d6b4441ee..e9c2f9f4a1 100644
--- a/script/docker-compose/hertzbeat-postgresql-greptimedb/conf/sureness.yml
+++ b/script/docker-compose/hertzbeat-postgresql-greptimedb/conf/sureness.yml
@@ -22,6 +22,12 @@
resourceRole:
- /api/account/auth/refresh===post===[admin,user,guest]
- /api/apps/**===get===[admin,user,guest]
+ # the define yml routes persist the global collection templates
+ # post can only add a new type, put overwrites an existing one and
immediately
+ # redispatches its collect job to every monitor already using it - hence
admin only
+ - /api/apps/**===post===[admin,user]
+ - /api/apps/**===put===[admin]
+ - /api/apps/**===delete===[admin]
- /api/monitor/**===get===[admin,user,guest]
- /api/monitor/**===post===[admin,user]
- /api/monitor/**===put===[admin,user]
diff --git
a/script/docker-compose/hertzbeat-postgresql-victoria-metrics/conf/sureness.yml
b/script/docker-compose/hertzbeat-postgresql-victoria-metrics/conf/sureness.yml
index fe6f8938f9..f19414fb5b 100644
---
a/script/docker-compose/hertzbeat-postgresql-victoria-metrics/conf/sureness.yml
+++
b/script/docker-compose/hertzbeat-postgresql-victoria-metrics/conf/sureness.yml
@@ -22,6 +22,12 @@
resourceRole:
- /api/account/auth/refresh===post===[admin,user,guest]
- /api/apps/**===get===[admin,user,guest]
+ # the define yml routes persist the global collection templates
+ # post can only add a new type, put overwrites an existing one and
immediately
+ # redispatches its collect job to every monitor already using it - hence
admin only
+ - /api/apps/**===post===[admin,user]
+ - /api/apps/**===put===[admin]
+ - /api/apps/**===delete===[admin]
- /api/monitor/**===get===[admin,user,guest]
- /api/monitor/**===post===[admin,user]
- /api/monitor/**===put===[admin,user]
diff --git a/script/sureness.yml b/script/sureness.yml
index 2d6b4441ee..e9c2f9f4a1 100644
--- a/script/sureness.yml
+++ b/script/sureness.yml
@@ -22,6 +22,12 @@
resourceRole:
- /api/account/auth/refresh===post===[admin,user,guest]
- /api/apps/**===get===[admin,user,guest]
+ # the define yml routes persist the global collection templates
+ # post can only add a new type, put overwrites an existing one and
immediately
+ # redispatches its collect job to every monitor already using it - hence
admin only
+ - /api/apps/**===post===[admin,user]
+ - /api/apps/**===put===[admin]
+ - /api/apps/**===delete===[admin]
- /api/monitor/**===get===[admin,user,guest]
- /api/monitor/**===post===[admin,user]
- /api/monitor/**===put===[admin,user]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
