T09 · Insecure Skill Coding Practices
- Location
scripts/manage_profile.py:281- Finding
Unrestricted Recursive Deletion Through User-Controlled Data Directory
- Content
View full analysis
Vulnerability Details
File Location:
scripts/manage_profile.py, lines 281–295
Vulnerability Type: Unrestricted recursive deletion of an arbitrary directory
Risk Level: HighVulnerable Code
python def cmd_delete_all(args: argparse.Namespace) -> None: if not args.confirm: raise SleepRoutineError("Deletion requires --confirm") removed = [] if args.data_dir.exists(): for child in args.data_dir.iterdir(): if child.is_file(): child.unlink() removed.append(child.name) elif child.is_dir(): shutil.rmtree(child) removed.append(child.name + "/") try: args.data_dir.rmdir() except OSError: pass print_json({"deleted": True, "removed": sorted(removed)})The deletion target comes from the unrestricted global command-line argument declared at line 301:
python parser.add_argument("--data-dir", type=Path, default=default_data_dir())Technical Analysis
The
delete-alloperation assumes thatargs.data_diris an application-owned sleep-data directory, but it does not verify this assumption. The caller can supply any filesystem path accessible to the current process.The implementation enumerates every child of the supplied path. It invokes
Path.unlink()for files andshutil.rmtree()for directories, regardless of whether those entries belong to this Skill. Consequently, the operation is not limited to the documented application files:profile.jsonsleep-records.jsonreminders.jsonsleep-shift-plan.json
The
--confirmflag only acts as a mechanical Boolean gate. It does not prove that the resolved path is safe, belongs to the application, or matches a path that the user reviewed. There are also no checks rejecting filesystem roots, home directories, project roots, symbolic links, or directories lacking an application ownership marker.Attack Path
...[truncated 1639 chars]
- Remediation
View remediation
Remediation Suggestions
-
Delete only explicitly owned files. Replace directory-wide enumeration with an allowlist:
python OWNED_FILES = { "profile.json", "sleep-records.json", "reminders.json", "sleep-shift-plan.json", } for name in OWNED_FILES: target = args.data_dir / name if target.is_file(): target.unlink() -
Add an application ownership marker. Create a marker during initialization and require it before any destructive operation. Validate its expected schema and application identifier.
-
Canonicalize and validate the target. Resolve the path before deletion and reject:
- Filesystem roots
- The user's home directory
- Repository or project roots
- Paths outside an approved base directory
- Symbolic links and paths traversing symbolic links
- Directories without the ownership marker
-
Bind confirmation to the resolved path. Require confirmation containing the canonical target path or a generated confirmation token, rather than accepting a generic
--confirmflag. -
Preserve least-destructive behavior. Remove the data directory only if it contains no entries other than known application-owned files. Never recursively remove unknown subdirectories.
-
Add security tests. Verify that deletion fails safely when
--data-dirpoints to a home directory, root directory, repository, symlink, unmarked directory, or directory containing unknown files.
-