🛡️ Sentinel: [CRITICAL] Fix symlink attack vulnerability in apt installer#56
🛡️ Sentinel: [CRITICAL] Fix symlink attack vulnerability in apt installer#56
Conversation
Changed `tools/os_installers/apt.sh` to use `mktemp -d` instead of the predictable `/tmp/yq` hardcoded path when downloading and moving binaries. This prevents malicious local users from exploiting the `sudo mv` operation to overwrite critical files via symlink attacks. Added learning entry to `.jules/sentinel.md`. Co-authored-by: kidchenko <5432753+kidchenko@users.noreply.github.com>
|
👋 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 New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughA security vulnerability in the yq package installer is addressed by replacing hardcoded temporary file paths with securely generated temporary directories. Documentation detailing the symlink attack risk and prevention guidelines is added alongside the implementation fix in the apt installer script. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/os_installers/apt.sh (1)
234-238: Make temp-dir cleanup unconditional.With
set -e, any failure on Line 235, Line 236, or Line 237 exits before Line 238, so the temp directory is never removed. Register atrapright aftermktemp -dso cleanup still happens on error.Proposed fix
YQ_VERSION="v4.44.6" TMP_DIR=$(mktemp -d) + trap 'rm -rf -- "$TMP_DIR"' EXIT wget "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64" -O "$TMP_DIR/yq" sudo mv "$TMP_DIR/yq" /usr/local/bin/yq sudo chmod +x /usr/local/bin/yq - rm -rf "$TMP_DIR" + trap - EXIT + rm -rf -- "$TMP_DIR"🤖 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 TMP_DIR created by TMP_DIR=$(mktemp -d) isn't cleaned up if subsequent commands fail; immediately after creating TMP_DIR, add a trap that removes "$TMP_DIR" on EXIT (e.g., trap 'rm -rf "$TMP_DIR"' EXIT) so the temporary directory is always deleted even if wget, mv, or chmod fail; keep the existing rm -rf "$TMP_DIR" at the end or rely on the trap but ensure the trap is registered right after mktemp and before the wget/mv/chmod sequence that uses TMP_DIR.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.jules/sentinel.md:
- Around line 1-4: Promote the heading "2026-03-12 - Prevent Symlink Attacks in
Package Installers" from H2 to H1 (change "##" to "#") to satisfy MD041; then
fix MD022/MD013 by wrapping long paragraph lines to a reasonable width (≤80
chars), removing or consolidating extra blank lines so there are no multiple
consecutive blank lines, and ensure the <details> block and its summary maintain
single blank lines around them and wrapped content; edit the markdown
accordingly to eliminate MD041, MD022, and MD013 failures.
---
Nitpick comments:
In `@tools/os_installers/apt.sh`:
- Around line 234-238: The TMP_DIR created by TMP_DIR=$(mktemp -d) isn't cleaned
up if subsequent commands fail; immediately after creating TMP_DIR, add a trap
that removes "$TMP_DIR" on EXIT (e.g., trap 'rm -rf "$TMP_DIR"' EXIT) so the
temporary directory is always deleted even if wget, mv, or chmod fail; keep the
existing rm -rf "$TMP_DIR" at the end or rely on the trap but ensure the trap is
registered right after mktemp and before the wget/mv/chmod sequence that uses
TMP_DIR.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f730048c-3f3e-453a-a5e5-4b0a9420ab4a
📒 Files selected for processing (2)
.jules/sentinel.mdtools/os_installers/apt.sh
| ## 2026-03-12 - Prevent Symlink Attacks in Package Installers | ||
| **Vulnerability:** Hardcoded, predictable temporary file paths (e.g., `/tmp/yq`) used prior to `sudo mv` operations for binary installation. | ||
| **Learning:** Malicious local users can create symlinks at predictable `/tmp` locations before an installation script executes. When the elevated script (using `sudo`) writes to or moves from these predictable paths, it can be exploited to overwrite critical system files, leading to privilege escalation or denial of service. | ||
| **Prevention:** Always use `mktemp -d` to create secure, unpredictable temporary directories for downloading or processing files, especially when running with elevated privileges or performing cross-user operations. |
There was a problem hiding this comment.
Fix the markdownlint failures in this note.
This file currently fails MD041, MD022, and MD013, so the documentation check will stay red until the heading is promoted to H1 and the paragraphs are wrapped.
Proposed fix
-## 2026-03-12 - Prevent Symlink Attacks in Package Installers
-**Vulnerability:** Hardcoded, predictable temporary file paths (e.g., `/tmp/yq`) used prior to `sudo mv` operations for binary installation.
-**Learning:** Malicious local users can create symlinks at predictable `/tmp` locations before an installation script executes. When the elevated script (using `sudo`) writes to or moves from these predictable paths, it can be exploited to overwrite critical system files, leading to privilege escalation or denial of service.
-**Prevention:** Always use `mktemp -d` to create secure, unpredictable temporary directories for downloading or processing files, especially when running with elevated privileges or performing cross-user operations.
+# 2026-03-12 - Prevent Symlink Attacks in Package Installers
+
+**Vulnerability:** Hardcoded, predictable temporary file paths
+(e.g., `/tmp/yq`) used before `sudo mv` operations for binary installation.
+
+**Learning:** Malicious local users can create symlinks at predictable
+`/tmp` locations before an installation script executes. When the elevated
+script uses those paths, it can overwrite critical system files, leading to
+privilege escalation or denial of service.
+
+**Prevention:** Always use `mktemp -d` to create secure, unpredictable
+temporary directories for downloading or processing files, especially when
+running with elevated privileges or performing cross-user operations.📝 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.
| ## 2026-03-12 - Prevent Symlink Attacks in Package Installers | |
| **Vulnerability:** Hardcoded, predictable temporary file paths (e.g., `/tmp/yq`) used prior to `sudo mv` operations for binary installation. | |
| **Learning:** Malicious local users can create symlinks at predictable `/tmp` locations before an installation script executes. When the elevated script (using `sudo`) writes to or moves from these predictable paths, it can be exploited to overwrite critical system files, leading to privilege escalation or denial of service. | |
| **Prevention:** Always use `mktemp -d` to create secure, unpredictable temporary directories for downloading or processing files, especially when running with elevated privileges or performing cross-user operations. | |
| # 2026-03-12 - Prevent Symlink Attacks in Package Installers | |
| **Vulnerability:** Hardcoded, predictable temporary file paths | |
| (e.g., `/tmp/yq`) used before `sudo mv` operations for binary installation. | |
| **Learning:** Malicious local users can create symlinks at predictable | |
| `/tmp` locations before an installation script executes. When the elevated | |
| script uses those paths, it can overwrite critical system files, leading to | |
| privilege escalation or denial of service. | |
| **Prevention:** Always use `mktemp -d` to create secure, unpredictable | |
| temporary directories for downloading or processing files, especially when | |
| running with elevated privileges or performing cross-user operations. |
🧰 Tools
🪛 GitHub Check: Lint Documentation
[failure] 4-4: Line length
.jules/sentinel.md:4:81 MD013/line-length Line length [Expected: 80; Actual: 215] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
[failure] 3-3: Line length
.jules/sentinel.md:3:81 MD013/line-length Line length [Expected: 80; Actual: 326] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
[failure] 2-2: Line length
.jules/sentinel.md:2:81 MD013/line-length Line length [Expected: 80; Actual: 140] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
[failure] 1-1: First line in a file should be a top-level heading
.jules/sentinel.md:1 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "## 2026-03-12 - Prevent Symlin..."] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md041.md
[failure] 1-1: Headings should be surrounded by blank lines
.jules/sentinel.md:1 MD022/blanks-around-headings Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "## 2026-03-12 - Prevent Symlink Attacks in Package Installers"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md
🪛 LanguageTool
[style] ~2-~2: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...orary file paths (e.g., /tmp/yq) used prior to sudo mv operations for binary install...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.jules/sentinel.md around lines 1 - 4, Promote the heading "2026-03-12 -
Prevent Symlink Attacks in Package Installers" from H2 to H1 (change "##" to
"#") to satisfy MD041; then fix MD022/MD013 by wrapping long paragraph lines to
a reasonable width (≤80 chars), removing or consolidating extra blank lines so
there are no multiple consecutive blank lines, and ensure the <details> block
and its summary maintain single blank lines around them and wrapped content;
edit the markdown accordingly to eliminate MD041, MD022, and MD013 failures.
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: The script
tools/os_installers/apt.shused a hardcoded, predictable temporary file path (/tmp/yq) for downloading theyqbinary before executingsudo mvto place it in/usr/local/bin.🎯 Impact: Malicious local users could pre-create a symlink at
/tmp/yqpointing to a critical system file. When the script executed thesudo mvcommand, it could overwrite the targeted file, potentially leading to local privilege escalation or a denial of service.🔧 Fix: Replaced the predictable path with a securely generated temporary directory using
mktemp -d. The binary is downloaded inside this safe directory, moved securely, and the temporary directory is cleaned up.✅ Verification: Verified that
shellcheckruns clean onapt.shand./build.sh syntaxconfirms valid bash. Journaled the learning in.jules/sentinel.mdto prevent future recurrences.PR created automatically by Jules for task 16274124836540046494 started by @kidchenko
Summary by CodeRabbit
Bug Fixes
Documentation