T09 · Insecure Skill Coding Practices
- Location
scripts/cleanup.sh:10- Finding
Destructive Cleanup Executes by Default Without Confirmation
- Content
View full analysis
Vulnerability Details
File Location:
scripts/cleanup.sh:10-15,scripts/cleanup.sh:22-28,scripts/cleanup.sh:104-112; conflicting claims inSKILL.md:8-16
Vulnerability Type: Unsafe destructive default and ineffective confirmation control
Risk Level: HighVulnerable Code
bash DRY_RUN=false SKIP_KERNELS=false SKIP_SNAP=false SKIP_DOCKER=false SKIP_BREW=false AUTO_YES=falsebash for arg in "$@"; do case "$arg" in --dry-run) DRY_RUN=true ;; --skip-kernels) SKIP_KERNELS=true ;; --skip-snap) SKIP_SNAP=true ;; --skip-docker) SKIP_DOCKER=true ;; --skip-brew) SKIP_BREW=true ;; --yes|-y) AUTO_YES=true ;;bash do_clean() { local desc="$1" shift if $DRY_RUN; then echo "[DRY-RUN] Would: $desc" else echo "Cleaning: $desc" "$@" 2>/dev/null || true fi }Technical Analysis
The script initializes
DRY_RUNtofalse, so invoking it without arguments enables destructive operations immediately. Although it parses--yesintoAUTO_YES, that variable is never consulted before cleanup begins. There is also no interactive confirmation prompt.This behavior conflicts with the documentation's claims that the Skill is “safe by default” and that dry-run mode protects users from unintended changes. The implementation instead requires users to explicitly request safety with
--dry-run.The generic
do_cleanfunction executes every supplied deletion or cleanup command wheneverDRY_RUNis false. This includesrm -rf, package cleanup, journal vacuuming, snap removal, Docker pruning, and kernel package removal. Several Linux operations invokesudo, increasing the affected scope from user-owned files to system resources.Errors are suppressed through
2>/dev/null || true, which can also conceal partial failures and make it difficult for users to determine exactly which destruct ...[truncated 1717 chars]- Remediation
View remediation
Remediation Suggestions
-
Make dry-run behavior the default:
bash DRY_RUN=true -
Require explicit authorization before enabling destructive mode:
bash --yes|-y) AUTO_YES=true DRY_RUN=false ;; -
If neither
--dry-runnor--yesis supplied, show the planned operations and require an interactive confirmation. Abort when standard input is not an interactive terminal. -
Check
AUTO_YESbefore the first destructive operation rather than merely parsing it. -
Use separate opt-in flags for high-impact categories such as kernels, Docker, Xcode archives, snaps, and system logs. These should not be included in a generic default cleanup.
-
Avoid blanket error suppression. Log command failures and return a nonzero status when critical cleanup operations fail.
-
Update
SKILL.mdso the documented default behavior precisely matches the implementation and clearly identifies operations requiringsudo.
-