From ef1ef6073e8c1e899d3cd4842bca16c636b688e7 Mon Sep 17 00:00:00 2001 From: Noah Brier Date: Tue, 7 Oct 2025 11:19:41 -0400 Subject: [PATCH] fix: address PR #8 security and cross-platform issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .claude/commands/init-bootstrap.md | 189 ++++++++++++++++-- .../commands/install-claudesidian-command.md | 122 ++++++++--- 2 files changed, 271 insertions(+), 40 deletions(-) diff --git a/.claude/commands/init-bootstrap.md b/.claude/commands/init-bootstrap.md index 9ff957a..fbdca5e 100644 --- a/.claude/commands/init-bootstrap.md +++ b/.claude/commands/init-bootstrap.md @@ -52,12 +52,12 @@ Then generate a customized CLAUDE.md file tailored to their needs. 3. **Gather Vault Information** - Search common locations for existing Obsidian vaults (.obsidian folder) - Check these paths with appropriate depth limits: - - `~/Documents` (maxdepth 3) - - `~/Desktop` (maxdepth 3) + - `~/Documents` (maxdepth 3) - all platforms + - `~/Desktop` (maxdepth 3) - all platforms - `~/Library/Mobile Documents/iCloud~md~obsidian/Documents` (maxdepth 5 - - iCloud vaults) - - Home directory `~/` (maxdepth 2) - - Current directory parent (maxdepth 2) + **macOS only**, iCloud vaults) + - Home directory `~/` (maxdepth 2) - all platforms + - Current directory parent (maxdepth 2) - all platforms - If found, ask: "Found Obsidian vault at [path]. Is this the vault you want to import?" - 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 - Detect if using PARA, Zettelkasten, Johnny Decimal, or custom - If not the right one or none found: - - Ask: "Is your vault stored in iCloud Drive? (yes/no)" - - If yes: "Please enter the full path to your vault (e.g., ~/Library/Mobile - Documents/iCloud~md~obsidian/Documents/YourVault)" - - If no: "Please enter the path to your existing vault, or type 'skip' to - start fresh" + - **On macOS only:** Ask: "Is your vault stored in iCloud Drive? (yes/no)" + - If yes (macOS): "Please enter the full path to your vault (e.g., + ~/Library/Mobile Documents/iCloud~md~obsidian/Documents/YourVault)" + - If no, or on Linux/Windows: "Please enter the path to your existing vault, + 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 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 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 When searching for vaults, use this find command pattern: ```bash # 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 # 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) 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 - Escaped spaces in path name - 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 @@ -440,7 +588,8 @@ First-run marker removed Now let me ask you a few questions to customize your setup: 🔍 **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 @@ -489,10 +638,11 @@ User: yes 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. -### Case 3: No Vaults Found (iCloud Check) +### Case 3: No Vaults Found (Platform-Aware) 🔍 **No Obsidian vaults found in common locations.** +**On macOS:** Is your vault stored in iCloud Drive? (yes/no) 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) +**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] [Sampling notes to understand content] [Detecting naming patterns from recent files] diff --git a/.claude/commands/install-claudesidian-command.md b/.claude/commands/install-claudesidian-command.md index 49e6cf7..1ab605b 100644 --- a/.claude/commands/install-claudesidian-command.md +++ b/.claude/commands/install-claudesidian-command.md @@ -34,8 +34,11 @@ The command will be an alias that: - Changes to the vault directory: `cd /path/to/your/vault` - Tries to resume existing session: `claude --resume 2>/dev/null` - Falls back to new session if no existing one: `|| claude` -- All in one command: - `(cd /path/to/vault && (claude --resume 2>/dev/null || claude))` +- All in one command with properly escaped path: + `(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 a new one if not. @@ -56,32 +59,75 @@ Add the alias to the appropriate config file: ## Shell Detection +Detects the user's default shell, with support for command-line override: + ```bash -# Detect current shell -if [ -n "$ZSH_VERSION" ]; then - SHELL_TYPE="zsh" - CONFIG_FILE="$HOME/.zshrc" -elif [ -n "$BASH_VERSION" ]; then - SHELL_TYPE="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 -elif [ -n "$FISH_VERSION" ]; then - SHELL_TYPE="fish" - CONFIG_FILE="$HOME/.config/fish/config.fish" +# Check if shell specified as argument (/install-claudesidian-command zsh) +if [ -n "$1" ]; then + # User provided shell type as argument + SHELL_TYPE="$1" +else + # Auto-detect from $SHELL (user's default shell, not current shell) + SHELL_TYPE=$(basename "$SHELL") 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 -1. **Get vault path**: Use `pwd` to get current directory -2. **Check if already installed**: Search config file for existing +1. **Detect shell**: Use argument if provided, otherwise auto-detect from `$SHELL` +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 -3. **Add alias**: Append to config file if not present -4. **Show success message**: With instructions to reload shell +5. **Create backup**: Before modifying, create timestamped backup of config file + ```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 @@ -92,8 +138,10 @@ fi 🐚 Shell detected: zsh 📝 Config file: /home/user/.zshrc +💾 Backup created: /home/user/.zshrc.backup-20250107-143025 + ✅ 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: source ~/.zshrc @@ -103,6 +151,15 @@ fi ✨ 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 - 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 - If alias already exists, ask user if they want to replace it - 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 -Install for current shell: +Install for your default shell (auto-detected): ``` /install-claudesidian-command ``` -Install for specific shell: +Install for specific shell (override auto-detection): ``` /install-claudesidian-command zsh /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 The alias uses a clever pattern: ```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 error 3. `|| claude` - If resume fails (no session), starts new session