Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-04-06 - Unsafe File Downloads 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-04-06 - Unsafe File Do..."] 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-04-06 - Unsafe File Downloads in Installation Scripts"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md
**Vulnerability:** Installation scripts (`apt.sh`) were downloading executable artifacts directly to predictable locations (`/tmp/yq`) and the current working directory, risking symlink attacks, local privilege escalation, and overwriting existing files.

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: 254] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Learning:** Shell scripts running with elevated privileges must never use hardcoded or predictable paths for temporary files, and downloading files to the current working directory is dangerous when executing as root or in unknown environments.

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: 246] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md
**Prevention:** Always use secure, randomly generated temporary directories via `mktemp -d` for downloading and processing installation artifacts. Wrap the logic in a subshell `(...)` and pair it with a local trap (e.g., `trap 'rm -rf "$TMP_DIR"' EXIT`) to ensure automatic cleanup upon exit.

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

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 | 🟡 Minor

Fix markdownlint blockers (MD041, MD022, MD013).

The heading level/spacing and long lines currently fail documentation lint checks.

📝 Proposed fix
-## 2024-04-06 - Unsafe File Downloads in Installation Scripts
-**Vulnerability:** Installation scripts (`apt.sh`) were downloading executable artifacts directly to predictable locations (`/tmp/yq`) and the current working directory, risking symlink attacks, local privilege escalation, and overwriting existing files.
-**Learning:** Shell scripts running with elevated privileges must never use hardcoded or predictable paths for temporary files, and downloading files to the current working directory is dangerous when executing as root or in unknown environments.
-**Prevention:** Always use secure, randomly generated temporary directories via `mktemp -d` for downloading and processing installation artifacts. Wrap the logic in a subshell `(...)` and pair it with a local trap (e.g., `trap 'rm -rf "$TMP_DIR"' EXIT`) to ensure automatic cleanup upon exit.
+# 2024-04-06 - Unsafe File Downloads in Installation Scripts
+
+**Vulnerability:** Installation scripts (`apt.sh`) were downloading executable
+artifacts directly to predictable locations (`/tmp/yq`) and the current working
+directory, risking symlink attacks, local privilege escalation, and overwriting
+existing files.
+
+**Learning:** Shell scripts running with elevated privileges must never use
+hardcoded or predictable paths for temporary files, and downloading files to
+the current working directory is dangerous when executing as root or in unknown
+environments.
+
+**Prevention:** Always use secure, randomly generated temporary directories via
+`mktemp -d` for downloading and processing installation artifacts. Wrap logic
+in a subshell `(...)` and pair it with a local trap (e.g.,
+`trap 'rm -rf "$TMP_DIR"' EXIT`) to ensure automatic cleanup upon exit.
📝 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-04-06 - Unsafe File Downloads in Installation Scripts
**Vulnerability:** Installation scripts (`apt.sh`) were downloading executable artifacts directly to predictable locations (`/tmp/yq`) and the current working directory, risking symlink attacks, local privilege escalation, and overwriting existing files.
**Learning:** Shell scripts running with elevated privileges must never use hardcoded or predictable paths for temporary files, and downloading files to the current working directory is dangerous when executing as root or in unknown environments.
**Prevention:** Always use secure, randomly generated temporary directories via `mktemp -d` for downloading and processing installation artifacts. Wrap the logic in a subshell `(...)` and pair it with a local trap (e.g., `trap 'rm -rf "$TMP_DIR"' EXIT`) to ensure automatic cleanup upon exit.
# 2024-04-06 - Unsafe File Downloads in Installation Scripts
**Vulnerability:** Installation scripts (`apt.sh`) were downloading executable
artifacts directly to predictable locations (`/tmp/yq`) and the current working
directory, risking symlink attacks, local privilege escalation, and overwriting
existing files.
**Learning:** Shell scripts running with elevated privileges must never use
hardcoded or predictable paths for temporary files, and downloading files to
the current working directory is dangerous when executing as root or in unknown
environments.
**Prevention:** Always use secure, randomly generated temporary directories via
`mktemp -d` for downloading and processing installation artifacts. Wrap logic
in a subshell `(...)` and pair it with a local trap (e.g.,
`trap 'rm -rf "$TMP_DIR"' EXIT`) to ensure automatic cleanup upon exit.
🧰 Tools
🪛 GitHub Check: Lint Documentation

[failure] 4-4: Line length
.jules/sentinel.md:4:81 MD013/line-length Line length [Expected: 80; Actual: 292] 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: 246] 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: 254] 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-04-06 - Unsafe File Do..."] 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-04-06 - Unsafe File Downloads in Installation Scripts"] 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, Fix the markdownlint errors in
.jules/sentinel.md by adjusting the heading and spacing: change the existing
heading "2024-04-06 - Unsafe File Downloads in Installation Scripts" to an
appropriate top-level or consistent heading level (e.g., prefix with a single
'#'), ensure there is a blank line before and after every heading to satisfy
MD022/MD041, and wrap or hard-break long lines (the
vulnerability/learning/prevention paragraphs) to a reasonable width (MD013, ~80
chars) so no lines exceed the limit; keep the existing content and detail block
structure but reflow text and add the required blank lines around headings and
lists.

64 changes: 40 additions & 24 deletions tools/os_installers/apt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,15 @@ fi
# Install Go
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"
(
TMP_DIR="$(mktemp -d)"
trap 'rm -rf "$TMP_DIR"' EXIT
cd "$TMP_DIR" || exit 1
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"
Comment on lines +211 to +214

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify current external-download blocks and nearby integrity checks.
rg -n -C2 'go\.dev/dl/go|mikefarah/yq/releases/download|lsd-rs/lsd/releases/download|sha256sum|checksum|gpg|cosign' tools/os_installers/apt.sh

Repository: kidchenko/dotfiles

Length of output: 3117


Add SHA256 checksum verification for Go, yq, and lsd downloads before privileged installation.

These packages download external artifacts and immediately extract or install them as root without verifying checksums. Other tools in this same file (GitHub CLI, Docker, Yarn, Terraform, Composer) implement integrity verification; these three should follow the same pattern.

Apply checksum verification immediately after each download:

  • Go (line 212): Download .tar.gz, verify SHA256 before extraction
  • yq (line 242): Download checksum file, verify downloaded binary against it
  • lsd (line 256): Download .deb, verify SHA256 before installation

Store SHA256 values as script variables (e.g., GO_SHA256, LSD_SHA256) and use sha256sum -c to verify, matching the pattern already used for Composer on line 279.

🤖 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 211 - 214, Add SHA256 checksum
verification before any privileged install for Go, yq, and lsd: introduce
variables like GO_SHA256 and LSD_SHA256 (and a yq checksum variable) and
download corresponding .sha256 or checksum values immediately after wget for the
artifacts referenced by GO_VERSION, yq download block, and lsd .deb download;
then run sha256sum -c (or equivalent) to verify the downloaded files before
running sudo tar -xzf /usr/local installation or sudo dpkg -i, and fail the
script if verification fails—follow the existing Composer pattern that uses
sha256sum -c to validate downloads.

)
echo "NOTE: Add 'export PATH=\$PATH:/usr/local/go/bin' to your shell profile"
fi

Expand All @@ -230,19 +234,28 @@ fi
# Install yq
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
(
TMP_DIR="$(mktemp -d)"
trap 'rm -rf "$TMP_DIR"' EXIT
cd "$TMP_DIR" || exit 1
YQ_VERSION="v4.44.6"
wget "https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64" -O "yq"
sudo mv "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"
(
TMP_DIR="$(mktemp -d)"
trap 'rm -rf "$TMP_DIR"' EXIT
cd "$TMP_DIR" || exit 1
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"
)
fi

# Install Tesseract OCR
Expand All @@ -252,17 +265,20 @@ 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
cd "$TMP_DIR" || exit 1
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
else
>&2 echo 'ERROR: Invalid installer checksum for Composer'
fi
Comment on lines +276 to +280

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 fast on Composer checksum mismatch.

On checksum mismatch, the script only logs and continues. This should exit non-zero so the install doesn’t report success with missing/compromised Composer.

🛠️ Proposed fix
         if [ "$EXPECTED_CHECKSUM" = "$ACTUAL_CHECKSUM" ]; then
             sudo php 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-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-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 276 - 280, The else branch that
handles the Composer checksum mismatch currently only writes an error but
continues; update the block that compares EXPECTED_CHECKSUM and ACTUAL_CHECKSUM
so that on mismatch it prints the error and then exits with a non-zero status
(e.g., exit 1) to fail fast and prevent reporting a successful install; modify
the conditional around the composer install (composer-setup.php invocation) to
ensure only the successful-check path executes and the mismatch path terminates
immediately.

)
fi

# Clean up
Expand Down
Loading