refactor: Security fixes, complexity reduction, and UX improvements (#58)

Security:
- Fix command injection in modal/lib/common.sh (run_server, upload_file, interactive_session)
- Fix command injection in fly/lib/common.sh (run_server, upload_file, interactive_session)
- All container providers now use printf '%q' for proper shell escaping

Complexity:
- Extract _api_should_retry_on_error() helper in shared/common.sh (-19 lines)
- Refactor scaleway_api and upcloud_api to use shared retry helper (-24 lines)
- Extract _save_fly_token() helper in fly/lib/common.sh (-11 lines)
- Extract validateAndGetAgent() in commands.ts, reducing cmdRun/cmdAgentInfo duplication
- Refactor cmdList column width calculation to use calculateColumnWidth()

UX:
- Add actionable next steps to error messages in shared/common.sh
- Improve CLI bash fallback error messages with guidance (spawn.sh)
- Add OAuth progress indicator during browser authentication wait
- Show invalid model ID value and link to openrouter.ai/models
- Add troubleshooting steps for agent installation failures

Tests:
- Update test assertions in test/run.sh to match refactored patterns
- All tests passing: 74 TypeScript + 75 bash = 149 total, 0 failures

Co-authored-by: A <6723574+louisgv@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
A 2026-02-08 17:09:27 -08:00 committed by GitHub
parent 44cafc7cc5
commit bbbe815035
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 164 additions and 129 deletions

View file

@ -232,11 +232,15 @@ ensure_manifest() {
# Offline fallback: use stale cache if available
if [[ -f "${SPAWN_MANIFEST}" ]]; then
log_warn "Using cached manifest (offline fallback)"
log_warn "Using cached manifest (offline or network issue)"
return 0
fi
log_error "No manifest available. Check your internet connection."
log_error "No manifest available and no cached version found"
echo "" >&2
echo "Check your internet connection and try again." >&2
echo "If the problem persists, file an issue at:" >&2
echo " https://github.com/${SPAWN_REPO}/issues" >&2
exit 1
}
@ -334,18 +338,25 @@ cmd_run() {
if [[ -z "${agent_name}" ]]; then
log_error "Unknown agent: ${agent}"
echo "Run 'spawn agents' to see available agents." >&2
echo "" >&2
echo "Run 'spawn agents' to see all available agents." >&2
echo "Run 'spawn help' for usage information." >&2
exit 1
fi
if [[ -z "${cloud_name}" ]]; then
log_error "Unknown cloud: ${cloud}"
echo "Run 'spawn clouds' to see available clouds." >&2
echo "" >&2
echo "Run 'spawn clouds' to see all available clouds." >&2
echo "Run 'spawn help' for usage information." >&2
exit 1
fi
status=$(manifest_matrix_status "${cloud}" "${agent}")
if [[ "${status}" != "implemented" ]]; then
log_error "${agent_name} on ${cloud_name} is not yet implemented"
echo "" >&2
echo "Run 'spawn list' to see all available combinations." >&2
echo "Run 'spawn ${agent}' to see which clouds support ${agent_name}." >&2
exit 1
fi
@ -633,7 +644,9 @@ main() {
agent_name=$(manifest_agent_name "${agent}")
if [[ -z "${agent_name}" ]]; then
log_error "Unknown command or agent: ${agent}"
echo "Run 'spawn help' for usage." >&2
echo "" >&2
echo "Run 'spawn agents' to see all available agents." >&2
echo "Run 'spawn help' for usage information." >&2
exit 1
fi

View file

@ -98,6 +98,24 @@ function validateAgent(manifest: Manifest, agent: string): asserts agent is keyo
}
}
// Validate and load agent - consolidates the pattern used by cmdRun and cmdAgentInfo
function validateAndGetAgent(agent: string): Promise<[manifest: Manifest, agentKey: string]> {
return (async () => {
try {
validateIdentifier(agent, "Agent name");
} catch (err) {
p.log.error(getErrorMessage(err));
process.exit(1);
}
validateNonEmptyString(agent, "Agent name", "spawn agents");
const manifest = await loadManifestWithSpinner();
validateAgent(manifest, agent);
return [manifest, agent];
})();
}
function validateCloud(manifest: Manifest, cloud: string): asserts cloud is keyof typeof manifest.clouds {
if (!manifest.clouds[cloud]) {
p.log.error(`Unknown cloud: ${pc.bold(cloud)}`);
@ -165,16 +183,14 @@ export async function cmdRun(agent: string, cloud: string, prompt?: string): Pro
process.exit(1);
}
validateNonEmptyString(agent, "Agent name", "spawn agents");
validateNonEmptyString(cloud, "Cloud name", "spawn clouds");
const manifest = await loadManifestWithSpinner();
const [manifest, agentKey] = await validateAndGetAgent(agent);
validateAgent(manifest, agent);
validateCloud(manifest, cloud);
validateImplementation(manifest, cloud, agent);
validateImplementation(manifest, cloud, agentKey);
const agentName = manifest.agents[agent].name;
const agentName = manifest.agents[agentKey].name;
const cloudName = manifest.clouds[cloud].name;
if (prompt) {
@ -183,7 +199,7 @@ export async function cmdRun(agent: string, cloud: string, prompt?: string): Pro
p.log.step(`Launching ${pc.bold(agentName)} on ${pc.bold(cloudName)}...`);
}
await execScript(cloud, agent, prompt);
await execScript(cloud, agentKey, prompt);
}
async function downloadScriptWithFallback(primaryUrl: string, fallbackUrl: string): Promise<string> {
@ -316,22 +332,15 @@ export async function cmdList(): Promise<void> {
const agents = agentKeys(manifest);
const clouds = cloudKeys(manifest);
// Calculate column widths without creating intermediate arrays
let agentColWidth = MIN_AGENT_COL_WIDTH;
for (const a of agents) {
const width = manifest.agents[a].name.length + COL_PADDING;
if (width > agentColWidth) {
agentColWidth = width;
}
}
let cloudColWidth = MIN_CLOUD_COL_WIDTH;
for (const c of clouds) {
const width = manifest.clouds[c].name.length + COL_PADDING;
if (width > cloudColWidth) {
cloudColWidth = width;
}
}
// Calculate column widths
const agentColWidth = calculateColumnWidth(
agents.map((a) => manifest.agents[a].name),
MIN_AGENT_COL_WIDTH
);
const cloudColWidth = calculateColumnWidth(
clouds.map((c) => manifest.clouds[c].name),
MIN_CLOUD_COL_WIDTH
);
console.log();
console.log(renderMatrixHeader(clouds, manifest, agentColWidth, cloudColWidth));
@ -381,21 +390,9 @@ export async function cmdClouds(): Promise<void> {
// ── Agent Info ─────────────────────────────────────────────────────────────────
export async function cmdAgentInfo(agent: string): Promise<void> {
// SECURITY: Validate input argument for injection attacks
try {
validateIdentifier(agent, "Agent name");
} catch (err) {
p.log.error(getErrorMessage(err));
process.exit(1);
}
const [manifest, agentKey] = await validateAndGetAgent(agent);
validateNonEmptyString(agent, "Agent name", "spawn agents");
const manifest = await loadManifestWithSpinner();
validateAgent(manifest, agent);
const a = manifest.agents[agent];
const a = manifest.agents[agentKey];
console.log();
console.log(`${pc.bold(a.name)} ${pc.dim("\u2014")} ${a.description}`);
console.log();
@ -404,10 +401,10 @@ export async function cmdAgentInfo(agent: string): Promise<void> {
let found = false;
for (const cloud of cloudKeys(manifest)) {
const status = matrixStatus(manifest, cloud, agent);
const status = matrixStatus(manifest, cloud, agentKey);
if (status === "implemented") {
const c = manifest.clouds[cloud];
console.log(` ${pc.green(c.name.padEnd(NAME_COLUMN_WIDTH))} ${pc.dim("spawn " + agent + " " + cloud)}`);
console.log(` ${pc.green(c.name.padEnd(NAME_COLUMN_WIDTH))} ${pc.dim("spawn " + agentKey + " " + cloud)}`);
found = true;
}
}