Skip to content

🛡️ Sentinel: [CRITICAL] Fix predictable /tmp path vulnerability in apt.sh #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kidchenko wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🛡️ Sentinel: [CRITICAL] Fix predictable /tmp path vulnerability in apt.sh #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-05-24 - [Avoid predictable /tmp files in installation scripts]

Check failure on line 1 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

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-05-24 - [Avoid predict..."] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md041.md

Check failure on line 1 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

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-05-24 - [Avoid predictable /tmp files in installation scripts]"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md
**Vulnerability:** In `tools/os_installers/apt.sh`, the `yq` binary was downloaded directly to a predictable path `/tmp/yq` before being moved with `sudo` to a global path. This can expose the system to symlink attacks or local privilege escalation since `/tmp` is a world-writable directory.

Check failure on line 2 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

Line length 

.jules/sentinel.md:2:81 MD013/line-length Line length [Expected: 80; Actual: 292] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Learning:** Hardcoding paths in `/tmp` for temporary files, especially those executed or moved with `sudo` privileges, introduces a vulnerability where malicious users could preemptively create a symlink or a file at that location.

Check failure on line 3 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

Line length 

.jules/sentinel.md:3:81 MD013/line-length Line length [Expected: 80; Actual: 233] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Prevention:** Always use `mktemp -d` to create securely generated, isolated temporary directories (e.g. `tmp_dir=$(mktemp -d)`) and use these directories to store downloaded artifacts, ensuring they are deleted via `rm -rf` afterwards.

Check failure on line 4 in .jules/sentinel.md

View workflow job for this annotation

GitHub Actions / Lint Documentation

Line length 

.jules/sentinel.md:4:81 MD013/line-length Line length [Expected: 80; Actual: 237] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
Comment on lines +1 to +4
Copy link

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix markdownlint blockers (MD041, MD022, MD013) in this new file.

CI currently fails because Line 1 is not a top-level heading, heading spacing is missing, and Lines 2–4 exceed max line length.

📝 Suggested lint-compliant rewrite 
-## 2024-05-24 - [Avoid predictable /tmp files in installation scripts]
-**Vulnerability:** In `tools/os_installers/apt.sh`, the `yq` binary was downloaded directly to a predictable path `/tmp/yq` before being moved with `sudo` to a global path. This can expose the system to symlink attacks or local privilege escalation since `/tmp` is a world-writable directory.
-**Learning:** Hardcoding paths in `/tmp` for temporary files, especially those executed or moved with `sudo` privileges, introduces a vulnerability where malicious users could preemptively create a symlink or a file at that location.
-**Prevention:** Always use `mktemp -d` to create securely generated, isolated temporary directories (e.g. `tmp_dir=$(mktemp -d)`) and use these directories to store downloaded artifacts, ensuring they are deleted via `rm -rf` afterwards.
+# Sentinel Security Learnings
+
+## 2024-05-24 - Avoid predictable /tmp files in installation scripts
+
+**Vulnerability:** In `tools/os_installers/apt.sh`, the `yq` binary was
+downloaded to predictable `/tmp/yq` before `sudo` move to a global path.
+This enables symlink attacks and local privilege escalation in `/tmp`.
+
+**Learning:** Hardcoding `/tmp` paths for temporary files—especially files
+executed or moved with `sudo`—allows pre-creation of malicious files or
+symlinks by local users.
+
+**Prevention:** Use `mktemp -d` to create isolated temp directories
+(`tmp_dir=$(mktemp -d)`), store artifacts there, and delete them with
+`rm -rf` afterward.
📝 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
## 2024-05-24 - [Avoid predictable /tmp files in installation scripts]
**Vulnerability:** In `tools/os_installers/apt.sh`, the `yq` binary was downloaded directly to a predictable path `/tmp/yq` before being moved with `sudo` to a global path. This can expose the system to symlink attacks or local privilege escalation since `/tmp` is a world-writable directory.
**Learning:** Hardcoding paths in `/tmp` for temporary files, especially those executed or moved with `sudo` privileges, introduces a vulnerability where malicious users could preemptively create a symlink or a file at that location.
**Prevention:** Always use `mktemp -d` to create securely generated, isolated temporary directories (e.g. `tmp_dir=$(mktemp -d)`) and use these directories to store downloaded artifacts, ensuring they are deleted via `rm -rf` afterwards.
# Sentinel Security Learnings
## 2024-05-24 - Avoid predictable /tmp files in installation scripts
**Vulnerability:** In `tools/os_installers/apt.sh`, the `yq` binary was
downloaded to predictable `/tmp/yq` before `sudo` move to a global path.
This enables symlink attacks and local privilege escalation in `/tmp`.
**Learning:** Hardcoding `/tmp` paths for temporary files—especially files
executed or moved with `sudo`—allows pre-creation of malicious files or
symlinks by local users.
**Prevention:** Use `mktemp -d` to create isolated temp directories
(`tmp_dir=$(mktemp -d)`), store artifacts there, and delete them with
`rm -rf` afterward.
🧰 Tools
🪛 GitHub Check: Lint Documentation

[failure] 4-4: Line length
.jules/sentinel.md:4:81 MD013/line-length Line length [Expected: 80; Actual: 237] 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: 233] 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: 292] 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-05-24 - [Avoid predict..."] 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-05-24 - [Avoid predictable /tmp files in installation scripts]"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md

🪛 LanguageTool

[locale-violation] ~4-~4: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ... ensuring they are deleted via rm -rf afterwards.

(AFTERWARDS_US)

🤖 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, Make the first line a top-level
heading and fix heading spacing and line lengths: change the opening line to a
top-level H1 (e.g., "# 2024-05-24 - Avoid predictable /tmp files in installation
scripts"), ensure there is a blank line after that heading (MD022), then reflow
the subsequent lines so no line exceeds the repo's max (wrap the Vulnerability,
Learning, and Prevention sections to fit MD013) and keep each section as
separate subheadings or bold labels to preserve structure; edit the content in
.jules/sentinel.md accordingly and keep the same wording but split long
sentences across lines to satisfy the linters (addresses MD041, MD022, MD013).

6 changes: 4 additions & 2 deletions tools/os_installers/apt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,11 @@ fi
echo "Installing yq..."
if ! command -v yq &> /dev/null; then
YQ_VERSION="v4.44.6"
wget "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64" -O /tmp/yq
sudo mv /tmp/yq /usr/local/bin/yq
tmp_dir=$(mktemp -d)
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
Comment on lines +235 to +236
Copy link

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

For GitHub release yq v4.44.6, what is the official checksum (or signature) verification workflow for the yq_linux_amd64 binary, including the exact checksum file URL and verification command?

💡 Result:

The official checksum verification workflow for the yq_linux_amd64 binary in GitHub release v4.44.6 of mikefarah/yq is as follows: 1. Download the binary from: https://github.com/mikefarah/yq/releases/download/v4.44.6/yq_linux_amd64 2. Download the checksum files: - checksums: https://github.com/mikefarah/yq/releases/download/v4.44.6/checksums - checksums_hashes_order: https://github.com/mikefarah/yq/releases/download/v4.44.6/checksums_hashes_order - (optionally) extract-checksum.sh: https://github.com/mikefarah/yq/releases/download/v4.44.6/extract-checksum.sh 3. Extract the SHA-256 checksum using the provided script (recommended): ./extract-checksum.sh SHA-256 yq_linux_amd64 This outputs the expected SHA-256 hash for the binary (e.g., "acebc9d07aa2d0e482969b2c080ee306e8f58efbd6f2d857eefbce6469da1473" from example usage). 4. Verify the checksum: sha256sum yq_linux_amd64 Compare the output to the extracted hash. Alternatively, pipe the extracted line to a verifier like sha256sum --check or rhash -c -. This workflow uses multiple hash algorithms in a single 'checksums' file, with 'checksums_hashes_order' defining the column order. The extract script simplifies retrieval of the specific SHA-256 value without depending on tool versions. No GPG signatures are provided; verification relies on these checksums. v4.44.6 exists as confirmed by release references and actions (Dec 2024), following the standard pattern seen in later releases like v4.52.4.

Citations:

🏁 Script executed:

# Check the file size and examine the relevant lines and context
wc -l tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 92

🏁 Script executed:

# Read the context around lines 235-236 to see YQ_VERSION and any verification
sed -n '220,250p' tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 948

🏁 Script executed:

# Search for any checksum or signature verification in the entire file
grep -n "checksum\|signature\|sha256\|md5\|gpg\|verify" tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 1484

Add checksum verification for yq binary before installation.

Line 235 downloads yq binary from GitHub and line 236 installs it to /usr/local/bin with sudo without integrity verification. While this script implements verification for other packages (Composer, Docker, Yarn, etc.), yq installation skips this critical step.

The yq project provides official checksums at: https://github.com/mikefarah/yq/releases/download/v4.44.6/checksums

Verify the downloaded binary before privileged installation:

wget "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/checksums" -O "$tmp_dir/checksums"
sha256sum "$tmp_dir/yq" | grep -f <(grep yq_linux_amd64 "$tmp_dir/checksums") || { echo "Checksum mismatch"; exit 1; }
🤖 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 235 - 236, Add integrity
verification for the yq binary before moving it to /usr/local/bin: after
downloading "${YQ_VERSION}/yq_linux_amd64" to "$tmp_dir/yq", also download the
corresponding "checksums" file for that release into "$tmp_dir/checksums",
compute and compare the sha256 of "$tmp_dir/yq" against the entry for
"yq_linux_amd64" in "$tmp_dir/checksums" (fail the script with an error message
and non‑zero exit if the checksum does not match), and only then run the
privileged install step (sudo mv "$tmp_dir/yq" /usr/local/bin/yq); reference the
YQ_VERSION and tmp_dir variables and the "$tmp_dir/yq" artifact when
implementing the verification and failure path.

sudo chmod +x /usr/local/bin/yq
rm -rf "$tmp_dir"
Comment on lines +234 to +238
Copy link

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether yq installation block has trap-based cleanup around mktemp usage.
rg -n -C3 'Installing yq|mktemp -d|trap .*EXIT|rm -rf "\$tmp_dir"|wget .*yq_linux_amd64|sudo (mv|install) .*yq' tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 484

🏁 Script executed:

# Check if set -e is enabled at line 7
head -10 tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 283

🏁 Script executed:

# Get broader context around the yq installation block to check for error handling
sed -n '230,240p' tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 399

Ensure temporary directory cleanup runs on failure paths.

Because set -e is enabled (Line 7), any failure on Line 235–Line 237 exits before Line 238, leaving the temp directory behind. Add an EXIT trap right after creating tmp_dir.

💡 Suggested fix 
-    tmp_dir=$(mktemp -d)
+    tmp_dir="$(mktemp -d)"
+    cleanup_yq_tmp() { rm -rf "$tmp_dir"; }
+    trap cleanup_yq_tmp 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"
+    cleanup_yq_tmp
+    trap - EXIT
🤖 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, After creating tmp_dir
with tmp_dir=$(mktemp -d), add a trap on EXIT to always remove it (e.g., trap
'rm -rf "$tmp_dir"' EXIT) so failures between the wget/move/chmod steps won't
leave the temp dir behind; place the trap immediately after the mktemp line and
ensure the trap references the tmp_dir variable (and is robust if tmp_dir is
empty or unset).

fi

# Install lsd (LSDeluxe)
Expand Down
Loading