🛡️ Sentinel: [MEDIUM] Fix predictable temp paths in apt.sh#64
🛡️ Sentinel: [MEDIUM] Fix predictable temp paths in apt.sh#64
Conversation
This commit replaces the use of predictable temporary file paths (e.g., `/tmp/yq`) and direct downloads into the current working directory with securely generated random temporary directories using `mktemp -d` in the `tools/os_installers/apt.sh` script. This prevents potential symlink attacks and arbitrary file overwrites, improving the overall security posture of the installation process. Co-authored-by: kidchenko <5432753+kidchenko@users.noreply.github.com>
📝 WalkthroughWalkthroughThis pull request enhances security by replacing predictable temporary file paths with randomized directories. A vulnerability entry is added to document the issue, while the installation script is modified to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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)
258-270: Composer installation still uses current working directory.For consistency with the security fixes applied to Go, yq, and lsd, consider applying the same
mktemp -dpattern here. Thecomposer-setup.phpis currently downloaded to the cwd.♻️ Optional: Apply same pattern to Composer installation
if ! command -v composer &> /dev/null; then + tmp_dir=$(mktemp -d) + cd "${tmp_dir}" EXPECTED_CHECKSUM="$(php -r 'copy("https://composer.github.io/installer.sig", "php://stdout");')" php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" ACTUAL_CHECKSUM="$(php -r "echo hash_file('sha384', 'composer-setup.php');")" if [ "$EXPECTED_CHECKSUM" = "$ACTUAL_CHECKSUM" ]; then sudo php composer-setup.php --quiet --install-dir=/usr/local/bin --filename=composer - rm composer-setup.php else >&2 echo 'ERROR: Invalid installer checksum for Composer' - rm composer-setup.php fi + cd - > /dev/null + rm -rf "${tmp_dir}" fi🤖 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 258 - 270, The Composer installer is currently downloaded into the current working directory (composer-setup.php); change the block in tools/os_installers/apt.sh to create a private temp directory with mktemp -d, download EXPECTED_CHECKSUM and composer-setup.php into that temp dir, compute ACTUAL_CHECKSUM against the temp file, run php on the temp composer-setup.php (installing to /usr/local/bin), and then rm the temp file and rmdir the temp directory (use a trap to ensure cleanup on exit); update references to composer-setup.php in the EXPECTED_CHECKSUM/ACTUAL_CHECKSUM and install/remove steps so they use the temp-dir path.
🤖 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: Update the sentinel entry to satisfy markdown lint rules:
change the date in the heading from "2024-03-20" to "2026-03-20", make the first
heading a top-level heading by replacing "##" with "#" for the title line, add a
blank line before and after that heading to satisfy MD022, and wrap long lines
in the body (the vulnerability, learning, and prevention lines) so each line is
<=80 characters; refer to the heading text "Hardcoded and Predictable Temporary
File Paths" and the three bullet lines (Vulnerability, Learning, Prevention)
when editing.
---
Nitpick comments:
In `@tools/os_installers/apt.sh`:
- Around line 258-270: The Composer installer is currently downloaded into the
current working directory (composer-setup.php); change the block in
tools/os_installers/apt.sh to create a private temp directory with mktemp -d,
download EXPECTED_CHECKSUM and composer-setup.php into that temp dir, compute
ACTUAL_CHECKSUM against the temp file, run php on the temp composer-setup.php
(installing to /usr/local/bin), and then rm the temp file and rmdir the temp
directory (use a trap to ensure cleanup on exit); update references to
composer-setup.php in the EXPECTED_CHECKSUM/ACTUAL_CHECKSUM and install/remove
steps so they use the temp-dir path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3bc8daf9-58f9-473c-87f0-85ca9c497698
📒 Files selected for processing (2)
.jules/sentinel.mdtools/os_installers/apt.sh
| ## 2024-03-20 - [Hardcoded and Predictable Temporary File Paths] | ||
| **Vulnerability:** Use of predictable temporary file paths like `/tmp/yq`, and downloading executables/archives (`go...tar.gz`, `lsd...deb`) into the current working directory. | ||
| **Learning:** These paths can be predicted by an attacker to conduct a symlink attack or file overwriting, especially when operations like `sudo mv /tmp/yq ...` or `sudo dpkg -i ...` are performed, which can lead to local privilege escalation. Downloading to `cwd` can also clutter the directory or overwrite existing files unintentionally. | ||
| **Prevention:** Use `mktemp -d` to securely generate a random, isolated temporary directory for downloading and manipulating files before moving them or installing them. |
There was a problem hiding this comment.
Fix linting issues flagged by static analysis.
The documentation content is accurate, but there are several markdown lint failures:
- Line 1: Date appears incorrect - should be
2026-03-20based on PR creation date - MD041: First line should be a top-level heading (
#not##) - MD022: Heading should be surrounded by blank lines
- MD013: Lines 2-4 exceed the 80-character limit
📝 Proposed fix for linting issues
-## 2024-03-20 - [Hardcoded and Predictable Temporary File Paths]
-**Vulnerability:** Use of predictable temporary file paths like `/tmp/yq`, and downloading executables/archives (`go...tar.gz`, `lsd...deb`) into the current working directory.
-**Learning:** These paths can be predicted by an attacker to conduct a symlink attack or file overwriting, especially when operations like `sudo mv /tmp/yq ...` or `sudo dpkg -i ...` are performed, which can lead to local privilege escalation. Downloading to `cwd` can also clutter the directory or overwrite existing files unintentionally.
-**Prevention:** Use `mktemp -d` to securely generate a random, isolated temporary directory for downloading and manipulating files before moving them or installing them.
+# Sentinel Security Findings
+
+## 2026-03-20 - Hardcoded and Predictable Temporary File Paths
+
+**Vulnerability:** Use of predictable temporary file paths like `/tmp/yq`,
+and downloading executables/archives (`go...tar.gz`, `lsd...deb`) into the
+current working directory.
+
+**Learning:** These paths can be predicted by an attacker to conduct a
+symlink attack or file overwriting, especially when operations like
+`sudo mv /tmp/yq ...` or `sudo dpkg -i ...` are performed, which can lead
+to local privilege escalation. Downloading to `cwd` can also clutter the
+directory or overwrite existing files unintentionally.
+
+**Prevention:** Use `mktemp -d` to securely generate a random, isolated
+temporary directory for downloading and manipulating files before moving
+them or installing them.🧰 Tools
🪛 GitHub Check: Lint Documentation
[failure] 4-4: Line length
.jules/sentinel.md:4:81 MD013/line-length Line length [Expected: 80; Actual: 169] 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: 340] 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: 176] 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: "## 2024-03-20 - [Hardcoded and..."] 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: "## 2024-03-20 - [Hardcoded and Predictable Temporary File Paths]"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md
🤖 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, Update the sentinel entry to satisfy
markdown lint rules: change the date in the heading from "2024-03-20" to
"2026-03-20", make the first heading a top-level heading by replacing "##" with
"#" for the title line, add a blank line before and after that heading to
satisfy MD022, and wrap long lines in the body (the vulnerability, learning, and
prevention lines) so each line is <=80 characters; refer to the heading text
"Hardcoded and Predictable Temporary File Paths" and the three bullet lines
(Vulnerability, Learning, Prevention) when editing.
1 participant
🚨 Severity: MEDIUM
💡 Vulnerability: Use of predictable temporary file paths (
/tmp/yq) and downloading executables/archives into the current working directory intools/os_installers/apt.sh.🎯 Impact: These paths can be predicted by an attacker to conduct a symlink attack or file overwriting, especially when operations like
sudo mv /tmp/yq ...orsudo dpkg -i ...are performed, which can lead to local privilege escalation.🔧 Fix: Replaced hardcoded
/tmppaths andcwddownloads with securely generated isolated temporary directories usingmktemp -d.✅ Verification: Run
./build.shto ensure scripts pass linting (shellcheck). Examinetools/os_installers/apt.shto verify Go, yq, and lsd installations usetmp_dir=$(mktemp -d)and thatrm -rf "${tmp_dir}"cleans up properly.PR created automatically by Jules for task 1803863750734523242 started by @kidchenko
Summary by CodeRabbit
Bug Fixes
Documentation