T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/sync-openrouter-models.py:185
- Finding
- Configuration Replacement May Weaken Permissions on Files Containing API Credentials<![CDATA[ ## Vulnerability Details **File Location**: `scripts/sync-openrouter-models.py`, lines 185-195 **Vulnerability Type**: Unsafe temporary-file permissions and failure to preserve destination permissions **Risk Level**: Medium ### Vulnerable Code ```python def safe_write_json(filepath: str, data: dict) -> None: """Write JSON atomically: write to temp, then rename.""" tmp = filepath + ".tmp" try: with open(tmp, "w") as f: json.dump(data, f, indent=2) os.replace(tmp, filepath) # atomic on same filesystem except Exception: if os.path.exists(tmp): os.remove(tmp) raise ``` ### Technical Analysis The function writes updated configuration data to a newly created temporary file and then replaces the original configuration with `os.replace()`. The temporary file is created using the process's default umask-derived permissions; the function neither applies a restrictive mode nor preserves the original destination file's mode. For example, under a common `022` umask, the temporary file will ordinarily be created with mode `0644`. Replacing an original configuration file whose mode was `0600` does not preserve that original mode—the replacement retains the temporary file's permissions. This is security-sensitive because the affected OpenClaw configuration files may contain an OpenRouter API credential. The script explicitly retrieves such credentials from these files: ```python key = root.get("providers", {}).get("openrouter", {}).get("apiKey") ``` Consequently, a normal model synchronization operation can unintentionally make a previously owner-only credential readable by other local users. Exploitation depends on another user being able to traverse the parent directories and read the resulting file. The authenticated OpenRouter request itself is consistent with the Skill's declared model-verification functionality: the credential is sent only to the hard-coded `https://openrouter.ai/a ...[truncated 1391 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Use a securely created temporary file in the destination directory and explicitly preserve or restrict its permissions before replacement. Recommended hardening steps: 1. Read the original file's mode with `os.stat(filepath).st_mode` when it exists. 2. Create the temporary file using `tempfile.mkstemp(dir=destination_directory)` to avoid a predictable shared temporary filename. 3. Apply the original mode with `os.fchmod()`, capped to a secure maximum, or enforce mode `0600` for configuration files containing credentials. 4. Flush buffered data and call `os.fsync()` before replacement if durability is required. 5. Replace the destination atomically only after serialization and permission assignment succeed. 6. Remove the temporary file on every error path. 7. Add a regression test that begins with a `0600` configuration file, performs an update, and verifies that the final file remains `0600`. Example hardened pattern: ```python import os import stat import tempfile def safe_write_json(filepath: str, data: dict) -> None: directory = os.path.dirname(filepath) or "." original_mode = 0o600 if os.path.exists(filepath): original_mode = stat.S_IMODE(os.stat(filepath).st_mode) fd, tmp = tempfile.mkstemp(prefix=".openclaw-", suffix=".tmp", dir=directory) try: os.fchmod(fd, original_mode & 0o600) with os.fdopen(fd, "w") as f: json.dump(data, f, indent=2) f.flush() os.fsync(f.fileno()) os.replace(tmp, filepath) except Exception: try: os.close(fd) except OSError: pass if os.path.exists(tmp): os.remove(tmp) raise ``` If broader permissions are intentionally supported, preserve the exact original mode instead of applying `original_mode & 0o600`; however, credential-bearing configuration files should normally remain owner-readable and owner-writable only. ]]>