diff --git a/README.md b/README.md index 73d03ad..e8c0c6d 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@ - [1.0 Introduction](#10-introduction) - [2.0 How it works](#20-how-it-works) + - [2.1 ECS Exec is not used](#21-ecs-exec-is-not-used) - [3.0 Setup](#30-setup) - [3.1 Environment](#31-environment) - [3.2 Registration](#32-registration) @@ -34,6 +35,20 @@ Each requested _task run_ is placed on the **Node**. Which means, anything described inside that task will be running on **Node** (self-hosted/external instance). Only, _task definition_ will live on _AWS ECS_. +### 2.1 ECS Exec is not used + +Workflow tasks are never launched with `enableExecuteCommand`, so ECS Exec is +unused on a runner. AWS's `ecs-anywhere-install.sh` nevertheless stages the SSM +session binaries into `/var/lib/ecs/deps/execute-command` on every install — the +call is unconditional and the script offers no flag to skip it — which leaves +unused binaries on disk for vulnerability scanners to flag. + +Registration therefore skips that step and removes the directory if anything was +staged anyway. This is not configurable: there is no supported setup in which the +runner needs those binaries. The ECS agent treats them as optional and simply +stops advertising the `ecs.capability.execute-command` attribute when they are +absent. + ## 3.0 Setup > **IMPORTANT:** diff --git a/main.sh b/main.sh index 0b73d05..5b44aff 100755 --- a/main.sh +++ b/main.sh @@ -29,8 +29,9 @@ readonly SG_DOCKER_NETWORK="sg-net" ECS_CONFIG_DIR="${ECS_CONFIG_DIR:=/etc/ecs}" ECS_LOG_DIR="${ECS_LOG_DIR:=/var/log/ecs}" ECS_DATA_DIR="${ECS_DATA_DIR:=/var/lib/ecs/data}" +ECS_EXEC_DEPS_DIR="${ECS_EXEC_DEPS_DIR:=/var/lib/ecs/deps/execute-command}" REGISTRATION_DIR="${REGISTRATION_DIR:=/var/log/registration}" -readonly ECS_CONFIG_DIR ECS_LOG_DIR ECS_DATA_DIR REGISTRATION_DIR +readonly ECS_CONFIG_DIR ECS_LOG_DIR ECS_DATA_DIR ECS_EXEC_DEPS_DIR REGISTRATION_DIR # diagnostics SG_DIAGNOSTIC_DIR="${SG_DIAGNOSTIC_DIR:=/var/lib/sg-runner}" @@ -734,6 +735,80 @@ EOF } #}}}: configure_http_proxy +# StackGuardian never launches ECS tasks with enableExecuteCommand, so ECS Exec +# is unused on runners. AWS's ecs-anywhere-install.sh stages the SSM session +# binaries under ECS_EXEC_DEPS_DIR regardless: its exec-setup call is +# unconditional and the script exposes no flag to skip it. That leaves unused +# binaries on disk which trip vulnerability scanners, e.g. CVE-2026-71556 in the +# go-git version vendored by amazon-ssm-agent 3.3.4624.0. +# +# The ECS agent treats these dependencies as optional: when the directory is +# absent it starts normally and simply stops advertising the +# ecs.capability.execute-command attribute (see appendExecCapabilities in +# amazon-ecs-agent), which we never rely on. + +disable_ecs_exec_setup() { #{{{ + # Best effort: neuter the installer's `exec-setup` call so the SSM binaries + # are never downloaded. remove_ecs_exec_deps is the backstop if this misses. + local script="$1" + + if ! grep -qx 'exec-setup' "$script"; then + debug "No exec-setup call found in" "$(basename "$script")" "- skipping patch." + return 0 + fi + + # Write-and-move rather than `sed -i`: the in-place flag is not portable. + if sed 's/^exec-setup$/: # exec-setup disabled: ECS Exec is unused/' \ + "$script" >"${script}.patched" && mv "${script}.patched" "$script"; then + debug "Disabled ECS Exec dependency staging in" "$(basename "$script")" + else + rm -f "${script}.patched" + debug "Could not patch" "$(basename "$script")" "- relying on cleanup." + fi + + # debug() is a no-op returning non-zero unless --debug is set; never let that + # become this function's exit status. + return 0 +} +#}}}: disable_ecs_exec_setup + +is_ecs_exec_deps_path() { #{{{ + # Guard for the root-run `rm -rf` in remove_ecs_exec_deps. ECS_EXEC_DEPS_DIR + # is overridable for testing, which makes it the one place in this script + # where an env var supplies a whole deletion path rather than a fixed literal. + # Accept only an absolute path that actually names a deps directory, so a + # stray "/" or "/etc" in the environment can never reach rm. + # + # Kept as a pure predicate so the dangerous inputs are unit-testable without + # any test ever pointing rm at them. A trailing slash is rejected too: the + # refusal is logged and harmless, unlike the alternative. + case "${1:-}" in + /*/execute-command) return 0 ;; + *) return 1 ;; + esac +} +#}}}: is_ecs_exec_deps_path + +remove_ecs_exec_deps() { #{{{ + # Authoritative cleanup: drop whatever exec-setup managed to stage. + if ! is_ecs_exec_deps_path "$ECS_EXEC_DEPS_DIR"; then + debug "Refusing to remove unexpected ECS_EXEC_DEPS_DIR:" "$ECS_EXEC_DEPS_DIR" + return 0 + fi + + [[ -d "$ECS_EXEC_DEPS_DIR" ]] || return 0 + + if rm -rf "$ECS_EXEC_DEPS_DIR"; then + debug "Removed unused ECS Exec dependencies:" "$ECS_EXEC_DEPS_DIR" + else + debug "Failed to remove ECS Exec dependencies:" "$ECS_EXEC_DEPS_DIR" + fi + + # Hardening only: a failure here must not fail an otherwise good registration. + return 0 +} +#}}}: remove_ecs_exec_deps + #}}}: Local configuration #{{{ Registration / deregistration @@ -830,6 +905,9 @@ register_instance() { #{{{ rm -f "$ecs_install_script" die "Downloaded script appears invalid" "missing bash shebang" fi + + disable_ecs_exec_setup "$ecs_install_script" + spinner_msg "Downloading support files" 0 check_systemctl_ecs_status @@ -869,6 +947,8 @@ register_instance() { #{{{ done & spinner "$!" "Verifying registration of this runner" + remove_ecs_exec_deps + # setup_cron save_registration_details } diff --git a/test/README.md b/test/README.md index ea414c1..f27dca9 100644 --- a/test/README.md +++ b/test/README.md @@ -28,8 +28,10 @@ test/ - **Path overrides via env.** Every path it writes to is overridable: `LOG_FILE`, `SG_DIAGNOSTIC_DIR` (`SG_DIAGNOSTIC_FILE` / `SG_DIAGNOSTIC_TMP_FILE` derive from it), `ECS_CONFIG_DIR`, `ECS_LOG_DIR`, - `ECS_DATA_DIR`, `REGISTRATION_DIR`. `load.bash` points all of these at a - per-test temp dir, so a test never touches real system paths. + `ECS_DATA_DIR`, `ECS_EXEC_DEPS_DIR`, `REGISTRATION_DIR`. `load.bash` points + all of these at a per-test temp dir, so a test never touches real system + paths. This matters most for `ECS_EXEC_DEPS_DIR`: `remove_ecs_exec_deps` + runs `rm -rf` on it. - **Root bypass.** `SG_SKIP_ROOT_CHECK=true` makes `is_root()` succeed without root. `load.bash` sets it. - **API + debug.** `SG_BASE_API` overrides the API base; `LOG_DEBUG=true` diff --git a/test/fixtures/ecs/ecs-anywhere-install.sh.sample b/test/fixtures/ecs/ecs-anywhere-install.sh.sample new file mode 100644 index 0000000..724d5d7 --- /dev/null +++ b/test/fixtures/ecs/ecs-anywhere-install.sh.sample @@ -0,0 +1,50 @@ +#!/bin/bash +# Trimmed stand-in for AWS's ecs-anywhere-install-latest.sh, preserving the +# parts disable_ecs_exec_setup cares about: the exec-setup function definition, +# its helper functions, and the unconditional top-level `exec-setup` call in the +# script's main flow. Sourced facts (function names, the hardcoded +# BINARY_VERSION, the call order) mirror the upstream script. + +set -e + +SKIP_REGISTRATION=false +DOCKER_SOURCE="" + +ok() { echo "ok"; } + +install-ssm-agent() { echo "install-ssm-agent"; } + +install-docker() { echo "install-docker $1"; } + +exec-setup() { + find-copy-certs-exec + download-ssm-binaries-exec +} + +find-copy-certs-exec() { + CERTS_PATH="/var/lib/ecs/deps/execute-command/certs" + echo "Copying certs for exec feature to ${CERTS_PATH}" + ok +} + +download-ssm-binaries-exec() { + BINARY_VERSION="3.3.4624.0" + BINARY_PATH="/var/lib/ecs/deps/execute-command/bin/${BINARY_VERSION}" + echo "Downloading SSM binaries for exec feature to ${BINARY_PATH}" + ok +} + +install-ecs-agent() { echo "install-ecs-agent"; } + +wait-agent-start() { echo "wait-agent-start"; } + +show-license() { echo "show-license"; } + +if ! $SKIP_REGISTRATION; then + install-ssm-agent +fi +install-docker "$DOCKER_SOURCE" +exec-setup +install-ecs-agent +wait-agent-start +show-license diff --git a/test/helpers/load.bash b/test/helpers/load.bash index feee4df..6ea57c4 100644 --- a/test/helpers/load.bash +++ b/test/helpers/load.bash @@ -47,6 +47,7 @@ export SG_DIAGNOSTIC_DIR="${SG_TEST_TMPDIR}/sg-runner" export ECS_CONFIG_DIR="${SG_TEST_TMPDIR}/etc-ecs" export ECS_LOG_DIR="${SG_TEST_TMPDIR}/var-log-ecs" export ECS_DATA_DIR="${SG_TEST_TMPDIR}/var-lib-ecs-data" +export ECS_EXEC_DEPS_DIR="${SG_TEST_TMPDIR}/var-lib-ecs-deps/execute-command" export REGISTRATION_DIR="${SG_TEST_TMPDIR}/registration" # Bypass the root requirement. diff --git a/test/unit/ecs_exec_deps.bats b/test/unit/ecs_exec_deps.bats new file mode 100644 index 0000000..55deb40 --- /dev/null +++ b/test/unit/ecs_exec_deps.bats @@ -0,0 +1,270 @@ +#!/usr/bin/env bats +# +# Unit tests for the ECS Exec dependency handling. +# +# StackGuardian never sets enableExecuteCommand on a task, but AWS's +# ecs-anywhere-install.sh stages the SSM session binaries under +# ECS_EXEC_DEPS_DIR unconditionally. Two functions keep them off the runner: +# +# disable_ecs_exec_setup - rewrites the installer's top-level `exec-setup` +# call so the download never happens. +# remove_ecs_exec_deps - removes anything that got staged anyway. +# +# load.bash points ECS_EXEC_DEPS_DIR at the per-test tmpdir, so the rm -rf here +# can never reach the real /var/lib/ecs/deps/execute-command. + +setup() { + load "${BATS_TEST_DIRNAME}/../helpers/load.bash" + INSTALLER="${SG_TEST_TMPDIR}/ecs-anywhere-install.sh" + cp "${BATS_TEST_DIRNAME}/../fixtures/ecs/ecs-anywhere-install.sh.sample" "$INSTALLER" +} + +#{{{ disable_ecs_exec_setup + +@test "the unpatched fixture does stage exec dependencies" { + # Guards the tests below: if upstream's shape changes so the fixture stops + # exercising exec-setup, this fails first and explains why. + run bash "$INSTALLER" + assert_success + assert_output --partial "Downloading SSM binaries for exec feature" +} + +@test "patched installer no longer stages exec dependencies" { + load_main + + run disable_ecs_exec_setup "$INSTALLER" + assert_success + + run bash "$INSTALLER" + assert_success + refute_output --partial "Downloading SSM binaries for exec feature" + refute_output --partial "Copying certs for exec feature" +} + +@test "patched installer still runs every other install step" { + load_main + + run disable_ecs_exec_setup "$INSTALLER" + assert_success + + run bash "$INSTALLER" + assert_success + assert_output --partial "install-ssm-agent" + assert_output --partial "install-docker" + assert_output --partial "install-ecs-agent" + assert_output --partial "wait-agent-start" +} + +@test "patched installer is still valid bash" { + load_main + + run disable_ecs_exec_setup "$INSTALLER" + assert_success + + run bash -n "$INSTALLER" + assert_success +} + +@test "only the top-level call is rewritten, the function stays defined" { + load_main + + run disable_ecs_exec_setup "$INSTALLER" + assert_success + + run grep -c '^exec-setup$' "$INSTALLER" + assert_output "0" + + # The definition and its helpers must survive untouched. + run grep -q '^exec-setup() {$' "$INSTALLER" + assert_success + run grep -q '^download-ssm-binaries-exec() {$' "$INSTALLER" + assert_success +} + +@test "no leftover .patched temp file" { + load_main + + run disable_ecs_exec_setup "$INSTALLER" + assert_success + + assert [ ! -e "${INSTALLER}.patched" ] +} + +@test "is idempotent: a second run leaves the file unchanged" { + load_main + + run disable_ecs_exec_setup "$INSTALLER" + assert_success + cp "$INSTALLER" "${SG_TEST_TMPDIR}/after-first" + run disable_ecs_exec_setup "$INSTALLER" + assert_success + + run diff "${SG_TEST_TMPDIR}/after-first" "$INSTALLER" + assert_success +} + +@test "installer without an exec-setup call is left byte-identical" { + load_main + printf '#!/bin/bash\ninstall-ecs-agent\n' >"$INSTALLER" + cp "$INSTALLER" "${SG_TEST_TMPDIR}/before" + + run disable_ecs_exec_setup "$INSTALLER" + assert_success + + run diff "${SG_TEST_TMPDIR}/before" "$INSTALLER" + assert_success +} + +@test "an indented or suffixed exec-setup mention is not rewritten" { + # Only a bare top-level call is a call; anything else is prose or a definition. + load_main + printf '#!/bin/bash\n# see exec-setup below\n exec-setup\nexec-setup() { :; }\n' >"$INSTALLER" + cp "$INSTALLER" "${SG_TEST_TMPDIR}/before" + + run disable_ecs_exec_setup "$INSTALLER" + assert_success + + run diff "${SG_TEST_TMPDIR}/before" "$INSTALLER" + assert_success +} + +#}}}: disable_ecs_exec_setup + +#{{{ remove_ecs_exec_deps + +@test "removes the staged dependency directory" { + load_main + mkdir -p "${ECS_EXEC_DEPS_DIR}/bin/3.3.4624.0" + touch "${ECS_EXEC_DEPS_DIR}/bin/3.3.4624.0/ssm-session-worker" + + run remove_ecs_exec_deps + assert_success + + assert [ ! -e "$ECS_EXEC_DEPS_DIR" ] +} + +@test "succeeds when the directory was never created" { + load_main + assert [ ! -e "$ECS_EXEC_DEPS_DIR" ] + + run remove_ecs_exec_deps + assert_success +} + +@test "is idempotent" { + load_main + mkdir -p "${ECS_EXEC_DEPS_DIR}/bin" + + run remove_ecs_exec_deps + assert_success + run remove_ecs_exec_deps + assert_success + assert [ ! -e "$ECS_EXEC_DEPS_DIR" ] +} + +@test "leaves sibling ECS state untouched" { + load_main + mkdir -p "${ECS_EXEC_DEPS_DIR}/bin" "$ECS_DATA_DIR" "$ECS_CONFIG_DIR" + touch "${ECS_DATA_DIR}/ecs_agent_data.json" "${ECS_CONFIG_DIR}/ecs.config" + + run remove_ecs_exec_deps + assert_success + + assert [ -e "${ECS_DATA_DIR}/ecs_agent_data.json" ] + assert [ -e "${ECS_CONFIG_DIR}/ecs.config" ] +} + +#}}}: remove_ecs_exec_deps + +#{{{ is_ecs_exec_deps_path (rm -rf guard) + +# ECS_EXEC_DEPS_DIR is overridable, so remove_ecs_exec_deps is the one place a +# stray env var could hand `rm -rf` a whole path while running as root. The +# guard is a pure predicate precisely so the catastrophic inputs below can be +# asserted without any test pointing rm at them. + +@test "guard accepts the production default path" { + load_main + run is_ecs_exec_deps_path "/var/lib/ecs/deps/execute-command" + assert_success +} + +@test "guard accepts the redirected test path" { + load_main + run is_ecs_exec_deps_path "$ECS_EXEC_DEPS_DIR" + assert_success +} + +@test "guard rejects root" { + load_main + run is_ecs_exec_deps_path "/" + assert_failure +} + +@test "guard rejects a bare system directory" { + load_main + for bad in /etc /var /usr /var/lib /var/lib/ecs; do + run is_ecs_exec_deps_path "$bad" + assert_failure + done +} + +@test "guard rejects empty and unset" { + load_main + run is_ecs_exec_deps_path "" + assert_failure + run is_ecs_exec_deps_path + assert_failure +} + +@test "guard rejects a relative path" { + load_main + run is_ecs_exec_deps_path "execute-command" + assert_failure + run is_ecs_exec_deps_path "./execute-command" + assert_failure +} + +@test "guard rejects a path merely containing the name" { + load_main + run is_ecs_exec_deps_path "/var/lib/ecs/deps/execute-command-backup" + assert_failure + run is_ecs_exec_deps_path "/execute-command/etc" + assert_failure +} + +@test "remove_ecs_exec_deps refuses a wrongly-shaped dir and leaves it intact" { + # Absolute but not a deps dir. Kept inside the test tmpdir so that a + # regression here destroys a sentinel, never anything real. + export ECS_EXEC_DEPS_DIR="${SG_TEST_TMPDIR}/not-a-deps-dir" + load_main + mkdir -p "$ECS_EXEC_DEPS_DIR" + touch "${ECS_EXEC_DEPS_DIR}/canary" + + run remove_ecs_exec_deps + assert_success + + assert [ -e "${ECS_EXEC_DEPS_DIR}/canary" ] +} + +#}}}: is_ecs_exec_deps_path + +#{{{ wiring + +@test "ECS_EXEC_DEPS_DIR defaults to the production path" { + run env -u ECS_EXEC_DEPS_DIR bash -c \ + 'source "${SG_MAIN_SH}"; printf "%s" "${ECS_EXEC_DEPS_DIR}"' + assert_success + assert_output "/var/lib/ecs/deps/execute-command" +} + +@test "register_instance patches the installer and sweeps the deps dir" { + # No flag gates this: both calls must be present in the registration path. + run bash -c \ + 'sed -n "/^register_instance() {/,/^#}}}: register_instance/p" "${SG_MAIN_SH}"' + assert_success + assert_output --partial 'disable_ecs_exec_setup "$ecs_install_script"' + assert_output --partial "remove_ecs_exec_deps" +} + +#}}}: wiring