From fbd5f68ff046c468319e69036f5982c3aca93b60 Mon Sep 17 00:00:00 2001 From: Maoyin Sun Date: Fri, 24 Jul 2026 04:13:26 +0200 Subject: [PATCH 01/77] fix(security): eliminate command injection in -O option parsing --- helpers/parse_arguments.sh | 21 ++++++--- helpers/parse_arguments.test.sh | 80 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 helpers/parse_arguments.test.sh diff --git a/helpers/parse_arguments.sh b/helpers/parse_arguments.sh index edcd6df..839e561 100644 --- a/helpers/parse_arguments.sh +++ b/helpers/parse_arguments.sh @@ -126,15 +126,22 @@ parse_arguments() { IFS=',' read -r -a other_options_array </dev/null 2>&1; + printf '%s' "${X}"; +)"; +if [ -e "${MARKER}" ]; then + _assert_fail "-O value is not executed as a command"; + rm -f "${MARKER}"; +else + _assert_pass "-O value is not executed as a command"; +fi +assert_equals "-O value kept verbatim (not evaluated)" '$(touch '"'${MARKER}'"')' "${value_x}"; + +# Handling -O must not print anything to stdout. +stdout="$(parse_arguments -O 'A=b' "prompt" 2>/dev/null)"; +assert_equals "-O produces no stdout" "" "${stdout}"; + +# A normal KEY=VALUE assignment works. +value_a="$(parse_arguments -O 'A=b' "prompt" >/dev/null 2>&1; printf '%s' "${A}")"; +assert_equals "-O assigns a simple value" "b" "${value_a}"; + +# A single -O may carry several comma-separated pairs. +values_cd="$(parse_arguments -O 'C=d,E=f' "prompt" >/dev/null 2>&1; printf '%s|%s' "${C}" "${E}")"; +assert_equals "-O splits comma-separated pairs" "d|f" "${values_cd}"; + +# The value may itself contain '=' (split on the first '=' only). +value_eq="$(parse_arguments -O 'C=d=e' "prompt" >/dev/null 2>&1; printf '%s' "${C}")"; +assert_equals "-O value may contain '='" "d=e" "${value_eq}"; + +# An invalid variable name must be rejected, not assigned. '1bad' is not a +# legal shell identifier, so it can never appear in the environment; the parser +# must skip it and log an error instead of failing or assigning anything. +bad_env="$( + ELL_LOG_LEVEL=0 parse_arguments -O '1bad=x' "prompt" >/dev/null 2>&1; + # If any assignment had leaked, a variable whose name starts with a digit + # cannot exist, so scan the environment for the offending value instead. + env | grep -c '=x$' || true; +)"; +assert_equals "-O rejects invalid variable name" "0" "${bad_env}"; + +# The trailing prompt is still captured correctly alongside -O. +prompt="$(parse_arguments -O 'A=b' "hello world" >/dev/null 2>&1; printf '%s' "${USER_PROMPT}")"; +assert_equals "prompt captured with -O present" "hello world" "${prompt}"; + +assert_summary; From d3fafdc472c01ead23858f4c953b828f2d5f4f70 Mon Sep 17 00:00:00 2001 From: Maoyin Sun Date: Fri, 24 Jul 2026 04:52:24 +0200 Subject: [PATCH 02/77] fix(security): refuse to source untrusted config files --- helpers/load_config.sh | 71 ++++++++++++++++++++++++++++--------- helpers/load_config.test.sh | 65 +++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 16 deletions(-) create mode 100644 helpers/load_config.test.sh diff --git a/helpers/load_config.sh b/helpers/load_config.sh index 007b895..b3db589 100644 --- a/helpers/load_config.sh +++ b/helpers/load_config.sh @@ -9,6 +9,57 @@ # 3. $PWD/.ellrc (per-project config) # 4. $ELL_CONFIG (explicit override) +# _config_is_trusted +# Config files are sourced, i.e. executed as shell code. Only source a file +# that cannot have been planted or tampered with by another user: it must be +# owned by the current user (or root) and must not be writable by its group or +# by others. This closes the "run ell in a directory containing a hostile +# .ellrc" arbitrary-code-execution hole (the $PWD/.ellrc case in particular). +# +# If ownership/permissions cannot be determined (no usable stat), err on the +# side of caution and refuse to source the file. +_config_is_trusted() { + local file="${1}" owner perms; + + # Owner UID and octal permission bits, trying GNU stat then BSD/macOS stat. + owner="$(stat -c '%u' "${file}" 2>/dev/null || stat -f '%u' "${file}" 2>/dev/null)"; + perms="$(stat -c '%a' "${file}" 2>/dev/null || stat -f '%Lp' "${file}" 2>/dev/null)"; + + if [ -z "${owner}" ] || [ -z "${perms}" ]; then + logging_warn "Cannot verify ownership/permissions of ${file}; refusing to source it"; + return 1; + fi + + # Must be owned by us or by root (root-owned system config is trusted). + if [ "${owner}" != "$(id -u)" ] && [ "${owner}" != "0" ]; then + logging_warn "Ignoring ${file}: not owned by the current user or root"; + return 1; + fi + + # Reject group- or world-writable files. `perms` is an octal string like + # "644" or "0644"; the last two characters are the group and other digits. + # Each is a single octal digit, so its write bit is the 2's place. + local group_bit="${perms: -2:1}" other_bit="${perms: -1:1}"; + if [ "$(( 8#${group_bit:-0} & 2 ))" -ne 0 ] || [ "$(( 8#${other_bit:-0} & 2 ))" -ne 0 ]; then + logging_warn "Ignoring ${file}: writable by group or others (insecure permissions)"; + return 1; + fi + + return 0; +} + +# _load_config_file +# Source a config file only if it exists and passes the trust check. +_load_config_file() { + local file="${1}" desc="${2}"; + [ -f "${file}" ] || return 0; + if ! _config_is_trusted "${file}"; then + return 0; + fi + logging_debug "Loading config from ${file}${desc:+ }${desc}"; + . "${file}"; +} + load_config() { local current_env ELL_XDG_CONFIG; logging_debug "Storing current environment"; @@ -16,27 +67,15 @@ load_config() { set -o allexport; ELL_XDG_CONFIG="${XDG_CONFIG_HOME:-${HOME}/.config}/ell/config"; - if [ -f "${ELL_XDG_CONFIG}" ]; then - logging_debug "Loading config from ${ELL_XDG_CONFIG} (XDG)"; - . "${ELL_XDG_CONFIG}" - fi - - if [ -f "${HOME}/.ellrc" ]; then - logging_debug "Loading config from ${HOME}/.ellrc (from \$HOME, legacy)"; - . "${HOME}/.ellrc" - fi - - if [ -f "${PWD}/.ellrc" ]; then - logging_debug "Loading config from ${PWD}/.ellrc (from \$PWD)"; - . "${PWD}/.ellrc" - fi + _load_config_file "${ELL_XDG_CONFIG}" "(XDG)"; + _load_config_file "${HOME}/.ellrc" "(from \$HOME, legacy)"; + _load_config_file "${PWD}/.ellrc" "(from \$PWD)"; if [ -z "${ELL_CONFIG}" ]; then logging_debug "ELL_CONFIG is not set"; else if [ -f "${ELL_CONFIG}" ]; then - logging_debug "Loading config from ${ELL_CONFIG}"; - . "${ELL_CONFIG}" + _load_config_file "${ELL_CONFIG}" ""; else logging_fatal "Config file ${ELL_CONFIG} not found"; exit 1; diff --git a/helpers/load_config.test.sh b/helpers/load_config.test.sh new file mode 100644 index 0000000..44f3fcb --- /dev/null +++ b/helpers/load_config.test.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash + +# Self-checking tests for helpers/load_config.sh. +# +# Config files are *sourced*, i.e. executed as shell code, so a config file +# that another user could plant or tamper with is an arbitrary-code-execution +# vector (notably $PWD/.ellrc when ell is run inside an untrusted directory). +# +# load_config now refuses to source any config file that is not owned by the +# current user (or root) and that is writable by group or others. These tests +# lock in that trust check: insecure files must be skipped, secure files must +# still load, and a missing file must be a no-op. + +set -o posix; + +DIR="$(dirname "${0}")"; +. "${DIR}/../tests/assert.sh"; + +ELL_LOG_LEVEL=0; +export ELL_LOG_LEVEL; +. "${DIR}/logging.sh"; +. "${DIR}/load_config.sh"; + +echo "load_config tests"; +echo "================="; + +WORK="$(mktemp -d)"; +trap 'rm -rf "${WORK}"' EXIT; + +# A file owned by us with private (0600) permissions is trusted. +printf 'x=1\n' > "${WORK}/private"; +chmod 600 "${WORK}/private"; +assert_success "0600 file is trusted" _config_is_trusted "${WORK}/private"; + +# 0644 (readable by all but writable only by owner) is still trusted. +printf 'x=1\n' > "${WORK}/readable"; +chmod 644 "${WORK}/readable"; +assert_success "0644 file is trusted" _config_is_trusted "${WORK}/readable"; + +# Group-writable files are rejected. +printf 'x=1\n' > "${WORK}/group_w"; +chmod 660 "${WORK}/group_w"; +assert_failure "group-writable file is rejected" _config_is_trusted "${WORK}/group_w"; + +# World-writable files are rejected. +printf 'x=1\n' > "${WORK}/world_w"; +chmod 606 "${WORK}/world_w"; +assert_failure "world-writable file is rejected" _config_is_trusted "${WORK}/world_w"; + +# A trusted file is actually sourced by _load_config_file. +printf 'ELL_TEST_TRUSTED=loaded\n' > "${WORK}/trusted_cfg"; +chmod 600 "${WORK}/trusted_cfg"; +loaded="$(_load_config_file "${WORK}/trusted_cfg" "" >/dev/null 2>&1; printf '%s' "${ELL_TEST_TRUSTED}")"; +assert_equals "trusted config is sourced" "loaded" "${loaded}"; + +# A world-writable file is NOT sourced: its assignment must not take effect. +printf 'ELL_TEST_EVIL=pwned\n' > "${WORK}/evil_cfg"; +chmod 666 "${WORK}/evil_cfg"; +skipped="$(_load_config_file "${WORK}/evil_cfg" "" >/dev/null 2>&1; printf '%s' "${ELL_TEST_EVIL-unset}")"; +assert_equals "untrusted config is not sourced" "unset" "${skipped}"; + +# A missing file is a silent no-op (returns success, sources nothing). +assert_success "missing file is a no-op" _load_config_file "${WORK}/does_not_exist" ""; + +assert_summary; From 4277986e6ca2f024fe84386fc934aaf506264c9f Mon Sep 17 00:00:00 2001 From: Maoyin Sun Date: Fri, 24 Jul 2026 04:55:13 +0200 Subject: [PATCH 03/77] test: cover load_config precedence, export and env-override contract --- helpers/load_config.test.sh | 74 +++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/helpers/load_config.test.sh b/helpers/load_config.test.sh index 44f3fcb..2b77db7 100644 --- a/helpers/load_config.test.sh +++ b/helpers/load_config.test.sh @@ -62,4 +62,78 @@ assert_equals "untrusted config is not sourced" "unset" "${skipped}"; # A missing file is a silent no-op (returns success, sources nothing). assert_success "missing file is a no-op" _load_config_file "${WORK}/does_not_exist" ""; +# --- load_config() end-to-end behaviour ------------------------------------- +# +# load_config resolves files via HOME / XDG_CONFIG_HOME / PWD / ELL_CONFIG. +# Point HOME and XDG_CONFIG_HOME at an isolated tree so the developer's real +# config is never read, and run load_config in a subshell so its exports and +# option changes (allexport) do not leak between cases. + +HOME_DIR="${WORK}/home"; +XDG_DIR="${WORK}/xdg"; +mkdir -p "${HOME_DIR}" "${XDG_DIR}/ell"; + +# XDG config sets a value; the legacy ~/.ellrc overrides it. Later files win. +printf 'ELL_LLM_MODEL=from-xdg\nELL_FROM_XDG=1\n' > "${XDG_DIR}/ell/config"; +chmod 600 "${XDG_DIR}/ell/config"; +printf 'ELL_LLM_MODEL=from-home\n' > "${HOME_DIR}/.ellrc"; +chmod 600 "${HOME_DIR}/.ellrc"; + +result="$( + cd "${WORK}" || exit 1; # PWD has no .ellrc here + HOME="${HOME_DIR}" XDG_CONFIG_HOME="${XDG_DIR}"; + export HOME XDG_CONFIG_HOME; + unset ELL_LLM_MODEL ELL_FROM_XDG; + load_config >/dev/null 2>&1; + printf '%s|%s' "${ELL_LLM_MODEL}" "${ELL_FROM_XDG}"; +)"; +assert_equals "later config overrides earlier; values are set" "from-home|1" "${result}"; + +# Config assignments are exported (allexport), visible to child processes. +exported="$( + cd "${WORK}" || exit 1; + HOME="${HOME_DIR}" XDG_CONFIG_HOME="${XDG_DIR}"; + export HOME XDG_CONFIG_HOME; + unset ELL_FROM_XDG; + load_config >/dev/null 2>&1; + bash -c 'printf "%s" "${ELL_FROM_XDG}"'; +)"; +assert_equals "config values are exported to children" "1" "${exported}"; + +# A variable already set in the environment must NOT be overridden by config, +# even though the config file assigns it. This is load_config's core contract. +preserved="$( + cd "${WORK}" || exit 1; + HOME="${HOME_DIR}" XDG_CONFIG_HOME="${XDG_DIR}"; + ELL_LLM_MODEL=from-env; + export HOME XDG_CONFIG_HOME ELL_LLM_MODEL; + load_config >/dev/null 2>&1; + printf '%s' "${ELL_LLM_MODEL}"; +)"; +assert_equals "environment is not overridden by config" "from-env" "${preserved}"; + +# An explicit ELL_CONFIG that does not exist is fatal (non-zero exit). +# load_config calls `exit 1`, which terminates the command-substitution +# subshell, so run it in its own subshell and inspect that subshell's status. +( + cd "${WORK}" || exit 1; + HOME="${HOME_DIR}" XDG_CONFIG_HOME="${XDG_DIR}" ELL_CONFIG="${WORK}/nope.conf"; + export HOME XDG_CONFIG_HOME ELL_CONFIG; + load_config >/dev/null 2>&1; +); +assert_not_equals "missing ELL_CONFIG is fatal" "0" "${?}"; + +# An explicit, trusted ELL_CONFIG is loaded and wins over the other files. +printf 'ELL_LLM_MODEL=from-explicit\n' > "${WORK}/explicit.conf"; +chmod 600 "${WORK}/explicit.conf"; +explicit="$( + cd "${WORK}" || exit 1; + HOME="${HOME_DIR}" XDG_CONFIG_HOME="${XDG_DIR}" ELL_CONFIG="${WORK}/explicit.conf"; + export HOME XDG_CONFIG_HOME ELL_CONFIG; + unset ELL_LLM_MODEL; + load_config >/dev/null 2>&1; + printf '%s' "${ELL_LLM_MODEL}"; +)"; +assert_equals "explicit ELL_CONFIG is loaded last" "from-explicit" "${explicit}"; + assert_summary; From 16e0d7906fff530e7277380c8a14b44b9a8ed6ae Mon Sep 17 00:00:00 2001 From: Maoyin Sun Date: Fri, 24 Jul 2026 05:04:35 +0200 Subject: [PATCH 04/77] fix(security): validate ELL_API_STYLE before sourcing a backend --- llm_backends/generate_completion.sh | 32 ++++++++++- llm_backends/generate_completion.test.sh | 70 ++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 llm_backends/generate_completion.test.sh diff --git a/llm_backends/generate_completion.sh b/llm_backends/generate_completion.sh index 6f42c5f..e102148 100644 --- a/llm_backends/generate_completion.sh +++ b/llm_backends/generate_completion.sh @@ -1,4 +1,32 @@ #!/usr/bin/env bash -# Sourcing the generate_completion.sh script according to the selected API style -. "$(dirname ${0})/llm_backends/${ELL_API_STYLE}/generate_completion.sh"; \ No newline at end of file +# Dispatcher: source the backend implementation selected by ELL_API_STYLE. +# +# ELL_API_STYLE comes from the CLI/config/environment, and it used to be +# interpolated straight into a source path: +# . "$(dirname ${0})/llm_backends/${ELL_API_STYLE}/generate_completion.sh" +# so a value like "../../tmp/evil" would source an arbitrary file as shell +# code. Validate the style name against a strict allowlist pattern (a single +# path segment of [A-Za-z0-9._-], no slashes and no "." / ".." traversal) +# before using it, and resolve the backend relative to this file's own +# directory rather than the outer script's ${0}. + +# Directory that contains this dispatcher and the per-style backend folders. +# Prefer the caller-provided BASE_DIR (ell.sh), fall back to BASH_SOURCE so the +# file is also correct when sourced directly (e.g. from tests). +_ELL_BACKENDS_DIR="${BASE_DIR:-$(dirname "${BASH_SOURCE[0]}")}/llm_backends"; + +if ! [[ "${ELL_API_STYLE}" =~ ^[A-Za-z0-9._-]+$ ]] \ + || [ "${ELL_API_STYLE}" = "." ] || [ "${ELL_API_STYLE}" = ".." ]; then + logging_fatal "Invalid ELL_API_STYLE: '${ELL_API_STYLE}' (expected a backend name like 'openai', 'gemini' or 'ell_echo')"; + exit 1; +fi + +_ELL_BACKEND_FILE="${_ELL_BACKENDS_DIR}/${ELL_API_STYLE}/generate_completion.sh"; +if [ ! -f "${_ELL_BACKEND_FILE}" ]; then + logging_fatal "Unknown API style '${ELL_API_STYLE}': no backend at ${_ELL_BACKEND_FILE}"; + exit 1; +fi + +# shellcheck source=/dev/null +. "${_ELL_BACKEND_FILE}"; diff --git a/llm_backends/generate_completion.test.sh b/llm_backends/generate_completion.test.sh new file mode 100644 index 0000000..dad2891 --- /dev/null +++ b/llm_backends/generate_completion.test.sh @@ -0,0 +1,70 @@ +#!/usr/bin/env bash + +# Self-checking tests for llm_backends/generate_completion.sh (the dispatcher). +# +# The dispatcher sources a backend selected by ELL_API_STYLE. That value comes +# from the CLI/config/environment and used to be interpolated straight into a +# source path, so "../../tmp/evil" could source an arbitrary file as shell +# code. These tests lock in the validation: only a simple backend name that +# resolves to an existing backend file is accepted; slashes, "."/".." traversal +# and unknown names are rejected, and a traversal attempt never sources the +# targeted file. + +set -o posix; + +DIR="$(dirname "${0}")"; +. "${DIR}/../tests/assert.sh"; + +ELL_LOG_LEVEL=0; +export ELL_LOG_LEVEL; +. "${DIR}/../helpers/logging.sh"; + +# The dispatcher resolves backends relative to BASE_DIR. +BASE_DIR="$(cd "${DIR}/.." && pwd)"; +export BASE_DIR; + +DISPATCH="${DIR}/generate_completion.sh"; + +# dispatch