Skip to content

feat(security): harden simple-git against malicious repo config RCE#3742

Merged
gregpriday merged 3 commits intodevelopfrom
feature/issue-3704-harden-simple-git-against
Mar 19, 2026
Merged

feat(security): harden simple-git against malicious repo config RCE#3742
gregpriday merged 3 commits intodevelopfrom
feature/issue-3704-harden-simple-git-against

Conversation

@gregpriday
Copy link
Copy Markdown
Collaborator

Summary

  • Introduces createHardenedGit() utility that wraps simpleGit() with config overrides to neutralize dangerous git config options (core.fsmonitor, core.pager, protocol.ext.allow)
  • Strengthens validateCwd() to reject relative paths, enforcing path.isAbsolute() across all git IPC handlers
  • Applies the same hardening to GitHub IPC handlers that accept cwd from the renderer

Resolves #3704

Changes

  • electron/utils/hardenedGit.ts — New module exporting createHardenedGit(cwd) and HARDENED_GIT_CONFIG constant. Disables core.fsmonitor, core.pager, and protocol.ext.allow via -c flags on every git invocation
  • electron/utils/git.tsvalidateCwd() now requires absolute paths via path.isAbsolute()
  • electron/ipc/handlers/git-write.ts — All simpleGit(cwd) calls replaced with createHardenedGit(cwd); validateCwd imported from shared utility
  • electron/ipc/handlers/github.ts — Added path.isAbsolute() validation and createHardenedGit() for all git operations
  • electron/ipc/handlers/projectCrud.ts — Switched to createHardenedGit()
  • electron/ipc/handlers/worktree.ts — Switched to createHardenedGit()
  • electron/services/GitService.ts — Switched to createHardenedGit()
  • electron/utils/__tests__/hardenedGit.test.ts — 136-line test suite covering config overrides, absolute path enforcement, empty/whitespace rejection
  • electron/utils/__tests__/git.test.ts — Updated validateCwd tests for absolute path requirement
  • electron/services/__tests__/GitService.test.ts — Updated mocks to use hardenedGit

Testing

  • All new and updated unit tests pass
  • TypeScript typecheck passes cleanly
  • ESLint and Prettier report no issues

- Create createHardenedGit() factory in electron/utils/hardenedGit.ts with -c config overrides disabling core.fsmonitor, protocol.ext.allow, core.sshCommand, core.gitProxy, core.hooksPath, diff.external, core.askpass, credential.helper
- Move validateCwd() to hardenedGit.ts with path.isAbsolute() enforcement
- Replace all bare simpleGit() calls in git-write.ts (10), git.ts (5), projectCrud.ts (2), GitService.ts (3) with createHardenedGit()
- Add path.isAbsolute() checks to all github.ts handlers (11) and worktree.ts listCommits handler
- Update test mocks in git.test.ts and GitService.test.ts to mock createHardenedGit
- Add comprehensive tests for validateCwd and createHardenedGit in hardenedGit.test.ts
…uction constant

- Replace HARDENED_GIT_CONFIG import with explicit expected key list in test
- Prevents test from silently passing if a security-critical config key is removed from source
@gregpriday
Copy link
Copy Markdown
Collaborator Author

Review status: Rebased and ready

@gregpriday
Copy link
Copy Markdown
Collaborator Author

Cross-PR review note

After reviewing all 12 open PRs in this batch together, these interactions were identified:

@gregpriday gregpriday merged commit f5ec05a into develop Mar 19, 2026
4 checks passed
@gregpriday gregpriday deleted the feature/issue-3704-harden-simple-git-against branch March 19, 2026 03:22
@gregpriday
Copy link
Copy Markdown
Collaborator Author

Setting diff.external= (empty string) made git interpret it as "run an empty program as the diff tool." Fixed in #4221 by removing the broken override and using --no-ext-diff flags instead.

Regression audit for training data.

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.

Harden simple-git against malicious repo config RCE

1 participant