Uh oh!
There was an error while loading. Please reload this page.
test: add cron-triggered Telegram reminder to soak test - #2519
Conversation
Tests OpenClaw's ability to stay alive and execute scheduled tasks. Installs a one-shot cron on the VM before the 1h soak wait that sends a Telegram message at ~55 min, then verifies the message was sent after the wait completes. Also moves Telegram config injection before the soak wait so the cron can use the bot token immediately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replaces the raw system cron approach with OpenClaw's built-in cron scheduler (`openclaw cron add`). This properly tests that OpenClaw's gateway stays alive after 1 hour and can execute scheduled tasks. The test now: 1. Injects Telegram config + schedules an OpenClaw cron job (--at +55min) 2. Waits 1 hour (soak) 3. Verifies the job fired via `openclaw cron runs` and `openclaw cron list` Uses --delete-after-run for one-shot semantics. Verification checks both the run history and the auto-deletion as proof of execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of trusting OpenClaw's self-reported cron status, we now verify the message actually exists in the Telegram chat: 1. Extract message_id from OpenClaw's cron execution logs (tries `openclaw cron runs`, then ~/.openclaw/cron/ directory) 2. Call Telegram's forwardMessage API with that message_id 3. If Telegram can forward it → message EXISTS in the chat (proof from Telegram itself, not OpenClaw) This catches cases where OpenClaw reports success but the message never actually reached Telegram. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6283424 to
830e67dCompare
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: CHANGES REQUESTED
Commit: 830e67d
Findings
CRITICAL - sh/e2e/lib/soak.sh:247-248 — Command injection via unquoted SOAK_CRON_DELAY_SECONDS
# VULNERABLE:
fire_at=$(cloud_exec "${app}""date -u -d '+${SOAK_CRON_DELAY_SECONDS} seconds' ...")# FIX: Add quotes and validate
fire_at=$(cloud_exec "${app}""date -u -d '+\"${SOAK_CRON_DELAY_SECONDS}\" seconds' ...")Attacker-controlled SOAK_CRON_DELAY_SECONDS='3300; rm -rf /' would execute arbitrary commands.
HIGH - sh/e2e/lib/soak.sh:20-21 — Missing input validation on numeric environment variables
SOAK_WAIT_SECONDS="${SOAK_WAIT_SECONDS:-3600}"
SOAK_CRON_DELAY_SECONDS="${SOAK_CRON_DELAY_SECONDS:-3300}"These must be validated as positive integers before use in arithmetic or command construction.
HIGH - sh/e2e/lib/soak.sh:264-274 — Command injection via TELEGRAM_TEST_CHAT_ID in openclaw cron command
The TELEGRAM_TEST_CHAT_ID variable is interpolated into a command string and could contain shell metacharacters. Must be sanitized or validated to contain only alphanumeric/dash/underscore characters.
MEDIUM - sh/e2e/lib/soak.sh:243 — Arithmetic expansion with unvalidated input
$((SOAK_CRON_DELAY_SECONDS /60))This will fail if SOAK_CRON_DELAY_SECONDS is not a valid integer. Add validation before arithmetic operations.
LOW - sh/e2e/lib/soak.sh:285 — Race condition with /tmp/.spawn-cron-scheduled
Using a fixed filename in /tmp could conflict if multiple soak tests run on the same VM. Consider using mktemp or including the app name in the marker filename.
Required Changes
- Add input validation function at the top of the file:
validate_numeric_env() {
local var_name="$1"local var_value="$2"if!printf'%s'"${var_value}"| grep -qE '^[0-9]+$';then
log_err "${var_name} must be a positive integer, got: ${var_value}"return 1
fiif [ "${var_value}"-lt 1 ] || [ "${var_value}"-gt 86400 ];then
log_err "${var_name} out of range (1-86400), got: ${var_value}"return 1
fireturn 0
}- Validate environment variables after line 21:
if! validate_numeric_env "SOAK_WAIT_SECONDS""${SOAK_WAIT_SECONDS}";thenexit 1;fiif! validate_numeric_env "SOAK_CRON_DELAY_SECONDS""${SOAK_CRON_DELAY_SECONDS}";thenexit 1;fi- Fix command injection on line 247-248 by properly quoting:
fire_at=$(cloud_exec "${app}""date -u -d '+${SOAK_CRON_DELAY_SECONDS} seconds' '+%Y-%m-%dT%H:%M:%SZ' 2>/dev/null || date -u -v+${SOAK_CRON_DELAY_SECONDS}S '+%Y-%m-%dT%H:%M:%SZ'"2>&1)||trueNote: Since these are now validated as numeric-only, the injection risk is mitigated, but proper quoting is still best practice.
- Validate TELEGRAM_TEST_CHAT_ID format in soak_validate_telegram_env (line 40):
if!printf'%s'"${TELEGRAM_TEST_CHAT_ID}"| grep -qE '^-?[0-9]+$';then
log_err "TELEGRAM_TEST_CHAT_ID must be numeric (chat IDs are integers)"
missing=1
fi- Fix race condition on line 285:
cloud_exec "${app}""touch /tmp/.spawn-cron-scheduled-${app}"2>/dev/null ||trueTests
- bash -n: PASS
- curl|bash: N/A (not a standalone installer)
- macOS compat: OK (uses bash 3.x compatible patterns)
-- security/pr-reviewer
- Add validate_positive_int() and validate SOAK_WAIT_SECONDS +
SOAK_CRON_DELAY_SECONDS at startup (prevents command injection via
crafted env vars)
- Validate TELEGRAM_TEST_CHAT_ID is numeric in soak_validate_telegram_env
- Use per-app marker file /tmp/.spawn-cron-scheduled-${app} to avoid
race conditions when multiple soak tests run on the same VM
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: 40fe98f
Summary
All CRITICAL security issues from the prior review (commit 830e67d) have been resolved. The PR now includes comprehensive input validation that prevents command injection attacks.
Fixed Issues
- CRITICAL → FIXED (Lines 32-48) — Added
validate_positive_int()function with regex validation forSOAK_WAIT_SECONDSandSOAK_CRON_DELAY_SECONDS. Range check (1-86400) prevents extreme values. Validation happens before any command/arithmetic use. - CRITICAL → FIXED (Lines 66-68) — Added regex validation for
TELEGRAM_TEST_CHAT_ID(pattern:^-?[0-9]+$). Prevents injection via chat_id parameter in curl commands. - LOW (Lines 273-274, 311) —
appparameter interpolated into commands without local validation. Mitigation: parameter comes frommake_app_name()which should sanitize. Risk is LOW (requires attacker control of provisioning).
Tests
- bash -n: PASS (no syntax errors)
- bun test: N/A (no TypeScript changes)
- curl|bash: OK (no relative paths, self-contained functions)
- macOS compat: OK (no
echo -e,source <(...),((var++)), orset -u)
Security Properties
✓ Input validation for all user-controlled numeric parameters
✓ Regex-based validation prevents injection via special characters
✓ Base64 encoding used for sensitive tokens
✓ No new security issues introduced
-- security/pr-reviewer
Uh oh!
There was an error while loading. Please reload this page.
…bs#2519) * test: add cron-triggered Telegram reminder to soak test Tests OpenClaw's ability to stay alive and execute scheduled tasks. Installs a one-shot cron on the VM before the 1h soak wait that sends a Telegram message at ~55 min, then verifies the message was sent after the wait completes. Also moves Telegram config injection before the soak wait so the cron can use the bot token immediately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: use OpenClaw's cron scheduler instead of system crontab Replaces the raw system cron approach with OpenClaw's built-in cron scheduler (`openclaw cron add`). This properly tests that OpenClaw's gateway stays alive after 1 hour and can execute scheduled tasks. The test now: 1. Injects Telegram config + schedules an OpenClaw cron job (--at +55min) 2. Waits 1 hour (soak) 3. Verifies the job fired via `openclaw cron runs` and `openclaw cron list` Uses --delete-after-run for one-shot semantics. Verification checks both the run history and the auto-deletion as proof of execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: verify cron message on Telegram side via forwardMessage Instead of trusting OpenClaw's self-reported cron status, we now verify the message actually exists in the Telegram chat: 1. Extract message_id from OpenClaw's cron execution logs (tries `openclaw cron runs`, then ~/.openclaw/cron/ directory) 2. Call Telegram's forwardMessage API with that message_id 3. If Telegram can forward it → message EXISTS in the chat (proof from Telegram itself, not OpenClaw) This catches cases where OpenClaw reports success but the message never actually reached Telegram. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address security review findings in soak test - Add validate_positive_int() and validate SOAK_WAIT_SECONDS + SOAK_CRON_DELAY_SECONDS at startup (prevents command injection via crafted env vars) - Validate TELEGRAM_TEST_CHAT_ID is numeric in soak_validate_telegram_env - Use per-app marker file /tmp/.spawn-cron-scheduled-${app} to avoid race conditions when multiple soak tests run on the same VM Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…bs#2519) * test: add cron-triggered Telegram reminder to soak test Tests OpenClaw's ability to stay alive and execute scheduled tasks. Installs a one-shot cron on the VM before the 1h soak wait that sends a Telegram message at ~55 min, then verifies the message was sent after the wait completes. Also moves Telegram config injection before the soak wait so the cron can use the bot token immediately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: use OpenClaw's cron scheduler instead of system crontab Replaces the raw system cron approach with OpenClaw's built-in cron scheduler (`openclaw cron add`). This properly tests that OpenClaw's gateway stays alive after 1 hour and can execute scheduled tasks. The test now: 1. Injects Telegram config + schedules an OpenClaw cron job (--at +55min) 2. Waits 1 hour (soak) 3. Verifies the job fired via `openclaw cron runs` and `openclaw cron list` Uses --delete-after-run for one-shot semantics. Verification checks both the run history and the auto-deletion as proof of execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: verify cron message on Telegram side via forwardMessage Instead of trusting OpenClaw's self-reported cron status, we now verify the message actually exists in the Telegram chat: 1. Extract message_id from OpenClaw's cron execution logs (tries `openclaw cron runs`, then ~/.openclaw/cron/ directory) 2. Call Telegram's forwardMessage API with that message_id 3. If Telegram can forward it → message EXISTS in the chat (proof from Telegram itself, not OpenClaw) This catches cases where OpenClaw reports success but the message never actually reached Telegram. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address security review findings in soak test - Add validate_positive_int() and validate SOAK_WAIT_SECONDS + SOAK_CRON_DELAY_SECONDS at startup (prevents command injection via crafted env vars) - Validate TELEGRAM_TEST_CHAT_ID is numeric in soak_validate_telegram_env - Use per-app marker file /tmp/.spawn-cron-scheduled-${app} to avoid race conditions when multiple soak tests run on the same VM Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…bs#2519) * test: add cron-triggered Telegram reminder to soak test Tests OpenClaw's ability to stay alive and execute scheduled tasks. Installs a one-shot cron on the VM before the 1h soak wait that sends a Telegram message at ~55 min, then verifies the message was sent after the wait completes. Also moves Telegram config injection before the soak wait so the cron can use the bot token immediately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: use OpenClaw's cron scheduler instead of system crontab Replaces the raw system cron approach with OpenClaw's built-in cron scheduler (`openclaw cron add`). This properly tests that OpenClaw's gateway stays alive after 1 hour and can execute scheduled tasks. The test now: 1. Injects Telegram config + schedules an OpenClaw cron job (--at +55min) 2. Waits 1 hour (soak) 3. Verifies the job fired via `openclaw cron runs` and `openclaw cron list` Uses --delete-after-run for one-shot semantics. Verification checks both the run history and the auto-deletion as proof of execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: verify cron message on Telegram side via forwardMessage Instead of trusting OpenClaw's self-reported cron status, we now verify the message actually exists in the Telegram chat: 1. Extract message_id from OpenClaw's cron execution logs (tries `openclaw cron runs`, then ~/.openclaw/cron/ directory) 2. Call Telegram's forwardMessage API with that message_id 3. If Telegram can forward it → message EXISTS in the chat (proof from Telegram itself, not OpenClaw) This catches cases where OpenClaw reports success but the message never actually reached Telegram. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address security review findings in soak test - Add validate_positive_int() and validate SOAK_WAIT_SECONDS + SOAK_CRON_DELAY_SECONDS at startup (prevents command injection via crafted env vars) - Validate TELEGRAM_TEST_CHAT_ID is numeric in soak_validate_telegram_env - Use per-app marker file /tmp/.spawn-cron-scheduled-${app} to avoid race conditions when multiple soak tests run on the same VM Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
How it works
/tmp/spawn-cron-telegram-result.json"ok":trueTest plan
SOAK_WAIT_SECONDS=120 SOAK_CRON_DELAY_SECONDS=60 bash sh/e2e/e2e.sh --soakwith short timers to verify the cron fires?reason=soakon the QA server to run the full 1h cycle🤖 Generated with Claude Code