T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/sync.sh:14
- Finding
- Unsafe and Destructive Cloud Synchronization Defaults<![CDATA[ ## Vulnerability Details **File Locations**: - `scripts/sync.sh:14` - `scripts/sync.sh:83-103` - `scripts/setup.sh:147-150` - `SKILL.md:8-20` **Vulnerability Type**: Unsafe synchronization configuration and destructive default behavior **Risk Level**: High ### Complete Code Snippets `scripts/sync.sh:14` silently selects bidirectional synchronization: ```sh DIRECTION="bisync" ``` `scripts/sync.sh:83-103` disables bisync safety checks and uses destructive `rclone sync` operations for both one-way directions: ```sh case "$DIRECTION" in bisync) # Bisync: bidirectional sync # shellcheck disable=SC2086 rclone bisync "$LOCAL_DIR" "$REMOTE" \ $RCLONE_COMMON \ $RESYNC \ --remove-empty-dirs \ --check-access=false \ --no-check-dest \ 2>&1 ;; push) # Push: local -> remote (one-way) # shellcheck disable=SC2086 rclone sync "$LOCAL_DIR" "$REMOTE" \ $RCLONE_COMMON \ --remove-empty-dirs \ 2>&1 ;; pull) # Pull: remote -> local (one-way) # shellcheck disable=SC2086 rclone sync "$REMOTE" "$LOCAL_DIR" \ $RCLONE_COMMON \ --remove-empty-dirs \ 2>&1 ;; ``` `scripts/setup.sh:147-150` recommends an immediate resync as the first operation: ```sh log "Setup complete." log "Next steps:" log " 1. Run first sync: sh scripts/sync.sh --resync" log " 2. Check status: sh scripts/status.sh" ``` `SKILL.md:8-20` claims that a mode is required and recommends mailbox mode, which is inconsistent with the supplied script's implicit bisync default: ```md Sync the agent workspace with cloud storage. `mode` is required — choose `mailbox` (inbox/outbox, safest), `mirror` (remote->local), or `bisync` (bidirectional, advanced). ## Trigger Use this skill when the user asks to: - Sync workspace to/from cloud - Back up workspace files - Check sync status - Fix sync issues - Send files to the agent workspace ## Sync modes | Mode | Direction | Description | |-- ...[truncated 3697 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove the implicit bisync default. Require `--direction` or an explicit synchronization mode and reject execution when it is absent. 2. Default to a dry run or a non-destructive mailbox/copy workflow rather than bidirectional synchronization. 3. Do not recommend `--resync` as the normal first operation. Require an explicit warning and interactive confirmation before resync. 4. Remove `--check-access=false` and `--no-check-dest` unless a documented, provider-specific reason requires them. 5. Require `--dry-run` before destructive `sync`, `bisync`, or `--resync` operations, and display the proposed changes before confirmation. 6. For additive transfer semantics, use `rclone copy` instead of `rclone sync`. If deletion is intended, clearly identify it and require explicit authorization. 7. Reject the remote root as a default target. Require a dedicated, non-empty remote subdirectory and validate both local and remote path scope. 8. Detect dangerous local targets such as `/`, the workspace root, or a home directory and require additional confirmation. 9. Create or verify a recoverable backup before destructive synchronization. 10. Align the documentation with the implementation. Either implement the documented mailbox mode or remove claims that it is provided by these scripts. 11. Add automated tests covering destination-only files, unavailable remotes, empty paths, first-run resync, dry-run behavior, and interrupted synchronization. ]]>
