Security review: findings and minor improvements

[MrMichou]

Security Review Summary

A comprehensive security audit of the codebase was performed. No critical vulnerabilities were found. The application demonstrates strong security practices throughout.

---

Strengths

Zero unsafe code

The entire codebase is 100% safe Rust — all memory safety guarantees are enforced by the compiler.

Command injection prevention (src/shell/mod.rs)

• Whitelist-based SSH argument validation
• Explicit blocking of dangerous SSH options (ProxyCommand, LocalCommand, PermitLocalCommand)
• GCP resource name validation (alphanumeric + hyphens, max 63 chars)
• std::process::Command with argument array (no shell interpolation)
• 20+ unit tests covering injection attempts

Systematic URL encoding (src/resource/sdk_dispatch.rs)

• All resource IDs pass through urlencoding::encode() before URL insertion
• Query parameters are also encoded
• Monitoring API filter injection protection (double-quote blocking)

Credential handling (src/gcp/auth.rs)

• Delegates to gcp_auth (Application Default Credentials)
• Tokens held in memory only, never persisted to disk
• 60s expiry buffer to prevent using near-expired tokens
• Mutex-based cache prevents TOCTOU race conditions

File permissions (src/config.rs)

• Config directory: 0700
• Config file: 0600
• No secrets stored in config (only project ID, zone, theme, aliases)

Secure logging

• Logging disabled by default (--log-level off)
• Tokens never logged (only expiry duration)
• Response bodies truncated to 200 chars
• Error messages sanitized (generic messages for 401/403/404)

Destructive operation protection

• Explicit confirmation required for delete/stop/reset
• default_yes: false for all destructive actions
• --readonly mode enforced at event handler level
• Shell actions (SSH, console) correctly allowed in readonly mode

Path traversal prevention (src/gcp/auth.rs, src/theme/mod.rs)

• Gcloud config names validated (alphanumeric + hyphens only)
• Theme names validated before loading

---

Minor findings (fixed)

#	Finding	Location	Fix
1	Query param keys not URL-encoded (only values)	sdk_dispatch.rs:923,928	Now encoding keys via urlencoding::encode()
2	Monitoring filter only blocked " for instance IDs	sdk_dispatch.rs:728	Now uses strict allowlist (alphanumeric, hyphens, underscores)

Already addressed (no change needed)

#	Item	Status
3	cargo audit in CI	Already present in .github/workflows/security.yml (weekly + on push/PR)
4	Client-side rate limiting / retry with backoff	Already implemented in src/gcp/http.rs (429/502/503/504 with exponential backoff)

Remaining recommendations

• Monitor serde_yaml 0.9 for security advisories (covered by cargo audit in CI)

---

Fix branch: claude/security-review-rnmyt
Security Grade: A