Commit graph

7 commits

Author SHA1 Message Date
A
8174ed1547
fix: HIGH severity security issues (command injection + weak VNC password) (#1150)
Fixes #1120

1. Command injection in shared/key-request.sh:86
   - BEFORE: export "${var_name}=${val}" allowed injection via $(...)
   - AFTER: Use printf -v to safely assign the value
   - Impact: Prevents arbitrary command execution via crafted API key values

2. Weak VNC password in cloudsigma/lib/common.sh:266
   - BEFORE: openssl rand -hex 8 (64 bits of entropy)
   - AFTER: openssl rand -hex 16 (128 bits of entropy)
   - Impact: Strengthens VNC password against brute force attacks

Agent: security-auditor

Co-authored-by: spawn-refactor-bot <refactor@openrouter.ai>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-02-14 20:39:48 -05:00
A
0bb085214a
fix: Properly handle comma-separated auth vars in key-request.sh (#1083)
* fix: Properly handle comma-separated auth vars in key-request.sh

The tr command was incorrectly translating each character in '+,' to newline,
causing "ALIYUN_ACCESS_KEY_ID, ALIYUN_ACCESS_KEY_SECRET" to not be split properly.

Also updated get_cloud_env_vars to split on both + and , separators.

Fixes the error: "ALIYUN_ACCESS_KEY_ID, ALIYUN_ACCESS_KEY_SECRET: invalid variable name"

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: Revert sed to tr for macOS bash 3.x compatibility

As requested in security review - BSD sed treats \n in replacement
as literal backslash-n, not newline. tr already handles both + and ,
delimiters correctly on all platforms.

Addresses security review feedback.

---------

Co-authored-by: Spawn QA Bot <qa-bot@openrouter.ai>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Spawn Refactor Service <refactor@spawn.service>
2026-02-14 05:10:11 -05:00
A
f3ee7e271a
security: Fix command injection vulnerability in env var exports (#1086)
CRITICAL: Add validation to prevent command injection via malicious environment variable names in `export "${var_name}=..."` patterns.

Vulnerability Details:
- All instances of `export "${var_name}=${value}"` where var_name is derived from external sources (manifest.json auth fields, user input, API responses) were vulnerable to command injection
- If var_name contained shell metacharacters like `;`, `$()`, or backticks, arbitrary code could be executed
- Example exploit: var_name=`FOO; rm -rf /` would execute the rm command

Affected Files:
- shared/key-request.sh: _try_load_env_var() - var_name from manifest.json
- shared/common.sh: _load_token_from_config(), ensure_api_token_with_provider(), _multi_creds_load_config(), _multi_creds_prompt(), _poll_instance_once() - var_name from function parameters
- test/record.sh: _load_multi_config_from_file(), _try_load_cloud_config(), _prompt_cloud_creds_interactive() - var_name from test fixtures

Fix Applied:
- Added regex validation before all export statements: `^[A-Z_][A-Z0-9_]*$`
- This allowlist enforces standard POSIX environment variable naming (uppercase letters, digits, underscores only, must start with letter or underscore)
- Returns error if validation fails, preventing injection

Impact:
- While current usage passes hardcoded env var names (e.g., "HCLOUD_TOKEN"), the vulnerability existed in the implementation
- manifest.json is currently trusted, but defense-in-depth prevents supply chain attacks or accidental malformed entries
- Test infrastructure was also vulnerable to malicious fixture data

Agent: security-auditor

Co-authored-by: Spawn Refactor Service <refactor@spawn.service>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-02-14 04:01:25 -05:00
A
ab7754e11b
fix: Handle comma-separated auth vars in key-request.sh (#1081)
The auth parsing in _load_cloud_credentials() only handled '+' separators,
but some clouds (like alibabacloud) use comma-separated env var lists.

Changed `tr '+' '\n'` to `tr '+,' '\n'` to handle both formats.

Fixes error: "ALIYUN_ACCESS_KEY_ID, ALIYUN_ACCESS_KEY_SECRET: invalid variable name"

Co-authored-by: Spawn QA Bot <qa-bot@openrouter.ai>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-02-14 02:46:49 -05:00
A
8a5d03995b
fix: validate provider name in invalidate_cloud_key and improve key validation (#1017)
- Add regex validation (^[a-z0-9][a-z0-9._-]{0,63}$) to invalidate_cloud_key()
  in shared/key-request.sh to prevent path traversal attacks that could delete
  arbitrary files via crafted provider names (e.g., ../../etc/important)

- Improve validKeyVal() in key-server.ts to block control characters
  (U+0000-U+001F, U+007F-U+009F) and enforce a 4096-byte max length on
  API key values, preventing injection of null bytes, newlines, and
  excessively long values

Agent: security-auditor

Co-authored-by: A <6723574+louisgv@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-02-13 14:43:44 -08:00
A
b0ebaa94bb
refactor: decompose load_cloud_keys_from_config into focused helpers (#1007)
Extract three helpers from the 82-line, 14-conditional function:
- _parse_cloud_auths: extract cloud auth specs from manifest.json
- _try_load_env_var: load a single env var from env or config file
- _load_cloud_credentials: load all env vars for one cloud provider

The main function is now a 36-line orchestrator with clear flow:
validate prerequisites -> parse manifest -> iterate clouds -> summarize.

Agent: complexity-hunter

Co-authored-by: A <6723574+louisgv@users.noreply.github.com>
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
2026-02-13 13:29:53 -08:00
Ahmed Abushagur
1ad2371a25
feat: qa bot and emails (#565) 2026-02-11 20:19:45 -08:00