fix: address PR #8 security and cross-platform issues

Fixed all critical and high-priority issues from code review:

Security fixes:
- Fix shell injection vulnerability with proper path escaping
- Add timestamped backup creation before modifying shell configs

Reliability improvements:
- Add comprehensive user input path validation
- Add iCloud sync state checking with soft warnings
- Improve shell detection to use default shell (not current session)

Cross-platform support:
- Add platform detection for iCloud features (macOS only)
- Document error handling approach
- Add helpful error messages with actionable suggestions

All changes ensure the commands work safely across Linux, macOS, and
Windows while providing better UX and preventing common user mistakes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Noah Brier
2025-10-07 11:19:41 -04:00
co-authored by Claude
parent 1c9a9eead0
commit ef1ef6073e
2 changed files with 271 additions and 40 deletions
+176 -13
View File
@@ -52,12 +52,12 @@ Then generate a customized CLAUDE.md file tailored to their needs.
3. **Gather Vault Information** 3. **Gather Vault Information**
- Search common locations for existing Obsidian vaults (.obsidian folder) - Search common locations for existing Obsidian vaults (.obsidian folder)
- Check these paths with appropriate depth limits: - Check these paths with appropriate depth limits:
- `~/Documents` (maxdepth 3) - `~/Documents` (maxdepth 3) - all platforms
- `~/Desktop` (maxdepth 3) - `~/Desktop` (maxdepth 3) - all platforms
- `~/Library/Mobile Documents/iCloud~md~obsidian/Documents` (maxdepth 5 - - `~/Library/Mobile Documents/iCloud~md~obsidian/Documents` (maxdepth 5 -
iCloud vaults) **macOS only**, iCloud vaults)
- Home directory `~/` (maxdepth 2) - Home directory `~/` (maxdepth 2) - all platforms
- Current directory parent (maxdepth 2) - Current directory parent (maxdepth 2) - all platforms
- If found, ask: "Found Obsidian vault at [path]. Is this the vault you want - If found, ask: "Found Obsidian vault at [path]. Is this the vault you want
to import?" to import?"
- Count files correctly: `find [path] -type f -name "*.md" | wc -l` (no depth - Count files correctly: `find [path] -type f -name "*.md" | wc -l` (no depth
@@ -71,11 +71,12 @@ Then generate a customized CLAUDE.md file tailored to their needs.
- Identify most active folders by file count - Identify most active folders by file count
- Detect if using PARA, Zettelkasten, Johnny Decimal, or custom - Detect if using PARA, Zettelkasten, Johnny Decimal, or custom
- If not the right one or none found: - If not the right one or none found:
- Ask: "Is your vault stored in iCloud Drive? (yes/no)" - **On macOS only:** Ask: "Is your vault stored in iCloud Drive? (yes/no)"
- If yes: "Please enter the full path to your vault (e.g., ~/Library/Mobile - If yes (macOS): "Please enter the full path to your vault (e.g.,
Documents/iCloud~md~obsidian/Documents/YourVault)" ~/Library/Mobile Documents/iCloud~md~obsidian/Documents/YourVault)"
- If no: "Please enter the path to your existing vault, or type 'skip' to - If no, or on Linux/Windows: "Please enter the path to your existing vault,
start fresh" or type 'skip' to start fresh"
- **Validate user-provided paths** (see "User Path Validation" section below)
- If no existing vault or user skips, they're starting fresh - If no existing vault or user skips, they're starting fresh
4. **Ask Configuration Questions** 4. **Ask Configuration Questions**
@@ -360,16 +361,52 @@ If the user's response is unclear:
- Example: "I want to make sure I import the right vault. Please type the number - Example: "I want to make sure I import the right vault. Please type the number
of your choice (1, 2, or 3)." of your choice (1, 2, or 3)."
### Platform Compatibility
This command is designed to work across Linux, macOS, and Windows (WSL/Git Bash), with platform-specific features:
**All Platforms:**
- Search ~/Documents, ~/Desktop, home directory
- Standard Obsidian vault detection
- Full vault import and setup
**macOS Only:**
- iCloud Drive vault detection and import
- Obsidian's iCloud sync is macOS-only, so iCloud features are disabled on other platforms
**Platform Detection:**
```bash
# Check platform
if [[ "$OSTYPE" == "darwin"* ]]; then
# macOS - enable iCloud features
PLATFORM="macOS"
ICLOUD_SUPPORTED=true
elif [[ "$OSTYPE" == "linux-gnu"* ]]; then
# Linux
PLATFORM="Linux"
ICLOUD_SUPPORTED=false
elif [[ "$OSTYPE" == "msys" || "$OSTYPE" == "cygwin" ]]; then
# Windows (Git Bash or WSL)
PLATFORM="Windows"
ICLOUD_SUPPORTED=false
fi
```
### iCloud Vault Search Implementation ### iCloud Vault Search Implementation
When searching for vaults, use this find command pattern: When searching for vaults, use this find command pattern:
```bash ```bash
# Standard locations (shallow search) # Standard locations (shallow search)
# Note: 2>/dev/null suppresses expected permission errors from system directories
# If no vaults are found, we'll ask the user for their vault path
find ~/Documents ~/Desktop -maxdepth 3 -type d -name ".obsidian" 2>/dev/null find ~/Documents ~/Desktop -maxdepth 3 -type d -name ".obsidian" 2>/dev/null
# iCloud location (deeper search needed due to nested structure) # iCloud location (deeper search needed due to nested structure)
find ~/Library/Mobile\ Documents/iCloud~md~obsidian/Documents -maxdepth 5 -type d -name ".obsidian" 2>/dev/null # Only search on macOS
if [[ "$OSTYPE" == "darwin"* ]]; then
find ~/Library/Mobile\ Documents/iCloud~md~obsidian/Documents -maxdepth 5 -type d -name ".obsidian" 2>/dev/null
fi
# Home directory (shallow to avoid deep recursion) # Home directory (shallow to avoid deep recursion)
find ~ -maxdepth 2 -type d -name ".obsidian" 2>/dev/null find ~ -maxdepth 2 -type d -name ".obsidian" 2>/dev/null
@@ -380,6 +417,117 @@ The iCloud path requires:
- Higher maxdepth (5) due to nested folder structure - Higher maxdepth (5) due to nested folder structure
- Escaped spaces in path name - Escaped spaces in path name
- Silent error handling (2>/dev/null) as many users won't have iCloud - Silent error handling (2>/dev/null) as many users won't have iCloud
- Platform check (macOS only)
**Error Handling Note:** Permission errors are suppressed (2>/dev/null) because they're expected when searching system directories. If no vaults are found, the script gracefully prompts the user for their vault path.
### User Path Validation
When users manually provide a vault path, validate it thoroughly with helpful error messages:
```bash
# User provided path
USER_PATH="$1"
# Expand tilde and resolve to absolute path
USER_PATH="${USER_PATH/#\~/$HOME}"
REAL_PATH=$(realpath "$USER_PATH" 2>/dev/null)
# Validation 1: Path exists
if [ -z "$REAL_PATH" ]; then
echo "❌ Error: Path does not exist: $USER_PATH"
echo ""
echo "💡 Suggestions:"
echo " • Check for typos in the path"
echo " • Make sure you're using the full path (e.g., /Users/name/vault)"
echo " • You can use ~ for your home directory (e.g., ~/Documents/vault)"
exit 1
fi
# Validation 2: Is a directory
if [ ! -d "$REAL_PATH" ]; then
echo "❌ Error: Not a directory: $REAL_PATH"
echo ""
echo "💡 The path exists but points to a file, not a folder."
exit 1
fi
# Validation 3: Contains .obsidian folder
if [ ! -d "$REAL_PATH/.obsidian" ]; then
echo "❌ Error: Not a valid Obsidian vault (no .obsidian folder)"
echo " Looking in: $REAL_PATH"
echo ""
echo "💡 Suggestions:"
echo " • Make sure the path points to your vault root (not a subfolder)"
echo " • Check that you've opened this vault in Obsidian at least once"
echo " • Try the path without trailing slash"
echo " • For iCloud: ~/Library/Mobile Documents/iCloud~md~obsidian/Documents/YourVault"
exit 1
fi
# Validation 4: Readable permissions
if [ ! -r "$REAL_PATH/.obsidian" ]; then
echo "❌ Error: Cannot read vault directory (permission denied)"
echo " Path: $REAL_PATH"
echo ""
echo "💡 You may need to:"
echo " • Check file permissions with: ls -la \"$REAL_PATH\""
echo " • Make sure you own this directory"
exit 1
fi
# Show resolved path if different from input
if [ "$USER_PATH" != "$REAL_PATH" ]; then
echo "✓ Resolved path: $REAL_PATH"
fi
# Valid vault path
VAULT_PATH="$REAL_PATH"
echo "✓ Valid Obsidian vault found"
```
This validation:
- Expands `~` to home directory properly
- Resolves symlinks and relative paths to absolute paths
- Checks all essential requirements (exists, is directory, has .obsidian, readable)
- Provides helpful, actionable error messages with suggestions
- Shows the resolved path so users understand what's being checked
- Trusts users (allows symlinks, paths outside home directory)
- Cross-platform compatible (works on Linux, macOS, Windows/WSL)
### iCloud Sync State Checking
When a user selects an iCloud vault, check sync state and warn if needed:
```bash
# After user confirms vault selection
if [[ "$OSTYPE" == "darwin"* ]] && [[ "$vault_path" == *"iCloud"* ]]; then
# Check for common iCloud sync indicators
if [ -f "$vault_path/.icloud" ] || [ -f "$vault_path/.obsidian/.icloud" ]; then
echo ""
echo "📱 iCloud Sync Notice:"
echo " This vault appears to be still downloading from iCloud."
echo " For best results, open it in Obsidian first to ensure files are synced."
echo ""
read -p "Continue anyway? (yes/no): " sync_answer
if [[ ! "$sync_answer" =~ ^[Yy] ]]; then
echo "No problem! Open the vault in Obsidian, then re-run /init-bootstrap"
exit 0
fi
else
echo ""
echo "📱 iCloud vault detected. If import seems incomplete, make sure sync is complete."
echo ""
fi
fi
```
This provides a soft warning that:
- Only runs on macOS for iCloud paths
- Checks for placeholder files that indicate incomplete download
- Asks for confirmation if sync issues detected
- Gives gentle reminder even when no issues found
- Lets users proceed if they choose
## Interactive Example ## Interactive Example
@@ -440,7 +588,8 @@ First-run marker removed
Now let me ask you a few questions to customize your setup: Now let me ask you a few questions to customize your setup:
🔍 **Searching for existing Obsidian vaults...** [Searches ~/Documents, 🔍 **Searching for existing Obsidian vaults...** [Searches ~/Documents,
~/Desktop, iCloud Drive, home directory, and parent directories] ~/Desktop, home directory, and parent directories. On macOS, also searches iCloud
Drive]
### Case 1: Single Vault Found ### Case 1: Single Vault Found
@@ -489,10 +638,11 @@ User: yes
Great! I'll import your vault to OLD_VAULT/ where it will be safely preserved. Great! I'll import your vault to OLD_VAULT/ where it will be safely preserved.
You can migrate files to the PARA folders at your own pace. You can migrate files to the PARA folders at your own pace.
### Case 3: No Vaults Found (iCloud Check) ### Case 3: No Vaults Found (Platform-Aware)
🔍 **No Obsidian vaults found in common locations.** 🔍 **No Obsidian vaults found in common locations.**
**On macOS:**
Is your vault stored in iCloud Drive? (yes/no) Is your vault stored in iCloud Drive? (yes/no)
User: yes User: yes
@@ -509,6 +659,19 @@ Found vault at: ~/Library/Mobile Documents/iCloud~md~obsidian/Documents/MyVault
Would you like to import this vault? (yes/skip) Would you like to import this vault? (yes/skip)
**On Linux/Windows:**
Please enter the path to your existing Obsidian vault, or type 'skip' to start
fresh: (Example: ~/Documents/MyVault or /home/user/obsidian-vault)
User: ~/Documents/MyVault
[Validates path and shows vault stats]
Found vault at: ~/Documents/MyVault 📊 Vault stats: 1,248 markdown files, 523MB
total size
Would you like to import this vault? (yes/skip)
📦 **Analyzing your vault structure...** [Running tree to see folder hierarchy] 📦 **Analyzing your vault structure...** [Running tree to see folder hierarchy]
[Sampling notes to understand content] [Detecting naming patterns from recent [Sampling notes to understand content] [Detecting naming patterns from recent
files] files]
@@ -34,8 +34,11 @@ The command will be an alias that:
- Changes to the vault directory: `cd /path/to/your/vault` - Changes to the vault directory: `cd /path/to/your/vault`
- Tries to resume existing session: `claude --resume 2>/dev/null` - Tries to resume existing session: `claude --resume 2>/dev/null`
- Falls back to new session if no existing one: `|| claude` - Falls back to new session if no existing one: `|| claude`
- All in one command: - All in one command with properly escaped path:
`(cd /path/to/vault && (claude --resume 2>/dev/null || claude))` `(cd "/path/to/vault" && (claude --resume 2>/dev/null || claude))`
**Important:** The path must be properly escaped to handle spaces and special
characters.
This automatically enters resume mode if there's an existing session, or starts This automatically enters resume mode if there's an existing session, or starts
a new one if not. a new one if not.
@@ -56,32 +59,75 @@ Add the alias to the appropriate config file:
## Shell Detection ## Shell Detection
Detects the user's default shell, with support for command-line override:
```bash ```bash
# Detect current shell # Check if shell specified as argument (/install-claudesidian-command zsh)
if [ -n "$ZSH_VERSION" ]; then if [ -n "$1" ]; then
SHELL_TYPE="zsh" # User provided shell type as argument
CONFIG_FILE="$HOME/.zshrc" SHELL_TYPE="$1"
elif [ -n "$BASH_VERSION" ]; then else
SHELL_TYPE="bash" # Auto-detect from $SHELL (user's default shell, not current shell)
# Prefer .bashrc on Linux, .bash_profile on macOS SHELL_TYPE=$(basename "$SHELL")
if [ -f "$HOME/.bashrc" ]; then
CONFIG_FILE="$HOME/.bashrc"
else
CONFIG_FILE="$HOME/.bash_profile"
fi
elif [ -n "$FISH_VERSION" ]; then
SHELL_TYPE="fish"
CONFIG_FILE="$HOME/.config/fish/config.fish"
fi fi
# Validate shell type and set appropriate config file
case "$SHELL_TYPE" in
zsh)
CONFIG_FILE="$HOME/.zshrc"
;;
bash)
# Prefer .bashrc on Linux, .bash_profile on macOS
if [ -f "$HOME/.bashrc" ]; then
CONFIG_FILE="$HOME/.bashrc"
else
CONFIG_FILE="$HOME/.bash_profile"
fi
;;
fish)
CONFIG_FILE="$HOME/.config/fish/config.fish"
;;
*)
echo "❌ Unsupported shell: $SHELL_TYPE"
echo " Supported shells: bash, zsh, fish"
echo " Usage: /install-claudesidian-command [bash|zsh|fish]"
exit 1
;;
esac
echo "🐚 Installing for: $SHELL_TYPE"
echo "📝 Config file: $CONFIG_FILE"
``` ```
**Key improvements:**
- Uses `$SHELL` to detect default shell (not `$ZSH_VERSION`/`$BASH_VERSION` which detect current session)
- Supports command-line argument to override auto-detection
- Shows detected shell and config file for transparency
- Validates shell type and provides clear error message for unsupported shells
## Installation Steps ## Installation Steps
1. **Get vault path**: Use `pwd` to get current directory 1. **Detect shell**: Use argument if provided, otherwise auto-detect from `$SHELL`
2. **Check if already installed**: Search config file for existing 2. **Get vault path**: Use `pwd` to get current directory
3. **Escape the path**: Properly escape quotes and special characters for shell
safety
```bash
# Escape any double quotes in the path
ESCAPED_PATH="${VAULT_PATH//\"/\\\"}"
# Also escape backslashes
ESCAPED_PATH="${ESCAPED_PATH//\\/\\\\}"
```
4. **Check if already installed**: Search config file for existing
`claudesidian` alias `claudesidian` alias
3. **Add alias**: Append to config file if not present 5. **Create backup**: Before modifying, create timestamped backup of config file
4. **Show success message**: With instructions to reload shell ```bash
# Create backup with timestamp
BACKUP_FILE="$CONFIG_FILE.backup-$(date +%Y%m%d-%H%M%S)"
cp "$CONFIG_FILE" "$BACKUP_FILE"
echo "💾 Backup created: $BACKUP_FILE"
```
6. **Add alias**: Append to config file if not present, using double-quoted path
7. **Show success message**: With instructions to reload shell
## Example Output ## Example Output
@@ -92,8 +138,10 @@ fi
🐚 Shell detected: zsh 🐚 Shell detected: zsh
📝 Config file: /home/user/.zshrc 📝 Config file: /home/user/.zshrc
💾 Backup created: /home/user/.zshrc.backup-20250107-143025
✅ Installed! Added to /home/user/.zshrc: ✅ Installed! Added to /home/user/.zshrc:
alias claudesidian='(cd /home/user/my-vault && (claude --resume 2>/dev/null || claude))' alias claudesidian='(cd "/home/user/my-vault" && (claude --resume 2>/dev/null || claude))'
🔄 To activate, run: 🔄 To activate, run:
source ~/.zshrc source ~/.zshrc
@@ -103,6 +151,15 @@ fi
✨ Test it: Type 'claudesidian' from any directory! ✨ Test it: Type 'claudesidian' from any directory!
``` ```
## Handling Special Characters
The implementation properly handles paths with:
- Spaces: `/Users/noah/My Vault`
- Quotes: `/Users/noah/vault's backup`
- Special characters that need escaping
Paths are double-quoted and any embedded quotes/backslashes are escaped.
## Important Notes ## Important Notes
- The command uses a subshell `()` so it returns to your original directory - The command uses a subshell `()` so it returns to your original directory
@@ -110,32 +167,43 @@ fi
- Automatically tries to resume existing sessions, falls back to new session - Automatically tries to resume existing sessions, falls back to new session
- If alias already exists, ask user if they want to replace it - If alias already exists, ask user if they want to replace it
- Always show what will be added before modifying config files - Always show what will be added before modifying config files
- Create backup of config file before modifying - **Always create timestamped backup** of config file before modifying (format:
`YYYYMMDD-HHMMSS`)
- Backups are kept indefinitely - users can manually clean up old backups if
needed
- Show backup location so users know where to restore from if needed
## Usage Examples ## Usage Examples
Install for current shell: Install for your default shell (auto-detected):
``` ```
/install-claudesidian-command /install-claudesidian-command
``` ```
Install for specific shell: Install for specific shell (override auto-detection):
``` ```
/install-claudesidian-command zsh /install-claudesidian-command zsh
/install-claudesidian-command bash /install-claudesidian-command bash
/install-claudesidian-command fish
``` ```
**When to specify shell:**
- You use multiple shells and want to install for a specific one
- Auto-detection picked the wrong shell
- You're setting up for someone else
## How It Works ## How It Works
The alias uses a clever pattern: The alias uses a clever pattern:
```bash ```bash
alias claudesidian='(cd /path/to/vault && (claude --resume 2>/dev/null || claude))' alias claudesidian='(cd "/path/to/vault" && (claude --resume 2>/dev/null || claude))'
``` ```
1. `(cd /path/to/vault && ...)` - Subshell that changes directory temporarily 1. `(cd "/path/to/vault" && ...)` - Subshell that changes directory temporarily
(path is double-quoted for safety)
2. `claude --resume 2>/dev/null` - Tries to resume existing session, suppresses 2. `claude --resume 2>/dev/null` - Tries to resume existing session, suppresses
error error
3. `|| claude` - If resume fails (no session), starts new session 3. `|| claude` - If resume fails (no session), starts new session