Skip to content

🛡️ Sentinel: [CRITICAL] Fix insecure temporary file usage #73

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

Closed
kidchenko wants to merge 1 commit into from
Closed

🛡️ Sentinel: [CRITICAL] Fix insecure temporary file usage #73

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 temporary paths in shared locations like /tmp]

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 temporary paths in shared locations like /tmp]"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md
**Vulnerability:** A script uses a hardcoded, predictable path (e.g., `/tmp/yq`) when downloading tools, which could lead to a symlink attack or unauthorized file modification, especially when running with elevated privileges (like `sudo mv /tmp/yq /usr/local/bin/yq`).

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: 269] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Learning:** Hardcoding paths in world-writable directories (`/tmp`) makes the script vulnerable to a local attacker pre-creating the file (or a symlink) to perform malicious actions.

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: 184] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Prevention:** Always use securely generated temporary directories (e.g., `mktemp -d`) coupled with a cleanup trap to store downloaded files before moving them to their final destination.

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: 188] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
Comment on lines +1 to +4
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix markdown linting errors to pass CI.

The pipeline reports several markdownlint failures:

  1. MD041: First line should be a top-level heading (# instead of ##)
  2. MD022: Add a blank line after the heading
  3. MD013: Lines 2-4 exceed the 80-character limit
Proposed fix 
-## 2024-05-24 - [Avoid predictable temporary paths in shared locations like /tmp]
-**Vulnerability:** A script uses a hardcoded, predictable path (e.g., `/tmp/yq`) when downloading tools, which could lead to a symlink attack or unauthorized file modification, especially when running with elevated privileges (like `sudo mv /tmp/yq /usr/local/bin/yq`).
-**Learning:** Hardcoding paths in world-writable directories (`/tmp`) makes the script vulnerable to a local attacker pre-creating the file (or a symlink) to perform malicious actions.
-**Prevention:** Always use securely generated temporary directories (e.g., `mktemp -d`) coupled with a cleanup trap to store downloaded files before moving them to their final destination.
+# Sentinel Security Learnings
+
+## 2024-05-24 - Avoid predictable temporary paths in /tmp
+
+**Vulnerability:** A script uses a hardcoded, predictable path (e.g.,
+`/tmp/yq`) when downloading tools, which could lead to a symlink attack or
+unauthorized file modification, especially when running with elevated
+privileges (like `sudo mv /tmp/yq /usr/local/bin/yq`).
+
+**Learning:** Hardcoding paths in world-writable directories (`/tmp`) makes
+the script vulnerable to a local attacker pre-creating the file (or a
+symlink) to perform malicious actions.
+
+**Prevention:** Always use securely generated temporary directories (e.g.,
+`mktemp -d`) coupled with a cleanup trap to store downloaded files before
+moving them to their final destination.
📝 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 temporary paths in shared locations like /tmp]
**Vulnerability:** A script uses a hardcoded, predictable path (e.g., `/tmp/yq`) when downloading tools, which could lead to a symlink attack or unauthorized file modification, especially when running with elevated privileges (like `sudo mv /tmp/yq /usr/local/bin/yq`).
**Learning:** Hardcoding paths in world-writable directories (`/tmp`) makes the script vulnerable to a local attacker pre-creating the file (or a symlink) to perform malicious actions.
**Prevention:** Always use securely generated temporary directories (e.g., `mktemp -d`) coupled with a cleanup trap to store downloaded files before moving them to their final destination.
# Sentinel Security Learnings
## 2024-05-24 - Avoid predictable temporary paths in /tmp
**Vulnerability:** A script uses a hardcoded, predictable path (e.g.,
`/tmp/yq`) when downloading tools, which could lead to a symlink attack or
unauthorized file modification, especially when running with elevated
privileges (like `sudo mv /tmp/yq /usr/local/bin/yq`).
**Learning:** Hardcoding paths in world-writable directories (`/tmp`) makes
the script vulnerable to a local attacker pre-creating the file (or a
symlink) to perform malicious actions.
**Prevention:** Always use securely generated temporary directories (e.g.,
`mktemp -d`) coupled with a cleanup trap to store downloaded files before
moving them to their final destination.
🧰 Tools
🪛 GitHub Check: Lint Documentation

[failure] 4-4: Line length
.jules/sentinel.md:4:81 MD013/line-length Line length [Expected: 80; Actual: 188] 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: 184] 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: 269] 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 temporary paths in shared locations like /tmp]"] 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, Change the first line to a top-level
heading (replace leading "##" with "#") and insert a blank line after that
heading to satisfy MD041 and MD022; then reflow the subsequent paragraph lines
to be <=80 characters each (wrap the long sentence about
vulnerability/learning/prevention into multiple shorter lines) so MD013 is
fixed; reference the heading text "2024-05-24 - [Avoid predictable temporary
paths in shared locations like /tmp]" when updating .jules/sentinel.md.

54 changes: 33 additions & 21 deletions tools/os_installers/apt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,13 @@ fi
echo "Installing Go..."
if ! command -v go &> /dev/null; then
GO_VERSION="1.23.4"
wget "https://go.dev/dl/go${GO_VERSION}.linux-amd64.tar.gz"
sudo rm -rf /usr/local/go
sudo tar -C /usr/local -xzf "go${GO_VERSION}.linux-amd64.tar.gz"
rm "go${GO_VERSION}.linux-amd64.tar.gz"
(
GO_TMP_DIR=$(mktemp -d)
trap 'rm -rf "$GO_TMP_DIR"' EXIT
wget "https://go.dev/dl/go${GO_VERSION}.linux-amd64.tar.gz" -O "$GO_TMP_DIR/go.tar.gz"
sudo rm -rf /usr/local/go
sudo tar -C /usr/local -xzf "$GO_TMP_DIR/go.tar.gz"
)
echo "NOTE: Add 'export PATH=\$PATH:/usr/local/go/bin' to your shell profile"
fi

Expand All @@ -231,18 +234,25 @@ 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
sudo chmod +x /usr/local/bin/yq
(
YQ_TMP_DIR=$(mktemp -d)
trap 'rm -rf "$YQ_TMP_DIR"' EXIT
wget "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64" -O "$YQ_TMP_DIR/yq"
sudo mv "$YQ_TMP_DIR/yq" /usr/local/bin/yq
sudo chmod +x /usr/local/bin/yq
)
fi

# Install lsd (LSDeluxe)
echo "Installing lsd..."
if ! command -v lsd &> /dev/null; then
LSD_VERSION="1.1.5"
wget "https://github.com/lsd-rs/lsd/releases/download/v${LSD_VERSION}/lsd_${LSD_VERSION}_amd64.deb"
sudo dpkg -i "lsd_${LSD_VERSION}_amd64.deb"
rm "lsd_${LSD_VERSION}_amd64.deb"
(
LSD_TMP_DIR=$(mktemp -d)
trap 'rm -rf "$LSD_TMP_DIR"' EXIT
wget "https://github.com/lsd-rs/lsd/releases/download/v${LSD_VERSION}/lsd_${LSD_VERSION}_amd64.deb" -O "$LSD_TMP_DIR/lsd.deb"
sudo dpkg -i "$LSD_TMP_DIR/lsd.deb"
)
fi

# Install Tesseract OCR
Expand All @@ -252,17 +262,19 @@ sudo apt install -y tesseract-ocr
# Install PHP Composer
echo "Installing Composer..."
if ! command -v composer &> /dev/null; then
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
(
EXPECTED_CHECKSUM="$(php -r 'copy("https://composer.github.io/installer.sig", "php://stdout");')"
COMPOSER_TMP_DIR=$(mktemp -d)
trap 'rm -rf "$COMPOSER_TMP_DIR"' EXIT
php -r "copy('https://getcomposer.org/installer', '$COMPOSER_TMP_DIR/composer-setup.php');"
ACTUAL_CHECKSUM="$(php -r "echo hash_file('sha384', '$COMPOSER_TMP_DIR/composer-setup.php');")"

if [ "$EXPECTED_CHECKSUM" = "$ACTUAL_CHECKSUM" ]; then
sudo php "$COMPOSER_TMP_DIR/composer-setup.php" --quiet --install-dir=/usr/local/bin --filename=composer
else
>&2 echo 'ERROR: Invalid installer checksum for Composer'
fi
Comment on lines +272 to +276
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add exit 1 after checksum mismatch error.

When the checksum validation fails, the script prints an error but the subshell exits with code 0 (from the successful echo). This means the overall script continues silently without installing Composer, which could mask a security issue.

Proposed fix 
         if [ "$EXPECTED_CHECKSUM" = "$ACTUAL_CHECKSUM" ]; then
             sudo php "$COMPOSER_TMP_DIR/composer-setup.php" --quiet --install-dir=/usr/local/bin --filename=composer
         else
             >&2 echo 'ERROR: Invalid installer checksum for Composer'
+            exit 1
         fi
📝 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
if [ "$EXPECTED_CHECKSUM" = "$ACTUAL_CHECKSUM" ]; then
sudo php "$COMPOSER_TMP_DIR/composer-setup.php" --quiet --install-dir=/usr/local/bin --filename=composer
else
>&2 echo 'ERROR: Invalid installer checksum for Composer'
fi
if [ "$EXPECTED_CHECKSUM" = "$ACTUAL_CHECKSUM" ]; then
sudo php "$COMPOSER_TMP_DIR/composer-setup.php" --quiet --install-dir=/usr/local/bin --filename=composer
else
>&2 echo 'ERROR: Invalid installer checksum for Composer'
exit 1
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 272 - 276, The else branch that
handles the checksum mismatch currently only echoes an error and returns a zero
exit status; update the checksum verification block that compares
EXPECTED_CHECKSUM and ACTUAL_CHECKSUM to terminate the script with a non-zero
status on failure by adding an explicit exit 1 after the error message so the
installer does not continue silently (affecting the branch that would otherwise
run sudo php "$COMPOSER_TMP_DIR/composer-setup.php" --quiet
--install-dir=/usr/local/bin --filename=composer).

)
fi

# Clean up
Expand Down
Loading