T09 · Insecure Skill Coding Practices
Error
- Location
scripts/codex-accounts.py:556- Finding
Unrestricted account names allow path traversal and unintended credential file access
- Content
View full analysis
tuple[str, Path]: base = (base_name or "account").strip() or "account" target = ACCOUNTS_DIR / f"{base}.json" if not target.exists(): return base, target suffix = 2 while True: candidate_name = f"{base}-{suffix}" candidate = ACCOUNTS_DIR / f"{candidate_name}.json" if not candidate.exists(): return candidate_name, candidate suffix += 1 ``` The `compare` command uses caller-controlled names as read paths: ```python def cmd_compare(name_a: str, name_b: str, json_mode: bool = False): path_a = ACCOUNTS_DIR / f"{name_a}.json" path_b = ACCOUNTS_DIR / f"{name_b}.json" if not path_a.exists(): print(f"❌ Account snapshot not found for '{name_a}': {path_a}") return if not path_b.exists(): print(f"❌ Account snapshot not found for '{name_b}': {path_b}") return with open(path_a, "r") as f: a = json.load(f) with open(path_b, "r") as f: b = json.load(f) ``` The `use` command similarly accepts an unvalidated source path and copies it over the active authentication file: ```python def cmd_use(name, sync_openclaw: bool = False, agent_names: list[str] | None = None): ensure_dirs() source = ACCOUNTS_DIR / f"{name}.json" if not source.exists(): print(f"❌ Account '{name}' not found.") print("Available accounts:") for f in _iter_account_snapshot_files(): print(f" - {f.stem}") return # Backup current if it's not saved? # Maybe risky to overwrite silently, but that's what a switcher does ...[truncated 6818 chars]- Remediation
View remediation
str: value = name.strip() if not ACCOUNT_NAME_RE.fullmatch(value): raise ValueError("Invalid account name") return value ``` Apply this validator to `add --name`, `save`, `use`, `compare`, and all JWT-derived names. 2. **Enforce path containment** Construct paths through one helper, resolve the account directory and candidate path, and verify that the candidate is a direct child: ```python def account_path(name: str) -> Path: safe_name = validate_account_name(name) base = ACCOUNTS_DIR.resolve() candidate = (base / f"{safe_name}.json").resolve(strict=False) if candidate.parent != base: raise ValueError("Account path escapes the accounts directory") return candidate ``` Replace every direct expression of the form `ACCOUNTS_DIR / f"{name}.json"` with this helper. 3. **Reject symbolic links** Before reading or writing, use `lstat()` or `Path.is_symlink()` to reject symlink sources and destinations. Where supported, open files with no-follow semantics. 4. **Use atomic, restrictive credential writes** Write credentials to a temporary file created inside `ACCOUNTS_DIR`, set mode `0600`, flush and synchronize it, and atomically replace the validated destination. Explicitly create `ACCOUNTS_DIR` with mode `0700`. 5. **Treat JWT claims as untrusted input** Sanitize email local parts and user IDs before using them as filenames. If sanitization produces an empty or invalid name, require an explicit safe name instead of preserving path characters. 6. **Limit implicit writes** Reconsider running `sync_current_login_to_snapshot()` on every command ...[truncated 439 chars]