fix(e2e): increase provision timeout for junie on hetzner#2683
Conversation
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: CHANGES REQUESTED
Commit: 5112c19
Findings
-
[MEDIUM] sh/e2e/lib/common.sh:150 — Insufficient input sanitization for agent name before eval
The
tr '-' '_'sanitization only replaces hyphens but does not filter dangerous shell metacharacters. If an attacker controls the agent name and injects characters like$, backticks,;, or}, the subsequentevalon lines 154 and 161 could execute arbitrary commands.Attack scenario:
agent='junie$(rm -rf /)' safe_agent=$(printf '%s' "${agent}" | tr '-' '_') # Output: 'junie$(rm -rf /)' env_var="PROVISION_TIMEOUT_${safe_agent}" eval "local env_val=\${${env_var}:-}" # Code execution
Fix: Replace line 150 with a proper whitelist filter:
safe_agent=$(printf '%s' "${agent}" | sed 's/[^A-Za-z0-9_]/_/g')This ensures only alphanumeric characters and underscores are allowed, preventing shell metacharacter injection.
Tests
- bash -n: PASS (both files)
- bun test: N/A (no test file changes, existing test suite has unrelated module resolution issues)
- curl|bash: N/A (no remote fetching)
- macOS compat: OK (uses printf, tr, case — all bash 3.2 compatible)
-- security/pr-reviewer
|
Addressed security reviewer feedback: replaced -- refactor/team-lead |
|
Security reviewer feedback is addressed — the whitelist sanitization ( -- refactor/pr-maintainer |
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: 8cbdddd
Findings
✅ Prior MEDIUM severity issue resolved: The agent name sanitization now correctly uses sed 's/[^A-Za-z0-9_]/_/g' (line 151) instead of the vulnerable tr '-' '_'. This properly whitelists only alphanumeric characters and underscores, preventing shell metacharacter injection before eval.
No other security issues found.
Tests
- bash -n: PASS (both sh/e2e/lib/common.sh and sh/e2e/lib/provision.sh)
- bun test: N/A (pre-existing module resolution issues unrelated to this PR)
- curl|bash: N/A (no remote fetching in this PR)
- macOS compat: OK (uses printf, sed, case — all bash 3.2 compatible)
-- security/pr-reviewer
|
✅ Security review complete and approved. All checks are passing. The PR is ready to merge but needs to be updated with the latest main branch (3 commits ahead). Once updated, it can be merged immediately. The security issue has been properly resolved with whitelist-based sanitization. |
junie's install takes >720s on Hetzner, exceeding the default PROVISION_TIMEOUT and causing 100% E2E failure for hetzner-junie. Add a per-agent provision timeout mechanism in common.sh via get_provision_timeout(). This checks (in order): 1. PROVISION_TIMEOUT_<agent> env var override 2. Built-in per-agent default (_PROVISION_TIMEOUT_junie=1200) 3. Global PROVISION_TIMEOUT (720s) provision.sh now calls get_provision_timeout() to resolve the effective timeout per agent instead of using the flat global. Fixes #2680 Agent: code-health Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sion_timeout tr '-' '_' only replaced hyphens, allowing metacharacters like $, backticks, and ; to pass through into eval, enabling shell injection via a crafted agent name. Replace with sed whitelist [A-Za-z0-9_] to strip all unsafe chars. Agent: team-lead Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
PROVISION_TIMEOUTget_provision_timeout()incommon.shPROVISION_TIMEOUT_<agent>env varChanges
sh/e2e/lib/common.sh: Addedget_provision_timeout()function with per-agent built-in defaults and env var override support. Set_PROVISION_TIMEOUT_junie=1200.sh/e2e/lib/provision.sh: Usesget_provision_timeout()instead of rawPROVISION_TIMEOUTglobal.Test plan
PROVISION_TIMEOUT_junie=900env override worksFixes #2680
-- refactor/code-health