🛡️ Sentinel: [CRITICAL] Fix predictable temp file vulnerability in apt.sh

[kidchenko]

🚨 Severity: CRITICAL
💡 Vulnerability: Predictable temporary file path (/tmp/yq) used when downloading yq and moving it to /usr/local/bin using sudo. This makes the script vulnerable to symlink attacks, potentially allowing a malicious local user to overwrite arbitrary files as root.
🎯 Impact: Local Privilege Escalation / Arbitrary File Overwrite
🔧 Fix: Switched to using a securely generated temporary directory via mktemp -d, which creates a directory with strict permissions and random name, ensuring only the executing user can access it. Cleaned up the directory after use.
✅ Verification: Ran ./build.sh syntax check and reviewed the diff to ensure the sudo mv command now operates on a securely generated path.

---

PR created automatically by Jules for task 5349592463307284626 started by @kidchenko

Summary by CodeRabbit

• Chores
  • Improved the installation script's temporary file handling to use secure temporary directory creation instead of fixed paths, enhancing reliability and consistency of the installation process.

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change refactors the yq binary installation process in the apt.sh installer script to use a temporary directory created via mktemp instead of directly using /tmp/yq, improving temporary file handling safety while maintaining the same installation behavior.

Changes

Cohort / File(s)	Summary
Temporary File Handling tools/os_installers/apt.sh	Replaces direct /tmp/yq path with mktemp-created temporary directory (YQ_TMPDIR), downloads yq to that directory, then moves to /usr/local/bin/yq, followed by cleanup of the temp directory.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A safer path for yq to land,
No more /tmp—now mktemp's grand!
With tmpdir born, the binary flows,
To /usr/local/bin it goes,
Cleanup follows, tidy and clean! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the main security fix: replacing a predictable temp file with a secure temporary directory to prevent symlink attacks.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch sentinel/fix-tmp-yq-path-5349592463307284626

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/os_installers/apt.sh`:
- Around line 234-238: The temporary dir YQ_TMPDIR created by mktemp can be
orphaned if wget or sudo mv fails due to set -e; add a trap immediately after
creating YQ_TMPDIR to run rm -rf "$YQ_TMPDIR" on EXIT (and optionally on
INT/TERM) so cleanup always runs, ensure mktemp success is checked before
setting the trap, and if you need to preserve the directory on success remove or
reset the trap after the mv/chmod steps; reference the YQ_TMPDIR variable and
the sequence around mktemp, wget, sudo mv, sudo chmod and rm -rf to locate where
to add the trap.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 29e21bcf-92a5-4a04-bae2-1d17c8f4ba3d

📥 Commits

Reviewing files that changed from the base of the PR and between cb5949a and e9e9e33.

📒 Files selected for processing (1)

• tools/os_installers/apt.sh

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether yq temp-dir cleanup is protected by trap.
rg -n -C2 'YQ_TMPDIR|trap .*YQ_TMPDIR|wget .*yq_linux_amd64|sudo mv .*yq|rm -rf "\$YQ_TMPDIR"' tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 406

---

🏁 Script executed:

head -20 tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 579

---

🏁 Script executed:

grep -n "set -e\|set -o errexit" tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 70

---

🏁 Script executed:

sed -n '230,240p' tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 407

---

Add trap to guarantee temp dir cleanup on failure.

With set -e active (line 7), if wget or sudo mv fails at lines 235–236, the script exits immediately and skips the cleanup at line 238, orphaning the temporary directory. Use a trap to ensure cleanup runs unconditionally:

Proposed fix

     YQ_TMPDIR=$(mktemp -d)
+    trap 'rm -rf "$YQ_TMPDIR"' EXIT
     wget "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64" -O "$YQ_TMPDIR/yq"
     sudo mv "$YQ_TMPDIR/yq" /usr/local/bin/yq
     sudo chmod +x /usr/local/bin/yq
-    rm -rf "$YQ_TMPDIR"

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	YQ_TMPDIR=$(mktemp -d)
	wget "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64" -O "$YQ_TMPDIR/yq"
	sudo mv "$YQ_TMPDIR/yq" /usr/local/bin/yq
	sudo chmod +x /usr/local/bin/yq
	rm -rf "$YQ_TMPDIR"
	YQ_TMPDIR=$(mktemp -d)
	trap 'rm -rf "$YQ_TMPDIR"' EXIT
	wget "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64" -O "$YQ_TMPDIR/yq"
	sudo mv "$YQ_TMPDIR/yq" /usr/local/bin/yq
	sudo chmod +x /usr/local/bin/yq

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/os_installers/apt.sh` around lines 234 - 238, The temporary dir
YQ_TMPDIR created by mktemp can be orphaned if wget or sudo mv fails due to set
-e; add a trap immediately after creating YQ_TMPDIR to run rm -rf "$YQ_TMPDIR"
on EXIT (and optionally on INT/TERM) so cleanup always runs, ensure mktemp
success is checked before setting the trap, and if you need to preserve the
directory on success remove or reset the trap after the mv/chmod steps;
reference the YQ_TMPDIR variable and the sequence around mktemp, wget, sudo mv,
sudo chmod and rm -rf to locate where to add the trap.