T09 · Insecure Skill Coding Practices
Error
- Location
- lib/config.py:80
- Finding
- OpenClaw Configuration Permissions Are Not Preserved During Atomic Replacement## Vulnerability Details **File Location**: `lib/config.py:80-104` **Vulnerability Type**: `T09: Insecure Skill Coding Practices` **Risk Level**: High ### Vulnerable Code ```python def _write_config_locked(self, config: Dict) -> str: """Write config with file locking and backup. Args: config: Config dict to write Returns: Backup file path """ backup_path = f"{self.CONFIG_PATH}.bak-{int(time.time())}" shutil.copy(self.CONFIG_PATH, backup_path) lock_fd = open(self.LOCK_PATH, 'w') try: fcntl.flock(lock_fd, fcntl.LOCK_EX | fcntl.LOCK_NB) # Write to temp file then rename (atomic) tmp_path = f"{self.CONFIG_PATH}.tmp" with open(tmp_path, 'w') as f: json.dump(config, f, indent=2, ensure_ascii=False) os.replace(tmp_path, self.CONFIG_PATH) finally: fcntl.flock(lock_fd, fcntl.LOCK_UN) lock_fd.close() return backup_path ``` ### Technical Analysis The temporary configuration file is created with Python's default `open(..., 'w')` behavior. Its permissions are consequently determined by the process umask instead of inheriting the restrictive permissions of the original `~/.openclaw/openclaw.json`. For example, under a common `022` umask, the temporary file may be created with mode `0644`. The subsequent `os.replace()` operation replaces the original configuration inode with this newly created file, so a configuration previously protected with mode `0600` can become readable by other local users. The OpenClaw configuration contains Discord bot tokens and may contain additional account credentials. This write path is reached by direct permission-removal operations and by the fallback configuration-patching path. The lock also does not address the permission issue because it only coordinates cooperating writers; it does not control the ...[truncated 1477 chars]
- Remediation
- ## Remediation Suggestions - Preserve the original configuration's owner, group, and mode when replacing it. - Create the temporary file explicitly with mode `0600`, rather than relying on the current umask. For example, use `os.open()` with `O_WRONLY | O_CREAT | O_EXCL` and mode `0o600`, then wrap the descriptor with `os.fdopen()`. - Use a uniquely named temporary file in the same directory, such as one created through `tempfile.mkstemp(dir=config_directory)`. - Call `os.fchmod(fd, 0o600)` defensively before writing sensitive data. - Flush and call `os.fsync()` on the temporary file before replacement, then fsync the parent directory after `os.replace()`. - Acquire the lock before reading, copying, or modifying the configuration so the entire read-modify-write transaction is protected. - Verify after replacement that the resulting file has the expected owner and restrictive mode. Abort or correct the mode if it does not. - Apply equally restrictive permissions to the lock file and all timestamped backup files.