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 e7cc03602b7..ca22ebbd823 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 9364fda8e78..34068816a93 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 00000000000..8cb181eed0b --- /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 of(ModuleRequest request) { + return Arrays.asList( + request.getStartCommand(), + request.getStopCommand(), + request.getCheckCommand(), + request.getInstallCommand(), + request.getUninstallCommand()); + } + + public static List 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 00000000000..3094205a2b0 --- /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. + * + *

Unlike the agent-side validator, this class does not perform path-under-root + * checks, which are filesystem-dependent and only meaningful at the agent runtime. + * + *

Whitelist baseline: + * 

{@code
+ *   cd, sh, bash, ps, grep, awk, kill, rm, mkdir, cp, mv, ln,
+ *   tar, unzip, chmod, chown, echo, cat, test, [, true, false, java
+ * }
+ * + *

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 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. + * + *

+ */ + public enum WhitelistMode { + + STRICT, WARN, OFF; + } + + /** Lazily-computed effective whitelist (baseline + config extras). */ + private volatile Set effectiveWhitelist; + + private static Set buildImmutableSet(String... items) { + Set 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 getEffectiveWhitelist() { + if (effectiveWhitelist != null) { + return effectiveWhitelist; + } + synchronized (this) { + if (effectiveWhitelist != null) { + return effectiveWhitelist; + } + Set 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 not need to check the mode or wrap the exception themselves. + * + *

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}. + * + *

    + *
  • {@link WhitelistMode#OFF} — skip entirely, return {@code true}.
    • + *
  • {@link WhitelistMode#STRICT} — throw {@link BusinessException} on any + * violation.
    • + *
  • {@link WhitelistMode#WARN} — log a warning and let the update proceed.
    • + *
+ * + * @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 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 changedCommands(ModuleDTO oldDto, ModuleRequest request) { + List newCmds = ModuleCommandAccessors.of(request); + if (oldDto == null) { + return newCmds; + } + List oldCmds = ModuleCommandAccessors.of(oldDto); + List 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 " in []"} 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 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 whitelist = getEffectiveWhitelist(); + + // Split by ';' into sub-commands (quote-aware), then by '|' into pipe segments + List 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 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 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 splitTopLevel(String raw, char delim) { + List 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}. + * + *

For a shell-command whitelist/blacklist validator, mixed quoted/unquoted tokens + * (e.g. {@code abc"def"ghi}) are intentionally not 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 tokenize(String s) { + List 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 92579a24372..ff499d1e670 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 Integer save(ModuleRequest request, String operator) { } 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 Boolean update(ModuleRequest request, String operator) { 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 00000000000..da862d280e6 --- /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 00000000000..bd142c6fd3d --- /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). + * + *

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()); + } + } +}