T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/clean_c_drive.py:38
- Finding
- Destructive cleanup can bypass documented preview, confirmation, and recovery safeguards<![CDATA[ ## Vulnerability Details **File Location**: `scripts/clean_c_drive.py:38-45`, `scripts/clean_c_drive.py:191-197`, `scripts/clean_c_drive.py:202-220`; related security requirements in `SKILL.md:37-39`, `SKILL.md:57-61`, and `SKILL.md:115-125` **Vulnerability Type**: Unsafe recursive deletion and bypassable safety controls **Risk Level**: High ### Vulnerable Code ```python for item in path.iterdir(): try: if item.is_file(): item.unlink() elif item.is_dir(): shutil.rmtree(item) except (PermissionError, OSError) as e: self.errors.append(f"{dir_path}: {str(e)}") ``` ```python parser.add_argument("--level", default="safe", choices=["safe", "standard", "aggressive"], help="清理级别：safe=安全，standard=标准，aggressive=激进") parser.add_argument("--dry-run", action="store_true", help="预览模式，不实际删除") parser.add_argument("--yes", action="store_true", help="跳过确认") args = parser.parse_args() cleaner = CDriveCleaner(dry_run=args.dry_run) # 预览模式 if args.dry_run: cleaner.execute(args.level) print("\n预览完成，未执行实际清理") print("执行清理请运行：python clean_c_drive.py --level", args.level) return # 请求确认 if not args.yes: if not confirm_action(): print("\n已取消") return # 执行清理 result = cleaner.execute(args.level) ``` ### Technical Analysis The cleanup implementation recursively and permanently deletes all immediate files and directories under broad locations such as: - The current user's temporary directory - `C:\Windows\Temp` - `C:\Windows\SoftwareDistribution\Download` - Browser cache directories - Package-manager cache directories The implementation does not create a backup, create a restore point, quarantine deleted files, enforce a minimum file age, verify that files are inactive, or generate an exact deletion manifest for user approval. These omissions contradict the safeguards documented in `SKILL.md`, which state that cleanup must be explicitly confirmed, previewed by de ...[truncated 3168 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Make preview mode the mandatory default** - Require a separate explicit option such as `--execute` before any mutation. - Do not treat omission of `--dry-run` as authorization to delete. - Require preview generation before execution. 2. **Remove or restrict confirmation bypass** - Remove `--yes` for Agent-driven use. - If unattended operation is required, require a signed or uniquely generated approval token tied to the preview manifest. - Ensure confirmation comes from the end user rather than being inferred from an Agent-generated command. 3. **Generate an exact deletion manifest** - Resolve and list every candidate file before deletion. - Include path, size, age, category, and deletion reason. - Hash or otherwise identify the manifest and require confirmation of that exact version. - Abort if the filesystem changes materially between preview and execution. 4. **Enforce path-containment protections** - Resolve and canonicalize every configured root and candidate path. - Verify that each candidate remains strictly inside an approved cleanup root. - Reject symlinks, junctions, mount points, and Windows reparse points. - Use filesystem APIs that are resistant to link-swap and time-of-check/time-of-use attacks. 5. **Apply conservative deletion rules** - Delete only recognized cache artifacts. - Require a safe minimum age and skip files currently in use. - Use supported Windows maintenance APIs for Windows Update cleanup instead of recursively deleting its download directory. - Do not label broad system-cache deletion as risk-free. 6. **Implement recovery controls** - Create the documented restore point where supported. - Quarantine user-owned files before permanent deletion. - Record sufficient metadata to restore quarantined content. - If these controls will not be implemented, remove the corresponding claims from `SKILL.md`. 7. **Correct cleanup account ...[truncated 495 chars]