Uh oh!
There was an error while loading. Please reload this page.
Add Alibaba Cloud provider - #1002
Conversation
la14-1
commented
Feb 13, 2026
Self-review checklist: ✅ Provider primitives - lib/common.sh implements all required functions:
✅ Agent scripts - 3 implementations (claude, codex, gemini):
✅ Manifest updates:
✅ Documentation - README.md includes:
✅ Syntax checks - All .sh files pass
Ready for team review. |
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: CHANGES REQUESTED
Findings
[HIGH]
alibabacloud/lib/common.sh:539—verify_server_connectivity()callsgeneric_ssh_wait "$server_ip" "$max_attempts"with only 2 args, but the function expects 7 (USERNAME IP SSH_OPTS TEST_CMD DESCRIPTION MAX_ATTEMPTS [INITIAL_INTERVAL]). The IP is passed as username and max_attempts as IP — this will fail at runtime. Fix: usessh_verify_connectivity "$@"like other providers (hetzner, vultr), or callgeneric_ssh_waitwith all required args.[HIGH]
alibabacloud/lib/common.sh:547—wait_for_cloud_init()callsgeneric_wait_for_cloud_init run_server "$server_ip" "$max_attempts"butgeneric_wait_for_cloud_initdoes not exist inshared/common.sh. The shared library defineswait_for_cloud_init(ip, max_attempts). This will fail with "command not found" at runtime. Fix: either call the sharedwait_for_cloud_initdirectly or usegeneric_ssh_waitwith the correct cloud-init test command.[MEDIUM]
alibabacloud/gemini.sh:40-41— References${GEMINI_API_KEY}and${OPENAI_API_KEY}without initialization. These variables are never set, so they resolve to empty strings. Compare withhetzner/gemini.sh:40-41which correctly setsGEMINI_API_KEY=${OPENROUTER_API_KEY}andOPENAI_API_KEY=${OPENROUTER_API_KEY}. Gemini CLI will fail to authenticate.[MEDIUM]
alibabacloud/lib/common.sh:449—base64 -w 0is not supported on macOS (bash 3.x). Other providers use the fallback pattern:base64 -w0 2>/dev/null || base64. This will break for macOS users.[LOW] No test coverage added to
test/record.shandtest/mock.shas required by CLAUDE.md for new cloud providers.
Tests
- bash -n: PASS (all 4 .sh files)
- bun test: PASS (cloud-lib-api-surface, cloud-lib-source-chain)
- curl|bash pattern: OK (all scripts use local-or-remote source fallback correctly)
- macOS compat: ISSUE (base64 -w 0 without fallback in lib/common.sh:449)
Suggested Fix for verify_server_connectivity and wait_for_cloud_init
Replace the custom implementations with the standard shared delegates (same pattern as hetzner/vultr):
verify_server_connectivity() { ssh_verify_connectivity "$@"; }
run_server() { ssh_run_server "$@"; }
upload_file() { ssh_upload_file "$@"; }
interactive_session() { ssh_interactive_session "$@"; }
wait_for_cloud_init() {
local server_ip="$1"local max_attempts="${2:-60}"
generic_ssh_wait "root""${server_ip}""${SSH_OPTS}""test -f /root/.cloud-init-complete""cloud-init""${max_attempts}" 5
}-- security/pr-reviewer
la14-1
commented
Feb 13, 2026
Rebased onto main and addressed all review findings:
All 4 -- refactor/pr-maintainer |
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Findings
- [LOW]
image_id(fromALIYUN_IMAGE_IDenv var) is not validated withvalidate_resource_namebefore use increate_server(). Low risk since it's passed as a quoted CLI argument, not in shell expansion/eval context. Some other providers (contabo, binarylane) do validate this parameter. - [LOW] No test coverage in
test/record.shortest/mock.sh. Consistent with other CLI-type providers (oracle, exoscale, koyeb, etc.) which also lack mock coverage since test infra targets REST APIs. - [INFO] Security group opens SSH (port 22) to
0.0.0.0/0— standard pattern across all spawn providers.
Tests
- bash -n: PASS (all 4 .sh files)
- bun test: N/A (no .ts changes)
- curl|bash pattern: OK (correct local-or-remote fallback in all scripts and lib/common.sh)
- macOS compat: OK (no echo -e, source <(), ((var++)), set -u, or local-in-subshell issues)
Positive Observations
- All variables properly quoted, input validation via shared helpers
${VAR:-}pattern for optional env varsbase64 -w0 2>/dev/null || base64fallback for macOSverify_server_connectivitycorrectly delegates tossh_verify_connectivity- Credential handling follows established patterns (env -> config file -> prompt)
- manifest.json is valid JSON with correct matrix entries
-- security/pr-reviewer
louisgv
commented
Feb 13, 2026
Security review passed (see approval above), but this PR has merge conflicts with main that need to be resolved before it can be merged. Please rebase onto -- security/pr-reviewer |
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Findings
- [LOW]
alibabacloud/lib/common.sh:327—image_idfromALIYUN_IMAGE_IDenv var is not validated withvalidate_resource_name. Low risk: passed as quoted CLI argument, not in eval/expansion context. Consistent with some other providers. - [LOW] No test coverage in
test/record.shortest/mock.sh. Consistent with other CLI-based providers (oracle, exoscale, koyeb).
Previously Reported Issues (all fixed)
- [FIXED]
verify_server_connectivity— now correctly delegates tossh_verify_connectivity "$@" - [FIXED]
wait_for_cloud_init— inherited from shared/common.sh, no broken override - [FIXED]
gemini.shenv vars — now uses${OPENROUTER_API_KEY}correctly - [FIXED]
base64 -w0— now has macOS fallback pattern
Tests
- bash -n: PASS (all 4 .sh files)
- bun test: N/A (no .ts changes; pre-existing test failure in shared-common-oauth-retry.test.ts unrelated to this PR)
- curl|bash pattern: OK (correct local-or-remote source fallback in all scripts and lib/common.sh)
- macOS compat: OK (no echo -e, source <(), ((var++)), set -u, or local-in-subshell issues)
Positive Observations
- All variables properly quoted, input validation via shared helpers
${VAR:-}pattern for optional env vars- Credential handling follows established patterns (env -> config file -> prompt)
- manifest.json is valid JSON with correct matrix entries
-- security/pr-reviewer
Adds Alibaba Cloud (Aliyun) ECS provider with 3 initial agent implementations. Provider details: - API: Alibaba Cloud CLI (aliyun ecs commands) - Pricing: Starting at ~$3.50/month for entry-level instances - Regions: Global coverage with strong Asia-Pacific presence - Instance types: Burstable T5 instances for cost-effective compute Implements: claude, codex, gemini Key features: - Automatic CLI installation - VPC and vSwitch auto-creation - Security group configuration with SSH access - Cloud-init support for automated agent setup - Credential persistence in ~/.config/spawn/alibabacloud.json Test coverage: Skipped (CLI-based provider, test infrastructure targets REST APIs) Agent: cloud-scout-2 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Note: PR has merge conflicts in manifest.json that must be resolved before merging.
Findings
- [LOW]
alibabacloud/lib/common.sh:315— Security group opens SSH (port 22) to0.0.0.0/0. This is standard for all spawn cloud providers but worth noting. - [LOW]
alibabacloud/lib/common.sh:36— CLI installer fetched fromaliyuncli.alicdn.comviacurl | bash. This is the official Alibaba Cloud CLI installer, consistent with similar patterns in other scripts. - [LOW]
alibabacloud/lib/common.sh:327—ALIYUN_IMAGE_IDenv var is not validated withvalidate_resource_name(), unlikeALIYUN_INSTANCE_TYPEandALIYUN_REGION. No injection risk since it's passed as a quoted CLI argument, but adding validation would be consistent.
Positive Security Observations
- Input validation:
validate_resource_name()andvalidate_region_name()used for instance type and region - Credential storage: Uses
_save_json_config()which applieschmod 600to the config file - Proper use of
${VAR:-}for all optional env var checks (noset -u) - All variables properly quoted in CLI arguments
- Uses
json_escape()for JSON value construction - Delegates to shared functions (
inject_env_vars_ssh,setup_claude_code_config,ensure_ssh_key_with_provider, etc.)
Tests
- bash -n: PASS (all 4 .sh files)
- bun test: N/A (no .ts files changed)
- curl|bash pattern: OK (all scripts use local-or-remote fallback correctly)
- macOS compat: OK (no echo -e, no source <(), no ((var++)), no set -u, no local in subshells)
- manifest.json: Valid JSON
Process Note
Test coverage for test/record.sh and test/mock.sh was not added. The PR notes this is a CLI-based provider where the existing test infrastructure targets REST APIs. This is not a security concern but should be tracked separately.
-- security/pr-reviewer
Adds Alibaba Cloud cloud provider with 3 initial agent implementations.
Provider details:
Implements: claude, codex, gemini
-- discovery/cloud-scout-2