Skip to content

🛡️ Sentinel: [MEDIUM] Fix insecure artifact download path #93

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: [MEDIUM] Fix insecure artifact download path #93

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
18 changes: 17 additions & 1 deletion .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,34 @@ 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

**Vulnerability:** Shell scripts were downloading executable artifacts directly
to predictable temporary paths like `/tmp/yq` or the current working directory,
which risks local privilege escalation, symlink attacks, and overwriting
existing files when executed with elevated privileges (`sudo`).

**Learning:** Hardcoded temporary paths (`/tmp/...`) are insecure and
susceptible to symlink hijacking by local attackers. Additionally, downloading
directly to the current directory is poor practice and pollutes the workspace
or risks naming collisions.

**Prevention:** Always use securely generated random directories (e.g.,
`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-18 - Prevent Insecure Artifact Downloads in APT Setup

**Vulnerability:** Shell scripts were downloading executable artifacts directly
to the current working directory, which risks overwriting existing files or
executing attacker-controlled binaries when executed with elevated privileges
(`sudo`).

**Learning:** Downloading directly to the current directory is poor practice
and pollutes the workspace or risks naming collisions.

**Prevention:** Always use securely generated random directories (e.g.,
`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
Expand Down
4 changes: 3 additions & 1 deletion tools/backup-projects.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ sync_git_repos() {
local repo_dir
repo_dir=$(dirname "$git_dir")
local repo_name
# shellcheck disable=SC2034
repo_name=$(basename "$repo_dir")
local relative_path="${repo_dir#$HOME/}"

Expand All @@ -292,7 +293,8 @@ sync_git_repos() {
# Commit local changes first (so pull --rebase doesn't fail)
if [[ -n $(git -C "$repo_dir" status --porcelain 2>/dev/null) ]]; then
git -C "$repo_dir" add -A 2>/dev/null
local commit_msg="chore: auto-backup commit $(date '+%Y-%m-%d %H:%M')"
local commit_msg
commit_msg="chore: auto-backup commit $(date '+%Y-%m-%d %H:%M')"
git -C "$repo_dir" commit -m "$commit_msg" &>/dev/null || true
fi

Expand Down
2 changes: 2 additions & 0 deletions tools/dotfiles
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ if [[ -t 1 ]]; then
BOLD='\033[1m'
NC='\033[0m'
else
# shellcheck disable=SC2034
RED='' GREEN='' YELLOW='' BLUE='' CYAN='' DIM='' BOLD='' NC=''
fi

Expand Down Expand Up @@ -344,6 +345,7 @@ cmd_logs() {
}

cmd_cron() {
# shellcheck disable=SC2034
local CRON_DIR="$DOTFILES_DIR/cron"
local CONFIG_FILE="${XDG_CONFIG_HOME:-$HOME/.config}/dotfiles/config.yaml"

Expand Down
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 18, 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 when Composer checksum verification fails.

The mismatch branch only echoes an error, so it returns success and lets the installer script continue. Add a non-zero exit, and guard against an empty checksum.

🛡️ Proposed fix 
-        if [ "$EXPECTED_CHECKSUM" = "$ACTUAL_CHECKSUM" ]; then
+        if [ -n "$EXPECTED_CHECKSUM" ] && [ "$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 [ -n "$EXPECTED_CHECKSUM" ] && [ "$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 echoes an error and allows the installer to continue;
update the logic around EXPECTED_CHECKSUM and ACTUAL_CHECKSUM (used in the
comparison and TMP_DIR/composer-setup.php installation) to validate that neither
checksum is empty and if they differ (or either is empty) print a clear error
and exit with a non-zero status (e.g., exit 1) so the script fails closed
instead of continuing; ensure the successful branch still runs sudo php
"$TMP_DIR/composer-setup.php" only when the checks are present and equal.

)
fi

# Clean up
Expand Down
Loading