fix(security): replace shell exec with execFile to prevent command injection

[bhaktofmahakal]

Summary

Hardens command execution across three services by replacing exec() (shell-based) with execFile() (array-based, no shell interpretation) to eliminate shell injection vectors. Also replaces weak Math.random() ID generation with cryptographically secure crypto.randomUUID().

Fixes

Fixes shell injection vulnerabilities in GitHubService, RepositoryManager, and weak ID generation in DatabaseService.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Refactor (does not change functionality, e.g. code style improvements, linting)

What Changed

Shell injection prevention (HIGH severity)

• GitHubService.ts: Refactored execGH() helper from exec(commandString) to execFileAsync('gh', argsArray). All 15+ call sites updated. Removed exec import entirely — now uses only execFile and spawn.
• GitHubService.ts: Converted cloneRepository(), initializeNewProject(), ensurePullRequestBranch() from exec() to execFileAsync().
• GitHubService.ts: Converted logout() from shell pipe (echo Y | gh auth logout) to spawn() with stdin write.
• RepositoryManager.ts: Replaced exec(\cd "${path}" && git ...`)withexecFileAsync('git', [...], { cwd: path })`.

Weak ID generation → crypto.randomUUID() (MEDIUM severity)

• DatabaseService.ts: Conversation IDs, comment IDs, and SSH connection IDs now use crypto.randomUUID() instead of Math.random().toString(36).
• RepositoryManager.ts: Repository IDs now use crypto.randomUUID().

Test updates

• GitHubService.test.ts: Updated mocks from exec → execFile API. Added missing electron mock that was causing a pre-existing test suite failure (3 tests now pass that weren't before).

Before / After

// BEFORE — shell injection possible via repoUrl or localPath
await execAsync(`git clone "${repoUrl}" "${localPath}"`);

// AFTER — args passed directly to binary, no shell interpretation
await execFileAsync('git', ['clone', repoUrl, localPath]);

[vercel]

@Copilot is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

[Copilot]

Pull request overview

Hardens process invocation and identifier generation in the Electron main-process services by migrating from shell-based exec() usage to argument-array execFile()/spawn(), reducing command injection risk, and by replacing weak/random IDs with UUIDs.

Changes:

• Replaced shell-interpreted command execution with execFile() (and spawn() for stdin piping) in GitHubService and RepositoryManager.
• Switched several persisted IDs in DatabaseService (and repo IDs in RepositoryManager) from Math.random()-based strings to crypto.randomUUID().
• Updated GitHubService unit tests to mock execFile/spawn and added a missing electron mock.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/main/services/GitHubService.ts	Reworks gh/git command execution to use execFile/spawn (no shell), updates helper API (execGH).
src/main/services/RepositoryManager.ts	Removes shell cd && git ... usage by using execFile with cwd, and switches repo IDs to UUIDs.
src/main/services/DatabaseService.ts	Replaces Math.random()-based identifiers for conversations/comments/SSH connections with UUID-based IDs.
src/test/main/GitHubService.test.ts	Updates mocks and assertions to align with execFile usage and adds electron mocking needed for the suite.

Comments suppressed due to low confidence (1)

src/main/services/GitHubService.ts:768

• getRepositories() still generates GitHubRepo.id via Math.random(), which can collide and is non-deterministic across calls. Since this PR is already migrating other IDs to crypto.randomUUID(), consider switching to a stable/unique identifier here as well (e.g., a UUID or a deterministic hash of nameWithOwner) to avoid key collisions downstream.

      return repos.map((repo: any) => ({
        id: Math.random(), // gh CLI doesn't provide ID, so we generate one
        name: repo.name,

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR hardens command execution across GitHubService, RepositoryManager, and DatabaseService by replacing shell-based exec() with execFile() (args passed as arrays, no shell interpreter involved), and by upgrading weak Math.random() ID generation to crypto.randomUUID(). These are well-motivated security improvements with correct implementation across all call sites.

Key changes:

• GitHubService.ts: execGH() refactored from accepting a raw command string to accepting a string[] args array; all call sites updated; logout() converted from echo Y | gh auth logout shell pipe to spawn() with stdin write; exec import removed entirely.
• RepositoryManager.ts: cd "${path}" && git ... shell chaining replaced with execFileAsync('git', [...], { cwd }). The getDefaultBranch() sed pipe is also eliminated in favor of pure-JS startsWith/slice.
• DatabaseService.ts: Conversation, comment, and SSH connection IDs now use crypto.randomUUID() instead of Math.random().toString(36).
• GitHubService.test.ts: Mocks updated to match the new execFile API; a missing electron mock is added; spawn mock added.

One minor issue in the logout() implementation: non-zero exit codes from gh auth logout are silently ignored without logging, inconsistent with the authenticate() method which properly checks exit codes.

Confidence Score: 4/5

• The PR is safe to merge; the security improvements (exec → execFile, Math.random → crypto.randomUUID) are correctly implemented with no injection vectors remaining. One minor inconsistency in error handling in logout() worth addressing in a follow-up.
• The core exec → execFile refactor is correctly implemented across all call sites with no shell injection vectors remaining. The crypto.randomUUID() upgrade is straightforward and safe. One minor regression exists in the logout() spawn implementation where non-zero exit codes are silently ignored, inconsistent with the authenticate() method's exit code checking, but neither affects correctness for typical usage.
• src/main/services/GitHubService.ts — the logout() implementation should check and log non-zero exit codes for consistency with authenticate()

_{Last reviewed commit: 1da1111}