Skip to content

🛡️ Sentinel: [security improvement] Secure composer-setup.php download with mktemp #95

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

Merged
kidchenko merged 1 commit into from
Apr 21, 2026
Merged

🛡️ Sentinel: [security improvement] Secure composer-setup.php download with mktemp #95

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
15 changes: 14 additions & 1 deletion .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ second.
**Prevention:** Wrap commands that create sensitive files in a subshell using
`umask 077` to ensure the file is created with secure permissions (`600`)
natively.
# Sentinel Security Journal

## 2026-04-16 - Prevent TOCTOU and Symlink Attacks via Insecure Temporary Directories

Expand All @@ -29,3 +28,17 @@ or risks naming collisions.
`TMP_DIR=$(mktemp -d)`) wrapped in a subshell `(...)` and paired with a local
trap (`trap 'rm -rf "$TMP_DIR"' EXIT`) to ensure isolation and automatic
cleanup upon exit.

## 2026-04-20 - Insecure executable artifact download location

**Vulnerability:** Downloaded executable script (`composer-setup.php`) directly
to the current working directory in an installation script.

**Learning:** Downloading files directly to the current directory is insecure
because it might overwrite existing files or leave executable artifacts
susceptible to modification before execution, especially in scripts that may run
with elevated privileges.

**Prevention:** Always use securely generated isolated temporary directories via
`mktemp -d`, and wrap the setup in a subshell `(...)` with an automatic `trap`
to ensure secure handling and cleanup.
24 changes: 13 additions & 11 deletions tools/os_installers/apt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,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
(
TMP_DIR=$(mktemp -d)
trap 'rm -rf "$TMP_DIR"' EXIT
EXPECTED_CHECKSUM="$(php -r 'copy("https://composer.github.io/installer.sig", "php://stdout");')"
php -r "copy('https://getcomposer.org/installer', '$TMP_DIR/composer-setup.php');"
ACTUAL_CHECKSUM="$(php -r "echo hash_file('sha384', '$TMP_DIR/composer-setup.php');")"

if [ "$EXPECTED_CHECKSUM" = "$ACTUAL_CHECKSUM" ]; then
sudo php "$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

@coderabbitai coderabbitai Bot Apr 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fail closed on checksum mismatch.

Line 275 logs the checksum failure, but echo returns 0, so the subshell and script continue as successful. Exit non-zero here so a tampered or failed Composer download cannot be reported as an installation success.

Proposed fix 
         if [ "$EXPECTED_CHECKSUM" = "$ACTUAL_CHECKSUM" ]; then
             sudo php "$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 "$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 "$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 checksum mismatch
branch currently only writes an error to stderr but exits 0, so change the else
branch (the check comparing EXPECTED_CHECKSUM and ACTUAL_CHECKSUM around
composer-setup.php in tools/os_installers/apt.sh) to terminate the script with a
non-zero exit (e.g., call exit 1) after logging the error about an invalid
installer checksum for Composer (referencing TMP_DIR/composer-setup.php if
helpful) so the install fails hard on tampering or download corruption.

)
fi

# Clean up
Expand Down
Loading