-
Notifications
You must be signed in to change notification settings - Fork 0
✨ Add curl | sh self-installer and clean up documentation #4
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.
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
base: main
Are you sure you want to change the base?
Conversation
- Remove MIGRATION.md file that was added in previous PR - Add install.sh script for easy one-line installation - Installer downloads only git-qcommit script to ~/.local/bin/ - Installer creates ~/.local/bin/ directory if it doesn't exist - Installer makes the script executable - Installer can be run via: curl <url> | sh - Update README.md with quick install instructions - Add optional step to download commitprompts separately
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. You can monitor the review status in the checks section at the bottom of this pull request. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security and Reliability Review
This PR adds a convenient curl | sh installer, but introduces several critical security vulnerabilities that must be addressed before merge:
🚨 Critical Issues
- No integrity verification - The installer downloads and executes code without checksum validation, creating a significant attack vector
- Insufficient error handling - Multiple failure points could leave the system in an inconsistent state
- Silent failures - Suppressed error output could hide important failure information
📋 Summary of Required Changes
- Add checksum verification for downloaded scripts
- Implement proper error checking for directory creation and file downloads
- Improve error reporting by removing silent flags from curl/wget
- Verify downloaded file exists and has content before making executable
✅ Positive Aspects
- Clean removal of unnecessary MIGRATION.md file
- Well-structured installer with clear user feedback
- Proper PATH verification and guidance
- Good fallback between curl and wget
The installer concept is solid, but the security issues must be resolved before this can be safely deployed to users.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| mkdir -p "$INSTALL_DIR" | ||
| fi | ||
|
|
||
| # Download the script | ||
| echo "Downloading git-qcommit from $SCRIPT_URL" | ||
| if command -v curl >/dev/null 2>&1; then | ||
| curl -fsSL "$SCRIPT_URL" -o "$INSTALL_DIR/$SCRIPT_NAME" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛑 Security Vulnerability: The installer downloads and executes code from a hardcoded GitHub URL without any integrity verification. This creates a significant security risk as the script could be compromised or intercepted during download1. Add checksum verification or use signed releases to ensure the downloaded script hasn't been tampered with.
| mkdir -p "$INSTALL_DIR" | |
| fi | |
| # Download the script | |
| echo "Downloading git-qcommit from $SCRIPT_URL" | |
| if command -v curl >/dev/null 2>&1; then | |
| curl -fsSL "$SCRIPT_URL" -o "$INSTALL_DIR/$SCRIPT_NAME" | |
| # Download the script with integrity verification | |
| echo "Downloading git-qcommit from $SCRIPT_URL" | |
| if command -v curl >/dev/null 2>&1; then | |
| curl -fsSL "$SCRIPT_URL" -o "$INSTALL_DIR/$SCRIPT_NAME" | |
| # TODO: Add checksum verification here | |
| # Expected checksum should be provided via a separate endpoint or release | |
| elif command -v wget >/dev/null 2>&1; then | |
| wget -qO "$INSTALL_DIR/$SCRIPT_NAME" "$SCRIPT_URL" | |
| # TODO: Add checksum verification here | |
| else | |
| echo "Error: Neither curl nor wget found. Please install one of them and try again." | |
| exit 1 | |
| fi |
Footnotes
-
CWE-494: Download of Code Without Integrity Check - https://cwe.mitre.org/data/definitions/494.html ↩
| INSTALL_DIR="$HOME/.local/bin" | ||
| SCRIPT_NAME="git-qcommit" | ||
| REPO_OWNER="jsamuel1" | ||
| REPO_NAME="git-qcommit" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded branch reference "main" could cause issues if the repository's default branch changes or if users want to install from a specific version. Consider making the branch configurable via environment variable or use a more stable reference like a release tag.
| REPO_NAME="git-qcommit" | |
| REPO_BRANCH="${GIT_QCOMMIT_BRANCH:-main}" |
| echo " git-qcommit Installer" | ||
| echo "=====================================" | ||
| echo "" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mkdir -p command could fail due to permission issues or filesystem problems, but the script continues execution without verifying the directory was actually created. Add error checking to ensure the installation directory exists before proceeding.
| echo " git-qcommit Installer" | |
| echo "=====================================" | |
| echo "" | |
| # Create installation directory if it doesn't exist | |
| if [ ! -d "$INSTALL_DIR" ]; then | |
| echo "Creating directory: $INSTALL_DIR" | |
| if ! mkdir -p "$INSTALL_DIR"; then | |
| echo "Error: Failed to create directory $INSTALL_DIR" | |
| echo "Please check permissions and try again." | |
| exit 1 | |
| fi | |
| fi |
| else | ||
| echo "Error: Neither curl nor wget found. Please install one of them and try again." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script doesn't verify that the download was successful before attempting to make the file executable. If the download fails silently, chmod +x will be applied to a non-existent or empty file. Add verification that the downloaded file exists and has content.
| else | |
| echo "Error: Neither curl nor wget found. Please install one of them and try again." | |
| # Verify the download was successful | |
| if [ ! -f "$INSTALL_DIR/$SCRIPT_NAME" ] || [ ! -s "$INSTALL_DIR/$SCRIPT_NAME" ]; then | |
| echo "Error: Download failed or file is empty" | |
| exit 1 | |
| fi | |
| # Make the script executable | |
| echo "Making script executable" | |
| chmod +x "$INSTALL_DIR/$SCRIPT_NAME" |
| mkdir -p "$INSTALL_DIR" | ||
| fi | ||
|
|
||
| # Download the script | ||
| echo "Downloading git-qcommit from $SCRIPT_URL" | ||
| if command -v curl >/dev/null 2>&1; then | ||
| curl -fsSL "$SCRIPT_URL" -o "$INSTALL_DIR/$SCRIPT_NAME" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The curl and wget commands use flags that suppress error output (-s in curl, -q in wget), which could hide important error messages from users. If the download fails, users won't see helpful error information. Consider using more verbose error reporting.
| mkdir -p "$INSTALL_DIR" | |
| fi | |
| # Download the script | |
| echo "Downloading git-qcommit from $SCRIPT_URL" | |
| if command -v curl >/dev/null 2>&1; then | |
| curl -fsSL "$SCRIPT_URL" -o "$INSTALL_DIR/$SCRIPT_NAME" | |
| echo "Downloading git-qcommit from $SCRIPT_URL" | |
| if command -v curl >/dev/null 2>&1; then | |
| if ! curl -fL "$SCRIPT_URL" -o "$INSTALL_DIR/$SCRIPT_NAME"; then | |
| echo "Error: Failed to download script from $SCRIPT_URL" | |
| exit 1 | |
| fi | |
| elif command -v wget >/dev/null 2>&1; then | |
| if ! wget -O "$INSTALL_DIR/$SCRIPT_NAME" "$SCRIPT_URL"; then | |
| echo "Error: Failed to download script from $SCRIPT_URL" | |
| exit 1 | |
| fi | |
| else | |
| echo "Error: Neither curl nor wget found. Please install one of them and try again." | |
| exit 1 | |
| fi |
Summary
This PR adds a curl-style self-installer script and removes the extra markdown file added in the previous PR, making git-qcommit easier to install and maintain.
Changes
Added
git-qcommitscript (not other files)~/.local/bin/directory if it doesn't existcurl https://raw.githubusercontent.com/jsamuel1/git-qcommit/main/install.sh | shRemoved
Installation Usage
Users can now install git-qcommit with a single command:
curl https://raw.githubusercontent.com/jsamuel1/git-qcommit/main/install.sh | shThe installer will:
~/.local/bin/if it doesn't existgit-qcommitscriptTesting
The installer script includes:
set -eRequirements Implemented
✅ Removed the extra markdown file (MIGRATION.md) from the previous PR
✅ Created a curl | sh style self-installer script
✅ Installer handles creating the ~/.local/bin/ directory if it doesn't exist
✅ Installer makes the installed script executable
✅ Installer is downloadable and executable via curl