Fix command injection vulnerability in editor launch (execSync → spawnSync)#4
Conversation
…mpFile Co-authored-by: seabearDEV <40605056+seabearDEV@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a command injection vulnerability in the editEntry function by replacing execSync with spawnSync and passing the temporary file path via an environment variable instead of direct string interpolation.
Changes:
- Replaced
execSyncwithspawnSyncto launch the editor, isolating the temp file path from shell word-splitting and expansion by passing it via theCODEX_TMPFILEenvironment variable
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/commands/entries.ts
Outdated
| const result = spawnSync('sh', ['-c', `${editor} "$CODEX_TMPFILE"`], { | ||
| stdio: 'inherit', | ||
| env: { ...process.env, CODEX_TMPFILE: tmpFile }, | ||
| }); | ||
| if (result.error) throw result.error; |
There was a problem hiding this comment.
The hardcoded 'sh' shell will fail on Windows systems. Consider using process.platform === 'win32' ? 'cmd' : 'sh' or following the pattern from line 107 which uses process.env.SHELL ?? '/bin/sh' to respect the user's configured shell. However, note that Windows may require different handling (e.g., using 'cmd' with /c or 'powershell' with -Command).
There was a problem hiding this comment.
Applied in commit f15d1bb — shell is now cmd /c on Windows (with %CODEX_TMPFILE%) and $SHELL ?? '/bin/sh' with -c on Unix (with "$CODEX_TMPFILE").
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: seabearDEV <40605056+seabearDEV@users.noreply.github.com>
Applied both changes in commit f15d1bb:
|
|
@copilot apply changes based on the comments in this thread |
These changes were already applied in commit f15d1bb — Windows-aware shell selection and the |
83b4c59
into
claude/verify-codexcli-readme-qVZ44
execSyncwas used to launch the user's editor by interpolating$EDITOR/$VISUALand the temp file path directly into a shell string, making it vulnerable to failures or exploitation if either contained spaces or shell metacharacters.Changes
src/commands/entries.ts: ReplaceexecSyncwithspawnSync, passing the temp file path via a dedicated env var (CODEX_TMPFILE) and referencing it with double-quoting in the shell command. The shell is selected based on the current platform (cmd /con Windows,$SHELL ?? '/bin/sh'on Unix). A check onresult.statusensures that non-zero editor exit codes (e.g. user cancels) are treated as errors rather than silent successes:This isolates
tmpFilefrom shell word-splitting and expansion, works correctly on both Unix and Windows, and propagates editor failures properly.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.