Skip to content

fix: prevent command injection and fix cross-platform compatibility#19

Open
VivanRajath wants to merge 1 commit intoopen-gitagent:mainfrom
VivanRajath:fix/command-injection-and-cross-platform
Open

fix: prevent command injection and fix cross-platform compatibility#19
VivanRajath wants to merge 1 commit intoopen-gitagent:mainfrom
VivanRajath:fix/command-injection-and-cross-platform

Conversation

@VivanRajath
Copy link

fix: prevent command injection and fix cross-platform compatibility

Problem

Multiple files use execSync() with string interpolation to run git commands. User-controlled inputs (branch names, commit messages, URLs) are interpolated directly into shell strings, enabling arbitrary command execution:

// Before — vulnerable to injection via branch name
function git(args: string, cwd: string): string {
    return execSync(`git ${args}`, { cwd, stdio: "pipe", encoding: "utf-8" }).trim();
}
git(`checkout -b ${branch}`, dir);  // branch = "; rm -rf /" → executes arbitrary command

Additionally:

  • Shell tools (cli, hooks, tool-loader) use sh which doesn't exist on Windows, breaking core command execution on that platform.
  • The hook path traversal guard uses hardcoded / instead of path.sep, completely bypassing the security check on Windows (which uses \).

Solution

1. Command Injection → Safe Argument Arrays

Replaced every execSync with string interpolation with execFileSync using argument arrays. Inputs are passed as separate OS-level arguments and are never interpreted by a shell:

// After — safe, inputs cannot be interpreted as shell commands
function git(args: string[], cwd: string): string {
    return execFileSync("git", args, { cwd, stdio: "pipe", encoding: "utf-8" }).trim();
}
git(["checkout", "-b", branch], dir);  // branch is always treated as a literal string

For compound git operations (e.g., git add && git commit), the single shell command was split into separate execFileSync calls:

// Before — single shell command with string interpolation
execSync(`git add "${memoryPath}" && git commit -m "${commitMsg.replace(/"/g, '\\"')}"`, ...);

// After — separate safe calls, no quote escaping needed
execFileSync("git", ["add", memoryPath], { cwd, stdio: "pipe" });
execFileSync("git", ["commit", "-m", commitMsg], { cwd, stdio: "pipe" });

2. Cross-Platform Shell Execution

Shell-spawning tools now detect the platform and use the appropriate shell:

const isWin = process.platform === "win32";
const child = spawn(isWin ? "cmd" : "sh", isWin ? ["/c", command] : ["-c", command], ...);

3. Path Traversal Guard Fix

-if (!resolvedScript.startsWith(allowedBase + "/") && resolvedScript !== allowedBase) {
+if (!resolvedScript.startsWith(allowedBase + sep) && resolvedScript !== allowedBase) {

Files Changed (12)

File Change
src/session.ts git() helper rewritten + all 15 call sites
src/loader.ts git clone in resolveInheritance() and resolveDependencies()
src/index.ts isGitRepo(), ensureRepo() git init/add/commit
src/tools/memory.ts Memory save git add + commit
src/sandbox.ts git remote get-url origin
src/tools/skill-learner.ts gitCommit() helper + delete action
src/tools/capture-photo.ts Photo commit git add + commit
src/plugin-cli.ts Removed unused execSync import
src/tools/cli.ts Cross-platform shell (sh/cmd)
src/hooks.ts Path traversal guard (path.sep) + cross-platform shell
src/tool-loader.ts Declarative tool scripts cross-platform shell

Not Included

src/voice/server.ts still uses execSync — it's a 113KB monolith that needs a refactor before these changes can be applied safely. Planned for a follow-up PR.

Testing

-tsc --noEmit passes with zero compilation errors
-npm test passes with zero compilation errors

  • No behavioral changes — all functions maintain identical semantics
  • The only difference is that inputs can no longer escape their argument boundaries

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