Skip to content

fix(security): replace variable-stored shell code with named function in verify.sh#3073

Merged
louisgv merged 1 commit intomainfrom
fix/issue-3070
Mar 28, 2026
Merged

fix(security): replace variable-stored shell code with named function in verify.sh#3073
louisgv merged 1 commit intomainfrom
fix/issue-3070

Conversation

@la14-1
Copy link
Copy Markdown
Member

@la14-1 la14-1 commented Mar 28, 2026

Why: Removes a command injection risk pattern where shell code was stored in a variable and executed via ${port_check} — if this variable ever accepted dynamic input, it would be directly exploitable.

Fixes #3070

Changes

  • Extract the port check logic from port_check / port_check_r local variables into a _check_port_18789() function defined inside each cloud_exec remote command
  • Replace all if ${port_check}; then usages with if _check_port_18789; then
  • Affected functions: _openclaw_ensure_gateway, _openclaw_restart_gateway, _openclaw_verify_gateway_resilience
  • Pattern is now safe: no shell code stored in variables, no eval-equivalent execution

Testing

  • bash -n sh/e2e/lib/verify.sh passes (syntax check)

-- refactor/security-auditor

… in verify.sh

Fixes #3070

The port_check / port_check_r variables stored executable shell code as
strings and expanded them via ${port_check} inside cloud_exec commands.
This is an eval-equivalent pattern: if any part of the variable were ever
derived from dynamic input, it would be directly exploitable as command
injection.

Replace the pattern with _check_port_18789() remote function definitions
inside each cloud_exec call. The function is defined and called entirely
on the remote side — no shell code is stored in local bash variables.

Affected functions:
- _openclaw_ensure_gateway (2 usages)
- _openclaw_restart_gateway (1 usage)
- _openclaw_verify_gateway_resilience (3 usages)

Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@louisgv louisgv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: cdb1228

Findings

No security issues found. This change is a security improvement.

The PR replaces variable-stored shell code (local port_check='...'${port_check}) with inline function definitions (_check_port_18789() { ...; }; _check_port_18789), eliminating command injection risk vectors.

Specific improvements:

  • ✅ Eliminates variable expansion-based command execution
  • ✅ Maintains proper shell quoting (double quotes outer, single quotes inner)
  • ✅ Preserves macOS bash 3.x compatibility
  • ✅ No introduction of new security risks

Tests

  • bash -n: PASS (syntax check clean)
  • bun test: N/A (shell script, no TypeScript tests affected)
  • curl|bash: N/A (E2E test library, not a bootstrap script)
  • macOS compat: OK (uses portable function syntax, no bash 4+ features)

-- security/pr-reviewer

@louisgv louisgv added the security-approved Security review approved label Mar 28, 2026
@louisgv louisgv merged commit a29d0d8 into main Mar 28, 2026
6 checks passed
@louisgv louisgv deleted the fix/issue-3070 branch March 28, 2026 04:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approved Security review approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: Command injection risk in verify.sh openclaw port check

2 participants