T09 · Insecure Skill Coding Practices
Error
- Location
- operations/cleaner.py:66
- Finding
- Arbitrary Recursive Directory Deletion Without Enforced Authorization or Confirmation<![CDATA[ ## Vulnerability Details **File Location**: `mcp/server.py:122-145`; `operations/cleaner.py:25-33`; `operations/cleaner.py:66-120` **Vulnerability Type**: Unrestricted destructive filesystem operation **Risk Level**: High ### Vulnerable Code ```python # mcp/server.py:122-145 @mcp.tool() def clean_directory( path: str, dry_run: bool = True, ) -> dict[str, Any]: """ Clean a directory. Args: path: Directory path to clean dry_run: If True, only preview what would be deleted Returns: Clean result with space that would be freed """ result = cleaner.clean(path, dry_run=dry_run) return result.to_dict() ``` ```python # operations/cleaner.py:25-33 self.protected_paths = set() if protected_paths: for p in protected_paths: self.protected_paths.add(str(Path(p).expanduser().resolve())) # Add default protected paths home = str(Path.home().resolve()) self.protected_paths.add(home) self.protected_paths.add(os.path.join(home, "Documents")) self.protected_paths.add(os.path.join(home, "Desktop")) ``` ```python # operations/cleaner.py:66-120 path_obj = Path(path).expanduser().resolve() path_str = str(path_obj) # Check if path exists if not path_obj.exists(): return CleanResult( success=False, path=path_str, error="Path does not exist", dry_run=dry_run, ) # Check if protected if self.is_protected(path_str): return CleanResult( success=False, path=path_str, error="Path is protected and cannot be deleted", dry_run=dry_run, ) # Get size before deletion size = self.get_size(path_str) if dry_run: return CleanResult( success=True, path=path_str, freed_bytes=size, dry_run=True, ) # Perform deletion try: if keep_root: # Delete contents but keep directory for item in path_obj.iterdir(): if item.is_dir(): shutil.rmtree(item) ...[truncated 2981 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Require a server-generated confirmation token before destructive execution: - A dry run should return a short-lived token bound to the canonical path, operation, caller, and previewed contents. - Reject `dry_run=False` unless the caller supplies the valid token. - Invalidate the token if the path or directory state changes. 2. Apply a deny-by-default path policy: - Deny filesystem roots, drive roots, home-directory ancestors, operating-system directories, program directories, and sensitive credential/configuration trees. - Protect both exact paths and all descendants or ancestors where appropriate. - Maintain platform-specific protected-path sets. 3. Restrict cleanup to explicit allowlisted cache or temporary directories. Unknown paths should require an out-of-band human approval mechanism. 4. Validate the path without following links: - Inspect the original path with `lstat()` or `Path.is_symlink()`. - Reject symlinks, junctions, mount points, and reparse points for recursive cleanup. - Revalidate immediately before deletion to reduce time-of-check/time-of-use races. 5. Run the MCP server under a dedicated, least-privileged account with access only to approved cleanup locations. 6. Record auditable details for destructive operations, including caller identity, canonical path, preview result, confirmation identifier, and outcome. ]]>