T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/sync-openrouter-models.py:185- Finding
Configuration Replacement May Weaken Permissions on Files Containing API Credentials
- Content
View full analysis
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
View remediation
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. ]]>