T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/shrink.sh:27
- Finding
- Unsafe and Overbroad Memory File Archival## Vulnerability Details **File Location**: `scripts/shrink.sh`, lines 27-34 **Vulnerability Type**: Unsafe pathname handling, archive overwrite, and insufficient retention controls **Risk Level**: Medium ```bash OLD_FILES=$(find "$MEMORY_DIR" -maxdepth 1 -name "*.md" -mtime +7 2>/dev/null) if [ -n "$OLD_FILES" ]; then for file in $OLD_FILES; do basename=$(basename "$file") mv "$file" "$ARCHIVE_DIR/${TIMESTAMP}_${basename}" echo "Archived: $basename" done echo "Archive complete." ``` ### Technical Analysis The output of `find` is stored as newline-delimited text and subsequently expanded through the unquoted expression `$OLD_FILES`. Shell word splitting therefore treats spaces, tabs, and newlines within filenames as separators. A valid memory filename containing such characters can be interpreted as multiple paths, causing incorrect moves or terminating the script partway because `set -e` is enabled. Archive destinations use a timestamp with only minute-level precision and the original basename. The script invokes `mv` without collision protection, so an existing regular file with the same destination name may be silently replaced. The destination is predictable from the execution time and source basename. The selection rule also archives every top-level Markdown file older than seven days without inspecting its content or excluding protected files. This conflicts with the documented requirement to retain unfinished-task progress, current team state, and ongoing discussions. Consequently, an old file that still contains operationally active information can be removed from the live memory directory. The workspace is derived from a caller-controlled first argument at line 7: ```bash WORKSPACE="${1:-/root/.openclaw/workspace-code_analyst}" ``` No approved-root or canonical-path validation is applied. A caller capable of invoking the script can therefore direct its file-moving be ...[truncated 1508 chars]
- Remediation
- ## Remediation Suggestions - Process paths using null delimiters rather than command substitution and shell word splitting: ```bash find "$MEMORY_DIR" -maxdepth 1 -type f -name '*.md' -mtime +7 -print0 | while IFS= read -r -d '' file; do name=${file##*/} destination="$ARCHIVE_DIR/${TIMESTAMP}_${name}" mv -n -- "$file" "$destination" done ``` - Use a collision-resistant archive name containing seconds or a securely generated unique suffix. Check whether the destination exists and fail safely instead of overwriting it. Use `mv -n --` where supported. - Explicitly exclude protected files such as `MEMORY.md` and evaluate documented retention criteria before moving a record. File age alone must not determine whether active operational information is archived. - Canonicalize the supplied workspace path and require it to reside under an approved workspace root. Reject paths outside that boundary, including paths redirected through symbolic links. - Validate that the memory and archive directories are not symbolic links before modifying them. - Record each selected source and destination, report failures accurately, and avoid leaving partially completed archival operations. Consider staging changes and committing them only after all validation succeeds.