fix: eliminate shell injection patterns across clis#830
Open
fix: eliminate shell injection patterns across clis#830
Conversation
- spotify/spotify.ts: replace shell-interpolated `open "${url}"` with
execFileSync to prevent URL-based shell injection
- chatgpt/ax.ts: replace execSync shell strings with execFileSync for
osascript calls (activate + delay)
- chatgpt/ask.ts: replace execSync `sleep` with spawnSync to avoid
shell interpolation
- record.ts: fix misleading "idempotent injection" comment — the
function is safe to call multiple times but not strictly idempotent
Same shell injection pattern as fix.ts — prompt with $, backticks, or other metacharacters would be expanded by the shell when interpolated into the command string.
- autoresearch/debug.ts: prompt via stdin instead of shell interpolation - autoresearch/engine.ts: git commit via execFileSync instead of shell string with quote escaping - autoresearch/eval-skill.ts: use execFileSync with argument array, pass prompt via stdin - src/plugin.ts: use execFileSync for esbuild PATH lookup
jackwener
force-pushed
the
fix/shell-injection-cleanup
branch
from
015c159 to
a359d2c
Compare
git add fails with fatal error when a pathspec glob doesn't match any files (e.g. 'extension/src/**/*.ts' in repos without that dir). Try each glob individually and skip non-matching ones silently. Fixes regression from #826.
Windows `start` is a cmd.exe builtin, not an executable —
execFileSync('start', ...) would ENOENT. Route through
cmd.exe with /c flag instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
openBrowser(url)was interpolating external URLs into shell commands (open "${url}"). Replaced withexecFileSync('open', [url])to prevent shell injection via crafted URLs.activateChatGPT()usedexecSyncwith shell string interpolation for osascript calls. Replaced both withexecFileSync('osascript', [...]).execSync(\sleep ${pollInterval}`). Replaced withspawnSync('sleep', [String(pollInterval)])`.Follows up on #826 which fixed the same class of issues in
engine.tsandfix.ts.Test plan
npx tsc --noEmitpassesnpm test— 541 passed, 1 skipped