T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/dev-reflect.sh:45
- Finding
- Destructive synchronization proceeds without enforcing the documented same-repository guard## Vulnerability Details **File Location**: `scripts/dev-reflect.sh:45-65` **Vulnerability Type**: Missing safety validation before destructive file synchronization **Risk Level**: High ### Technical Analysis The helper derives the destination from the user-supplied marketplace name and then synchronizes component directories using `rsync -a --delete`. It validates only that the source manifest and destination directory exist. It does not resolve symlinks, compare canonical paths, or compare Git common directories before performing the destructive synchronization. ```bash SRC_MP="$SOURCE/.claude-plugin/marketplace.json" CLONE="$HOME/.claude/plugins/marketplaces/$MARKETPLACE" CLONE_MP="$CLONE/.claude-plugin/marketplace.json" [ -f "$SRC_MP" ] || { echo "[dev-reflect] not a marketplace source (no $SRC_MP)" >&2; exit 1; } [ -d "$CLONE" ] || { echo "[dev-reflect] marketplace clone not found: $CLONE" >&2; exit 1; } [ -f "$CLONE_MP" ] || { echo "[dev-reflect] clone has no marketplace.json: $CLONE_MP" >&2; exit 1; } command -v jq >/dev/null || { echo "[dev-reflect] jq required" >&2; exit 1; } run() { if [ "$DRYRUN" = 1 ]; then echo "DRY: $*"; else eval "$*"; fi; } # 1. Sync component dirs HAVE_RSYNC=0; command -v rsync >/dev/null && HAVE_RSYNC=1 for dir in skills agents commands hooks plugins; do [ -d "$SOURCE/$dir" ] || continue if [ "$HAVE_RSYNC" = 1 ]; then run "rsync -a --delete \"$SOURCE/$dir/\" \"$CLONE/$dir/\"" else run "mkdir -p \"$CLONE/$dir\"" run "command cp -r \"$SOURCE/$dir/.\" \"$CLONE/$dir/\"" ``` The associated documentation recognizes this exact hazard in `dev-reflect.md:23-50`: a marketplace path can be a symlink to the source repository or can refer to another worktree of the same repository. It labels the comparison a “HARD STOP,” but the executable helper does not implement that check. Consequently, the prose warning does not protect direct script invocation by the Agent ...[truncated 1745 chars]
- Remediation
- ## Remediation Suggestions 1. Resolve the source and destination with canonical, symlink-aware paths before any write operation, and abort if they are identical. 2. For Git repositories, obtain and canonicalize both the worktree root and common Git directory. Abort when the source and destination share the same repository, including when they are different worktrees. 3. Perform these checks inside `scripts/dev-reflect.sh`; do not rely solely on documentation or the calling Agent. 4. Make dry-run the default and require an explicit flag such as `--apply` or `--confirm-delete` before using `rsync --delete`. 5. Display the canonical source and destination paths and the deletion plan before requesting confirmation. 6. Refuse unsafe destination paths, including an empty path, the home directory, the Claude configuration root, or a destination outside the expected marketplaces directory. 7. Replace the string-based `eval` helper with direct command execution using argument arrays. This preserves argument boundaries and reduces the risk of future command-injection defects. 8. Add regression tests covering direct path equality, symlink aliases, nested paths, separate worktrees sharing a Git common directory, and genuinely independent repositories.