Back to skill

Security audit

delete-recovery

Security checks for vulnerabilities and agentic risk

Overview

The skill has a coherent backup-and-recovery purpose, but its scripts contain data-loss and out-of-scope deletion risks that should be reviewed before installation.

Install only if you are comfortable reviewing or patching the deletion paths first. In particular, restrict delete_backup to validated backup folder names and do not rely on workspace_cleaner fallback backups for recoverability until directory restore and metadata format handling are fixed.

Vulnerability Patterns
  • Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
  • Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
  • Agent Memory PoisoningWrites attacker-controlled rules into memory that affect later sessions
  • Remote Payload Retrieval and ExecutionFetches external code whose behavior can change after review
  • Embedded Malicious CodeShips malicious scripts inside the skill and executes them locally
Findings (2)

T09 · Insecure Skill Coding Practices

Error
Location
scripts/delete_recovery.py:735
Finding

Arbitrary Directory Deletion Through Unvalidated Backup Folder Argument

Content
View full analysis

Vulnerability Details

File Location: scripts/delete_recovery.py:735-743
Vulnerability Type: Path traversal and unrestricted recursive directory deletion
Risk Level: High

Vulnerable Code

python
def delete_backup_folder(backup_folder):
    """Delete a specific backup folder manually; also removes its manifest entries."""
    backup_dir = BACKUP_ROOT / backup_folder
    if backup_dir.exists():
        shutil.rmtree(backup_dir)
        # v0.6.0: remove all manifest entries for this folder
        # Use tombstone approach for large manifest
        removed = _remove_manifest_entry(backup_folder, None)

The value of backup_folder originates from the command-line argument supplied to the delete_backup action. It is joined to BACKUP_ROOT without validation before being passed to shutil.rmtree().

Technical Analysis

The implementation assumes that backup_folder is a simple backup identifier, such as a timestamp, but does not enforce that assumption. It does not reject:

  • Absolute paths
  • Parent-directory components such as ..
  • Path separators
  • Symbolic links resolving outside the backup root
  • Non-backup directory names

With pathlib, joining a base path with an absolute path discards the base path. For example, BACKUP_ROOT / "/tmp/target" evaluates to /tmp/target. A relative value containing sufficient ../ components can also resolve outside BACKUP_ROOT.

The restore and verification code uses the same unsafe construction at scripts/delete_recovery.py:589-593 and scripts/delete_recovery.py:674-678. Those paths create additional out-of-root file access opportunities, although recursive deletion is the most severe confirmed sink.

Attack Path

  1. An attacker or untrusted caller gains the ability to invoke the Skill command with controlled arguments.
  2. The caller supplies an absolute directory or a traversal path as the backup folder:
    bash

...[truncated 1218 chars]

Remediation
View remediation

Remediation Suggestions

  1. Accept only strict backup identifiers:

    python
    import re
    
    if not re.fullmatch(r"\d{12}|\d{14}", backup_folder):
        raise ValueError("Invalid backup folder identifier")
    
  2. Reject absolute paths, separators, . and .. components before constructing the target.

  3. Resolve the candidate and verify containment using path semantics rather than string-prefix checks:

    python
    root = BACKUP_ROOT.resolve()
    candidate = (root / backup_folder).resolve(strict=True)
    
    if candidate == root or not candidate.is_relative_to(root):
        raise SafePathError("Backup folder is outside the backup root")
    
  4. Require the target to be a direct child of BACKUP_ROOT and a real directory:

    python
    if candidate.parent != root or not candidate.is_dir() or candidate.is_symlink():
        raise SafePathError("Invalid backup directory")
    
  5. Apply equivalent containment validation to backup_folder and safe_name in restore and verification operations.

  6. Where supported, use descriptor-relative filesystem operations and avoid following symbolic links to reduce time-of-check/time-of-use race exposure.

  7. Add regression tests covering absolute paths, nested traversal, symbolic links, malformed timestamp names, and attempts to delete BACKUP_ROOT itself.

T09 · Insecure Skill Coding Practices

Error
Location
scripts/workspace_cleaner.py:619
Finding

Workspace Cleaner Deletes Data After Producing Backups That the Restore Path Cannot Reliably Recover

Content
View full analysis

Vulnerability Details

File Location: scripts/workspace_cleaner.py:619-659; related restore logic at scripts/delete_recovery.py:589-638 and metadata parser at scripts/safe_path.py:150-190
Vulnerability Type: Destructive cleanup with incompatible and non-restorable backup format
Risk Level: High

Vulnerable Code

The manual fallback treats a copied directory as a successful backup even though it creates no SHA256 record:

python
def _manual_backup(src: Path, rel_str: str) -> bool:
    """
    v0.11.0: Fixed to handle directories (Finding newB, 91% confidence).

    Fallback: manually copy file or directory to delete_backup/timestamp/ when
    delete_recovery.py is unavailable or fails. Mirrors the backup format.

    For files: uses shutil.copy2 with SHA256 + .path record.
    For directories: uses shutil.copytree, records only .path (no SHA256 for dirs).
    If any step fails, returns False — the file/dir will NOT be deleted.
    """
    try:
        timestamp = datetime.now().strftime("%Y%m%d%H%M%S")
        backup_dir = BACKUP_ROOT / timestamp
        backup_dir.mkdir(exist_ok=True)
        safe_name = rel_str.replace("/", "__").replace("\\", "__").replace(":", "")
        dest = backup_dir / safe_name

        if src.is_dir():
            # Directory backup: copytree preserves structure
            shutil.copytree(src, dest)
            path_file = backup_dir / (safe_name + ".path")
            path_file.write_text(str(src.resolve()), encoding="utf-8")
            # No SHA256 for directories (not a file)
            return True
        else:
            # File backup: copy2 + SHA256 + path record
            shutil.copy2(src, dest)
            path_file = backup_dir / (safe_name + ".path")
            path_file.write_text(str(src.resolve()), encoding="utf-8")
            h = hashlib.sha256()
            with open(src, "rb") as f_read:
                for chunk i
...[truncated 4538 chars]
Remediation
View remediation

Remediation Suggestions

  1. Implement a documented, versioned directory-backup format and a directory-aware restore operation.

  2. Compute a deterministic tree manifest or tree hash covering relative paths, file contents, entry types, and relevant metadata. Validate it before restoring a directory.

  3. Make restore logic distinguish files from directories:

    python
    if backup_path.is_dir():
        shutil.copytree(backup_path, dest)
    else:
        shutil.copy2(backup_path, dest)
    
  4. Handle existing destination directories safely. Do not use os.remove() for a directory, and do not destroy an existing target until its preservation copy has been verified.

  5. Use the canonical writer for fallback file metadata:

    python
    SafePathValidator.write_sha256_file(
        sha256_file,
        SafePathValidator.compute_sha256(dest),
        str(src.resolve()),
    )
    
  6. Hash the completed backup copy rather than the source after copying. This confirms the object that will actually be restored.

  7. Before deleting the source, run a backup-format validation that confirms:

    • The copied object exists.
    • Its type matches the source.
    • Required metadata is present and parseable.
    • Its content or tree hash matches.
    • The normal restore implementation supports that backup type.
  8. Narrow --force handling so it bypasses only a genuinely missing legacy record. Malformed, empty, partially written, or invalid records must remain blocking errors.

  9. Use atomic writes for .path and .sha256 metadata, and remove partial backup artifacts when any backup step fails.

  10. Add end-to-end tests that back up, delete, and restore both files and non-empty directories through the primary and fallback paths.

Vulnerability Patterns
  • Behavioral ASTexec() Call, eval() Call, Dynamic Import
  • MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
  • Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
  • Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
  • Privilege EscalationExcessive Permissions, Sudo/Root Execution, Credential Access
Findings (4)

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
96% confidence
Finding

The _search_manifest docstring says it returns entries whose filename, description, or path contains the keyword, implying meaningful path-based search over original paths. However, earlier code stores path as an HMAC-based irreversible value, so the search only matches the opaque token rather than the original path string. This is an active documentation/behavior contradiction, not merely an omitted detail.

Content

No source excerpt is available for this finding.

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
95% confidence
Finding

The class docstring says restore destinations must be within the backup_root tree unless allowed_roots is explicitly set. However, init sets self.allowed_roots to [WORKSPACE_ROOT] when allowed_roots is None, and no code derives a default restore boundary from backup_root. This is an active contradiction in security-relevant behavior.

Content

No source excerpt is available for this finding.

subprocess module call

Medium
Category
Dangerous Code Execution
Confidence
70% confidence
Finding

subprocess module calls execute external commands. Without careful input validation, this enables command injection.

Content

Scanner excerpt · scripts/workspace_cleaner.py (reported line 377)May include surrounding context.

python
try:
        encoded = str(file_path).replace("\\", "/")
        original = original_path_str.replace("\\", "/")
        result = subprocess.run(
            [sys.executable, str(DELETE_RECOVERY_SCRIPT),
             "backup", encoded, original, "workspace_cleaner"],
            capture_output=True, text=True, timeout=60,

Natural-Language Policy Violations

Low
Category
Not specified by scanner
Confidence
91% confidence
Finding

The document is structured to provide behavior and usage in fixed Chinese and English sections, but nowhere offers a user-selectable language preference or states that language follows user choice. Under the policy, forcing a specific language/locale without opt-in is a natural-language policy concern.

Content

No source excerpt is available for this finding.

Static analysis

No suspicious patterns detected.