[3.3.6] backport Linux non-root packaging scripts#35145
[3.3.6] backport Linux non-root packaging scripts#35145
Conversation
- Add setup_env() to install.sh: detects user vs root mode, selects
service dirs (systemctl --user for non-root), initialises all path
variables accordingly, and writes .install_path sentinel.
- Add validate_safe_path(), .install_path discovery, user_mode
detection, and sysctl_cmd (systemctl --user for non-root) to
remove.sh; clean_service_on_systemd_of now uses ${sysctl_cmd}.
- Add smokeTest/test_non_root_server_packaging_backport.sh regression
guard (6 checks, all passing).
- 3.3.6 tool lists, service names, and runtime boundaries preserved;
no xnode SQL behaviour introduced.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- clear csudo in user_mode=1 branch of setup_env() (install.sh) and
user_mode=1 branch (remove.sh) so non-root installs never run file
operations with sudo
- replace bare systemctl / ${csudo}systemctl with ${sysctl_cmd} in
clean_service_on_systemd(), install_service_on_systemd(), and the
updateProduct() stop path so non-root installs target the user bus
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Override csudo to empty after the top-level sudo detection so that non-root (user_mode=1) uninstalls never invoke sudo for file operations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the unconditional reassignment of service_config_dir to /etc/systemd/system inside the root-mode directory initialization block of setup_env(). The earlier service-manager detection block already sets service_config_dir=/etc/init.d when sysvinit is in use; overwriting it here broke that detection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- packaging/tools/install.sh: drop csudo variable and all sudo invocations; direct commands run under current user context which is already correct (root owns system dirs, non-root owns user-local dirs set by setup_env) - packaging/tools/remove.sh: same; also remove the csudo init block and the non-root override that was masking the underlying sudo calls - packaging/smokeTest/test_non_root_server_packaging_backport.sh: extend regression guard with check_absent() helper and two new checks that fail if sudo or csudo appear in either target script Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request introduces non-root (user-mode) support for TDengine server and client packaging scripts, including new regression tests. Key changes involve environment detection, directory initialization for non-root users, and updating systemd service management to use per-user sessions. Feedback highlights a critical security risk where /etc/hosts is made world-writable, and identifies several portability issues with ps and xargs commands that will cause failures on macOS (Darwin) systems.
| chmod 666 /etc/hosts | ||
| echo "127.0.0.1 $1" >>/etc/hosts |
There was a problem hiding this comment.
The chmod 666 /etc/hosts command is a significant security risk as it makes a critical system file world-writable. Additionally, this operation will fail when the script is run in non-root mode (user_mode=1), causing the installation to abort due to set -e. Since the script runs as root for system-wide installations, the chmod is unnecessary to append to the file. For non-root mode, this modification should be skipped entirely as the user lacks permissions to modify /etc/hosts.
| pids=$(ps -eo user=,pid=,comm= | awk -v svc="$_service" -v current_user="$current_user" '$1 == current_user && $3 == svc {print $2}') | ||
| else | ||
| config_dir_escaped=${config_dir%/} | ||
| local allow_default_root_config=0 | ||
| if [ "$config_dir_escaped" = "/etc/${PREFIX}" ]; then | ||
| allow_default_root_config=1 | ||
| fi | ||
| pids=$(ps -eo pid=,args= | awk -v svc="$_service" -v config_dir="$config_dir_escaped" -v allow_default_root_config="$allow_default_root_config" ' |
There was a problem hiding this comment.
The ps command uses the = suffix (e.g., pid=,args=) to suppress headers, which is a GNU-specific extension. This script explicitly handles Darwin (macOS) in several places (e.g., lines 22, 474), but this syntax is not supported on macOS and will cause the command to fail. Consider using standard ps output formats and filtering the header with awk 'NR>1' to maintain cross-platform compatibility.
| pids=$(ps -eo pid=,user=,comm= | awk -v current_user="$current_user" -v process_name="$process_name" '$2 == current_user && $3 == process_name {print $1}' || true) | ||
| else | ||
| pids=$(ps -eo pid=,user=,comm= | awk -v process_name="$process_name" '$3 == process_name {print $1}' || true) | ||
| fi | ||
|
|
||
| if [ -n "$pids" ]; then | ||
| echo "$pids" | xargs -r kill -9 2>/dev/null || : |
There was a problem hiding this comment.
The ps command uses non-portable header suppression (pid=,user=,comm=), and xargs -r is a GNU extension not available on macOS. Since this script explicitly supports Darwin (see lines 39, 64, 81), these commands will fail on that platform. To ensure compatibility, consider checking if the PID list is non-empty before calling xargs and using standard ps output options.
There was a problem hiding this comment.
Pull request overview
Backports main-branch non-root (user-mode) Linux packaging behavior into the 3.3.6 community install/remove scripts, adding safeguards around conflicting system-wide installs and adding smoke tests to prevent regressions.
Changes:
- Add user-mode awareness across install/remove scripts (per-user install dirs,
systemctl --user, safer process cleanup,.install_pathinstall discovery). - Add uninstall/install safety improvements (path validation, preserve cfg/log/data in user-mode unless explicitly removed, tighter process matching).
- Add bash smoke-test guards to validate key non-root backport behaviors for server and client packaging scripts.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packaging/tools/startPre.sh | Support user-mode systemd unit location and systemctl --user reloads. |
| packaging/tools/install.sh | Add user-mode setup, conflicting system install detection, .install_path, user-mode unit rewrites. |
| packaging/tools/install_client.sh | Add user-mode env setup, conflict checks, .install_path, and per-user link locations. |
| packaging/tools/remove.sh | Add .install_path discovery, user-mode cleanup logic, safer process-kill scoping, and path validation. |
| packaging/tools/remove_client.sh | Add .install_path discovery, user-mode dirs, safer process matching, and path validation. |
| packaging/smokeTest/test_non_root_server_packaging_backport.sh | Regression guard for server packaging non-root backport expectations. |
| packaging/smokeTest/test_non_root_client_packaging_backport.sh | Regression guard for client packaging non-root backport expectations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fi | ||
|
|
||
| if [ -n "$pids" ]; then | ||
| echo "$pids" | xargs -r kill -9 2>/dev/null || : |
| taosd=${serviceDir}/${serverName}.service | ||
| line=$(grep StartLimitBurst ${taosd}) | ||
| num=${line##*=} | ||
| #echo "burst num: ${num}" |
| elif ((${initd_mod} == 2)); then | ||
| ${csudo}insserv $1} || : | ||
| ${csudo}insserv -d $1 || : | ||
| insserv $1} || : |
| return | ||
| else | ||
| ${csudo}chmod 666 /etc/hosts | ||
| ${csudo}echo "127.0.0.1 $1" >>/etc/hosts | ||
| chmod 666 /etc/hosts | ||
| echo "127.0.0.1 $1" >>/etc/hosts | ||
| fi |
| if [ -d "/usr/local/Cellar/" ]; then | ||
| installDir="/usr/local/Cellar/tdengine/${verNumber}" | ||
| elif [ -d "/opt/homebrew/Cellar/" ]; then | ||
| installDir="/opt/homebrew/Cellar/tdengine/${verNumber}" | ||
| else |
| case "$path" in | ||
| ""|"/"|"/etc"|"/bin"|"/lib"|"/usr"|"/sbin"|"/boot"|"/root"|"/home"|"/var"|"/tmp"|"/dev"|"/proc"|"/sys"|"/run") |
| validate_safe_path() { | ||
| local path="$1" | ||
| case "$path" in | ||
| ""|"/"|"/etc"|"/bin"|"/lib"|"/usr"|"/sbin"|"/boot"|"/root"|"/home"|"/var"|"/tmp"|"/dev"|"/proc"|"/sys"|"/run") |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Backports main-branch non-root (user-mode) Linux packaging behavior into the 3.3.6 community install/remove scripts, adding safety checks and smoke-test guards to prevent regressions in non-root lifecycle and uninstall behavior.
Changes:
- Add user-mode support across install/remove scripts (per-user dirs,
systemctl --user,.install_pathinstallDir discovery). - Harden uninstall behavior (safer process cleanup, preserve cfg/log/data for user installs unless explicitly requested).
- Add packaging smoke-test regression guards for server/client non-root backport behavior and uninstall entrypoint correctness.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| packaging/tools/startPre.sh | Adds user-mode systemd unit path selection and systemctl --user support. |
| packaging/tools/remove_client.sh | Adds .install_path discovery, user-mode directories, safer process kill logic, and safer install-dir cleanup. |
| packaging/tools/remove.sh | Adds .install_path discovery, user-mode dirs + systemctl --user, safer process targeting, and install-dir cleanup logic. |
| packaging/tools/install_client.sh | Adds user-mode install layout, blocks non-root install if system-wide install exists, writes .install_path, updates shell rc. |
| packaging/tools/install.sh | Adds user-mode install layout, blocks non-root install if system-wide install exists, writes .install_path, rewrites systemd units for user-mode. |
| packaging/smokeTest/test_non_root_server_packaging_backport.sh | New regression guard verifying server scripts contain expected non-root backport behavior. |
| packaging/smokeTest/test_non_root_client_packaging_backport.sh | New regression guard verifying client scripts contain expected non-root backport behavior. |
| packaging/smokeTest/test_install_sh_client_uninstall_entrypoint.sh | New guard ensuring install.sh “client” uses remove.sh as uninstall backend. |
| packaging/smokeTest/test_install_sh_client_uninstall_entrypoint_guard_selftest.sh | New self-test validating the guard script catches intended good/bad fixtures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function kill_client() { | ||
| pid=$(ps -ef | grep "${clientName}" | grep -v "grep" | awk '{print $2}') | ||
| if [ -n "$pid" ]; then | ||
| ${csudo}kill -9 $pid || : | ||
| kill -9 "$pid" || : | ||
| fi |
| return | ||
| else | ||
| ${csudo}chmod 666 /etc/hosts | ||
| ${csudo}echo "127.0.0.1 $1" >>/etc/hosts | ||
| chmod 666 /etc/hosts | ||
| echo "127.0.0.1 $1" >>/etc/hosts | ||
| fi |
| elif ((${initd_mod} == 2)); then | ||
| ${csudo}insserv $1} || : | ||
| ${csudo}insserv -d $1 || : | ||
| insserv $1} || : |
| fi | ||
|
|
||
| if [ -n "$pids" ]; then | ||
| echo "$pids" | xargs -r kill -9 2>/dev/null || : |
| function install_main_path() { | ||
| #create install main dir and all sub dir | ||
| ${csudo}rm -rf ${install_main_dir}/cfg || : | ||
| ${csudo}rm -rf ${install_main_dir}/bin || : | ||
| ${csudo}rm -rf ${driver_path}/ || : | ||
| ${csudo}rm -rf ${install_main_dir}/examples || : | ||
| ${csudo}rm -rf ${install_main_dir}/include || : | ||
| ${csudo}rm -rf ${install_main_dir}/share || : | ||
| ${csudo}rm -rf ${install_main_dir}/log || : | ||
|
|
||
| ${csudo}mkdir -p ${install_main_dir} | ||
| ${csudo}mkdir -p ${install_main_dir}/cfg | ||
| ${csudo}mkdir -p ${install_main_dir}/bin | ||
| # ${csudo}mkdir -p ${install_main_dir}/connector | ||
| ${csudo}mkdir -p ${driver_path}/ | ||
| ${csudo}mkdir -p ${install_main_dir}/examples | ||
| ${csudo}mkdir -p ${install_main_dir}/include | ||
| ${csudo}mkdir -p ${configDir} | ||
| # ${csudo}mkdir -p ${install_main_dir}/init.d | ||
| if [[ $user_mode -eq 0 ]]; then | ||
| rm -rf ${install_main_dir}/cfg || : | ||
| rm -rf ${install_main_dir}/bin || : | ||
| rm -rf ${driver_path}/ || : | ||
| rm -rf ${install_main_dir}/examples || : |
| case "$path" in | ||
| ""|"/"|"/etc"|"/bin"|"/lib"|"/usr"|"/sbin"|"/boot"|"/root"|"/home"|"/var"|"/tmp"|"/dev"|"/proc"|"/sys"|"/run") | ||
| echo -e "${RED}Refusing to operate on dangerous system path: $path${NC}" | ||
| exit 1 | ||
| ;; | ||
| esac |
|
|
||
| if [ -d "${config_dir}" ]; then | ||
| ${csudo}rm -rf ${config_dir} | ||
| rm -rf ${config_dir} |
| rm -f ${install_main_dir}/uninstall_${PREFIX}x.sh ${install_main_dir}/uninstall.sh ${install_main_dir}/.install_path || : | ||
|
|
||
| if [ "$user_mode" -eq 1 ] && [ X$remove_flag != X"true" ]; then | ||
| find "${install_main_dir}" -mindepth 1 -maxdepth 1 ! \( -name cfg -o -name log -o -name data \) -exec rm -rf {} + || : | ||
| return 0 | ||
| fi | ||
|
|
||
| rm -rf ${install_main_dir} || : |
| taosd=${serviceDir}/${serverName}.service | ||
| line=$(grep StartLimitBurst ${taosd}) | ||
| num=${line##*=} |
| case "$path" in | ||
| ""|"/"|"/etc"|"/bin"|"/lib"|"/usr"|"/sbin"|"/boot"|"/root"|"/home"|"/var"|"/tmp"|"/dev"|"/proc"|"/sys"|"/run") | ||
| echo -e "${RED}Refusing to operate on dangerous system path: $path${NC}" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| } | ||
| validate_safe_path "$installDir" |
- install.sh: remove chmod 666 /etc/hosts, use -w check instead - install.sh: fix insserv $1} stray brace typo - remove.sh: fix undefined verNumber in macOS Cellar paths - remove.sh: quote all rm -rf variable expansions for safety - install_client.sh: fix kill_client() multi-PID handling with while-read loop - remove_client.sh: replace GNU xargs -r with portable while-read loop - test: update guard pattern to match quoted rm -f form Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- install.sh: kill_process() use pgrep -x + while-read + || true - install_client.sh: kill_client() use pgrep -x + || true - remove_client.sh: add || : to while-read pipeline Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Backports main-branch non-root (user-mode) Linux packaging behaviors into the 3.3.6 community install/remove scripts, adds guardrails around root vs non-root lifecycle safety, and introduces smoke tests to prevent regressions.
Changes:
- Add user-mode install/uninstall support via
.install_pathdiscovery, per-user systemd units (systemctl --user), and per-user install locations. - Harden uninstall behavior to better scope process termination and avoid deleting config/log/data unintentionally.
- Add packaging smoke-test scripts to validate the backport invariants (non-root behavior, uninstall entrypoint correctness).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packaging/tools/startPre.sh | Adjust service/log locations and systemctl invocation for user-mode systemd units. |
| packaging/tools/remove_client.sh | Add .install_path discovery, user-mode paths, safer process killing, and selective uninstall cleanup. |
| packaging/tools/remove.sh | Add .install_path discovery, user-mode support, scoped process killing, and safer uninstall cleanup logic. |
| packaging/tools/install_client.sh | Add user-mode install layout, conflict detection vs system installs, .install_path writing, and PATH/LD_LIBRARY_PATH setup. |
| packaging/tools/install.sh | Add user-mode setup (setup_env), conflict detection, user-mode systemd unit rewriting, and .install_path writing. |
| packaging/smokeTest/test_non_root_server_packaging_backport.sh | Regression guard for server-side non-root backport behavior. |
| packaging/smokeTest/test_non_root_client_packaging_backport.sh | Regression guard for client-side non-root backport behavior. |
| packaging/smokeTest/test_install_sh_client_uninstall_entrypoint.sh | Guard ensuring install.sh client mode uses remove.sh (not remove_client.sh) as uninstall backend. |
| packaging/smokeTest/test_install_sh_client_uninstall_entrypoint_guard_selftest.sh | Self-test harness validating the uninstall entrypoint guard script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| validate_safe_path() { | ||
| local path="$1" | ||
| case "$path" in | ||
| ""|"/"|"/etc"|"/bin"|"/lib"|"/usr"|"/sbin"|"/boot"|"/root"|"/home"|"/var"|"/tmp"|"/dev"|"/proc"|"/sys"|"/run") | ||
| echo -e "${RED}Refusing to operate on dangerous system path: $path${NC}" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| } | ||
| validate_safe_path "$installDir" |
| ${csudo}systemctl disable ${_service} &>/dev/null || echo &>/dev/null | ||
| ${csudo}rm -f ${_service_config} | ||
| ${sysctl_cmd} disable ${_service} &>/dev/null || echo &>/dev/null | ||
| rm -f ${_service_config} |
| validate_safe_path() { | ||
| local path="$1" | ||
| case "$path" in | ||
| ""|"/"|"/etc"|"/bin"|"/lib"|"/usr"|"/sbin"|"/boot"|"/root"|"/home"|"/var"|"/tmp"|"/dev"|"/proc"|"/sys"|"/run") | ||
| echo -e "${RED}Refusing to operate on dangerous system path: $path${NC}" | ||
| exit 1 | ||
| ;; | ||
| esac |
| echo | ||
| echo -e "\033[44;32;1m${productName2} client is updated successfully!${NC}" | ||
|
|
||
| rm -rf $(tar -tf ${tarName} | grep -Ev "^\./$|^\/") | ||
| rm -rf $(tar -tf "${tarName}" | grep -Ev "^\./$|^\/") | ||
| } |
| echo | ||
| echo -e "\033[44;32;1m${productName2} client is installed successfully!${NC}" | ||
|
|
||
| rm -rf $(tar -tf ${tarName} | grep -Ev "^\./$|^\/") | ||
| rm -rf $(tar -tf "${tarName}" | grep -Ev "^\./$|^\/") | ||
| } |
| taosd=${serviceDir}/${serverName}.service | ||
| line=$(grep StartLimitBurst ${taosd}) | ||
| num=${line##*=} |
- Check systemd >= 232 before non-root install, clear error for CentOS 7 - Show linger hint after install if not enabled (needed for auto-start on reboot) - Improve user session error message Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
User-level systemd does not have multi-user.target. Services must use WantedBy=default.target for auto-start on boot (with linger enabled). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Backports main-branch non-root (user-mode) packaging behavior into the 3.3.6 community install/remove scripts, adds safeguards around conflicting system-wide installs, and introduces smoke-test guards to prevent regressions in this backport.
Changes:
- Add user-mode (non-root) install/uninstall support across server/client scripts, including
.install_path-based installDir discovery and systemd--userhandling. - Tighten uninstall safety (scoped process killing, config/log preservation behavior) and block non-root installs when a system-wide install is detected.
- Add packaging smoke tests to assert key backport invariants and guard against future drift.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packaging/tools/startPre.sh | Adjusts log/service dirs and systemctl invocation for user-mode systemd units. |
| packaging/tools/install.sh | Adds setup_env() user-mode detection, systemd user-unit rewrites, .install_path, and conflict detection. |
| packaging/tools/remove.sh | Adds .install_path discovery, user-mode support, safer process scoping, and revised removal logic. |
| packaging/tools/install_client.sh | Adds non-root client install behavior (user-local dirs, rc-file PATH/LD_LIBRARY_PATH updates, .install_path, conflict detection). |
| packaging/tools/remove_client.sh | Adds .install_path discovery, user-mode directory handling, safer process scoping, and improved cleanup behavior. |
| packaging/smokeTest/test_non_root_server_packaging_backport.sh | New regression guard verifying server-side user-mode backport invariants. |
| packaging/smokeTest/test_non_root_client_packaging_backport.sh | New regression guard verifying client-side user-mode backport invariants. |
| packaging/smokeTest/test_install_sh_client_uninstall_entrypoint.sh | New guard ensuring install.sh client flow uses remove.sh as uninstall backend. |
| packaging/smokeTest/test_install_sh_client_uninstall_entrypoint_guard_selftest.sh | Self-test harness validating the uninstall-entrypoint guard script behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rm -rf "${bin_link_dir}/${_service}" | ||
| rm -rf "${installDir}/bin/${_service}" | ||
| rm -rf "${local_bin_link_dir}/${_service}" |
| validate_safe_path() { | ||
| local path="$1" | ||
| case "$path" in | ||
| ""|"/"|"/etc"|"/bin"|"/lib"|"/usr"|"/sbin"|"/boot"|"/root"|"/home"|"/var"|"/tmp"|"/dev"|"/proc"|"/sys"|"/run") | ||
| echo -e "${RED}Refusing to operate on dangerous system path: $path${NC}" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| } | ||
| validate_safe_path "$installDir" |
| if [ -d "/usr/local/Cellar/" ]; then | ||
| installDir="/usr/local/Cellar/tdengine/" | ||
| elif [ -d "/opt/homebrew/Cellar/" ]; then | ||
| installDir="/opt/homebrew/Cellar/tdengine/" | ||
| else | ||
| installDir="/usr/local/${PREFIX}" | ||
| fi |
| validate_safe_path() { | ||
| local path="$1" | ||
| case "$path" in | ||
| ""|"/"|"/etc"|"/bin"|"/lib"|"/usr"|"/sbin"|"/boot"|"/root"|"/home"|"/var"|"/tmp"|"/dev"|"/proc"|"/sys"|"/run") | ||
| echo -e "${RED}Refusing to operate on dangerous system path: $path${NC}" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| } |
| if [[ "$(id -u)" -ne 0 ]]; then | ||
| # Check systemd version >= 232 for user service support | ||
| local sd_ver | ||
| sd_ver=$(systemctl --version 2>/dev/null | head -1 | awk '{print $2}') | ||
| if [ -z "$sd_ver" ] || [ "$sd_ver" -lt 232 ] 2>/dev/null; then | ||
| echo -e "${RED}Non-root install requires systemd >= 232, current version: ${sd_ver:-unknown}${NC}" | ||
| echo -e "Supported: CentOS/RHEL 8+, Ubuntu 18.04+, Debian 9+, SUSE 15+" | ||
| echo -e "CentOS/RHEL 7 (systemd 219) does not support non-root installation." | ||
| exit 1 | ||
| fi | ||
| if ! systemctl --user show-environment &>/dev/null; then | ||
| echo -e "${RED}Current user is not root and no systemd user session (user bus) is available.${NC}" |
| taosd=${serviceDir}/${serverName}.service | ||
| line=$(grep StartLimitBurst ${taosd}) | ||
| num=${line##*=} |
Summary
Test Plan
https://project.feishu.cn/taosdata_td/job/detail/6961071591