Skip to content

Commit 6ec0401

Browse files
committed
chore(security): refactor command execution to prevent shell injection
Replace shell-based command execution with safer spawn-based approach in plugin and repository managers. Add new runSafeCommand utility that passes arguments as an array instead of shell strings, eliminating command injection vulnerabilities. Update multiple dependencies including @modelcontextprotocol/sdk, firebase, openai, and @vscode/vsce.
1 parent 5155712 commit 6ec0401

17 files changed

Lines changed: 217 additions & 105 deletions

.github/workflows/vscode-release.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ jobs:
3737
bun run package
3838
3939
- name: Create Release
40-
uses: softprops/action-gh-release@v1
40+
uses: softprops/action-gh-release@v2
4141
with:
42-
files: vscode-extension/*.vsix
42+
files: |
43+
vscode-extension/dist/*.vsix
4344
generate_release_notes: true
45+
fail_on_unmatched_files: true

bun.lock

Lines changed: 54 additions & 33 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,23 @@
6363
"test:watch": "vitest",
6464
"typecheck": "tsc --noEmit"
6565
},
66+
"overrides": {
67+
"@modelcontextprotocol/sdk": "^1.26.0",
68+
"brace-expansion": "^2.0.1",
69+
"minimatch": "^9.0.5"
70+
},
6671
"dependencies": {
6772
"@google/generative-ai": "^0.24.1",
68-
"@modelcontextprotocol/sdk": "^1.25.3",
73+
"@modelcontextprotocol/sdk": "^1.26.0",
6974
"@types/mime": "^4.0.0",
7075
"axios": "^1.13.4",
7176
"bcrypt": "^6.0.0",
7277
"cfonts": "^3.3.1",
7378
"chalk": "^5.6.2",
7479
"commander": "^14.0.3",
7580
"conventional-changelog-cli": "^5.0.0",
76-
"dotenv": "^17.2.3",
77-
"firebase": "^12.8.0",
81+
"dotenv": "^17.2.4",
82+
"firebase": "^12.9.0",
7883
"fs-extra": "^11.3.3",
7984
"ignore": "^7.0.5",
8085
"ink": "^6.6.0",
@@ -83,7 +88,7 @@
8388
"marked-terminal": "^7.3.0",
8489
"mime": "^4.1.0",
8590
"open": "^11.0.0",
86-
"openai": "^6.17.0",
91+
"openai": "^6.18.0",
8792
"react": "^19.2.4",
8893
"ripgrep-node": "^1.0.0",
8994
"shell-quote": "^1.8.3",
@@ -92,7 +97,7 @@
9297
},
9398
"devDependencies": {
9499
"@types/fs-extra": "^11.0.4",
95-
"@types/node": "^25.1.0",
100+
"@types/node": "^25.2.1",
96101
"@types/react": "18.3.27",
97102
"@types/ws": "^8.18.1",
98103
"@typescript-eslint/eslint-plugin": "^8.54.0",
@@ -102,7 +107,7 @@
102107
"prettier": "^3.8.1",
103108
"prettier-plugin-organize-imports": "^4.3.0",
104109
"prettier-plugin-packagejson": "^3.0.0",
105-
"prettier-plugin-sort-imports": "^1.8.9",
110+
"prettier-plugin-sort-imports": "^1.8.10",
106111
"react-devtools-core": "^7.0.1",
107112
"tsx": "^4.21.0",
108113
"typescript": "^5.9.3",

src/hooks/use-input-handler.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -442,14 +442,21 @@ export function useInputHandler({
442442

443443
// Strip placeholders like <name>, [action], etc.
444444
// Examples: "/chat save <name>" -> "/chat save ", "/mcp <action>" -> "/mcp "
445-
let completedCommand = selectedCommand.command;
446-
447-
// Remove everything starting from the first space followed by a placeholder
448-
// or just placeholders themselves
449-
completedCommand = completedCommand
450-
.replace(/\s*<[^>]+>/g, "")
451-
.replace(/\s*\[[^\]]+\]/g, "");
445+
const stripCommandPlaceholders = (command: string): string => {
446+
let previous: string;
447+
let current = command;
448+
do {
449+
previous = current;
450+
current = current
451+
.replace(/\s*<[^>]+>/g, "")
452+
.replace(/\s*\[[^\]]+\]/g, "");
453+
} while (current !== previous);
454+
return current;
455+
};
452456

457+
let completedCommand = stripCommandPlaceholders(
458+
selectedCommand.command,
459+
);
453460
// If it doesn't end with a space and it was a placeholder-heavy command, add a space
454461
if (
455462
!completedCommand.endsWith(" ") &&

src/plugins/manager.ts

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
import { getSettingsManager } from "../utils/settings-manager";
22
import { SuperAgent } from "../agent/super-agent";
3+
import { runSafeCommand } from "../utils/exec";
34
import { SuperAgentPlugin } from "./types";
45
import { SuperAgentTool } from "../types";
5-
import { exec } from "child_process";
6-
import { promisify } from "util";
6+
import execAsync from "node:process";
77
import * as path from "path";
88
import fs from "fs-extra";
99

10-
const execAsync = promisify(exec);
11-
1210
export class PluginManager {
1311
private static instance: PluginManager;
1412
private plugins: Map<string, SuperAgentPlugin> = new Map();
@@ -156,13 +154,15 @@ export class PluginManager {
156154
`Repository already cloned at ${targetDir}. Pulling latest...`,
157155
);
158156
try {
159-
await execAsync(`cd "${targetDir}" && git pull`);
157+
await runSafeCommand("git", ["pull"], {
158+
cwd: targetDir,
159+
});
160160
} catch (error) {
161161
console.warn(`Failed to pull updates: ${error}`);
162162
}
163163
} else {
164164
// Clone the repository
165-
await execAsync(`git clone "${gitUrl}" "${targetDir}"`);
165+
await runSafeCommand("git", ["clone", gitUrl, targetDir]);
166166
}
167167

168168
// Try to build if package.json exists
@@ -174,20 +174,18 @@ export class PluginManager {
174174
const hasBun = await fs.pathExists(path.join(targetDir, "bun.lockb"));
175175
const hasYarn = await fs.pathExists(path.join(targetDir, "yarn.lock"));
176176

177-
const installCmd = hasBun
178-
? "bun install"
179-
: hasYarn
180-
? "yarn install"
181-
: "npm install";
182-
await execAsync(`cd "${targetDir}" && ${installCmd}`);
177+
const installCmd = hasBun ? "bun" : hasYarn ? "yarn" : "npm";
178+
const installArgs = ["install"];
179+
await runSafeCommand(installCmd, installArgs, {
180+
cwd: targetDir,
181+
});
183182

184183
console.log("🔨 Building plugin...");
185-
const buildCmd = hasBun
186-
? "bun run build"
187-
: hasYarn
188-
? "yarn build"
189-
: "npm run build";
190-
await execAsync(`cd "${targetDir}" && ${buildCmd}`);
184+
const buildCmd = hasBun ? "bun" : hasYarn ? "yarn" : "npm";
185+
const buildArgs = ["run", "build"];
186+
await runSafeCommand(buildCmd, buildArgs, {
187+
cwd: targetDir,
188+
});
191189
} catch (error: any) {
192190
console.warn(`Build failed: ${error.message}`);
193191
}
@@ -222,20 +220,18 @@ export class PluginManager {
222220
path.join(absolutePath, "yarn.lock"),
223221
);
224222

225-
const installCmd = hasBun
226-
? "bun install"
227-
: hasYarn
228-
? "yarn install"
229-
: "npm install";
230-
await execAsync(`cd "${absolutePath}" && ${installCmd}`);
223+
const installCmd = hasBun ? "bun" : hasYarn ? "yarn" : "npm";
224+
const installArgs = ["install"];
225+
await runSafeCommand(installCmd, installArgs, {
226+
cwd: absolutePath,
227+
});
231228

232229
console.log("🔨 Building plugin...");
233-
const buildCmd = hasBun
234-
? "bun run build"
235-
: hasYarn
236-
? "yarn build"
237-
: "npm run build";
238-
await execAsync(`cd "${absolutePath}" && ${buildCmd}`);
230+
const buildCmd = hasBun ? "bun" : hasYarn ? "yarn" : "npm";
231+
const buildArgs = ["run", "build"];
232+
await runSafeCommand(buildCmd, buildArgs, {
233+
cwd: absolutePath,
234+
});
239235
} catch (error: any) {
240236
console.warn(`Build failed: ${error.message}`);
241237
}

src/plugins/repository-manager.ts

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
import { exec } from "child_process";
2-
import { promisify } from "util";
1+
import { runSafeCommand } from "../utils/exec";
32
import * as path from "path";
43
import fs from "fs-extra";
54

6-
const execAsync = promisify(exec);
7-
85
export interface RepositoryConfig {
96
type: "agents" | "skills" | "hooks" | "mcp";
107
url: string;
@@ -89,27 +86,35 @@ export class RepositoryManager {
8986

9087
if (!(await fs.pathExists(gitDirPath))) {
9188
// Not in a git repo, just clone instead of using submodule
92-
await execAsync(`git clone ${repoConfig.url} ${targetDir}`, {
89+
await runSafeCommand("git", ["clone", repoConfig.url, targetDir], {
9390
cwd: process.cwd(),
9491
});
9592
} else {
9693
// Add as git submodule
9794
try {
98-
await execAsync(`git submodule add ${repoConfig.url} ${targetDir}`, {
99-
cwd: process.cwd(),
100-
});
95+
await runSafeCommand(
96+
"git",
97+
["submodule", "add", repoConfig.url, targetDir],
98+
{
99+
cwd: process.cwd(),
100+
},
101+
);
101102
} catch (submoduleError: any) {
102103
// If submodule add fails, fall back to regular clone
103-
await execAsync(`git clone ${repoConfig.url} ${targetDir}`, {
104+
await runSafeCommand("git", ["clone", repoConfig.url, targetDir], {
104105
cwd: process.cwd(),
105106
});
106107
}
107108

108109
// Update .gitmodules if needed
109110
try {
110-
await execAsync(`git submodule update --init --recursive`, {
111-
cwd: process.cwd(),
112-
});
111+
await runSafeCommand(
112+
"git",
113+
["submodule", "update", "--init", "--recursive"],
114+
{
115+
cwd: process.cwd(),
116+
},
117+
);
113118
} catch {
114119
// Ignore update errors
115120
}
@@ -134,9 +139,13 @@ export class RepositoryManager {
134139
const targetDir = path.join(this.reposDir, config.type);
135140
try {
136141
if (await fs.pathExists(targetDir)) {
137-
await execAsync(`git pull origin ${config.branch || "main"}`, {
138-
cwd: targetDir,
139-
});
142+
await runSafeCommand(
143+
"git",
144+
["pull", "origin", config.branch || "main"],
145+
{
146+
cwd: targetDir,
147+
},
148+
);
140149
updated.push(key);
141150
}
142151
} catch (error: any) {

src/utils/confirmation-service.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
1-
import { exec, spawn } from "child_process";
21
import { strict as assert } from "assert";
2+
import { spawn } from "child_process";
33
import { EventEmitter } from "events";
4-
import { promisify } from "util";
5-
6-
const execAsync = promisify(exec);
74

85
export interface ConfirmationOptions {
96
operation: string;

src/utils/exec.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { spawn } from "child_process";
2+
3+
/**
4+
* Options for executing a command
5+
*/
6+
export interface ExecOptions {
7+
cwd?: string;
8+
env?: NodeJS.ProcessEnv;
9+
input?: string;
10+
}
11+
12+
/**
13+
* Result of a command execution
14+
*/
15+
export interface ExecResult {
16+
stdout: string;
17+
stderr: string;
18+
}
19+
20+
/**
21+
* Safely execute a command using child_process.spawn.
22+
* This avoids shell injection vulnerabilities by passing arguments as an array
23+
* and disabling shell execution by default.
24+
*/
25+
export async function runSafeCommand(
26+
command: string,
27+
args: string[],
28+
options: ExecOptions = {},
29+
): Promise<ExecResult> {
30+
return new Promise((resolve, reject) => {
31+
const child = spawn(command, args, {
32+
cwd: options.cwd || process.cwd(),
33+
env: { ...process.env, ...options.env },
34+
shell: false, // Disabling shell execution prevents injection
35+
});
36+
37+
let stdout = "";
38+
let stderr = "";
39+
40+
if (child.stdout) {
41+
child.stdout.on("data", data => {
42+
stdout += data.toString();
43+
});
44+
}
45+
46+
if (child.stderr) {
47+
child.stderr.on("data", data => {
48+
stderr += data.toString();
49+
});
50+
}
51+
52+
if (options.input && child.stdin) {
53+
child.stdin.write(options.input);
54+
child.stdin.end();
55+
}
56+
57+
child.on("close", code => {
58+
if (code === 0) {
59+
resolve({ stdout, stderr });
60+
} else {
61+
const error = new Error(
62+
`Command "${command} ${args.join(" ")}" failed with exit code ${code}.
63+
Stderr: ${stderr}`,
64+
);
65+
(error as any).code = code;
66+
(error as any).stdout = stdout;
67+
(error as any).stderr = stderr;
68+
reject(error);
69+
}
70+
});
71+
72+
child.on("error", err => {
73+
reject(err);
74+
});
75+
});
76+
}
201 KB
Loading
177 KB
Loading

0 commit comments

Comments
 (0)