From c4ed1f48c428d9a8d41b07b0f1d78c6de6ad9560 Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Thu, 2 Jul 2026 14:35:11 +0800 Subject: [PATCH 1/7] [INLONG-12149][Manager] [Agent] Add command whitelist validation in Manager (#12149) --- .../manager/common/enums/ErrorCodeEnum.java | 2 + .../module/ModuleCommandValidator.java | 388 ++++++++++++++++++ .../service/module/ModuleServiceImpl.java | 45 +- .../module/ModuleCommandValidatorTest.java | 211 ++++++++++ .../service/module/ModuleServiceImplTest.java | 178 ++++++++ 5 files changed, 823 insertions(+), 1 deletion(-) create mode 100644 inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandValidator.java create mode 100644 inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleCommandValidatorTest.java create mode 100644 inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleServiceImplTest.java 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..c83c4578146 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, + "Command validation failed: '%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/ModuleCommandValidator.java b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandValidator.java new file mode 100644 index 00000000000..9cc31d6f9b5 --- /dev/null +++ b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandValidator.java @@ -0,0 +1,388 @@ +/* + * 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 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}") + private String whitelistModeConfig; + + /** + * Whitelist enforcement mode. + * + *

+ */ + public enum WhitelistMode { + + STRICT, WARN, OFF; + + public static WhitelistMode fromConfig(String val) { + if (StringUtils.isBlank(val)) { + return WARN; + } + String upper = val.trim().toUpperCase(); + try { + return valueOf(upper); + } catch (IllegalArgumentException e) { + LOGGER.warn("ModuleCommandValidator: invalid module.command.whitelistMode '{}'," + + " falling back to STRICT", val); + return STRICT; + } + } + } + + /** 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(",")) { + 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; + } + + /** + * Get the current whitelist enforcement mode from configuration. + */ + public WhitelistMode getMode() { + return WhitelistMode.fromConfig(whitelistModeConfig); + } + + /** + * Incremental validation — only validates command fields that have actually changed + * between the existing module config (from DB) and the incoming update request. + *

+ * This avoids blocking updates for existing modules that only change non-command + * fields (e.g. version number, package id). + * + * @param oldDto the existing ModuleDTO parsed from DB extParams (may be null if + * extParams was empty — all fields are treated as changed) + * @param request the incoming update request + * @return the offending field name + command, or {@code null} if all changed commands pass + */ + public String validateChanged(ModuleDTO oldDto, ModuleRequest request) { + if (oldDto == null) { + // No existing data — validate everything (same as save) + return validateAll(request.getStartCommand(), request.getStopCommand(), + request.getCheckCommand(), request.getInstallCommand(), + request.getUninstallCommand()); + } + + String[] fields = {"startCommand", "stopCommand", "checkCommand", "installCommand", "uninstallCommand"}; + String[] oldValues = { + oldDto.getStartCommand(), oldDto.getStopCommand(), oldDto.getCheckCommand(), + oldDto.getInstallCommand(), oldDto.getUninstallCommand()}; + String[] newValues = { + request.getStartCommand(), request.getStopCommand(), request.getCheckCommand(), + request.getInstallCommand(), request.getUninstallCommand()}; + + for (int i = 0; i < fields.length; i++) { + String oldVal = oldValues[i]; + String newVal = newValues[i]; + // Skip if unchanged (both null or equal) + if (Objects.equals(oldVal, newVal)) { + continue; + } + String offending = validate(newVal); + if (offending != null) { + return fields[i] + ": '" + offending + "'"; + } + } + 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). + * + * @param rawCmd the raw command string (may be null or blank — skipped) + * @return error description, or {@code null} if valid + */ + public String validate(String rawCmd) { + if (StringUtils.isBlank(rawCmd)) { + return null; + } + + // metacharacter blacklist (whole string, before splitting) + String metaHit = firstMetaCharHit(rawCmd); + if (metaHit != null) { + if ("*".equals(metaHit) || "?".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('\n') >= 0 || rawCmd.indexOf('\r') >= 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, ';'); + if (segments == null) { + return "DISALLOWED_META_CHAR: unterminated quote"; + } + for (String seg : segments) { + String trimmed = seg == null ? "" : seg.trim(); + if (trimmed.isEmpty()) { + continue; + } + List pipeSegs = splitTopLevel(trimmed, '|'); + if (pipeSegs == null) { + return "DISALLOWED_META_CHAR: unterminated quote in pipe segment"; + } + for (String pipeSeg : pipeSegs) { + String pipeTrimmed = pipeSeg == null ? "" : pipeSeg.trim(); + if (pipeTrimmed.isEmpty()) { + continue; + } + String[] argv = tokenize(pipeTrimmed); + if (argv.length == 0) { + continue; + } + String cmdName = argv[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.length > 1) { + for (int i = 1; i < argv.length; i++) { + if (argv[i].startsWith("-c")) { + return "FORBIDDEN_SH_C_FLAG: '" + cmdName + " " + argv[i] + "'"; + } + } + } + } + } + return null; + } + + /** + * Thoroughly validate all commands in a module request. Returns {@code null} if all + * commands pass, or a descriptive message like {@code "startCommand: 'python3'"} if + * one fails. + */ + public String validateAll(String startCommand, String stopCommand, String checkCommand, + String installCommand, String uninstallCommand) { + String[] fields = {"startCommand", "stopCommand", "checkCommand", "installCommand", "uninstallCommand"}; + String[] values = {startCommand, stopCommand, checkCommand, installCommand, uninstallCommand}; + for (int i = 0; i < fields.length; i++) { + String offending = validate(values[i]); + if (offending != null) { + return fields[i] + ": '" + offending + "'"; + } + } + 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 (raw == null) { + out.add(""); + return out; + } + StringBuilder cur = new StringBuilder(); + 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 == '\'' || c == '"') { + 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. (Same logic as the + * agent-side validator.) + */ + private static String[] tokenize(String s) { + List tokens = new ArrayList<>(); + StringBuilder cur = new StringBuilder(); + char quote = 0; + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + if (quote != 0) { + if (c == quote) { + quote = 0; + } else { + cur.append(c); + } + continue; + } + if (c == '\'' || c == '"') { + quote = c; + continue; + } + if (Character.isWhitespace(c)) { + if (cur.length() > 0) { + tokens.add(cur.toString()); + cur.setLength(0); + } + continue; + } + cur.append(c); + } + if (cur.length() > 0) { + tokens.add(cur.toString()); + } + return tokens.toArray(new String[0]); + } +} 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..77c6e7f4782 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,28 @@ 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); + + // Validate commands against whitelist at save time. + // STRICT and WARN both block; only OFF skips. + ModuleCommandValidator.WhitelistMode mode = commandValidator.getMode(); + if (mode != ModuleCommandValidator.WhitelistMode.OFF) { + String violation = commandValidator.validateAll( + request.getStartCommand(), request.getStopCommand(), + request.getCheckCommand(), request.getInstallCommand(), + request.getUninstallCommand()); + if (violation != null) { + throw new BusinessException(ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST, + String.format(ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST.getMessage(), + violation)); + } + } + ModuleConfigEntity moduleConfigEntity = CommonBeanUtils.copyProperties(request, ModuleConfigEntity::new); try { ModuleDTO dto = ModuleDTO.getFromRequest(request, moduleConfigEntity.getExtParams(), @@ -68,7 +86,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 +101,30 @@ 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 validation (mode-aware): only check command fields + // that actually changed compared to the stored extParams. + ModuleCommandValidator.WhitelistMode mode = commandValidator.getMode(); + if (mode != ModuleCommandValidator.WhitelistMode.OFF) { + ModuleDTO oldDto = null; + if (moduleConfigEntity.getExtParams() != null) { + oldDto = ModuleDTO.getFromJson(moduleConfigEntity.getExtParams()); + } + String violation = commandValidator.validateChanged(oldDto, request); + if (violation != null) { + if (mode == ModuleCommandValidator.WhitelistMode.STRICT) { + throw new BusinessException(ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST, + String.format( + ErrorCodeEnum.MODULE_COMMAND_NOT_IN_WHITELIST.getMessage(), + violation)); + } else { + // WARN mode: log only, do not block the update + LOGGER.warn("ModuleCommandValidator: non-whitelisted command in update " + + "(mode=WARN, not blocking): moduleId={}, {}", request.getId(), + violation); + } + } + } 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..5bca14143fe --- /dev/null +++ b/inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleCommandValidatorTest.java @@ -0,0 +1,211 @@ +/* + * 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", "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); + } + + /* ---------------- validateAll: returns fieldName + offending detail ---------------- */ + + @Test + public void validateAll_starInStartCommand_shouldReturnFieldQualifiedError() { + String r = validator.validateAll("rm /a/*.log", "echo stop", "echo check", + "echo install", "echo uninstall"); + Assertions.assertNotNull(r); + Assertions.assertTrue(r.startsWith("startCommand:"), + "must be prefixed with field name 'startCommand:', got: " + r); + Assertions.assertTrue(r.contains("DISALLOWED_META_CHAR: '*'"), + "must contain the underlying rule + char, got: " + r); + Assertions.assertTrue(r.contains("glob wildcards are not supported"), + "must contain the user-facing wildcard hint, got: " + r); + } + + @Test + public void validateAll_questionInInstallCommand_shouldPinpointField() { + String r = validator.validateAll("echo start", "echo stop", "echo check", + "cp /a/b?.txt /c/", "echo uninstall"); + Assertions.assertNotNull(r); + Assertions.assertTrue(r.startsWith("installCommand:"), r); + Assertions.assertTrue(r.contains("'?'"), r); + Assertions.assertTrue(r.contains("glob wildcards are not supported"), r); + } + + @Test + public void validateAll_allCommandsClean_shouldReturnNull() { + Assertions.assertNull(validator.validateAll( + "echo start", "echo stop", "echo check", "echo install", "echo uninstall")); + } +} 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..adf83145e4d --- /dev/null +++ b/inlong-manager/manager-service/src/test/java/org/apache/inlong/manager/service/module/ModuleServiceImplTest.java @@ -0,0 +1,178 @@ +/* + * 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 "Command validation failed: '%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", "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("Command validation failed:"), + "API caller should see the wrapped error, got: " + msg); + // pinpoint the offending field + Assertions.assertTrue(msg.contains("startCommand:"), + "must tell the user which field is offending, 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("Command validation failed:"), msg); + Assertions.assertTrue(msg.contains("installCommand:"), 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("Command validation failed:"), msg); + Assertions.assertTrue(msg.contains("checkCommand:"), 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("Command validation failed:"), msg); + Assertions.assertTrue(msg.contains("stopCommand:"), 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("Command validation failed:"), msg); + Assertions.assertTrue(msg.contains("uninstallCommand:"), 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", "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("Command validation failed:"), + "OFF mode must not surface the whitelist wrapper, got: " + e.getMessage()); + } + } +} From 6a9d0b845f3d6229429280681046d241671dbeb7 Mon Sep 17 00:00:00 2001 From: spirit_sx Date: Thu, 9 Jul 2026 14:18:58 +0800 Subject: [PATCH 2/7] Update inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/ErrorCodeEnum.java Co-authored-by: fuweng11 <76141879+fuweng11@users.noreply.github.com> --- .../org/apache/inlong/manager/common/enums/ErrorCodeEnum.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c83c4578146..5a8988e6bdb 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 @@ -170,7 +170,7 @@ 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, - "Command validation failed: '%s'"), + "Module command not in whitlist: '%s'"), PACKAGE_NOT_FOUND(7001, "Package does not exist/no operation authority"), PACKAGE_INFO_INCORRECT(7002, "Package info was incorrect"), From f848ad63ae334b14600f8670632a064829da848a Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Thu, 9 Jul 2026 16:11:26 +0800 Subject: [PATCH 3/7] [INLONG-12149][Manager] [Manager] fix review comment (#12149) --- .../common/consts/InlongConstants.java | 14 ++ .../manager/common/enums/ErrorCodeEnum.java | 2 +- .../manager/service/module/CommandField.java | 43 ++++++ .../module/ModuleCommandAccessors.java | 92 +++++++++++++ .../module/ModuleCommandValidator.java | 125 ++++++++---------- .../service/module/ModuleServiceImpl.java | 15 +-- .../module/ModuleCommandValidatorTest.java | 32 ++++- .../service/module/ModuleServiceImplTest.java | 20 +-- 8 files changed, 248 insertions(+), 95 deletions(-) create mode 100644 inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/CommandField.java create mode 100644 inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandAccessors.java 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 5a8988e6bdb..aedd3d1824b 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 @@ -170,7 +170,7 @@ 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 whitlist: '%s'"), + "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/CommandField.java b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/CommandField.java new file mode 100644 index 00000000000..26a7e38566f --- /dev/null +++ b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/CommandField.java @@ -0,0 +1,43 @@ +/* + * 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; + +/** + * The five command fields validated by {@link ModuleCommandValidator}. Only carries the + * display label used in error messages; how to read the value from a given carrier + * (DTO / Request / ...) is supplied by {@link ModuleCommandAccessors}, so this enum is + * decoupled from any concrete POJO type. + * + *

Package-private on purpose — it is an implementation detail shared between + * {@link ModuleCommandValidator} and {@link ModuleCommandAccessors} and should not leak + * to callers. + */ +enum CommandField { + + START("startCommand"), + STOP("stopCommand"), + CHECK("checkCommand"), + INSTALL("installCommand"), + UNINSTALL("uninstallCommand"); + + final String label; + + CommandField(String label) { + this.label = label; + } +} 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..e6b21004be0 --- /dev/null +++ b/inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/module/ModuleCommandAccessors.java @@ -0,0 +1,92 @@ +/* + * 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.EnumMap; +import java.util.Map; +import java.util.function.Function; + +/** + * The single place in this package that knows how to read the five command fields off a + * concrete POJO. Given a carrier ({@link ModuleRequest} / {@link ModuleDTO} / ...), it + * returns a {@code Function} that {@link ModuleCommandValidator} + * can drive uniformly. + * + *

Design notes: + *

    + *
  • The field \u2192 getter mapping is registered once per carrier type in a + * {@link EnumMap} \u2014 no more per-carrier {@code switch} statements.
    • + *
  • Adding a new carrier (e.g. {@code ModuleResponse}) = one extra static map + one + * extra {@code of(...)} overload. Nothing in {@link ModuleCommandValidator} + * changes.
    • + *
  • Package-private on purpose \u2014 this is an internal helper, not part of any + * public API.
    • + *
+ */ +public final class ModuleCommandAccessors { + + private static final Map> REQUEST_GETTERS = + new EnumMap<>(CommandField.class); + private static final Map> DTO_GETTERS = + new EnumMap<>(CommandField.class); + + static { + REQUEST_GETTERS.put(CommandField.START, ModuleRequest::getStartCommand); + REQUEST_GETTERS.put(CommandField.STOP, ModuleRequest::getStopCommand); + REQUEST_GETTERS.put(CommandField.CHECK, ModuleRequest::getCheckCommand); + REQUEST_GETTERS.put(CommandField.INSTALL, ModuleRequest::getInstallCommand); + REQUEST_GETTERS.put(CommandField.UNINSTALL, ModuleRequest::getUninstallCommand); + + DTO_GETTERS.put(CommandField.START, ModuleDTO::getStartCommand); + DTO_GETTERS.put(CommandField.STOP, ModuleDTO::getStopCommand); + DTO_GETTERS.put(CommandField.CHECK, ModuleDTO::getCheckCommand); + DTO_GETTERS.put(CommandField.INSTALL, ModuleDTO::getInstallCommand); + DTO_GETTERS.put(CommandField.UNINSTALL, ModuleDTO::getUninstallCommand); + } + + private ModuleCommandAccessors() { + } + + /** + * Return a reader over the command fields of the given {@link ModuleRequest}. + */ + static Function of(ModuleRequest request) { + return readerFor(request, REQUEST_GETTERS); + } + + /** + * Return a reader over the command fields of the given {@link ModuleDTO}. + */ + static Function of(ModuleDTO dto) { + return readerFor(dto, DTO_GETTERS); + } + + private static Function readerFor(T carrier, + Map> getters) { + return field -> { + Function getter = getters.get(field); + if (getter == null) { + throw new IllegalStateException("No getter registered for command field: " + field); + } + return getter.apply(carrier); + }; + } +} 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 index 9cc31d6f9b5..00f7f47df77 100644 --- 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 @@ -17,6 +17,7 @@ package org.apache.inlong.manager.service.module; +import org.apache.inlong.manager.common.consts.InlongConstants; import org.apache.inlong.manager.pojo.module.ModuleDTO; import org.apache.inlong.manager.pojo.module.ModuleRequest; @@ -33,6 +34,8 @@ import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.function.Function; +import java.util.function.Predicate; /** * Manager-side command whitelist validator. Validates that command names (argv[0]) in module @@ -84,7 +87,7 @@ public class ModuleCommandValidator { /** Whitelist enforcement mode: STRICT (block), WARN (log only), OFF (skip). */ @Value("${module.command.whitelistMode:WARN}") - private String whitelistModeConfig; + private WhitelistMode whitelistModeConfig; /** * Whitelist enforcement mode. @@ -99,20 +102,6 @@ public class ModuleCommandValidator { public enum WhitelistMode { STRICT, WARN, OFF; - - public static WhitelistMode fromConfig(String val) { - if (StringUtils.isBlank(val)) { - return WARN; - } - String upper = val.trim().toUpperCase(); - try { - return valueOf(upper); - } catch (IllegalArgumentException e) { - LOGGER.warn("ModuleCommandValidator: invalid module.command.whitelistMode '{}'," - + " falling back to STRICT", val); - return STRICT; - } - } } /** Lazily-computed effective whitelist (baseline + config extras). */ @@ -138,7 +127,7 @@ private Set getEffectiveWhitelist() { } Set merged = new LinkedHashSet<>(BASELINE_WHITELIST); if (StringUtils.isNotBlank(extraWhitelist)) { - for (String item : extraWhitelist.split(",")) { + for (String item : extraWhitelist.split(InlongConstants.COMMA)) { String trimmed = item.trim(); if (trimmed.isEmpty()) { continue; @@ -157,7 +146,7 @@ private Set getEffectiveWhitelist() { * Get the current whitelist enforcement mode from configuration. */ public WhitelistMode getMode() { - return WhitelistMode.fromConfig(whitelistModeConfig); + return whitelistModeConfig; } /** @@ -175,29 +164,31 @@ public WhitelistMode getMode() { public String validateChanged(ModuleDTO oldDto, ModuleRequest request) { if (oldDto == null) { // No existing data — validate everything (same as save) - return validateAll(request.getStartCommand(), request.getStopCommand(), - request.getCheckCommand(), request.getInstallCommand(), - request.getUninstallCommand()); + return validateAll(request); } + Function oldReader = ModuleCommandAccessors.of(oldDto); + Function newReader = ModuleCommandAccessors.of(request); + // Skip fields whose value is unchanged (both null or equal). + return validateFields(newReader, + f -> Objects.equals(oldReader.apply(f), newReader.apply(f))); + } - String[] fields = {"startCommand", "stopCommand", "checkCommand", "installCommand", "uninstallCommand"}; - String[] oldValues = { - oldDto.getStartCommand(), oldDto.getStopCommand(), oldDto.getCheckCommand(), - oldDto.getInstallCommand(), oldDto.getUninstallCommand()}; - String[] newValues = { - request.getStartCommand(), request.getStopCommand(), request.getCheckCommand(), - request.getInstallCommand(), request.getUninstallCommand()}; - - for (int i = 0; i < fields.length; i++) { - String oldVal = oldValues[i]; - String newVal = newValues[i]; - // Skip if unchanged (both null or equal) - if (Objects.equals(oldVal, newVal)) { + /** + * Core validation loop: iterate through every {@link CommandField}, read its current + * value via {@code accessor}, and run {@link #validate(String)}. Fields for which + * {@code skip} returns {@code true} are bypassed (used for incremental validation). + * + * @return {@code "