Skip to content

fixed: download template failed due to spaces in project name#14

Merged
jwfing merged 1 commit intomainfrom
fix/command-args
Mar 7, 2026
Merged

fixed: download template failed due to spaces in project name#14
jwfing merged 1 commit intomainfrom
fix/command-args

Conversation

@jwfing
Copy link
Contributor

@jwfing jwfing commented Mar 7, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of special characters in project configuration parameters to ensure more reliable command execution.

@coderabbitai
Copy link

coderabbitai bot commented Mar 7, 2026

Walkthrough

The downloadTemplate function in src/commands/create.ts now uses a new esc function to safely escape and wrap shell command arguments in single quotes, replacing direct string interpolation to mitigate shell injection risks.

Changes

Cohort / File(s) Summary
Shell Command Safety
src/commands/create.ts
Added esc function to escape single quotes and wrap arguments for safe shell command construction. Command now escapes targetDir, projectConfig.oss_host, and anonKey arguments to npx create-insforge-app.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 With quotes now safe and arguments bright,
No shells shall crack from injection's bite,
The rabbit hops with glee today—
Security wrapped in a single-quote way! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix in the changeset: adding safe shell-escaping to handle spaces and special characters in project names in the template download command.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/command-args

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/commands/create.ts (1)

265-273: ⚠️ Potential issue | 🟠 Major

Switch to execFile with an argv array to fix Windows compatibility.

This still breaks on Windows. exec() uses cmd.exe on Windows, which does not recognize single quotes ('...') for argument quoting—only double quotes ("). Arguments with spaces (like "My App") will still fail. To fix spaces reliably across all platforms, avoid shell parsing entirely by using execFile with an argv array.

Suggested fix
-import { exec } from 'node:child_process';
+import { execFile } from 'node:child_process';
 ...
-const execAsync = promisify(exec);
+const execFileAsync = promisify(execFile);
 ...
-    const esc = (s: string) => `'${s.replace(/'/g, "'\\''")}'`;
-    const command = `npx create-insforge-app ${esc(targetDir)} --frame ${frame} --base-url ${esc(projectConfig.oss_host)} --anon-key ${esc(anonKey)} --skip-install`;
+    const cmd = process.platform === 'win32' ? 'npx.cmd' : 'npx';
+    const args = [
+      'create-insforge-app',
+      targetDir,
+      '--frame',
+      frame,
+      '--base-url',
+      projectConfig.oss_host,
+      '--anon-key',
+      anonKey,
+      '--skip-install',
+    ];
 ...
-    await execAsync(command, {
+    await execFileAsync(cmd, args, {
       maxBuffer: 10 * 1024 * 1024,
       cwd: tempDir,
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/create.ts` around lines 265 - 273, The current use of execAsync
with a shell-quoted command (variables: esc, command) breaks on Windows; replace
this with a shell-free execFile call so arguments are passed as an argv array.
Remove the manual quoting via esc and instead call execFile (or a promisified
execFile) with executable "npx" and args ["create-insforge-app", targetDir,
"--frame", frame, "--base-url", projectConfig.oss_host, "--anon-key", anonKey,
"--skip-install"], preserving options {cwd: tempDir, maxBuffer: 10*1024*1024};
ensure you import or use the promisified execFile equivalent and update the call
site that currently uses execAsync(command, ...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/commands/create.ts`:
- Around line 265-273: The current use of execAsync with a shell-quoted command
(variables: esc, command) breaks on Windows; replace this with a shell-free
execFile call so arguments are passed as an argv array. Remove the manual
quoting via esc and instead call execFile (or a promisified execFile) with
executable "npx" and args ["create-insforge-app", targetDir, "--frame", frame,
"--base-url", projectConfig.oss_host, "--anon-key", anonKey, "--skip-install"],
preserving options {cwd: tempDir, maxBuffer: 10*1024*1024}; ensure you import or
use the promisified execFile equivalent and update the call site that currently
uses execAsync(command, ...).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d86f3825-bc30-468a-b8a9-e7326ff3ace2

📥 Commits

Reviewing files that changed from the base of the PR and between 70564d5 and a0c0ef5.

📒 Files selected for processing (1)
  • src/commands/create.ts

@jwfing jwfing merged commit ecbafda into main Mar 7, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant