Conversation
install_rofi_greenclip and create_gruvbox_terminal_profile downloaded to fixed paths in the world-writable /tmp and then executed or installed the result -- create_gruvbox_terminal_profile ran 'bash /tmp/create-gruvbox' directly. Another local user can pre-create or replace those paths between the download and the use. 'mktemp -d' gives a 0700 directory owned by the invoking user, which closes the window. Clean up afterwards so repeated runs do not litter. install_ripgrep has the same pattern; it is dropped entirely on the ripgrep-from-apt branch rather than fixed here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new temp directories are only cleaned up on the success path; adding failure-safe cleanup (e.g., an EXIT trap scoped via subshell) would match the stated cleanup intent and avoid leaving artifacts behind on errors.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens the setup bootstrap script by avoiding predictable, world-writable /tmp/... staging paths for downloaded artifacts and instead staging downloads in per-run mktemp -d directories to mitigate local TOCTOU/symlink attacks.
Changes:
- Stage the greenclip binary download in a
mktemp -ddirectory before installing to/usr/local/bin. - Stage the
create-gruvboxscript download in amktemp -ddirectory before executing it. - Remove the temporary staging directory after use.
File summaries
| File | Description |
|---|---|
setup |
Replaces fixed /tmp download targets with mktemp -d staging directories for safer download-and-use flows. |
Review details
Suppressed comments (1)
setup:556
- Cleanup only happens on the success path; if the downloaded script fails (or download fails) under
set -e, the temp directory will be left behind. Using a subshell with anEXITtrap ensures the mktemp directory is removed even when the function aborts early.
local tmp
tmp=$(mktemp -d)
get_prestobuntu_file "create-gruvbox" "$tmp/create-gruvbox"
bash "$tmp/create-gruvbox"
rm -rf "$tmp"
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Temporary directory cleanup in create_gruvbox_terminal_profile is not guaranteed on error due to set -e, which undermines the PR’s stated cleanup goal.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
| local tmp | ||
| tmp=$(mktemp -d) | ||
| get_prestobuntu_file "create-gruvbox" "$tmp/create-gruvbox" | ||
| bash "$tmp/create-gruvbox" | ||
| rm -rf "$tmp" |
install_rofi_greenclip and create_gruvbox_terminal_profile downloaded to
fixed paths in the world-writable /tmp and then executed or installed
the result -- create_gruvbox_terminal_profile ran 'bash /tmp/create-gruvbox'
directly. Another local user can pre-create or replace those paths
between the download and the use.
'mktemp -d' gives a 0700 directory owned by the invoking user, which
closes the window. Clean up afterwards so repeated runs do not litter.
install_ripgrep has the same pattern; it is dropped entirely on the
ripgrep-from-apt branch rather than fixed here.
🤖 Generated with Claude Code