Skip to content

Commit 90e0106

Browse files
committed
fix: use execFile instead of exec to prevent command injection
1 parent d53c066 commit 90e0106

File tree

1 file changed

+40
-20
lines changed

1 file changed

+40
-20
lines changed

packages/agent/src/worktree-manager.ts

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import { exec, execFile } from "node:child_process";
1+
import { execFile } from "node:child_process";
22
import * as crypto from "node:crypto";
33
import * as fs from "node:fs/promises";
44
import * as path from "node:path";
55
import { promisify } from "node:util";
66
import type { WorktreeInfo } from "./types.js";
77
import { Logger } from "./utils/logger.js";
88

9-
const execAsync = promisify(exec);
109
const execFileAsync = promisify(execFile);
1110

1211
export interface WorktreeConfig {
@@ -512,14 +511,14 @@ export class WorktreeManager {
512511
return this.worktreeBasePath !== null;
513512
}
514513

515-
private async runGitCommand(command: string): Promise<string> {
514+
private async runGitCommand(args: string[]): Promise<string> {
516515
try {
517-
const { stdout } = await execAsync(`git ${command}`, {
516+
const { stdout } = await execFileAsync("git", args, {
518517
cwd: this.mainRepoPath,
519518
});
520519
return stdout.trim();
521520
} catch (error) {
522-
throw new Error(`Git command failed: ${command}\n${error}`);
521+
throw new Error(`Git command failed: git ${args.join(" ")}\n${error}`);
523522
}
524523
}
525524

@@ -607,9 +606,9 @@ export class WorktreeManager {
607606
private async getDefaultBranch(): Promise<string> {
608607
// Try all methods in parallel for speed
609608
const [symbolicRef, mainExists, masterExists] = await Promise.allSettled([
610-
this.runGitCommand("symbolic-ref refs/remotes/origin/HEAD"),
611-
this.runGitCommand("rev-parse --verify main"),
612-
this.runGitCommand("rev-parse --verify master"),
609+
this.runGitCommand(["symbolic-ref", "refs/remotes/origin/HEAD"]),
610+
this.runGitCommand(["rev-parse", "--verify", "main"]),
611+
this.runGitCommand(["rev-parse", "--verify", "master"]),
613612
]);
614613

615614
// Prefer symbolic ref (most accurate)
@@ -681,15 +680,27 @@ export class WorktreeManager {
681680
const gitStart = Date.now();
682681
if (this.usesExternalPath()) {
683682
// Use absolute path for external worktrees
684-
await this.runGitCommand(
685-
`worktree add --quiet -b "${branchName}" "${worktreePath}" "${baseBranch}"`,
686-
);
683+
await this.runGitCommand([
684+
"worktree",
685+
"add",
686+
"--quiet",
687+
"-b",
688+
branchName,
689+
worktreePath,
690+
baseBranch,
691+
]);
687692
} else {
688693
// Use relative path from repo root for in-repo worktrees
689-
const relativePath = `${WORKTREE_FOLDER_NAME}/${worktreeName}`;
690-
await this.runGitCommand(
691-
`worktree add --quiet -b "${branchName}" "./${relativePath}" "${baseBranch}"`,
692-
);
694+
const relativePath = `./${WORKTREE_FOLDER_NAME}/${worktreeName}`;
695+
await this.runGitCommand([
696+
"worktree",
697+
"add",
698+
"--quiet",
699+
"-b",
700+
branchName,
701+
relativePath,
702+
baseBranch,
703+
]);
693704
}
694705
const gitTime = Date.now() - gitStart;
695706

@@ -784,7 +795,7 @@ export class WorktreeManager {
784795
try {
785796
await fs.rm(worktreePath, { recursive: true, force: true });
786797
// Also prune the worktree list
787-
await this.runGitCommand("worktree prune");
798+
await this.runGitCommand(["worktree", "prune"]);
788799
this.logger.info("Worktree cleaned up manually", { worktreePath });
789800
} catch (cleanupError) {
790801
this.logger.error("Failed to cleanup worktree", {
@@ -799,7 +810,11 @@ export class WorktreeManager {
799810
async getWorktreeInfo(worktreePath: string): Promise<WorktreeInfo | null> {
800811
try {
801812
// Parse the worktree list to find info about this worktree
802-
const output = await this.runGitCommand("worktree list --porcelain");
813+
const output = await this.runGitCommand([
814+
"worktree",
815+
"list",
816+
"--porcelain",
817+
]);
803818
const worktrees = this.parseWorktreeList(output);
804819

805820
const worktree = worktrees.find((w) => w.worktreePath === worktreePath);
@@ -812,7 +827,11 @@ export class WorktreeManager {
812827

813828
async listWorktrees(): Promise<WorktreeInfo[]> {
814829
try {
815-
const output = await this.runGitCommand("worktree list --porcelain");
830+
const output = await this.runGitCommand([
831+
"worktree",
832+
"list",
833+
"--porcelain",
834+
]);
816835
return this.parseWorktreeList(output);
817836
} catch (error) {
818837
this.logger.debug("Failed to list worktrees", { error });
@@ -862,8 +881,9 @@ export class WorktreeManager {
862881

863882
async isWorktree(repoPath: string): Promise<boolean> {
864883
try {
865-
const { stdout } = await execAsync(
866-
"git rev-parse --is-inside-work-tree",
884+
const { stdout } = await execFileAsync(
885+
"git",
886+
["rev-parse", "--is-inside-work-tree"],
867887
{ cwd: repoPath },
868888
);
869889
if (stdout.trim() !== "true") {

0 commit comments

Comments
 (0)