Harden InstallTarballPackage.sh with input & path validation, safer downloads, stricter error handling#27557
Open
emixor wants to merge 7 commits into
Open
Harden InstallTarballPackage.sh with input & path validation, safer downloads, stricter error handling#27557emixor wants to merge 7 commits into
emixor wants to merge 7 commits into
Conversation
…p cleanup, and quoting
### Motivation
- Improve robustness and safety of the tarball installation script by adding strict shell options and explicit input validation.
- Prevent unsafe filenames and malformed version strings from being used to construct download URLs or filesystem paths.
- Ensure temporary artifacts are cleaned up and downloads use secure, fail-fast curl options.
### Description
- Enabled strict error handling with `set -eu` and added parameter defaults using `${1:-}` and `${2:-}` for `POWERSHELL_VERSION` and `POWERSHELL_PACKAGE`.
- Added validation `case` checks to reject invalid `POWERSHELL_VERSION` and `POWERSHELL_PACKAGE` values and to enforce that the package ends with `.tar.gz`.
- Create a secure temporary directory via `mktemp -d`, register a `cleanup` function with `trap` to remove it, and download the package into the temp directory using `curl --fail --location --show-error --proto '=https' --tlsv1.2`.
- Use variables for `DOWNLOAD_URL`, `PACKAGE_PATH`, and `INSTALL_DIR`, update `tar` and `ln -s` invocations to use quoted variables, and ensure `/etc/shells` modifications are quoted and guarded with `grep`.
### Testing
- No automated tests were run on this change.
…p dir, and atomic symlink ### Motivation - Make the PowerShell tarball install script more robust and secure by adding strict error handling and input validation. - Avoid partial installs and leftover temporary files by using a temporary directory with cleanup on exit and making installations atomic. - Improve download reliability and harden handling of filenames and paths to prevent injection or malformed input issues. ### Description - Enable strict shell behavior with `set -eu` and validate `POWERSHELL_VERSION` and `POWERSHELL_PACKAGE` inputs with `case` patterns to reject unsafe values. - Require the package to end with `.tar.gz` and construct `DOWNLOAD_URL`, `PACKAGE_PATH`, and `INSTALL_DIR` variables to centralize paths and filenames. - Use `mktemp -d` to create a temporary directory and `trap` a `cleanup` function to remove it on `EXIT`, `HUP`, `INT`, and `TERM`. - Download with `curl --fail --location --show-error --proto '=https' --tlsv1.2` to ensure secure and reliable transfers and extract the package using the variable paths. - Create the symlink with `ln -sfn` for atomic updates and quote variables when writing or appending the path to `/etc/shells` while guarding duplicates with `grep -q`. ### Testing - No automated tests were added or executed for this change.
…ion, and safer download/cleanup
### Motivation
- Improve security and robustness of the PowerShell tarball installer by validating inputs and verifying release checksums.
- Ensure downloaded artifacts are retrieved over a hardened TLS configuration and any temporary files are cleaned up.
- Make the script fail fast on unset variables and provide clearer error messages for invalid parameters.
### Description
- Turn on strict shell options with `set -eu` and validate positional parameters using `POWERSHELL_VERSION=${1:-}` and `POWERSHELL_PACKAGE=${2:-}`.
- Add input validation for `POWERSHELL_VERSION` and `POWERSHELL_PACKAGE`, including allowed character checks and enforcing a `.tar.gz` package extension.
- Download the package and `hashes.sha256` using secure `curl` options and verify the package SHA-256 against the release `hashes.sha256` file; introduce `get_file_sha256` helper to support `sha256sum` or `shasum`.
- Use a `mktemp` temporary directory with a `cleanup` trap for safe removal, use `INSTALL_DIR` and `PACKAGE_PATH` variables, and extract to the target directory; create an atomic symlink with `ln -sfn` and quote variables when writing to `/etc/shells`.
### Testing
- Ran `shellcheck` on the modified script and fixed issues flagged by the linter, which passed without errors.
- Executed an automated smoke test in a container that runs the script with a known release (example `7.6.2` / `powershell-7.6.2-linux-x64.tar.gz`), which completed with exit code `0` and created the expected install directory and `/usr/bin/pwsh` symlink.
- Verified checksum mismatch handling by an automated negative test that supplies a tampered package, which correctly failed checksum validation with a non-zero exit code.
Fixed 1 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
…xor/powershell Harden InstallTarballPackage.sh: input validation, safe download, temp cleanup, and quoting
Author
|
@microsoft-github-policy-service agree |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds safer PowerShell tarball installation behavior and introduces a shell-based test harness to validate symlink handling.
Changes:
- Hardened
InstallTarballPackage.shwith input validation, checksum verification, temp-dir cleanup, and safer symlink updates. - Added Docker-focused shell tests for symlink creation/replacement/refusal scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| docker/InstallTarballPackage.sh | Adds validation, SHA-256 verification, temp dir cleanup, and non-destructive symlink handling. |
| test/docker/InstallTarballPackage.Tests.sh | Introduces isolated shell tests using fake curl/tar to validate installer symlink behavior without network access. |
Comment on lines
9
to
12
| usage() { | ||
| echo "usage: $0 <powershell version> <powershell package name>" | ||
| exit 1 | ||
| } |
Comment on lines
+64
to
+74
| # Print the SHA-256 hash for the file path passed as the first argument. | ||
| get_file_sha256() { | ||
| if command -v sha256sum >/dev/null 2>&1; then | ||
| sha256sum "$1" | awk '{ print $1 }' | ||
| elif command -v shasum >/dev/null 2>&1; then | ||
| shasum -a 256 "$1" | awk '{ print $1 }' | ||
| else | ||
| echo "Unable to verify package integrity: sha256sum or shasum is required." >&2 | ||
| return 1 | ||
| fi | ||
| } |
| exit 1 | ||
| } | ||
|
|
||
| actual_hash=$(get_file_sha256 "$PACKAGE_PATH") |
Comment on lines
+105
to
+107
| mkdir -p "$INSTALL_DIR" | ||
| # Expand powershell to the target folder | ||
| tar zxf /tmp/powershell.tar.gz -C /opt/microsoft/powershell/$POWERSHELL_VERSION | ||
| tar zxf "$PACKAGE_PATH" -C "$INSTALL_DIR" |
Comment on lines
+111
to
+122
| if [ -L "$POWERSHELL_LINKFILE" ]; then | ||
| current_target=$(readlink "$POWERSHELL_LINKFILE") | ||
| if [ "$current_target" != "$POWERSHELL_TARGET" ]; then | ||
| rm -- "$POWERSHELL_LINKFILE" | ||
| ln -s "$POWERSHELL_TARGET" "$POWERSHELL_LINKFILE" | ||
| fi | ||
| elif [ -e "$POWERSHELL_LINKFILE" ]; then | ||
| echo "$POWERSHELL_LINKFILE already exists and is not a symbolic link; refusing to replace it." >&2 | ||
| exit 1 | ||
| else | ||
| ln -s "$POWERSHELL_TARGET" "$POWERSHELL_LINKFILE" | ||
| fi |
Comment on lines
+121
to
+122
| test_keeps_matching_symlink() { | ||
| work_dir=$(mktemp -d) |
Comment on lines
+133
to
+134
| test_replaces_different_symlink() { | ||
| work_dir=$(mktemp -d) |
Comment on lines
+145
to
+146
| test_refuses_regular_file() { | ||
| work_dir=$(mktemp -d) |
| fi | ||
| } | ||
|
|
||
| PACKAGE_HASH=$(get_string_sha256 "$PACKAGE_CONTENT") |
| sh "$SCRIPT_PATH" "$POWERSHELL_VERSION" "$POWERSHELL_PACKAGE" | ||
| } | ||
|
|
||
| test_creates_symlink_when_missing() { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
This PR hardens the PowerShell tarball installer script by validating
POWERSHELL_VERSIONandPOWERSHELL_PACKAGE, downloading into a temporary directory with cleanup, restricting downloads to HTTPS/TLS, and tightening quoting around paths and shell registration. It also adds test coverage for the installer’s symlink and file-handling behavior.PR Context
This change improves the robustness, safety and reliability of the tarball install path without changing the public PowerShell API surface.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header