This is an automated email from the ASF dual-hosted git repository.
dockerzhang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/inlong.git
The following commit(s) were added to refs/heads/master by this push:
new 4e56b8df2a [INLONG-12149][Improve] [Manager] Add command whitelist
validation in Manager (#12155)
4e56b8df2a is described below
commit 4e56b8df2a9abbca10f8404d3e88c7d9ec9b2ddb
Author: spirit_sx <[email protected]>
AuthorDate: Wed Jul 15 14:19:41 2026 +0800
[INLONG-12149][Improve] [Manager] Add command whitelist validation in
Manager (#12155)
---
.../manager/common/consts/InlongConstants.java | 14 +
.../inlong/manager/common/enums/ErrorCodeEnum.java | 2 +
.../service/module/ModuleCommandAccessors.java | 52 +++
.../service/module/ModuleCommandValidator.java | 411 +++++++++++++++++++++
.../manager/service/module/ModuleServiceImpl.java | 18 +-
.../service/module/ModuleCommandValidatorTest.java | 203 ++++++++++
.../service/module/ModuleServiceImplTest.java | 180 +++++++++
7 files changed, 879 insertions(+), 1 deletion(-)
diff --git
a/inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/consts/InlongConstants.java
b/inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/consts/InlongConstants.java
index e7cc03602b..ca22ebbd82 100644
---
a/inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/consts/InlongConstants.java
+++
b/inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/consts/InlongConstants.java
@@ -74,10 +74,24 @@ public class InlongConstants {
public static final String AMPERSAND = "&";
+ public static final String ASTERISK = "*";
+
public static final String NEW_LINE = "\n";
public static final String REGEX_WHITESPACE = "\\s";
+ public static final char SINGLE_QUOTE_CHAR = '\'';
+
+ public static final char DOUBLE_QUOTE_CHAR = '"';
+
+ public static final char NEW_LINE_CHAR = '\n';
+
+ public static final char CARRIAGE_RETURN_CHAR = '\r';
+
+ public static final char SEMICOLON_CHAR = ';';
+
+ public static final char PIPE_CHAR = '|';
+
public static final String ADMIN_USER = "admin";
public static final Integer AFFECTED_ONE_ROW = 1;
diff --git
a/inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/ErrorCodeEnum.java
b/inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/ErrorCodeEnum.java
index 9364fda8e7..34068816a9 100644
---
a/inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/ErrorCodeEnum.java
+++
b/inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/ErrorCodeEnum.java
@@ -169,6 +169,8 @@ public enum ErrorCodeEnum {
MODULE_NOT_FOUND(6001, "Module does not exist/no operation authority"),
MODULE_INFO_INCORRECT(6002, "Module info was incorrect"),
+ MODULE_COMMAND_NOT_IN_WHITELIST(6003,
+ "Module command not in whitelist: %s"),
PACKAGE_NOT_FOUND(7001, "Package does not exist/no operation authority"),
PACKAGE_INFO_INCORRECT(7002, "Package info was incorrect"),
diff --git
a/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandAccessors.java
b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandAccessors.java
new file mode 100644
index 0000000000..8cb181eed0
--- /dev/null
+++
b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandAccessors.java
@@ -0,0 +1,52 @@
+/*
+ * 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 regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.inlong.manager.service.module;
+
+import org.apache.inlong.manager.pojo.module.ModuleDTO;
+import org.apache.inlong.manager.pojo.module.ModuleRequest;
+
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * Flattens a carrier ({@link ModuleRequest} / {@link ModuleDTO} / ...) into a
plain list
+ * of raw command strings.
+ */
+public final class ModuleCommandAccessors {
+
+ private ModuleCommandAccessors() {
+ }
+
+ public static List<String> of(ModuleRequest request) {
+ return Arrays.asList(
+ request.getStartCommand(),
+ request.getStopCommand(),
+ request.getCheckCommand(),
+ request.getInstallCommand(),
+ request.getUninstallCommand());
+ }
+
+ public static List<String> of(ModuleDTO dto) {
+ return Arrays.asList(
+ dto.getStartCommand(),
+ dto.getStopCommand(),
+ dto.getCheckCommand(),
+ dto.getInstallCommand(),
+ dto.getUninstallCommand());
+ }
+}
diff --git
a/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandValidator.java
b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandValidator.java
new file mode 100644
index 0000000000..3094205a2b
--- /dev/null
+++
b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandValidator.java
@@ -0,0 +1,411 @@
+/*
+ * 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 regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.inlong.manager.service.module;
+
+import org.apache.inlong.manager.common.consts.InlongConstants;
+import org.apache.inlong.manager.common.enums.ErrorCodeEnum;
+import org.apache.inlong.manager.common.exceptions.BusinessException;
+import org.apache.inlong.manager.pojo.module.ModuleDTO;
+import org.apache.inlong.manager.pojo.module.ModuleRequest;
+
+import lombok.Getter;
+import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.stereotype.Component;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+/**
+ * Manager-side command whitelist validator. Validates that command names
(argv[0]) in module
+ * commands (start/stop/check/install/uninstall) are in the allowed whitelist.
This catches
+ * misconfiguration at save time rather than waiting until the agent tries to
execute.
+ *
+ * <p>Unlike the agent-side validator, this class does <b>not</b> perform
path-under-root
+ * checks, which are filesystem-dependent and only meaningful at the agent
runtime.
+ *
+ * <p>Whitelist baseline:
+ * <pre>{@code
+ * cd, sh, bash, ps, grep, awk, kill, rm, mkdir, cp, mv, ln,
+ * tar, unzip, chmod, chown, echo, cat, test, [, true, false, java
+ * }</pre>
+ *
+ * <p>Extend via {@code module.command.extraWhitelist} (comma-separated) in
application
+ * properties, e.g.: {@code module.command.extraWhitelist=python3,nohup,curl}
+ */
+@Component
+public class ModuleCommandValidator {
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(ModuleCommandValidator.class);
+
+ /** Baseline argv[0] whitelist — kept in sync with agent-side
ModuleCommandValidator. */
+ private static final Set<String> BASELINE_WHITELIST = buildImmutableSet(
+ "cd", "sh", "bash", "ps", "grep", "awk", "kill", "rm", "mkdir",
"cp", "mv", "ln",
+ "tar", "unzip", "chmod", "chown", "echo", "cat", "test", "[",
"true", "false", "java");
+
+ /**
+ * Metacharacter substring blacklist — kept in sync with agent-side
+ * {@code ModuleCommandValidator.META_CHAR_BLACKLIST}. These characters
indicate shell
+ * injection attempts (command substitution, chaining, redirection,
escaping). Unlike
+ * path-under-root checks, metacharacter validation is
filesystem-independent and
+ * belongs on both the Manager and Agent side.
+ */
+ private static final String[] META_CHAR_BLACKLIST = new String[]{
+ "`", "$(", "${", "&&", "||", ">>", ">", "<", "\\", "\u0000",
+ /*
+ * glob wildcards — Agent uses ProcessBuilder without shell, so
'*' and '?' are NOT expanded. Passing e.g.
+ * 'rm *.log' as argv[1] would delete a literal file named "*.log"
(or silently no-op), which is far more
+ * dangerous than an explicit error. Reject them here with a clear
hint.
+ */
+ "*", "?"
+ };
+
+ /** Extra whitelist entries from Spring config, comma-separated. */
+ @Value("${module.command.extraWhitelist:}")
+ private String extraWhitelist;
+
+ /** Whitelist enforcement mode: STRICT (block), WARN (log only), OFF
(skip). */
+ @Value("${module.command.whitelistMode:WARN}")
+ @Getter
+ private WhitelistMode whitelistModeConfig;
+
+ /**
+ * Whitelist enforcement mode.
+ *
+ * <ul>
+ * <li>{@code STRICT} — save + update both block non-whitelisted
commands.</li>
+ * <li>{@code WARN} — save blocks, update only logs a warning (for
+ * gradually tightening existing modules).</li>
+ * <li>{@code OFF} — all validation is skipped.</li>
+ * </ul>
+ */
+ public enum WhitelistMode {
+
+ STRICT, WARN, OFF;
+ }
+
+ /** Lazily-computed effective whitelist (baseline + config extras). */
+ private volatile Set<String> effectiveWhitelist;
+
+ private static Set<String> buildImmutableSet(String... items) {
+ Set<String> s = new HashSet<>(items.length * 2);
+ Collections.addAll(s, items);
+ return Collections.unmodifiableSet(s);
+ }
+
+ /**
+ * Get the effective (merged) whitelist — baseline + config extras.
Lazy-init and
+ * cached after the first call.
+ */
+ private Set<String> getEffectiveWhitelist() {
+ if (effectiveWhitelist != null) {
+ return effectiveWhitelist;
+ }
+ synchronized (this) {
+ if (effectiveWhitelist != null) {
+ return effectiveWhitelist;
+ }
+ Set<String> merged = new LinkedHashSet<>(BASELINE_WHITELIST);
+ if (StringUtils.isNotBlank(extraWhitelist)) {
+ for (String item :
extraWhitelist.split(InlongConstants.COMMA)) {
+ String trimmed = item.trim();
+ if (trimmed.isEmpty()) {
+ continue;
+ }
+ merged.add(trimmed);
+ }
+ }
+ effectiveWhitelist = Collections.unmodifiableSet(new
HashSet<>(merged));
+ LOGGER.info("ModuleCommandValidator initialized: whitelist={}",
+ new java.util.TreeSet<>(effectiveWhitelist));
+ }
+ return effectiveWhitelist;
+ }
+
+ /**
+ * Save-time entry point. Runs the full command whitelist check and throws
+ * {@link BusinessException} on the first violation; otherwise returns
{@code true}.
+ * Callers do <b>not</b> need to check the mode or wrap the exception
themselves.
+ *
+ * <p>In {@link WhitelistMode#OFF} the check is skipped and {@code true}
is returned.
+ * In {@link WhitelistMode#STRICT} and {@link WhitelistMode#WARN} the save
path is
+ * always blocking (WARN only softens the update path).
+ */
+ public void validateOnSave(ModuleRequest request) {
+ if (whitelistModeConfig == WhitelistMode.OFF || request == null) {
+ return;
+ }
+ String violation =
findFirstViolation(ModuleCommandAccessors.of(request));
+ if (violation != null) {
+ throw new
BusinessException(ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST,
+
String.format(ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST.getMessage(),
+ violation));
+ }
+ }
+
+ /**
+ * Update-time entry point. Only validates command fields that actually
changed
+ * between {@code oldDto} (existing DB row) and the incoming {@code
request}.
+ *
+ * <ul>
+ * <li>{@link WhitelistMode#OFF} — skip entirely, return {@code
true}.</li>
+ * <li>{@link WhitelistMode#STRICT} — throw {@link BusinessException} on
any
+ * violation.</li>
+ * <li>{@link WhitelistMode#WARN} — log a warning and let the update
proceed.</li>
+ * </ul>
+ *
+ * @param oldDto ModuleDTO parsed from stored extParams; {@code null}
means "treat
+ * every field as changed" (behaves like save-time)
+ * @param request the incoming update request
+ */
+ public void validateOnUpdate(ModuleDTO oldDto, ModuleRequest request) {
+ if (whitelistModeConfig == WhitelistMode.OFF || request == null) {
+ return;
+ }
+ List<String> toCheck = changedCommands(oldDto, request);
+ String violation = findFirstViolation(toCheck);
+ if (violation == null) {
+ return;
+ }
+ if (whitelistModeConfig == WhitelistMode.STRICT) {
+ throw new
BusinessException(ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST,
+
String.format(ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST.getMessage(),
+ violation));
+ }
+ // WARN mode: do not block the update, only surface the reason in logs.
+ LOGGER.warn("ModuleCommandValidator: non-whitelisted command in update
"
+ + "(mode=WARN, not blocking): moduleId={}, {}",
request.getId(),
+ violation);
+ }
+
+ /**
+ * Compute the sub-list of new-side commands whose value differs from the
stored
+ * one, positionally (both accessors return the five command fields in the
same
+ * order). A {@code null} {@code oldDto} means "no baseline", so every
field is
+ * treated as changed — which mirrors save-time semantics.
+ */
+ private static List<String> changedCommands(ModuleDTO oldDto,
ModuleRequest request) {
+ List<String> newCmds = ModuleCommandAccessors.of(request);
+ if (oldDto == null) {
+ return newCmds;
+ }
+ List<String> oldCmds = ModuleCommandAccessors.of(oldDto);
+ List<String> changed = new ArrayList<>(newCmds.size());
+ for (int i = 0; i < newCmds.size(); i++) {
+ String oldRaw = i < oldCmds.size() ? oldCmds.get(i) : null;
+ if (!Objects.equals(oldRaw, newCmds.get(i))) {
+ changed.add(newCmds.get(i));
+ }
+ }
+ return changed;
+ }
+
+ /**
+ * Iterate the given commands and return {@code "<reason> in [<raw>]"} for
the
+ * first offending one, or {@code null} if all pass. The raw command is
echoed
+ * verbatim so operators can immediately locate which configured line
triggered
+ * the rejection.
+ */
+ private String findFirstViolation(List<String> commands) {
+ for (String raw : commands) {
+ String reason = validate(raw);
+ if (StringUtils.isBlank(reason)) {
+ continue;
+ }
+ return reason + " in [" + raw + "]";
+ }
+ return null;
+ }
+
+ /**
+ * Scan the raw command string for any metacharacter substring from
+ * {@link #META_CHAR_BLACKLIST}. Returns the first hit, or {@code null} if
clean.
+ * (Same logic as the agent-side validator.)
+ */
+ private static String firstMetaCharHit(String raw) {
+ for (String meta : META_CHAR_BLACKLIST) {
+ if (raw.contains(meta)) {
+ return meta;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Validate a raw command string. Returns a descriptive error string if
invalid, or
+ * {@code null} if valid. The error prefix indicates the rejection reason:
+ * {@code DISALLOWED_META_CHAR}, {@code FORBIDDEN_SH_C_FLAG}, or just the
offending
+ * command name (NOT_IN_WHITELIST).
+ * Package-private for the unit tests.
+ */
+ String validate(String rawCmd) {
+ if (StringUtils.isBlank(rawCmd)) {
+ return null;
+ }
+
+ // metacharacter blacklist (whole string, before splitting).
+ // Offending chars are wrapped with [] so callers can unambiguously
see the boundary,
+ // e.g. "DISALLOWED_META_CHAR: [`]" rather than a bare backtick which
is easy to miss.
+ String metaHit = firstMetaCharHit(rawCmd);
+ if (StringUtils.isNotBlank(metaHit)) {
+ if (InlongConstants.ASTERISK.equals(metaHit) ||
InlongConstants.QUESTION_MARK.equals(metaHit)) {
+ return "DISALLOWED_META_CHAR: [" + metaHit + "]"
+ + " — glob wildcards are not supported (Agent runs
commands without a shell,"
+ + " so [*] and [?] will NOT be expanded); please
specify explicit file paths";
+ }
+ return "DISALLOWED_META_CHAR: [" + metaHit + "]";
+ }
+ if (rawCmd.indexOf(InlongConstants.NEW_LINE_CHAR) >= 0
+ || rawCmd.indexOf(InlongConstants.CARRIAGE_RETURN_CHAR) >= 0) {
+ return "DISALLOWED_META_CHAR: line-break";
+ }
+
+ Set<String> whitelist = getEffectiveWhitelist();
+
+ // Split by ';' into sub-commands (quote-aware), then by '|' into pipe
segments
+ List<String> segments = splitTopLevel(rawCmd,
InlongConstants.SEMICOLON_CHAR);
+ if (segments == null) {
+ return "DISALLOWED_META_CHAR: unterminated quote";
+ }
+ for (String seg : segments) {
+ if (StringUtils.isBlank(seg)) {
+ continue;
+ }
+ List<String> pipeSegs = splitTopLevel(seg,
InlongConstants.PIPE_CHAR);
+ if (pipeSegs == null) {
+ return "DISALLOWED_META_CHAR: unterminated quote in pipe
segment";
+ }
+ for (String pipeSeg : pipeSegs) {
+ if (StringUtils.isBlank(pipeSeg)) {
+ continue;
+ }
+ List<String> argv = tokenize(pipeSeg);
+ if (argv.isEmpty()) {
+ continue;
+ }
+ String cmdName = argv.get(0);
+
+ // argv[0] whitelist
+ if (!whitelist.contains(cmdName)) {
+ return cmdName;
+ }
+
+ // forbid sh/bash -c flag (inline script execution)
+ if (("sh".equals(cmdName) || "bash".equals(cmdName)) &&
argv.size() > 1) {
+ for (int i = 1; i < argv.size(); i++) {
+ if (argv.get(i).startsWith("-c")) {
+ return "FORBIDDEN_SH_C_FLAG: " + cmdName + " " +
argv.get(i);
+ }
+ }
+ }
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Quote-aware split of {@code raw} on the top-level {@code delim}
character. Characters
+ * inside a single-quoted or double-quoted region are treated as literals
and do not
+ * split the string. Returns {@code null} when the input has an
unterminated quote.
+ */
+ static List<String> splitTopLevel(String raw, char delim) {
+ List<String> out = new ArrayList<>();
+ if (StringUtils.isEmpty(raw)) {
+ return out;
+ }
+ StringBuilder cur = new StringBuilder();
+ // current quote char (0 if not in a quote)
+ char quote = 0;
+ for (int i = 0; i < raw.length(); i++) {
+ char c = raw.charAt(i);
+ if (quote != 0) {
+ cur.append(c);
+ if (c == quote) {
+ quote = 0;
+ }
+ continue;
+ }
+ if (c == InlongConstants.SINGLE_QUOTE_CHAR || c ==
InlongConstants.DOUBLE_QUOTE_CHAR) {
+ quote = c;
+ cur.append(c);
+ continue;
+ }
+ if (c == delim) {
+ out.add(cur.toString());
+ cur.setLength(0);
+ continue;
+ }
+ cur.append(c);
+ }
+ if (quote != 0) {
+ return null;
+ }
+ out.add(cur.toString());
+ return out;
+ }
+
+ /**
+ * Whitespace-based tokenizer that honours single/double quotes. Uses
{@code substring}
+ * to avoid per-character copying into a {@code StringBuilder}.
+ *
+ * <p>For a shell-command whitelist/blacklist validator, mixed
quoted/unquoted tokens
+ * (e.g. {@code abc"def"ghi}) are intentionally <b>not</b> supported — it
is safer
+ * that way. A quote character always starts a fresh token, and any
unquoted run
+ * terminates as soon as a quote character is encountered.
+ */
+ private static List<String> tokenize(String s) {
+ List<String> tokens = new ArrayList<>();
+ int i = 0;
+ int n = s.length();
+ while (i < n) {
+ while (i < n && Character.isWhitespace(s.charAt(i))) {
+ i++;
+ }
+ if (i >= n) {
+ break;
+ }
+ char c = s.charAt(i);
+ if (c == InlongConstants.SINGLE_QUOTE_CHAR || c ==
InlongConstants.DOUBLE_QUOTE_CHAR) {
+ int start = i + 1;
+ int end = s.indexOf(c, start);
+ if (end < 0) {
+ tokens.add(s.substring(start));
+ break;
+ }
+ tokens.add(s.substring(start, end));
+ i = end + 1;
+ } else {
+ int start = i;
+ while (i < n && !Character.isWhitespace(s.charAt(i))
+ && s.charAt(i) != InlongConstants.SINGLE_QUOTE_CHAR
+ && s.charAt(i) != InlongConstants.DOUBLE_QUOTE_CHAR) {
+ i++;
+ }
+ tokens.add(s.substring(start, i));
+ }
+ }
+ return tokens;
+ }
+}
diff --git
a/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleServiceImpl.java
b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleServiceImpl.java
index 92579a2437..ff499d1e67 100644
---
a/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleServiceImpl.java
+++
b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleServiceImpl.java
@@ -52,10 +52,17 @@ public class ModuleServiceImpl implements ModuleService {
private ModuleConfigEntityMapper moduleConfigEntityMapper;
@Autowired
private ObjectMapper objectMapper;
+ @Autowired
+ private ModuleCommandValidator commandValidator;
@Override
public Integer save(ModuleRequest request, String operator) {
LOGGER.info("begin to save module info: {}", request);
+
+ // Whitelist check — mode handling and exception wrapping live inside
the
+ // validator, so a violation surfaces as a BusinessException directly.
+ commandValidator.validateOnSave(request);
+
ModuleConfigEntity moduleConfigEntity =
CommonBeanUtils.copyProperties(request, ModuleConfigEntity::new);
try {
ModuleDTO dto = ModuleDTO.getFromRequest(request,
moduleConfigEntity.getExtParams(),
@@ -68,7 +75,8 @@ public class ModuleServiceImpl implements ModuleService {
}
moduleConfigEntity.setCreator(operator);
moduleConfigEntity.setModifier(operator);
- int id = moduleConfigEntityMapper.insert(moduleConfigEntity);
+ moduleConfigEntityMapper.insert(moduleConfigEntity);
+ int id = moduleConfigEntity.getId();
LOGGER.info("success to save module info: {}", request);
return id;
@@ -82,6 +90,14 @@ public class ModuleServiceImpl implements ModuleService {
throw new BusinessException(ErrorCodeEnum.MODULE_NOT_FOUND,
String.format("Module does not exist with id=%s",
request.getId()));
}
+
+ // Incremental whitelist check — the validator internally picks the
enforcement
+ // mode (STRICT throws, WARN logs, OFF skips) so no branching is
needed here.
+ ModuleDTO oldDto = null;
+ if (moduleConfigEntity.getExtParams() != null) {
+ oldDto = ModuleDTO.getFromJson(moduleConfigEntity.getExtParams());
+ }
+ commandValidator.validateOnUpdate(oldDto, request);
CommonBeanUtils.copyProperties(request, moduleConfigEntity, true);
try {
ModuleDTO dto = ModuleDTO.getFromRequest(request,
moduleConfigEntity.getExtParams(),
diff --git
a/inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleCommandValidatorTest.java
b/inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleCommandValidatorTest.java
new file mode 100644
index 0000000000..da862d280e
--- /dev/null
+++
b/inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleCommandValidatorTest.java
@@ -0,0 +1,203 @@
+/*
+ * 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 regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.inlong.manager.service.module;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.test.util.ReflectionTestUtils;
+
+/**
+ * Pure unit tests for {@link ModuleCommandValidator} — no Spring context.
+ * Focuses on:
+ * 1. rule detection (meta-char / whitelist / sh -c);
+ * 2. exact error message format returned to the caller (which becomes the
+ * user-facing message once wrapped by {@code
ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST}).
+ */
+public class ModuleCommandValidatorTest {
+
+ private ModuleCommandValidator validator;
+
+ @BeforeEach
+ public void setUp() {
+ validator = new ModuleCommandValidator();
+ // Force STRICT so getMode() != OFF and validation actually happens.
+ ReflectionTestUtils.setField(validator, "whitelistModeConfig",
+ ModuleCommandValidator.WhitelistMode.STRICT);
+ ReflectionTestUtils.setField(validator, "extraWhitelist", "");
+ }
+
+ /* ---------------- happy path ---------------- */
+
+ @Test
+ public void validate_null_shouldReturnNull() {
+ Assertions.assertNull(validator.validate(null));
+ Assertions.assertNull(validator.validate(""));
+ Assertions.assertNull(validator.validate(" "));
+ }
+
+ @Test
+ public void validate_simpleWhitelistedCommand_shouldPass() {
+ Assertions.assertNull(validator.validate("ps -ef"));
+ Assertions.assertNull(validator.validate("cd /opt/inlong"));
+ Assertions.assertNull(validator.validate("mkdir -p /tmp/x"));
+ }
+
+ @Test
+ public void validate_pipeChainOfWhitelistedCommands_shouldPass() {
+ Assertions.assertNull(validator.validate("ps -ef | grep java | awk
'{print $2}'"));
+ }
+
+ @Test
+ public void validate_semicolonChainOfWhitelistedCommands_shouldPass() {
+ Assertions.assertNull(validator.validate("cd /opt; mkdir logs; echo
done"));
+ }
+
+ /* ---------------- P0: glob wildcards must be rejected with a helpful
hint ---------------- */
+
+ @Test
+ public void validate_starWildcard_shouldRejectWithGlobHint() {
+ String r = validator.validate("rm /opt/inlong/*.log");
+ Assertions.assertNotNull(r);
+ Assertions.assertTrue(r.startsWith("DISALLOWED_META_CHAR: [*]"),
+ "should start with DISALLOWED_META_CHAR: [*], got: " + r);
+ Assertions.assertTrue(r.contains("glob wildcards are not supported"),
+ "must explain wildcards are not supported, got: " + r);
+ Assertions.assertTrue(r.contains("will NOT be expanded"),
+ "must warn user the wildcard will not be expanded, got: " + r);
+ Assertions.assertTrue(r.contains("explicit file paths"),
+ "must suggest explicit file paths, got: " + r);
+ }
+
+ @Test
+ public void validate_questionMarkWildcard_shouldRejectWithGlobHint() {
+ String r = validator.validate("ls /opt/inlong/?.txt");
+ Assertions.assertNotNull(r);
+ Assertions.assertTrue(r.startsWith("DISALLOWED_META_CHAR: [?]"),
+ "should start with DISALLOWED_META_CHAR: [?], got: " + r);
+ Assertions.assertTrue(r.contains("glob wildcards are not supported"),
+ "must explain wildcards are not supported, got: " + r);
+ }
+
+ @Test
+ public void validate_starAsArgv_shouldRejectEvenIfArgvIsSingleChar() {
+ String r = validator.validate("rm *");
+ Assertions.assertNotNull(r);
+ Assertions.assertTrue(r.contains("*"), "must mention '*', got: " + r);
+ Assertions.assertTrue(r.contains("glob wildcards are not supported"),
+ "must include glob hint, got: " + r);
+ }
+
+ /* ---------------- other meta chars — reject but WITHOUT the glob hint
---------------- */
+
+ @Test
+ public void validate_backtick_shouldRejectWithoutGlobHint() {
+ String r = validator.validate("echo `whoami`");
+ Assertions.assertNotNull(r);
+ Assertions.assertTrue(r.startsWith("DISALLOWED_META_CHAR: [`]"), r);
+ Assertions.assertFalse(r.contains("glob wildcards"),
+ "non-glob meta char must NOT get the glob hint, got: " + r);
+ }
+
+ @Test
+ public void validate_dollarParen_shouldReject() {
+ String r = validator.validate("echo $(whoami)");
+ Assertions.assertNotNull(r);
+ Assertions.assertTrue(r.startsWith("DISALLOWED_META_CHAR: [$(]"), r);
+ }
+
+ @Test
+ public void validate_doubleAmp_shouldReject() {
+ String r = validator.validate("echo a && echo b");
+ Assertions.assertNotNull(r);
+ Assertions.assertTrue(r.startsWith("DISALLOWED_META_CHAR: [&&]"), r);
+ }
+
+ @Test
+ public void validate_redirect_shouldReject() {
+ String r = validator.validate("echo hi > /tmp/x");
+ Assertions.assertNotNull(r);
+ Assertions.assertTrue(r.startsWith("DISALLOWED_META_CHAR: [>]"), r);
+ }
+
+ @Test
+ public void validate_lineBreak_shouldReject() {
+ String r = validator.validate("echo a\necho b");
+ Assertions.assertNotNull(r);
+ Assertions.assertEquals("DISALLOWED_META_CHAR: line-break", r);
+ }
+
+ /* ---------------- argv[0] whitelist ---------------- */
+
+ @Test
+ public void validate_notInWhitelist_shouldReturnCmdName() {
+ String r = validator.validate("python3 script.py");
+ Assertions.assertEquals("python3", r,
+ "non-whitelisted argv[0] should be returned verbatim (no
prefix)");
+ }
+
+ @Test
+ public void validate_extraWhitelist_shouldExpandBaseline() {
+ ReflectionTestUtils.setField(validator, "extraWhitelist",
"python3,curl");
+ // reset lazy cache
+ ReflectionTestUtils.setField(validator, "effectiveWhitelist", null);
+ Assertions.assertNull(validator.validate("python3 script.py"));
+ Assertions.assertNull(validator.validate("curl http://x";));
+ }
+
+ /* ---------------- forbid sh -c / bash -c ---------------- */
+
+ @Test
+ public void validate_shDashC_shouldReject() {
+ String r = validator.validate("sh -c 'echo hi'");
+ Assertions.assertNotNull(r);
+ Assertions.assertTrue(r.startsWith("FORBIDDEN_SH_C_FLAG:"),
+ "should start with FORBIDDEN_SH_C_FLAG, got: " + r);
+ Assertions.assertTrue(r.contains("sh"), r);
+ }
+
+ @Test
+ public void validate_bashDashC_shouldReject() {
+ String r = validator.validate("bash -c 'ls'");
+ Assertions.assertNotNull(r);
+ Assertions.assertTrue(r.startsWith("FORBIDDEN_SH_C_FLAG:"), r);
+ Assertions.assertTrue(r.contains("bash"), r);
+ }
+
+ /* ---------------- tokenizer: mixed quoted/unquoted is intentionally NOT
supported ---------------- */
+
+ @Test
+ public void validate_mixedQuotedUnquoted_argv0IsUnquotedPrefix() {
+ // Under the tokenizer, a quote character always starts a fresh token
and any
+ // unquoted run terminates as soon as a quote character is encountered.
+ // So `python3"foo" bar` splits into: [python3, foo, bar] — argv[0] is
'python3',
+ // which is NOT in the baseline whitelist and must be rejected
verbatim.
+ String r = validator.validate("python3\"foo\" bar");
+ Assertions.assertEquals("python3", r,
+ "argv[0] should be the unquoted prefix 'python3', not the
concatenated 'python3foo'");
+ }
+
+ @Test
+ public void validate_mixedQuotedUnquoted_whitelistedPrefix_shouldPass() {
+ // `ps"aux"` splits into [ps, aux] — argv[0] 'ps' is whitelisted, so
this passes.
+ // (Note: under the old concat-tokenizer this would have been
argv[0]='psaux',
+ // which would have been rejected. The new stricter semantics are
intentional.)
+ Assertions.assertNull(validator.validate("ps\"aux\""));
+ }
+
+}
diff --git
a/inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleServiceImplTest.java
b/inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleServiceImplTest.java
new file mode 100644
index 0000000000..bd142c6fd3
--- /dev/null
+++
b/inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleServiceImplTest.java
@@ -0,0 +1,180 @@
+/*
+ * 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 regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.inlong.manager.service.module;
+
+import org.apache.inlong.manager.common.exceptions.BusinessException;
+import org.apache.inlong.manager.pojo.module.ModuleRequest;
+import org.apache.inlong.manager.service.ServiceBaseTest;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.util.ReflectionTestUtils;
+
+/**
+ * Integration test for {@link ModuleServiceImpl} — walks the real save() code
path
+ * that the {@code /api/module/save} endpoint invokes, and verifies that the
+ * {@link BusinessException#getMessage()} eventually surfaced to the API caller
+ * carries the expected user-facing error text (including the glob-wildcard
hint
+ * added for the P0 fix).
+ *
+ * <p>The wrapper format is defined by
+ * {@code ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST}: {@code "Module
command not in whitelist: '%s'"}.
+ */
+public class ModuleServiceImplTest extends ServiceBaseTest {
+
+ @Autowired
+ private ModuleService moduleService;
+
+ @Autowired
+ private ModuleCommandValidator commandValidator;
+
+ @BeforeEach
+ public void forceStrictMode() {
+ // Ensure the test is independent of application-*.properties: force
STRICT
+ // so the validator blocks rather than logs.
+ ReflectionTestUtils.setField(commandValidator, "whitelistModeConfig",
+ ModuleCommandValidator.WhitelistMode.STRICT);
+ }
+
+ private ModuleRequest baseRequest() {
+ ModuleRequest r = new ModuleRequest();
+ r.setName("m-" + System.nanoTime());
+ r.setType("AGENT");
+ r.setVersion("1.0.0");
+ r.setPackageId(1);
+ // sensible default: use only whitelisted commands
+ r.setStartCommand("echo start");
+ r.setStopCommand("echo stop");
+ r.setCheckCommand("echo check");
+ r.setInstallCommand("echo install");
+ r.setUninstallCommand("echo uninstall");
+ return r;
+ }
+
+ /* ---------- P0 focus: '*' rejected at save time with a helpful message
---------- */
+
+ @Test
+ public void save_startCommandContainsStar_apiCallerSeesGlobHint() {
+ ModuleRequest req = baseRequest();
+ req.setStartCommand("rm /opt/inlong/*.log");
+
+ BusinessException ex = Assertions.assertThrows(BusinessException.class,
+ () -> moduleService.save(req, "admin"));
+
+ String msg = ex.getMessage();
+ Assertions.assertNotNull(msg);
+ // ErrorCodeEnum wrapper
+ Assertions.assertTrue(msg.startsWith("Module command not in
whitelist:"),
+ "API caller should see the wrapped error, got: " + msg);
+ // pinpoint the offending command by echoing its raw text
+ Assertions.assertTrue(msg.contains("in [rm /opt/inlong/*.log]"),
+ "must echo the offending command verbatim, got: " + msg);
+ // pinpoint the offending char
+ Assertions.assertTrue(msg.contains("DISALLOWED_META_CHAR: [*]"),
+ "must tell the user which char triggered rejection, got: " +
msg);
+ // and — critically — WHY it is rejected
+ Assertions.assertTrue(msg.contains("glob wildcards are not supported"),
+ "must explain glob wildcards are not supported, got: " + msg);
+ Assertions.assertTrue(msg.contains("will NOT be expanded"),
+ "must warn the wildcard is not expanded, got: " + msg);
+ Assertions.assertTrue(msg.contains("explicit file paths"),
+ "must guide the user toward the workaround, got: " + msg);
+ }
+
+ @Test
+ public void
save_installCommandContainsQuestionMark_apiCallerSeesGlobHint() {
+ ModuleRequest req = baseRequest();
+ req.setInstallCommand("cp /pkg/a?.tar /opt/");
+
+ BusinessException ex = Assertions.assertThrows(BusinessException.class,
+ () -> moduleService.save(req, "admin"));
+
+ String msg = ex.getMessage();
+ Assertions.assertTrue(msg.startsWith("Module command not in
whitelist:"), msg);
+ Assertions.assertTrue(msg.contains("in [cp /pkg/a?.tar /opt/]"), msg);
+ Assertions.assertTrue(msg.contains("DISALLOWED_META_CHAR: [?]"), msg);
+ Assertions.assertTrue(msg.contains("glob wildcards are not
supported"), msg);
+ }
+
+ /* ---------- other meta chars: rejected, but WITHOUT the glob hint
---------- */
+
+ @Test
+ public void
save_backtickInCheckCommand_apiCallerSeesRejectionWithoutGlobHint() {
+ ModuleRequest req = baseRequest();
+ req.setCheckCommand("echo `whoami`");
+
+ BusinessException ex = Assertions.assertThrows(BusinessException.class,
+ () -> moduleService.save(req, "admin"));
+
+ String msg = ex.getMessage();
+ Assertions.assertTrue(msg.startsWith("Module command not in
whitelist:"), msg);
+ Assertions.assertTrue(msg.contains("in [echo `whoami`]"), msg);
+ Assertions.assertTrue(msg.contains("DISALLOWED_META_CHAR: [`]"), msg);
+ Assertions.assertFalse(msg.contains("glob wildcards"),
+ "non-glob meta char must NOT carry the glob hint, got: " +
msg);
+ }
+
+ @Test
+ public void save_shDashCInStopCommand_apiCallerSeesForbiddenShCFlag() {
+ ModuleRequest req = baseRequest();
+ req.setStopCommand("sh -c ls");
+
+ BusinessException ex = Assertions.assertThrows(BusinessException.class,
+ () -> moduleService.save(req, "admin"));
+
+ String msg = ex.getMessage();
+ Assertions.assertTrue(msg.startsWith("Module command not in
whitelist:"), msg);
+ Assertions.assertTrue(msg.contains("in [sh -c ls]"), msg);
+ Assertions.assertTrue(msg.contains("FORBIDDEN_SH_C_FLAG"), msg);
+ }
+
+ @Test
+ public void save_nonWhitelistedArgv0_apiCallerSeesCmdName() {
+ ModuleRequest req = baseRequest();
+ req.setUninstallCommand("python3 uninstall.py");
+
+ BusinessException ex = Assertions.assertThrows(BusinessException.class,
+ () -> moduleService.save(req, "admin"));
+
+ String msg = ex.getMessage();
+ Assertions.assertTrue(msg.startsWith("Module command not in
whitelist:"), msg);
+ Assertions.assertTrue(msg.contains("in [python3 uninstall.py]"), msg);
+ Assertions.assertTrue(msg.contains("python3"),
+ "must tell the user the offending command name, got: " + msg);
+ }
+
+ /* ---------- OFF mode: validation is completely skipped ---------- */
+
+ @Test
+ public void save_modeOff_shouldNotBlockEvenIfCommandsAreDirty() {
+ ReflectionTestUtils.setField(commandValidator, "whitelistModeConfig",
+ ModuleCommandValidator.WhitelistMode.OFF);
+ ModuleRequest req = baseRequest();
+ req.setStartCommand("rm /opt/*.log");
+ // In OFF mode, save must not throw due to validator. It may still
throw for
+ // other reasons; here we only assert the validator is NOT the throw
source.
+ try {
+ moduleService.save(req, "admin");
+ } catch (BusinessException e) {
+
Assertions.assertFalse(String.valueOf(e.getMessage()).startsWith("Module
command not in whitelist:"),
+ "OFF mode must not surface the whitelist wrapper, got: " +
e.getMessage());
+ }
+ }
+}
