Fix shell script tilde comparison security vulnerability in CLI preparation tools#24
Fix shell script tilde comparison security vulnerability in CLI preparation tools#24
Conversation
Co-authored-by: joe10832 <103850533+joe10832@users.noreply.github.com>
"$TARGET_DIR" == "$HOME" to properly check for the home directory."$TARGET_DIR" == "$HOME" 
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a security vulnerability in shell script directory comparisons and implements complete CLI preparation tools for wallet extensions. The vulnerability occurred when using "$TARGET_DIR" == "~" which never matches the home directory since ~ isn't expanded in quoted string comparisons.
- Replaces vulnerable
== "~"comparisons with secure== "$HOME"comparisons in all shell scripts - Implements complete wallet extension preparation scripts (MetaMask, Coinbase, Phantom) replacing placeholder functionality
- Updates Node.js wrapper scripts to execute the shell scripts with proper error handling
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cli/prepare-phantom.sh | New shell script implementing Phantom Wallet download/extraction with security fixes |
| src/cli/prepare-phantom.mjs | Updated Node.js wrapper to execute the shell script instead of placeholder |
| src/cli/prepare-metamask.sh | New shell script implementing MetaMask download/extraction with security fixes |
| src/cli/prepare-metamask.mjs | Updated Node.js wrapper to execute the shell script instead of placeholder |
| src/cli/prepare-coinbase.sh | New shell script implementing Coinbase Wallet download/extraction with security fixes |
| src/cli/prepare-coinbase.mjs | Updated Node.js wrapper to execute the shell script instead of placeholder |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| execSync(`chmod +x "${scriptPath}" && "${scriptPath}"`, { | ||
| stdio: "inherit", | ||
| cwd: process.cwd(), | ||
| }) |
There was a problem hiding this comment.
Command injection vulnerability: Using template literals with user-controllable path in shell command. Use array form or properly validate/sanitize the path before execution.
| execSync(`chmod +x "${scriptPath}" && "${scriptPath}"`, { | |
| stdio: "inherit", | |
| cwd: process.cwd(), | |
| }) | |
| execSync('chmod', ['+x', scriptPath], { | |
| stdio: "inherit", | |
| cwd: process.cwd(), | |
| }) | |
| execSync(scriptPath, { | |
| stdio: "inherit", | |
| cwd: process.cwd(), | |
| }) |
| // Execute the shell script for MetaMask preparation | ||
| const scriptPath = path.join(process.cwd(), "src", "cli", "prepare-metamask.sh") | ||
| try { | ||
| execSync(`chmod +x "${scriptPath}" && "${scriptPath}"`, { |
There was a problem hiding this comment.
Command injection vulnerability: Using template literals with user-controllable path in shell command. Use array form or properly validate/sanitize the path before execution.
This PR fixes a critical security vulnerability in shell script directory comparisons where using
"$TARGET_DIR" == "~"never matches the user's home directory because the shell doesn't expand~in string comparisons.The Problem
The vulnerable pattern:
Even when
$TARGET_DIRcontains the full home directory path (e.g.,/home/user), this comparison fails because~is not expanded within double quotes during string comparison.The Solution
Replace all instances with the secure pattern:
Implementation
This PR implements complete CLI preparation tools for wallet extensions that were previously just placeholder scripts:
prepare-metamask.sh) - Downloads and extracts MetaMask extension for testingprepare-coinbase.sh) - Downloads and extracts Coinbase Wallet extension for testingprepare-phantom.sh) - Downloads and extracts Phantom Wallet extension for testingEach script includes security checks that prevent dangerous directory operations by properly detecting:
/)$HOME) - now correctly identified.)..)The Node.js wrapper scripts (
.mjsfiles) have also been updated to properly execute these shell scripts with error handling.Validation
The fix has been validated to work correctly:
"$HOME" == "~"returnsfalse(vulnerable pattern)"$HOME" == "$HOME"returnstrue(secure pattern)Fixes #19.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.