Description
internal/git/git.go uses safeexec.LookPath to find the git executable, caching the result via sync.Once. This prevents PATH injection attacks on Windows where the current directory is implicitly included in PATH resolution.
internal/config/config.go calls exec.Command("git", ...) directly with a bare string across all of its methods (GetTrunk, SetParent, GetPR, GetForkPoint, GetPRBase, etc.), bypassing this protection.
Suggested Fix
Either:
- Export
resolveGitPath() (or an equivalent) from internal/git so internal/config can share the same cached, safe path, or
- Give
internal/config its own resolveGitPath() using safeexec.LookPath + sync.Once, mirroring the pattern already in internal/git.
Option 1 is cleaner since it avoids duplicating the cache.
Context
Noticed while adding GetPRBase/SetPRBase/RemovePRBase in #95's fix. The new methods follow the same pattern as the existing ones, so this is a pre-existing inconsistency rather than a regression — but worth closing.
Description
internal/git/git.gousessafeexec.LookPathto find thegitexecutable, caching the result viasync.Once. This prevents PATH injection attacks on Windows where the current directory is implicitly included in PATH resolution.internal/config/config.gocallsexec.Command("git", ...)directly with a bare string across all of its methods (GetTrunk,SetParent,GetPR,GetForkPoint,GetPRBase, etc.), bypassing this protection.Suggested Fix
Either:
resolveGitPath()(or an equivalent) frominternal/gitsointernal/configcan share the same cached, safe path, orinternal/configits ownresolveGitPath()usingsafeexec.LookPath+sync.Once, mirroring the pattern already ininternal/git.Option 1 is cleaner since it avoids duplicating the cache.
Context
Noticed while adding
GetPRBase/SetPRBase/RemovePRBasein #95's fix. The new methods follow the same pattern as the existing ones, so this is a pre-existing inconsistency rather than a regression — but worth closing.