T09 · Insecure Skill Coding Practices
Warning
- Location
- store.py:30
- Finding
- Habit and Conversation Data Stored Without Explicitly Restrictive File Permissions<![CDATA[ ## Vulnerability Details **File Location**: `store.py:30` and `store.py:52-60` **Vulnerability Type**: Plaintext sensitive-data storage with permissions dependent on the process umask **Risk Level**: Medium ### Vulnerable Code ```python os.makedirs(self.data_dir, exist_ok=True) ``` ```python def save(self, user_data: UserData) -> None: """保存用户数据，原子写入 + 文件锁""" tmp_file = self._data_file + ".tmp" try: data = user_data.to_dict() with open(tmp_file, "w", encoding="utf-8") as f: fcntl.flock(f.fileno(), fcntl.LOCK_EX) try: json.dump(data, f, ensure_ascii=False, indent=2) finally: fcntl.flock(f.fileno(), fcntl.LOCK_UN) # 原子替换 os.replace(tmp_file, self._data_file) ``` The persisted data includes user-controlled habit goals, check-in notes, coaching preferences, reminder information, and rationalization conversation history. For example, the conversation history is populated in `agent.py:112-117`: ```python if ai_message: rat.conversation.append(ConversationTurn(role="ai", content=ai_message)) if user_response: rat.conversation.append(ConversationTurn(role="user", content=user_response)) rat.round_count += 1 ``` ### Technical Analysis The data directory is created without an explicit mode, and the temporary JSON file is opened without setting or verifying restrictive permissions. Consequently: - Directory permissions are derived from the default creation mode and the process umask. - The temporary file is normally created from mode `0666`, modified only by the process umask. - `os.replace()` preserves the permissions of the temporary file when it becomes `habits.json`. - Corrupted and manually created backups use similarly inherited permissions. - All records are stored as unencrypted JSON. With a typical restrictive umask, practical exposure may be reduced. However, the code does not enforce that assumption. In environments wi ...[truncated 2004 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Create the data and backup directories with owner-only permissions: ```python os.makedirs(self.data_dir, mode=0o700, exist_ok=True) os.chmod(self.data_dir, 0o700) ``` Apply equivalent protection to the backup directory. 2. Create temporary files atomically with mode `0600`, rather than relying on the process umask: ```python fd = os.open( tmp_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600, ) with os.fdopen(fd, "w", encoding="utf-8") as f: json.dump(data, f, ensure_ascii=False, indent=2) ``` 3. After `os.replace()`, verify or enforce owner-only permissions: ```python os.replace(tmp_file, self._data_file) os.chmod(self._data_file, 0o600) ``` 4. Reject or warn about user-supplied data directories that are symlinks, shared, group-writable, world-writable, or owned by another user. 5. Apply mode `0600` to backup and error-log files. Ensure backup directories use mode `0700`. 6. Document that habit records and conversation history are stored locally in plaintext. Provide retention controls and a command to securely delete stored records and backups. 7. If the Skill is expected to store particularly sensitive health or behavioral information, encrypt records at rest using a key held outside the data directory. 8. Add automated tests that run under permissive umasks and verify that directories are `0700` and all data, temporary, backup, and log files are `0600`. ]]>