From bb44c5326ee237b57d53bc5bd849e810ddc49960 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 19 Jun 2026 01:02:59 +0200 Subject: [PATCH] fix(e2e): validate docker log limits --- scripts/lib/docker-e2e-logs.sh | 73 +++++++++++++++++------- scripts/lib/docker-e2e-package.sh | 4 +- test/scripts/docker-build-helper.test.ts | 61 ++++++++++++++++++++ 3 files changed, 115 insertions(+), 23 deletions(-) diff --git a/scripts/lib/docker-e2e-logs.sh b/scripts/lib/docker-e2e-logs.sh index c696ca9d4ae..ac715b1d242 100644 --- a/scripts/lib/docker-e2e-logs.sh +++ b/scripts/lib/docker-e2e-logs.sh @@ -4,14 +4,39 @@ # They centralize temporary log naming and the small success/failure print # pattern used by Docker scenario scripts. +docker_e2e_normalize_positive_int_value() { + local label="${1:?missing value label}" + local value="${2-}" + if [[ ! "$value" =~ ^[0-9]+$ ]] || (( 10#$value < 1 )); then + echo "invalid $label: $value" >&2 + return 2 + fi + printf '%s\n' "$((10#$value))" +} + +docker_e2e_read_positive_int_env() { + local name="${1:?missing environment variable name}" + local fallback="${2:?missing fallback value}" + local value="${!name-}" + if [ -z "${!name+x}" ]; then + value="$fallback" + fi + docker_e2e_normalize_positive_int_value "$name" "$value" +} + run_logged() { local label="$1" shift + docker_e2e_read_positive_int_env OPENCLAW_DOCKER_E2E_LOG_PRINT_BYTES 65536 >/dev/null || return $? local log_file log_file="$(docker_e2e_run_log "$label")" if ! "$@" >"$log_file" 2>&1; then - docker_e2e_print_log "$log_file" + local print_status=0 + docker_e2e_print_log "$log_file" || print_status="$?" rm -f "$log_file" + if [ "$print_status" -ne 0 ]; then + return "$print_status" + fi return 1 fi rm -f "$log_file" @@ -20,14 +45,23 @@ run_logged() { run_logged_print() { local label="$1" shift + docker_e2e_read_positive_int_env OPENCLAW_DOCKER_E2E_LOG_PRINT_BYTES 65536 >/dev/null || return $? local log_file log_file="$(docker_e2e_run_log "$label")" if ! "$@" >"$log_file" 2>&1; then - docker_e2e_print_log "$log_file" + local print_status=0 + docker_e2e_print_log "$log_file" || print_status="$?" rm -f "$log_file" + if [ "$print_status" -ne 0 ]; then + return "$print_status" + fi return 1 fi - docker_e2e_print_log "$log_file" + docker_e2e_print_log "$log_file" || { + local print_status="$?" + rm -f "$log_file" + return "$print_status" + } rm -f "$log_file" } @@ -35,11 +69,12 @@ run_logged_print_heartbeat() { local label="$1" local interval_seconds="$2" shift 2 - if ! [[ "$interval_seconds" =~ ^[0-9]+$ ]] || [ "$interval_seconds" -lt 1 ]; then - interval_seconds="30" - else - interval_seconds="$((10#$interval_seconds))" - fi + docker_e2e_read_positive_int_env OPENCLAW_DOCKER_E2E_LOG_PRINT_BYTES 65536 >/dev/null || return $? + interval_seconds="$(docker_e2e_normalize_positive_int_value "Docker E2E log heartbeat interval" "$interval_seconds")" || return $? + local heartbeat_term_grace_seconds + heartbeat_term_grace_seconds="$( + docker_e2e_read_positive_int_env OPENCLAW_DOCKER_E2E_HEARTBEAT_TERM_GRACE_SECONDS 30 + )" || return $? local log_file log_file="$(docker_e2e_run_log "$label")" local command_pid="" @@ -55,14 +90,8 @@ run_logged_print_heartbeat() { return 0 fi kill -TERM "$command_pid" 2>/dev/null || true - local grace_seconds="${OPENCLAW_DOCKER_E2E_HEARTBEAT_TERM_GRACE_SECONDS:-30}" - if ! [[ "$grace_seconds" =~ ^[0-9]+$ ]] || [ "$grace_seconds" -lt 1 ]; then - grace_seconds="30" - else - grace_seconds="$((10#$grace_seconds))" - fi local wait_attempt - for wait_attempt in $(seq 1 "$((grace_seconds * 10))"); do + for wait_attempt in $(seq 1 "$((heartbeat_term_grace_seconds * 10))"); do if ! kill -0 "$command_pid" 2>/dev/null; then return 0 fi @@ -130,7 +159,11 @@ run_logged_print_heartbeat() { wait "$command_pid" status=$? set -e - docker_e2e_print_log "$log_file" + docker_e2e_print_log "$log_file" || { + local print_status="$?" + cleanup_heartbeat_command 0 + return "$print_status" + } cleanup_heartbeat_command 0 return "$status" } @@ -144,12 +177,8 @@ docker_e2e_run_log() { docker_e2e_print_log() { local log_file="$1" - local max_bytes="${OPENCLAW_DOCKER_E2E_LOG_PRINT_BYTES:-65536}" - if ! [[ "$max_bytes" =~ ^[0-9]+$ ]] || [ "$max_bytes" -lt 1 ]; then - max_bytes="65536" - else - max_bytes="$((10#$max_bytes))" - fi + local max_bytes + max_bytes="$(docker_e2e_read_positive_int_env OPENCLAW_DOCKER_E2E_LOG_PRINT_BYTES 65536)" || return $? if [ ! -f "$log_file" ]; then return 0 fi diff --git a/scripts/lib/docker-e2e-package.sh b/scripts/lib/docker-e2e-package.sh index 6c8abaf55f9..b48ed2676d1 100644 --- a/scripts/lib/docker-e2e-package.sh +++ b/scripts/lib/docker-e2e-package.sh @@ -409,9 +409,11 @@ docker_e2e_run_logged_with_harness() { docker_e2e_run_logged_print_with_harness() { local label="$1" shift + local heartbeat_seconds + heartbeat_seconds="$(docker_e2e_read_positive_int_env OPENCLAW_DOCKER_E2E_LOG_HEARTBEAT_SECONDS 30)" || return $? run_logged_print_heartbeat \ "$label" \ - "${OPENCLAW_DOCKER_E2E_LOG_HEARTBEAT_SECONDS:-30}" \ + "$heartbeat_seconds" \ docker_e2e_run_with_harness \ "$@" } diff --git a/test/scripts/docker-build-helper.test.ts b/test/scripts/docker-build-helper.test.ts index ce469153b79..570c9c0bb2e 100644 --- a/test/scripts/docker-build-helper.test.ts +++ b/test/scripts/docker-build-helper.test.ts @@ -2625,6 +2625,67 @@ output="$(run_logged_print_heartbeat plugins-run 30 bash -c 'printf "DO_NOT_PRIN } }); + it.each([ + ["printed log bytes", "OPENCLAW_DOCKER_E2E_LOG_PRINT_BYTES", "64kb"], + ["heartbeat termination grace", "OPENCLAW_DOCKER_E2E_HEARTBEAT_TERM_GRACE_SECONDS", "soon"], + ])("rejects invalid Docker E2E %s before setup", (_label, envName, value) => { + const workDir = mkdtempSync(join(tmpdir(), "openclaw-docker-e2e-log-invalid-")); + + try { + const rootDir = process.cwd(); + const script = ` +set -euo pipefail +ROOT_DIR=${shellQuote(rootDir)} +TMPDIR=${shellQuote(workDir)} +export ROOT_DIR TMPDIR +export ${envName}=${shellQuote(value)} + +source "$ROOT_DIR/scripts/lib/docker-e2e-logs.sh" + +run_logged_print_heartbeat plugins-run 30 bash -c 'printf "should not print\\\\n"' +`; + + const result = spawnSync("bash", ["-lc", script], { encoding: "utf8" }); + + expect(result.status).toBe(2); + expect(result.stderr).toContain(`invalid ${envName}: ${value}`); + expect(result.stdout).toBe(""); + } finally { + rmSync(workDir, { recursive: true, force: true }); + } + }); + + it("rejects invalid Docker E2E log heartbeat env before harness setup", () => { + const workDir = mkdtempSync(join(tmpdir(), "openclaw-docker-e2e-log-heartbeat-invalid-")); + + try { + const rootDir = process.cwd(); + const script = ` +set -euo pipefail +ROOT_DIR=${shellQuote(rootDir)} +TMPDIR=${shellQuote(workDir)} +export ROOT_DIR TMPDIR +export OPENCLAW_DOCKER_E2E_LOG_HEARTBEAT_SECONDS=1e3 + +source "$ROOT_DIR/scripts/lib/docker-e2e-package.sh" + +docker_e2e_run_with_harness() { + echo "should not run" +} + +docker_e2e_run_logged_print_with_harness plugins-run image-name +`; + + const result = spawnSync("bash", ["-lc", script], { encoding: "utf8" }); + + expect(result.status).toBe(2); + expect(result.stderr).toContain("invalid OPENCLAW_DOCKER_E2E_LOG_HEARTBEAT_SECONDS: 1e3"); + expect(result.stdout).toBe(""); + } finally { + rmSync(workDir, { recursive: true, force: true }); + } + }); + it("prints heartbeat progress for long successful Docker E2E log captures", () => { const workDir = mkdtempSync(join(tmpdir(), "openclaw-docker-e2e-log-heartbeat-"));